* SUMMARY ASSERT reloading <http://www.apple.com>. * STEPS TO REPRODUCE 1. Load http://www.apple.com 2. Reload => ASSERT ASSERTION FAILED: m_inInvalidateNodeListAndCollectionCaches ? m_collectionsInvalidatedAtDocument.isEmpty() : m_collectionsInvalidatedAtDocument.contains(&collection) (lldb) bt * thread #1: tid = 0x6a39bc, 0x00000001133be3aa JavaScriptCore`WTFCrash + 42 at Assertions.cpp:329, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef) * frame #0: 0x00000001133be3aa JavaScriptCore`WTFCrash + 42 at Assertions.cpp:329 frame #1: 0x0000000114bbb6c7 WebCore`WebCore::Document::unregisterCollection(this=0x00007fd55c8b4000, collection=0x00007fd55c386f10) + 279 at Document.cpp:3516 frame #2: 0x0000000115037f1e WebCore`WebCore::HTMLCollection::~HTMLCollection(this=0x00007fd55c386f10) + 94 at HTMLCollection.cpp:156 frame #3: 0x00000001150380c5 WebCore`WebCore::HTMLCollection::~HTMLCollection(this=0x00007fd55c386f10) + 21 at HTMLCollection.cpp:154 frame #4: 0x00000001150380e9 WebCore`WebCore::HTMLCollection::~HTMLCollection(this=0x00007fd55c386f10) + 25 at HTMLCollection.cpp:154 frame #5: 0x0000000114693123 WebCore`WTF::RefCounted<WebCore::HTMLCollection>::deref(this=0x00007fd55c386f20) + 83 at RefCounted.h:146 frame #6: 0x00000001155eb039 WebCore`WebCore::JSHTMLCollection::releaseImplIfNotNull(this=0x000000011e048e10) + 57 at JSHTMLCollection.h:63 frame #7: 0x00000001155ea615 WebCore`WebCore::JSHTMLCollection::~JSHTMLCollection(this=0x000000011e048e10) + 21 at JSHTMLCollection.cpp:172 frame #8: 0x00000001155ea5f5 WebCore`WebCore::JSHTMLCollection::~JSHTMLCollection(this=0x000000011e048e10) + 21 at JSHTMLCollection.cpp:171 frame #9: 0x00000001155e9b3d WebCore`WebCore::JSHTMLCollection::destroy(cell=0x000000011e048e10) + 29 at JSHTMLCollection.cpp:167 frame #10: 0x00000001131ccf86 JavaScriptCore`void JSC::MarkedBlock::callDestructor<(this=0x000000011e040000, cell=0x000000011e048e10)2>(JSC::JSCell*) + 70 at MarkedBlock.cpp:65 frame #11: 0x00000001131ccd38 JavaScriptCore`JSC::MarkedBlock::FreeList JSC::MarkedBlock::specializedSweep<(this=0x000000011e040000)3, (JSC::MarkedBlock::SweepMode)1, (JSC::MarkedBlock::DestructorType)2>() + 216 at MarkedBlock.cpp:89 frame #12: 0x00000001131cbe9a JavaScriptCore`JSC::MarkedBlock::FreeList JSC::MarkedBlock::sweepHelper<(this=0x000000011e040000, sweepMode=SweepToFreeList)2>(JSC::MarkedBlock::SweepMode) + 282 at MarkedBlock.cpp:142 frame #13: 0x00000001131cb689 JavaScriptCore`JSC::MarkedBlock::sweep(this=0x000000011e040000, sweepMode=SweepToFreeList) + 233 at MarkedBlock.cpp:121 frame #14: 0x00000001131cab7e JavaScriptCore`JSC::MarkedAllocator::tryAllocateHelper(this=0x00007fd55f831990, bytes=32) + 270 at MarkedAllocator.cpp:80 frame #15: 0x00000001131c9122 JavaScriptCore`JSC::MarkedAllocator::tryAllocate(this=0x00007fd55f831990, bytes=32) + 114 at MarkedAllocator.cpp:129 frame #16: 0x00000001131c8a0e JavaScriptCore`JSC::MarkedAllocator::allocateSlowCase(this=0x00007fd55f831990, bytes=32) + 254 at MarkedAllocator.cpp:171 frame #17: 0x0000000114771b41 WebCore`JSC::MarkedAllocator::allocate(this=0x00007fd55f831990, bytes=32) + 81 at MarkedAllocator.h:95 frame #18: 0x00000001147f77f9 WebCore`JSC::MarkedSpace::allocateWithNormalDestructor(this=0x00007fd55f831928, bytes=32) + 41 at MarkedSpace.h:251 frame #19: 0x00000001147f77c6 WebCore`JSC::Heap::allocateWithNormalDestructor(this=0x00007fd55f831618, bytes=32) + 118 at HeapInlines.h:187 frame #20: 0x00000001155a0d37 WebCore`void* JSC::allocateCell<WebCore::JSEvent>(heap=0x00007fd55f831618, size=32) + 151 at JSCellInlines.h:122 frame #21: 0x00000001155a0c8f WebCore`void* JSC::allocateCell<WebCore::JSEvent>(heap=0x00007fd55f831618) + 31 at JSCellInlines.h:136 frame #22: 0x00000001155a0ade WebCore`WebCore::JSEvent::create(structure=0x0000000123852570, globalObject=0x000000011e02e870, impl=PassRefPtr<WebCore::Event> at 0x00007fff56892bb0) + 46 at JSEvent.h:36 frame #23: 0x0000000115592a66 WebCore`WebCore::JSDOMWrapper* WebCore::createWrapper<WebCore::JSEvent, WebCore::Event>(globalObject=0x000000011e02e870, node=0x00007fd55c2bcf70) + 214 at JSDOMBinding.h:229 frame #24: 0x0000000115591c87 WebCore`WebCore::toJS((null)=0x000000011e02e8b0, globalObject=0x000000011e02e870, event=0x00007fd55c2bcf70) + 455 at JSEventCustom.cpp:68 frame #25: 0x00000001155a56d6 WebCore`WebCore::JSEventListener::handleEvent(this=0x00007fd55c3b9af0, scriptExecutionContext=0x00007fd55c8b40a0, event=0x00007fd55c2bcf70) + 774 at JSEventListener.cpp:114 frame #26: 0x0000000114dac9cf WebCore`WebCore::EventTarget::fireEventListeners(this=0x00007fd55c0e61c0, event=0x00007fd55c2bcf70, d=0x00007fd55c0e61c8, entry=0x00007fd55c3cd980) + 1503 at EventTarget.cpp:246 frame #27: 0x0000000114dac29e WebCore`WebCore::EventTarget::fireEventListeners(this=0x00007fd55c0e61c0, event=0x00007fd55c2bcf70) + 334 at EventTarget.cpp:197 frame #28: 0x0000000114cd5c5b WebCore`WebCore::DOMWindow::dispatchEvent(this=0x00007fd55c0e61c0, prpEvent=<unavailable>, prpTarget=(m_ptr = WebCore::EventTarget * = 0x0000000000000000)) + 539 at DOMWindow.cpp:1897 frame #29: 0x0000000114f11667 WebCore`WebCore::FrameLoader::stopLoading(this=0x00007fd55c02a0b0, unloadEventPolicy=UnloadEventPolicyUnloadAndPageHide) + 983 at FrameLoader.cpp:464 frame #30: 0x0000000114f11c9f WebCore`WebCore::FrameLoader::closeURL(this=0x00007fd55c02a0b0) + 111 at FrameLoader.cpp:543 frame #31: 0x0000000114f1a2a9 WebCore`WebCore::FrameLoader::transitionToCommitted(this=0x00007fd55c02a0b0, cachedPage=0x0000000000000000) + 345 at FrameLoader.cpp:1901 frame #32: 0x0000000114f1993f WebCore`WebCore::FrameLoader::commitProvisionalLoad(this=0x00007fd55c02a0b0) + 2143 at FrameLoader.cpp:1798 frame #33: 0x0000000114c051ec WebCore`WebCore::DocumentLoader::commitIfReady(this=0x00007fd55b829400) + 60 at DocumentLoader.cpp:355 frame #34: 0x0000000114c0825c WebCore`WebCore::DocumentLoader::commitLoad(this=0x00007fd55b829400, data=0x00007fd55e18e000, length=10161) + 76 at DocumentLoader.cpp:763 frame #35: 0x0000000114c08b7d WebCore`WebCore::DocumentLoader::dataReceived(this=0x00007fd55b829400, resource=0x00007fd565eba3f0, data=0x00007fd55e18e000, length=10161) + 1693 at DocumentLoader.cpp:888 frame #36: 0x0000000114815031 WebCore`WebCore::CachedRawResource::notifyClientsDataWasReceived(this=0x00007fd565eba3f0, data=0x00007fd55e18e000, length=10161) + 161 at CachedRawResource.cpp:114 frame #37: 0x0000000114814f17 WebCore`WebCore::CachedRawResource::addDataBuffer(this=0x00007fd565eba3f0, data=0x00007fd55c2e5c80) + 215 at CachedRawResource.cpp:70 frame #38: 0x00000001162c07fe WebCore`WebCore::SubresourceLoader::didReceiveDataOrBuffer(this=0x00007fd565c9f8c0, data=0x00007fd55f916a50, length=10161, prpBuffer=PassRefPtr<WebCore::SharedBuffer> at 0x00007fff56893e60, encodedDataLength=-1, dataPayloadType=DataPayloadBytes) + 478 at SubresourceLoader.cpp:274 frame #39: 0x00000001162c0606 WebCore`WebCore::SubresourceLoader::didReceiveData(this=0x00007fd565c9f8c0, data=0x00007fd55f916a50, length=10161, encodedDataLength=-1, dataPayloadType=DataPayloadBytes) + 102 at SubresourceLoader.cpp:250 frame #40: 0x00000001109a77ff WebKit`WebKit::WebResourceLoader::didReceiveData(this=0x00007fd568644f50, data=0x00007fff56894020, encodedDataLength=-1) + 223 at WebResourceLoader.cpp:148 frame #41: 0x00000001109ab175 WebKit`void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (object=0x00007fd568644f50, function=0x00000001109a7720, args=0x00007fff56894020, (null)=index_sequence<0, 1> at 0x00007fff56893f38)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>, 0ul, 1ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>&&, std::index_sequence<0ul, 1ul>) + 181 at HandleMessage.h:16 frame #42: 0x00000001109ab018 WebKit`void IPC::callMemberFunction<WebKit::WebResourceLoader, void (args=0x00007fff56894020, object=0x00007fd568644f50, function=0x00000001109a7720)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>, std::make_index_sequence<2ul> >(std::__1::tuple<IPC::DataReference, long long>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)) + 88 at HandleMessage.h:22 frame #43: 0x00000001109aa1eb WebKit`void IPC::handleMessage<Messages::WebResourceLoader::DidReceiveData, WebKit::WebResourceLoader, void (decoder=0x00007fd565f18d80, object=0x00007fd568644f50, function=0x00000001109a7720)(IPC::DataReference const&, long long)>(IPC::MessageDecoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)) + 283 at HandleMessage.h:120 frame #44: 0x00000001109a97fc WebKit`WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(this=0x00007fd568644f50, (null)=0x00007fd55c3170f0, decoder=0x00007fd565f18d80) + 636 at WebResourceLoaderMessageReceiver.cpp:68 frame #45: 0x000000011046bf60 WebKit`WebKit::NetworkProcessConnection::didReceiveMessage(this=0x00007fd55c3090f0, connection=0x00007fd55c3170f0, decoder=0x00007fd565f18d80) + 160 at NetworkProcessConnection.cpp:61 frame #46: 0x00000001102698c3 WebKit`IPC::Connection::dispatchMessage(this=0x00007fd55c3170f0, decoder=0x00007fd565f18d80) + 51 at Connection.cpp:809 frame #47: 0x0000000110261ba0 WebKit`IPC::Connection::dispatchMessage(this=0x00007fd55c3170f0, message=unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> > at 0x00007fff568944a8) + 368 at Connection.cpp:828 frame #48: 0x000000011026969f WebKit`IPC::Connection::dispatchOneMessage(this=0x00007fd55c3170f0) + 1439 at Connection.cpp:856 frame #49: 0x000000011027a8f2 WebKit`WTF::FunctionWrapper<void (this=0x00007fd55c5f5c30, c=0x00007fd55c3170f0)()>::operator()(IPC::Connection*) + 114 at Functional.h:218 frame #50: 0x000000011027a875 WebKit`WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (this=0x00007fd55c5f5c20)()>, void (IPC::Connection*)>::operator()() + 53 at Functional.h:496 frame #51: 0x0000000110280a62 WebKit`WTF::Function<void (this=0x00007fd565f288d8)>::operator()() const + 114 at Functional.h:704 frame #52: 0x00000001102809dc WebKit`std::__1::__function::__func<WTF::Function<void ()>, std::__1::allocator<WTF::Function<void ()> >, void ()>::operator()() [inlined] decltype(this=0x00007fd565f288d8, __f=0x00007fd565f288d8)>&>(fp)(std::__1::forward<>(fp0))) std::__1::__invoke<WTF::Function<void ()>&>(WTF::Function<void ()>&&&) + 60 at __functional_base:413 frame #53: 0x00000001102809cb WebKit`std::__1::__function::__func<WTF::Function<void (this=0x00007fd565f288d0)>, std::__1::allocator<WTF::Function<void ()> >, void ()>::operator()() + 43 at functional:1370 frame #54: 0x00000001133e747a JavaScriptCore`std::__1::function<void (this=0x00007fff568949e0)>::operator()() const + 26 at functional:1755 frame #55: 0x00000001133f6e12 JavaScriptCore`WTF::RunLoop::performWork(this=0x00007fd55c30c460) + 306 at RunLoop.cpp:104 frame #56: 0x00000001133f8304 JavaScriptCore`WTF::RunLoop::performWork(context=0x00007fd55c30c460) + 36 at RunLoopCF.cpp:38 frame #57: 0x00007fff88c06111 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 frame #58: 0x00007fff88bf7c8d CoreFoundation`__CFRunLoopDoSources0 + 269 frame #59: 0x00007fff88bf72db CoreFoundation`__CFRunLoopRun + 923 frame #60: 0x00007fff88bf6cf8 CoreFoundation`CFRunLoopRunSpecific + 296 ... (lldb) up frame #1: 0x0000000114bbb6c7 WebCore`WebCore::Document::unregisterCollection(this=0x00007fd55c8b4000, collection=0x00007fd55c386f10) + 279 at Document.cpp:3516 3513 if (!collection.isRootedAtDocument()) 3514 return; 3515 -> 3516 ASSERT(m_inInvalidateNodeListAndCollectionCaches 3517 ? m_collectionsInvalidatedAtDocument.isEmpty() 3518 : m_collectionsInvalidatedAtDocument.contains(&collection)); 3519 m_collectionsInvalidatedAtDocument.remove(&collection); (lldb) p m_inInvalidateNodeListAndCollectionCaches (bool) $0 = false
FWIW, the collection is empty: (lldb) p m_collectionsInvalidatedAtDocument.isEmpty() (bool) $6 = true
The issue is not with the assertion itself, but with the HashSet being properly zeroed out in Document::invalidateNodeListAndCollectionCaches() after HashTable gained proper move constructor and assignment operator. The problematic HTMLCollection is of the DocLinks type. After it is created, the first change in any class attribute on www.apple.com (like hovering on/off a menu) results in the following call chain: #0 WebCore::HTMLCollection::invalidateCache (this=0x3936000, attrName=0x7ffe59fb0de8 <WebCore::HTMLNames::classAttr>) at ../../Source/WebCore/html/HTMLCollection.h:114 #1 0x00007ffe56d016b9 in WebCore::Document::invalidateNodeListAndCollectionCaches (this=0x2b80a40, attrName=0x7ffe59fb0de8 <WebCore::HTMLNames::classAttr>) at ../../Source/WebCore/dom/Node.cpp:722 #2 0x00007ffe56d017e5 in WebCore::Node::invalidateNodeListAndCollectionCachesInAncestors (this=0x2788210, attrName=0x7ffe59fb0de8 <WebCore::HTMLNames::classAttr>, attributeOwnerElement=0x2788210) at ../../Source/WebCore/dom/Node.cpp:742 #3 0x00007ffe56caeb78 in WebCore::Element::attributeChanged (this=0x2788210, name=..., oldValue=..., newValue=...) at ../../Source/WebCore/dom/Element.cpp:1103 #4 0x00007ffe56d52218 in WebCore::StyledElement::attributeChanged (this=0x2788210, name=..., oldValue=..., newValue=..., reason=WebCore::Element::ModifiedDirectly) at ../../Source/WebCore/dom/StyledElement.cpp:162 #5 0x00007ffe56cb2826 in WebCore::Element::didAddAttribute (this=0x2788210, name=..., value=...) at ../../Source/WebCore/dom/Element.cpp:2771 #6 0x00007ffe56cb27bb in WebCore::Element::addAttributeInternal (this=0x2788210, name=..., value=..., inSynchronizationOfLazyAttribute=WebCore::Element::NotInSynchronizationOfLazyAttribute) at ../../Source/WebCore/dom/Element.cpp:1842 #7 0x00007ffe56cb83fb in WebCore::Element::setAttributeInternal (this=0x2788210, index=4294967295, name=..., newValue=..., inSynchronizationOfLazyAttribute=WebCore::Element::NotInSynchronizationOfLazyAttribute) at ../../Source/WebCore/dom/Element.cpp:1032 #8 0x00007ffe56cae80a in WebCore::Element::setAttributeWithoutSynchronization (this=0x2788210, name=..., value=...) at ../../Source/WebCore/dom/Element.cpp:1014 #9 0x00007ffe57e489ff in WebCore::setJSElementClassName (exec=0x7fff008e45a0, baseObject=0x7ffe000bfdd0, thisValue=140728898809168, encodedValue=140728901566416) at DerivedSources/WebCore/JSElement.cpp:2276 #10 0x00007ffe5ac3e7c2 in JSC::callCustomSetter (exec=0x7fff008e45a0, customGetterSetter=..., base=0x7ffe000bfdd0, thisValue=..., value=...) at ../../Source/JavaScriptCore/runtime/CustomGetterSetter.cpp:44 #11 0x00007ffe5acb475e in JSC::JSObject::put (cell=0x7ffe0005ed50, exec=0x7fff008e45a0, propertyName=..., value=..., slot=...) at ../../Source/JavaScriptCore/runtime/JSObject.cpp:393 #12 0x00007ffe5a99c312 in JSC::JSValue::put (this=0x7fff008e4488, exec=0x7fff008e45a0, propertyName=..., value=..., slot=...) at ../../Source/JavaScriptCore/runtime/JSCJSValueInlines.h:729 #13 0x00007ffe5adbce45 in llint_slow_path_put_by_id (exec=0x7fff008e45a0, pc=0x3515a40) at ../../Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:637 #14 0x00007ffe5adc88a9 in llint_entry () from /home/zan/dvt/webkit/hackhack/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0 #15 0x00007ffe5adcbf2b in llint_entry () from /home/zan/dvt/webkit/hackhack/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0 #16 0x00007ffe5adc5706 in callToJavaScript () from /home/zan/dvt/webkit/hackhack/WebKitBuild/Debug/lib/libjavascriptcoregtk-3.0.so.0 HTMLCollection::invalidateCache() then does nothing since the collection won't be invalidated for any change to a class attribute (like it would be in case of a changed href attribute). The Document::invalidateNodeListAndCollectionCaches() has already zeroed out the m_collectionsInvalidatedAtDocument HashSet so the assertion in Document::unregisterCollection() fails when it gets called during the HTMLCollection's destruction. This wasn't a problem before the HashTable changes since the entries were simply copied, and the assertion wouldn't be hit.
I think that Document::invalidateNodeListAndCollectionCaches() should be considered as working as intended. I'd recommend relaxing the assertion in Document::unregisterCollection() instead. It should only check that the HashSet object is empty if the unregister action is coming from invalidateNodeListAndCollectionCaches(). Something like this: ASSERT(m_inInvalidateNodeListAndCollectionCaches && !m_listsInvalidatedAtDocument.isEmpty() || !m_inInvalidateNodeListAndCollectionCaches); Opinions?
Also: http://alistapart.com/blog/post/ten-css-one-liners-to-replace-native-apps/ <rdar://problem/17778184>
I'll try relaxing the assertion.
(In reply to comment #3) > Opinions? Concept sounds fine. I didn’t read your new assertion carefully to be sure I understood the logic.
Created attachment 235932 [details] Patch
Attachment 235932 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 235933 [details] Patch
Comment on attachment 235933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235933&action=review I’m going to say review+, but I think removing the assertion entirely would be better. > Source/WebCore/dom/Document.cpp:3527 > - ASSERT(m_inInvalidateNodeListAndCollectionCaches > - ? m_collectionsInvalidatedAtDocument.isEmpty() > - : m_collectionsInvalidatedAtDocument.contains(&collection)); > + // This only ensures that Document::invalidateNodeListAndCollectionCaches() empties out > + // the m_collectionsInvalidatedAtDocument HashSet but shouldn't assert that the collection > + // is contianed in the HashSet otherwise. See https://webkit.org/b/135168. > + ASSERT(m_inInvalidateNodeListAndCollectionCaches && m_collectionsInvalidatedAtDocument.isEmpty() > + || !m_inInvalidateNodeListAndCollectionCaches); I don’t think we need an assertion at all. This asserts so little that it’s pretty much useless. Also, lets not land the misspelling contianed.
Created attachment 236017 [details] Patch This patch removes the assertion altogether.
Comment on attachment 236017 [details] Patch Clearing flags on attachment: 236017 Committed r172210: <http://trac.webkit.org/changeset/172210>
All reviewed patches have been landed. Closing bug.
Mass moving XML DOM bugs to the "DOM" Component.