Title: [127347] trunk
- Revision
- 127347
- Author
- [email protected]
- Date
- 2012-08-31 18:09:02 -0700 (Fri, 31 Aug 2012)
Log Message
Source/WebCore: fast/loader/document-destruction-within-unload.html causes assertion failures on mac and qt.
https://bugs.webkit.org/show_bug.cgi?id=66783
Reviewed by Darin Adler.
This was a not-quite-regression from trac.webkit.org/changeset/93521, in that we hit asserts
in a case where we previously were use-after-freeing. Tweak how we handle cases where a Document
is cleared from within an unload handler.
No new tests, fixing fast/loader/document-destruction-within-unload.html on mac and qt.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::setDocumentLoader): Instead of trying to reattach a partially removed
DocumentLoader if it is detached before being fully added, leave the old one in place, completed.
(WebCore::FrameLoader::transitionToCommitted):
LayoutTests: Unskip fast/loader/document-destruction-within-unload.html
on mac and qt.
https://bugs.webkit.org/show_bug.cgi?id=66783
Reviewed by Darin Adler.
* platform/mac/Skipped:
* platform/qt/Skipped:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (127346 => 127347)
--- trunk/LayoutTests/ChangeLog 2012-09-01 01:00:10 UTC (rev 127346)
+++ trunk/LayoutTests/ChangeLog 2012-09-01 01:09:02 UTC (rev 127347)
@@ -1,3 +1,14 @@
+2012-08-31 Nate Chapin <[email protected]>
+
+ Unskip fast/loader/document-destruction-within-unload.html
+ on mac and qt.
+ https://bugs.webkit.org/show_bug.cgi?id=66783
+
+ Reviewed by Darin Adler.
+
+ * platform/mac/Skipped:
+ * platform/qt/Skipped:
+
2012-08-31 Roger Fong <[email protected]>
Unreviewed. CSS_VARIABLES are not enabled on Windows. Adding Windows specific results.
Modified: trunk/LayoutTests/platform/mac/Skipped (127346 => 127347)
--- trunk/LayoutTests/platform/mac/Skipped 2012-09-01 01:00:10 UTC (rev 127346)
+++ trunk/LayoutTests/platform/mac/Skipped 2012-09-01 01:09:02 UTC (rev 127347)
@@ -517,10 +517,6 @@
# This test verifies that a mismatch reftest will fail as intended if both results are same.
fast/harness/sample-fail-mismatch-reftest.html
-# https://bugs.webkit.org/show_bug.cgi?id=66783
-# Makes subsequent test crash
-fast/loader/document-destruction-within-unload.html
-
# https://bugs.webkit.org/show_bug.cgi?id=67716
media/media-controls-invalid-url.html
Modified: trunk/LayoutTests/platform/qt/Skipped (127346 => 127347)
--- trunk/LayoutTests/platform/qt/Skipped 2012-09-01 01:00:10 UTC (rev 127346)
+++ trunk/LayoutTests/platform/qt/Skipped 2012-09-01 01:09:02 UTC (rev 127347)
@@ -2782,10 +2782,6 @@
# https://bugs.webkit.org/show_bug.cgi?id=95432
svg/custom/clamped-masking-clipping.svg
-# [Qt] REGRESSION(r122175): fast/loader/document-destruction-within-unload.html makes the following test assert
-# https://bugs.webkit.org/show_bug.cgi?id=95441
-fast/loader/document-destruction-within-unload.html
-
# REGRESSION (r127202): http/tests/security/inactive-document-with-empty-security-origin.html failing on JSC ports
# https://bugs.webkit.org/show_bug.cgi?id=95530
http/tests/security/inactive-document-with-empty-security-origin.html
Modified: trunk/Source/WebCore/ChangeLog (127346 => 127347)
--- trunk/Source/WebCore/ChangeLog 2012-09-01 01:00:10 UTC (rev 127346)
+++ trunk/Source/WebCore/ChangeLog 2012-09-01 01:09:02 UTC (rev 127347)
@@ -1,3 +1,21 @@
+2012-08-31 Nate Chapin <[email protected]>
+
+ fast/loader/document-destruction-within-unload.html causes assertion failures on mac and qt.
+ https://bugs.webkit.org/show_bug.cgi?id=66783
+
+ Reviewed by Darin Adler.
+
+ This was a not-quite-regression from trac.webkit.org/changeset/93521, in that we hit asserts
+ in a case where we previously were use-after-freeing. Tweak how we handle cases where a Document
+ is cleared from within an unload handler.
+
+ No new tests, fixing fast/loader/document-destruction-within-unload.html on mac and qt.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::setDocumentLoader): Instead of trying to reattach a partially removed
+ DocumentLoader if it is detached before being fully added, leave the old one in place, completed.
+ (WebCore::FrameLoader::transitionToCommitted):
+
2012-08-31 Tony Chang <[email protected]>
Make computeBlockDirectionMargins const
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (127346 => 127347)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2012-09-01 01:00:10 UTC (rev 127346)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2012-09-01 01:09:02 UTC (rev 127347)
@@ -1614,24 +1614,21 @@
m_client->prepareForDataSourceReplacement();
detachChildren();
+
+ // detachChildren() can trigger this frame's unload event, and therefore
+ // script can run and do just about anything. For example, an unload event that calls
+ // document.write("") on its parent frame can lead to a recursive detachChildren()
+ // invocation for this frame. In that case, we can end up at this point with a
+ // loader that hasn't been deleted but has been detached from its frame. Such a
+ // DocumentLoader has been sufficiently detached that we'll end up in an inconsistent
+ // state if we try to use it.
+ if (loader && !loader->frame())
+ return;
+
if (m_documentLoader)
m_documentLoader->detachFromFrame();
m_documentLoader = loader;
-
- // The following abomination is brought to you by the unload event.
- // The detachChildren() call above may trigger a child frame's unload event,
- // which could do something obnoxious like call document.write("") on
- // the main frame, which results in detaching children while detaching children.
- // This can cause the new m_documentLoader to be detached from its Frame*, but still
- // be alive. To make matters worse, DocumentLoaders with a null Frame* aren't supposed
- // to happen when they're still alive (and many places below us on the stack think the
- // DocumentLoader is still usable). Ergo, we reattach loader to its Frame, and pretend
- // like nothing ever happened.
- if (m_documentLoader && !m_documentLoader->frame()) {
- ASSERT(!m_documentLoader->isLoading());
- m_documentLoader->setFrame(m_frame);
- }
}
void FrameLoader::setPolicyDocumentLoader(DocumentLoader* loader)
@@ -1805,6 +1802,12 @@
setDocumentLoader(m_provisionalDocumentLoader.get());
setProvisionalDocumentLoader(0);
+
+ if (pdl != m_documentLoader) {
+ ASSERT(m_state == FrameStateComplete);
+ return;
+ }
+
setState(FrameStateCommittedPage);
#if ENABLE(TOUCH_EVENTS)
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
