Bug 50312 - Assertion failure in WebCore::HTMLFrameElementBase::insertedIntoDocument()
Summary: Assertion failure in WebCore::HTMLFrameElementBase::insertedIntoDocument()
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.6
: P2 Normal
Assignee: Eric Seidel (no email)
URL: http://sports.yahoo.com/
: 54711 71177 75390 (view as bug list)
Depends on:
Blocks: 45049
  Show dependency treegraph
Reported: 2010-11-30 22:56 PST by Jeff Johnson
Modified: 2012-01-19 16:45 PST (History)
8 users (show)

See Also:

Crash log (44.07 KB, text/plain)
2010-11-30 22:56 PST, Jeff Johnson
no flags Details
Patch (3.91 KB, patch)
2012-01-05 14:44 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch (4.40 KB, patch)
2012-01-19 11:30 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff Johnson 2010-11-30 22:56:48 PST
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:
(/Users/Shared/source/WebKit/WebCore/html/HTMLFrameElementBase.cpp:188 virtual void WebCore::HTMLFrameElementBase::insertedIntoDocument())
Comment 1 Eric Seidel (no email) 2012-01-03 13:37:22 PST
*** Bug 75390 has been marked as a duplicate of this bug. ***
Comment 2 Eric Seidel (no email) 2012-01-03 13:38:23 PST
This does not reproduce for me in a Debug build, but it may be ad-related.
Comment 3 Eric Seidel (no email) 2012-01-03 13:39:18 PST
*** Bug 71177 has been marked as a duplicate of this bug. ***
Comment 4 Eric Seidel (no email) 2012-01-03 13:41:16 PST
*** Bug 54711 has been marked as a duplicate of this bug. ***
Comment 5 Eric Seidel (no email) 2012-01-03 13:42:05 PST
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. :)
Comment 6 Eric Seidel (no email) 2012-01-03 13:57:20 PST
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. :)
Comment 7 Eric Seidel (no email) 2012-01-03 13:58:32 PST
The ASSERT is from http://trac.webkit.org/changeset/67182 (as mentioned in several dups).
Comment 8 Eric Seidel (no email) 2012-01-03 14:00:25 PST
iframe loading being done in the renderer is also related to plugin (object/embed) loading being done in the renderer.  See bug 45049.
Comment 9 Eric Seidel (no email) 2012-01-03 14:01:26 PST
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.
Comment 10 Eric Seidel (no email) 2012-01-03 14:14:02 PST
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
Comment 11 Eric Seidel (no email) 2012-01-03 14:23:09 PST
It's possible that we're creating a renderer, then trying to move the frame.

void HTMLFrameElementBase::willRemove()
    if (m_remainsAliveOnRemovalFromTree)


can block the zeroing of renderer(), but then 

void HTMLFrameElementBase::insertedIntoDocument()

    if (m_remainsAliveOnRemovalFromTree) {

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)
Comment 12 Dmitry Titov 2012-01-03 15:33:40 PST
(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...
Comment 13 Dmitry Titov 2012-01-03 16:29:38 PST
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*)
Comment 14 Dmitry Titov 2012-01-03 17:04:52 PST
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.
Comment 15 Eric Seidel (no email) 2012-01-05 11:23:07 PST
Thank you.  I have a reduction now:

var subtree = document.createElement('div');
var styleElement = document.createElement('style');
styleElement.textContent = "iframe { border: 3px solid blue; }";
Comment 16 Eric Seidel (no email) 2012-01-05 14:44:00 PST
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())

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.
Comment 17 Eric Seidel (no email) 2012-01-05 14:44:56 PST
Created attachment 121333 [details]
Comment 18 Eric Seidel (no email) 2012-01-05 14:45:34 PST
We'll see if the EWS bots have any better luck running the tests than I did.
Comment 19 WebKit Review Bot 2012-01-06 02:05:36 PST
Comment on attachment 121333 [details]

Attachment 121333 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11147337

New failing tests:
Comment 20 Eric Seidel (no email) 2012-01-16 01:16:12 PST
Comment on attachment 121333 [details]

Sorry, I should have cleared the review.  I don't think this can be landed as-is.  I need to finish investigating.
Comment 21 Eric Seidel (no email) 2012-01-19 11:30:47 PST
Created attachment 123156 [details]
Comment 22 WebKit Review Bot 2012-01-19 16:44:57 PST
Comment on attachment 123156 [details]

Clearing flags on attachment: 123156

Committed r105463: <http://trac.webkit.org/changeset/105463>
Comment 23 WebKit Review Bot 2012-01-19 16:45:03 PST
All reviewed patches have been landed.  Closing bug.