Regression(r196145): Crash in getOwnPropertyDescriptor on http://www.history.com/shows/vikings: ASSERTION FAILED: maybeGetterSetter /Volumes/Data/WebKit/OpenSource/Source/JavaScriptCore/runtime/JSObject.cpp(2859) : bool JSC::JSObject::getOwnPropertyDescriptor(JSC::ExecState *, JSC::PropertyName, JSC::PropertyDescriptor &) 1 0x110ffeded WTFCrash 2 0x110b52342 JSC::JSObject::getOwnPropertyDescriptor(JSC::ExecState*, JSC::PropertyName, JSC::PropertyDescriptor&) 3 0x110ca3875 JSC::objectConstructorGetOwnPropertyDescriptor(JSC::ExecState*, JSC::JSObject*, JSC::Identifier const&) 4 0x110ca3f0d JSC::objectConstructorGetOwnPropertyDescriptor(JSC::ExecState*) 5 0x2a29de801028 6 0x110c35cd3 llint_entry 7 0x110c35d4d llint_entry 8 0x110c35d4d llint_entry 9 0x110c35d4d llint_entry 10 0x110c35d4d llint_entry 11 0x110c2f37e vmEntryToJavaScript 12 0x110a4ecb7 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) The page crashes when doing the following: Object.getOwnPropertyDescriptor(window, "indexedDB") getOwnPropertyDescriptor() expected getDirect() to return a CustomGetterSetter for CustomAccessors but it is not the case for window.indexedDB. The reason is that window.indexedDB is a special property currently, which is not part of the static table but returned by GetOwnPropertySlot() if IndexedDB feature is enabled. This weirdness is because we do not have proper support for [EnabledAtRuntime] properties on Window.
rdar://problem/25410767
Created attachment 275474 [details] WIP Patch for ews
Comment on attachment 275474 [details] WIP Patch for ews Attachment 275474 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1088208 New failing tests: js/cached-window-properties.html fast/dom/Window/window-postmessage-clone-frames.html
Created attachment 275476 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 275474 [details] WIP Patch for ews Attachment 275474 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1088210 New failing tests: js/cached-window-properties.html
Created attachment 275477 [details] Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 275474 [details] WIP Patch for ews Attachment 275474 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1088230 New failing tests: js/cached-window-properties.html fast/dom/Window/window-postmessage-clone-frames.html
Created attachment 275479 [details] Archive of layout-test-results from ews112 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 275488 [details] WIP Patch for ews
Created attachment 275502 [details] Patch
Comment on attachment 275502 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275502&action=review Can we add a bindings test? > Source/JavaScriptCore/runtime/CommonIdentifiers.h:31 > + macro(AnimationTimeline) \ It's strange that these things are listed in "common" identifiers table. Do we really need to add them here?
(In reply to comment #11) > Comment on attachment 275502 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=275502&action=review > > Can we add a bindings test? The change is specific to DOMWindow so this makes bindings test coverage difficult. We have no such coverage at the moment. > > > Source/JavaScriptCore/runtime/CommonIdentifiers.h:31 > > + macro(AnimationTimeline) \ > > It's strange that these things are listed in "common" identifiers table. > Do we really need to add them here? My initial implementation was creating those PropertyNames on the fly in JSDOMWindow::finishCreation() as so: PropertyName propertyString = Identifier::fromString(&vm, "indexedDB"); However, this lead to assertion hits in debug when destroying the PropertyTable: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread ASSERT_WITH_MESSAGE(iterator != atomicStringTable.end(), "The string being removed is atomic in the string table of an other thread!"); 0 com.apple.JavaScriptCore 0x0000000105fc4b47 WTFCrash + 39 (Assertions.cpp:322) 1 com.apple.JavaScriptCore 0x0000000105fc819b WTF::AtomicStringImpl::remove(WTF::AtomicStringImpl*) + 267 (AtomicStringImpl.cpp:453) 2 com.apple.JavaScriptCore 0x0000000106019643 WTF::StringImpl::~StringImpl() + 163 (StringImpl.cpp:117) 3 com.apple.JavaScriptCore 0x00000001060197b5 WTF::StringImpl::~StringImpl() + 21 (StringImpl.cpp:134) 4 com.apple.JavaScriptCore 0x00000001060197d5 WTF::StringImpl::destroy(WTF::StringImpl*) + 21 (StringImpl.cpp:139) 5 com.apple.JavaScriptCore 0x000000010505b654 WTF::StringImpl::deref() + 52 (StringImpl.h:601) 6 com.apple.JavaScriptCore 0x0000000105d95ed1 JSC::PropertyTable::~PropertyTable() + 97 (PropertyTable.cpp:127) 7 com.apple.JavaScriptCore 0x0000000105d95e65 JSC::PropertyTable::~PropertyTable() + 21 (PropertyTable.cpp:130) 8 com.apple.JavaScriptCore 0x0000000105d94fe5 JSC::PropertyTable::destroy(JSC::JSCell*) + 21 (PropertyTable.cpp:121) 9 com.apple.JavaScriptCore 0x0000000105bfcc98 JSC::MarkedBlock::callDestructor(JSC::JSCell*) + 200 (MarkedBlock.cpp:85) 10 com.apple.JavaScriptCore 0x0000000105bfcafb JSC::MarkedBlock::FreeList JSC::MarkedBlock::specializedSweep<(JSC::MarkedBlock::BlockState)3, (JSC::MarkedBlock::SweepMode)0, true>() + 267 (MarkedBlock.cpp:102) I did not understand why this would happen. Maybe a JSC expert can shade some light on this?
Comment on attachment 275502 [details] Patch Clearing flags on attachment: 275502 Committed r199017: <http://trac.webkit.org/changeset/199017>
All reviewed patches have been landed. Closing bug.
This layout test has been timing out on GTK since it was added, see bug #158925.
(In reply to comment #15) > This layout test has been timing out on GTK since it was added, see bug > #158925. Sorry, ignore that, I had my canned reply at the top of my head. Rather, this change appears to have broken the preexisting test media/media-source/media-source-loadedmetada-with-two-sourcebuffers.html. See bug #158925.