Bad interaction between document destruction and unload events

https://bugs.webkit.org/show_bug.cgi?id=64741

Patch by Scott Graham <scottmg@chromium.org> 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.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@92439 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 329523a6
2011-08-04 Scott Graham <scottmg@chromium.org>
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 <tkent@chromium.org>
Move <input type=week> tests to fast/forms/week/
For the test to pass there should be no crash.
<html>
<body>
<script>
if (window.layoutTestController) {
layoutTestController.dumpAsText();
layoutTestController.waitUntilDone();
}
</script>
<iframe src="resources/document-destruction-within-unload-iframe.html">
</iframe>
<p>For the test to pass there should be no crash.</p>
<script>
function done() {
if (window.layoutTestController)
layoutTestController.notifyDone();
}
</script>
</body>
</html>
<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="does_not_exist"></object>
<object data="document-destruction-within-unload.svg" id="root" onload="runTest();"></object>
</body>
</html>
<svg xmlns="http://www.w3.org/2000/svg">
</svg>
2011-08-04 Scott Graham <scottmg@chromium.org>
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 <simon.fraser@apple.com>
Add code to determine whether a Range in inside fixed position content
......@@ -1995,6 +1995,10 @@ void Document::implicitOpen()
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();
......
......@@ -1834,9 +1834,13 @@ void FrameLoader::transitionToCommitted(PassRefPtr<CachedPage> cachedPage)
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::frameLoadCompleted()
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)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment