- Revision
- 92439
- Author
- [email protected]
- Date
- 2011-08-04 18:32:24 -0700 (Thu, 04 Aug 2011)
Log Message
Bad interaction between document destruction and unload events
https://bugs.webkit.org/show_bug.cgi?id=64741
Patch by Scott Graham <[email protected]> on 2011-08-04
Reviewed by Adam Barth.
Source/WebCore:
Three different errors triggered by this test case. The case to
consider is a subdocument with an onunload on an element, that
destroys the parent document during the onunload. One fix was a
lifetime issue fixed by a protecting RefPtr, and another was an
additional cancel of event triggers. The main fix was that during the
transition to commited state, the documentLoader is being replaced by
the provisionalDocumentLoader. But, because during firing events in
the subdocument the parent is destroyed, that subevent caused the
provisionalDocumentLoader to be detached from its frame. By marking
the page as being in committed state before the parent documentLoader
is set, this is avoided.
Test: loader/document-destruction-within-unload.html
* dom/Document.cpp:
(WebCore::Document::implicitOpen):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::transitionToCommitted):
(WebCore::FrameLoader::detachChildren):
LayoutTests:
* loader/document-destruction-within-unload-expected.txt: Added.
* loader/document-destruction-within-unload.html: Added.
* loader/resources/document-destruction-within-unload-iframe.html: Added.
* loader/resources/document-destruction-within-unload.svg: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (92438 => 92439)
--- trunk/LayoutTests/ChangeLog 2011-08-05 01:07:04 UTC (rev 92438)
+++ trunk/LayoutTests/ChangeLog 2011-08-05 01:32:24 UTC (rev 92439)
@@ -1,3 +1,15 @@
+2011-08-04 Scott Graham <[email protected]>
+
+ Bad interaction between document destruction and unload events
+ https://bugs.webkit.org/show_bug.cgi?id=64741
+
+ Reviewed by Adam Barth.
+
+ * loader/document-destruction-within-unload-expected.txt: Added.
+ * loader/document-destruction-within-unload.html: Added.
+ * loader/resources/document-destruction-within-unload-iframe.html: Added.
+ * loader/resources/document-destruction-within-unload.svg: Added.
+
2011-08-04 Kent Tamura <[email protected]>
Move <input type=week> tests to fast/forms/week/
Added: trunk/LayoutTests/loader/document-destruction-within-unload-expected.txt (0 => 92439)
--- trunk/LayoutTests/loader/document-destruction-within-unload-expected.txt (rev 0)
+++ trunk/LayoutTests/loader/document-destruction-within-unload-expected.txt 2011-08-05 01:32:24 UTC (rev 92439)
@@ -0,0 +1,2 @@
+
+For the test to pass there should be no crash.
Added: trunk/LayoutTests/loader/document-destruction-within-unload.html (0 => 92439)
--- trunk/LayoutTests/loader/document-destruction-within-unload.html (rev 0)
+++ trunk/LayoutTests/loader/document-destruction-within-unload.html 2011-08-05 01:32:24 UTC (rev 92439)
@@ -0,0 +1,20 @@
+<html>
+ <body>
+ <script>
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+ </script>
+ <iframe src=""
+ </iframe>
+ <p>For the test to pass there should be no crash.</p>
+ <script>
+ function done() {
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }
+ </script>
+
+ </body>
+</html>
Property changes on: trunk/LayoutTests/loader/document-destruction-within-unload.html
___________________________________________________________________
Added: svn:executable
Added: trunk/LayoutTests/loader/resources/document-destruction-within-unload-iframe.html (0 => 92439)
--- trunk/LayoutTests/loader/resources/document-destruction-within-unload-iframe.html (rev 0)
+++ trunk/LayoutTests/loader/resources/document-destruction-within-unload-iframe.html 2011-08-05 01:32:24 UTC (rev 92439)
@@ -0,0 +1,16 @@
+<html>
+ <body _onload_="window.done()">
+ <script>
+ function runTest() {
+ var test = document.getElementById('root').contentDocument;
+ test.firstChild.setAttribute('onunload', "parent.clearUs();");
+ location.reload();
+ }
+ function clearUs() {
+ document.write();
+ }
+ </script>
+ <object data=""
+ <object data="" id="root" _onload_="runTest();"></object>
+ </body>
+</html>
Property changes on: trunk/LayoutTests/loader/resources/document-destruction-within-unload-iframe.html
___________________________________________________________________
Added: svn:executable
Added: trunk/LayoutTests/loader/resources/document-destruction-within-unload.svg (0 => 92439)
--- trunk/LayoutTests/loader/resources/document-destruction-within-unload.svg (rev 0)
+++ trunk/LayoutTests/loader/resources/document-destruction-within-unload.svg 2011-08-05 01:32:24 UTC (rev 92439)
@@ -0,0 +1,2 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+</svg>
Property changes on: trunk/LayoutTests/loader/resources/document-destruction-within-unload.svg
___________________________________________________________________
Added: svn:executable
Modified: trunk/Source/WebCore/ChangeLog (92438 => 92439)
--- trunk/Source/WebCore/ChangeLog 2011-08-05 01:07:04 UTC (rev 92438)
+++ trunk/Source/WebCore/ChangeLog 2011-08-05 01:32:24 UTC (rev 92439)
@@ -1,3 +1,30 @@
+2011-08-04 Scott Graham <[email protected]>
+
+ Bad interaction between document destruction and unload events
+ https://bugs.webkit.org/show_bug.cgi?id=64741
+
+ Reviewed by Adam Barth.
+
+ Three different errors triggered by this test case. The case to
+ consider is a subdocument with an onunload on an element, that
+ destroys the parent document during the onunload. One fix was a
+ lifetime issue fixed by a protecting RefPtr, and another was an
+ additional cancel of event triggers. The main fix was that during the
+ transition to commited state, the documentLoader is being replaced by
+ the provisionalDocumentLoader. But, because during firing events in
+ the subdocument the parent is destroyed, that subevent caused the
+ provisionalDocumentLoader to be detached from its frame. By marking
+ the page as being in committed state before the parent documentLoader
+ is set, this is avoided.
+
+ Test: loader/document-destruction-within-unload.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::implicitOpen):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::transitionToCommitted):
+ (WebCore::FrameLoader::detachChildren):
+
2011-08-04 Simon Fraser <[email protected]>
Add code to determine whether a Range in inside fixed position content
Modified: trunk/Source/WebCore/dom/Document.cpp (92438 => 92439)
--- trunk/Source/WebCore/dom/Document.cpp 2011-08-05 01:07:04 UTC (rev 92438)
+++ trunk/Source/WebCore/dom/Document.cpp 2011-08-05 01:32:24 UTC (rev 92439)
@@ -1995,6 +1995,10 @@
removeChildren();
+ // cancel again, as removeChildren can cause event triggers to be added
+ // again, which we don't want triggered on the old document.
+ cancelParsing();
+
setCompatibilityMode(NoQuirksMode);
m_parser = createParser();
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (92438 => 92439)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2011-08-05 01:07:04 UTC (rev 92438)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2011-08-05 01:32:24 UTC (rev 92439)
@@ -1834,9 +1834,13 @@
if (m_documentLoader)
m_documentLoader->stopLoadingPlugIns();
+ // State must be set before setting m_documentLoader to avoid
+ // m_provisionalDocumentLoader getting detached from the frame via a sub
+ // frame. See https://bugs.webkit.org/show_bug.cgi?id=64741 for more
+ // discussion.
+ setState(FrameStateCommittedPage);
setDocumentLoader(m_provisionalDocumentLoader.get());
setProvisionalDocumentLoader(0);
- setState(FrameStateCommittedPage);
#if ENABLE(TOUCH_EVENTS)
if (isLoadingMainFrame())
@@ -2332,12 +2336,14 @@
void FrameLoader::detachChildren()
{
+ typedef Vector<RefPtr<Frame> > FrameVector;
+ FrameVector protect;
+
// FIXME: Is it really necessary to do this in reverse order?
- Frame* previous;
- for (Frame* child = m_frame->tree()->lastChild(); child; child = previous) {
- previous = child->tree()->previousSibling();
- child->loader()->detachFromParent();
- }
+ for (Frame* child = m_frame->tree()->lastChild(); child; child = child->tree()->previousSibling())
+ protect.append(child);
+ for (FrameVector::iterator it = protect.begin(); it != protect.end(); ++it)
+ (*it)->loader()->detachFromParent();
}
void FrameLoader::closeAndRemoveChild(Frame* child)
