Created attachment 75255 [details] Crash log Steps to Reproduce: 1) Build WebKit svn r73008 Debug configuration from source 2) run-safari 3) Load http://sports.yahoo.com/ Build Date & Platform: Mac OS X 10.6.5, Safari 5.0.3, WebKit svn rr73008 Additional Information: ASSERTION FAILED: !renderer() (/Users/Shared/source/WebKit/WebCore/html/HTMLFrameElementBase.cpp:188 virtual void WebCore::HTMLFrameElementBase::insertedIntoDocument())
*** Bug 75390 has been marked as a duplicate of this bug. ***
This does not reproduce for me in a Debug build, but it may be ad-related.
*** Bug 71177 has been marked as a duplicate of this bug. ***
*** Bug 54711 has been marked as a duplicate of this bug. ***
Given the 4 reports, I'm sure folks are hitting this in a debug build. But w/o a consistent repro, it's very difficult to move forward on. Fixing this bug will help us better understand the insanity of our current plugin creation code. :)
I mailed webkit-dev asking for reproductions. Unfortunately, I typo'd. It's not plugins, but rather frames. Still likely caused by ads of course. :) I have plugins on the brain from working on HTMLEmbedElement I guess. :)
The ASSERT is from http://trac.webkit.org/changeset/67182 (as mentioned in several dups).
iframe loading being done in the renderer is also related to plugin (object/embed) loading being done in the renderer. See bug 45049.
The question with this ASSERT is to figure out why we're ending up with a renderer() between when the DOM object creation happens and when insertedIntoDocument is called.
We may be hitting this ASSERT due to bad interactions with the magic-iframe code. There was a security bug which added some of this complexity: https://bugs.webkit.org/show_bug.cgi?id=68267
It's possible that we're creating a renderer, then trying to move the frame. void HTMLFrameElementBase::willRemove() { if (m_remainsAliveOnRemovalFromTree) return; HTMLFrameOwnerElement::willRemove(); } can block the zeroing of renderer(), but then void HTMLFrameElementBase::insertedIntoDocument() { HTMLFrameOwnerElement::insertedIntoDocument(); if (m_remainsAliveOnRemovalFromTree) { updateOnReparenting(); setRemainsAliveOnRemovalFromTree(false); return; } should early-return with a different path (before the ASSERT). It's possible that m_remainsAliveOnRemovalFromTree is somehow changing between when the remove happens and when it's added back... But I don't see how that would work (since m_remainsAliveOnRemovalFromTree is set by setRemainsAliveOnRemovalFromTree which is only called during Document::adoptNode)
(In reply to comment #11) I seem to have it reproducible, at least occasionally, with a url from bug 71177 in debug build of Chrome. Will try to debug...
doesn't seem related to magic iframe. However, I'm not sure what to deduct from what I see, maybe this will help Eric: it appears that iframe element gets a renderer before it gets insertedIntoDocument() call, then the whole fragment gets inserted into a document... Here is the callstack where IFrame gets the renderer: 1 0x1a57597c WebCore::HTMLIFrameElement::createRenderer(WebCore::RenderArena*, WebCore::RenderStyle*) 2 0x1a4aa8fa WebCore::NodeRendererFactory::createRenderer() 3 0x1a4aace6 WebCore::NodeRendererFactory::createRendererIfNeeded() 4 0x1a46450d WebCore::Node::createRendererIfNeeded() 5 0x1a4181d0 WebCore::Element::attach() 6 0x1a56f860 WebCore::HTMLFrameElementBase::attach() 7 0x1a35df87 WebCore::ContainerNode::attach() 8 0x1a41820e WebCore::Element::attach() 9 0x1a35df87 WebCore::ContainerNode::attach() 10 0x1a41820e WebCore::Element::attach() 11 0x1a35df87 WebCore::ContainerNode::attach() 12 0x1a41820e WebCore::Element::attach() 13 0x1a35df87 WebCore::ContainerNode::attach() 14 0x1a41820e WebCore::Element::attach() 15 0x1a35df87 WebCore::ContainerNode::attach() 16 0x1a41820e WebCore::Element::attach() 17 0x1a35df87 WebCore::ContainerNode::attach() 18 0x1a41820e WebCore::Element::attach() 19 0x1a35df87 WebCore::ContainerNode::attach() 20 0x1a41820e WebCore::Element::attach() 21 0x1a35df87 WebCore::ContainerNode::attach() 22 0x1a41820e WebCore::Element::attach() 23 0x1a420d84 WebCore::Node::reattach() 24 0x1a418c2d WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 25 0x1a419313 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 26 0x1a419313 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 27 0x1a419313 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 28 0x1a419313 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 29 0x1a419313 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 30 0x1a419313 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) 31 0x1a38a5fc WebCore::Document::recalcStyle(WebCore::Node::StyleChange) 32 0x1a38be89 WebCore::Document::styleSelectorChanged(WebCore::StyleSelectorUpdateFlag) 33 0x1a391f17 WebCore::Document::removePendingSheet() 34 0x1a4fa71e WebCore::StyleElement::sheetLoaded(WebCore::Document*) 35 0x1a5e68f6 WebCore::HTMLStyleElement::sheetLoaded() 36 0x1ad7e6a4 WebCore::CSSStyleSheet::checkLoaded() 37 0x1a4fa4b0 WebCore::StyleElement::createSheet(WebCore::Element*, int, WTF::String const&) 38 0x1a4f99bf WebCore::StyleElement::process(WebCore::Element*) 39 0x1a4f96f3 WebCore::StyleElement::insertedIntoDocument(WebCore::Document*, WebCore::Element*) 40 0x1a5e631c WebCore::HTMLStyleElement::insertedIntoDocument() 41 0x1a35e15d WebCore::ContainerNode::insertedIntoDocument() 42 0x1a417bd1 WebCore::Element::insertedIntoDocument() 43 0x1a35e15d WebCore::ContainerNode::insertedIntoDocument() 44 0x1a417bd1 WebCore::Element::insertedIntoDocument() 45 0x1a35e15d WebCore::ContainerNode::insertedIntoDocument() 46 0x1a417bd1 WebCore::Element::insertedIntoDocument() 47 0x1a35bba2 WebCore::notifyChildInserted(WebCore::Node*) 48 0x1a35b1b1 WebCore::ContainerNode::appendChild(WTF::PassRefPtr<WebCore::Node>, int&, bool) 49 0x1a460666 WebCore::Node::appendChild(WTF::PassRefPtr<WebCore::Node>, int&, bool) 50 0x1ab8fa44 WebCore::V8Node::appendChildCallback(v8::Arguments const&) 51 0x19407466 v8::internal::MaybeObject* v8::internal::HandleApiCallHelper<false>(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) 52 0x19406f2a v8::internal::Builtin_Impl_HandleApiCall(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) 53 0x193fcd5d v8::internal::Builtin_HandleApiCall(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*)
One more bit of data: the fragment of DOM is first assembled from script, then attached to the actual document. This triggers a tree walk with insertedIntoDocument() on each element. Before the tree walk reaches the iframe, it reaches the Style element. Since the whole subtree is now InDocument(), that triggers sync style recalc on a subtree, which includes the iframe. The iframe gets a renderer. Then, during the same tree walk, the iframe gets insertedIntoDocument() and fires that ASSERT.
Thank you. I have a reduction now: <body> <script> var subtree = document.createElement('div'); var styleElement = document.createElement('style'); styleElement.textContent = "iframe { border: 3px solid blue; }"; subtree.appendChild(styleElement); subtree.appendChild(document.createElement('iframe')); document.body.appendChild(subtree); </script>
I originally wrote the following: // Loads may cause synchronous javascript execution (e.g. beforeload or // src=javascript), which could try to access the renderer before the normal // parser machinery would call lazyAttach() and set us as needing style // resolve. Any code which expects this to be attached will resolve style // before using renderer(), so this will make sure we attach in time. if (!renderer()) { // This recalc is unecessary if we already have a renderer, as can be the case // if an earlier element's insertedIntoDocument() already forced a style recalc. // FIXME: Normally lazyAttach marks the renderer as attached(), but we don't // want to do that here, as as callers expect to call attach() right after // this and attach() will ASSERT(!attached()) lazyAttach(DoNotSetAttached); } But I've since modified the code to just remove this lazyAttach. As far as I can tell, it's unnecessary. Unfortunately the tests are not behaving well on my machine, so I can't be 100% sure.
Created attachment 121333 [details] Patch
We'll see if the EWS bots have any better luck running the tests than I did.
Comment on attachment 121333 [details] Patch Attachment 121333 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11147337 New failing tests: fast/frames/frame-js-url-clientWidth.html fast/frames/frameElement-widthheight.html fast/frames/iframe-js-url-clientWidth.html fast/forms/basic-textareas.html
Comment on attachment 121333 [details] Patch Sorry, I should have cleared the review. I don't think this can be landed as-is. I need to finish investigating.
Created attachment 123156 [details] Patch
Comment on attachment 123156 [details] Patch Clearing flags on attachment: 123156 Committed r105463: <http://trac.webkit.org/changeset/105463>
All reviewed patches have been landed. Closing bug.