Bug 87746 - [Chromium] Remove assertions on state in Prerender.cpp
Summary: [Chromium] Remove assertions on state in Prerender.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Peters
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-29 07:53 PDT by Gavin Peters
Modified: 2012-05-31 10:34 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.22 KB, patch)
2012-05-29 07:58 PDT, Gavin Peters
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Peters 2012-05-29 07:53:47 PDT
Remove aassertions on state in Prerender.cpp
Comment 1 Gavin Peters 2012-05-29 07:58:00 PDT
Created attachment 144559 [details]
Patch
Comment 2 Gavin Peters 2012-05-29 08:01:37 PDT
Comment on attachment 144559 [details]
Patch

abarth, WDYT?

I'm most interested in knowing if I was really overzealous with my assertions like I thought: the alternative is that something freaky is happen by calling HTMLElement::removedFromDocument() after the DOM is stopped.
Comment 3 Adam Barth 2012-05-29 15:59:02 PDT
Comment on attachment 144559 [details]
Patch

I talked with Gavin directly abou this patch.  Here's what I said:

[[[
Yes, that's entirely possible.
The way this can happen is you take a reference to a DOM node in an
iframe and then either navigate to iframe to a new URL or remove the
iframe from the DOM entirely.  Now, stop() will be called on the DOM
node, but it will still be in the DOM tree.  You can then remove it
from the DOM tree using removeChild.
]]]

He's going to write a test for this in the Chromium repo where it can be an end-to-end test.  Ideally, we'd have a LayoutTest too, but Gavin tells me that there's not enough of the prerendering machinery in DumpRenderTree for that to be a meaningful test.

Gavin, once you add the test to Chromium, can you include a link in this bug?
Comment 4 WebKit Review Bot 2012-05-29 16:08:06 PDT
Comment on attachment 144559 [details]
Patch

Clearing flags on attachment: 144559

Committed r118848: <http://trac.webkit.org/changeset/118848>
Comment 5 WebKit Review Bot 2012-05-29 16:08:11 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Gavin Peters 2012-05-31 10:34:41 PDT
I opted to add enough machinery to DumpRenderTree to let this be tested; see https://bugs.webkit.org/show_bug.cgi?id=87860 for the new Mocks and the layout test for this issue.