- Revision
- 214798
- Author
- carlo...@webkit.org
- Date
- 2017-04-03 09:10:45 -0700 (Mon, 03 Apr 2017)
Log Message
Merge r214365 - Prevent new navigations during document unload
https://bugs.webkit.org/show_bug.cgi?id=169934
<rdar://problem/31247584>
Reviewed by Chris Dumez.
Source/WebCore:
Similar to our policy of preventing new navigations from onbeforeunload handlers
we should prevent new navigations that are initiated during the document unload
process.
The significant part of this change is the instantiation of the RAII object NavigationDisabler
in Document::prepareForDestruction(). The rest of this change just renames class
NavigationDisablerForBeforeUnload to NavigationDisabler now that this RAII class is
used to prevent navigation from both onbeforeunload event handlers and when unloading
a document.
Test: fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html
* dom/Document.cpp:
(WebCore::Document::prepareForDestruction): Disable new navigations when disconnecting
subframes. Also assert that the document is not in the page cache before we fall off
the end of the function.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::isNavigationAllowed): Update for renaming below.
(WebCore::FrameLoader::shouldClose): Ditto.
* loader/NavigationScheduler.cpp:
(WebCore::NavigationScheduler::shouldScheduleNavigation): Ditto.
* loader/NavigationScheduler.h:
(WebCore::NavigationDisabler::NavigationDisabler): Renamed class; formerly named NavigationDisablerForBeforeUnload.
(WebCore::NavigationDisabler::~NavigationDisabler): Ditto.
(WebCore::NavigationDisabler::isNavigationAllowed): Ditto.
(WebCore::NavigationDisablerForBeforeUnload::NavigationDisablerForBeforeUnload): Deleted.
(WebCore::NavigationDisablerForBeforeUnload::~NavigationDisablerForBeforeUnload): Deleted.
(WebCore::NavigationDisablerForBeforeUnload::isNavigationAllowed): Deleted.
LayoutTests:
Add a test to ensure that we do not cause an assertion fail when calling setTimeout
after starting a navigation from an onunload event handler.
* fast/frames/frame-unload-navigate-and-setTimeout-assert-fail-expected.txt: Added.
* fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html: Added.
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog (214797 => 214798)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-04-03 16:00:04 UTC (rev 214797)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-04-03 16:10:45 UTC (rev 214798)
@@ -1,3 +1,17 @@
+2017-03-24 Daniel Bates <daba...@apple.com>
+
+ Prevent new navigations during document unload
+ https://bugs.webkit.org/show_bug.cgi?id=169934
+ <rdar://problem/31247584>
+
+ Reviewed by Chris Dumez.
+
+ Add a test to ensure that we do not cause an assertion fail when calling setTimeout
+ after starting a navigation from an onunload event handler.
+
+ * fast/frames/frame-unload-navigate-and-setTimeout-assert-fail-expected.txt: Added.
+ * fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html: Added.
+
2017-03-24 Per Arne Vollan <pvol...@apple.com>
Text stroke is sometimes clipped on video captions.
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail-expected.txt (0 => 214798)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail-expected.txt 2017-04-03 16:10:45 UTC (rev 214798)
@@ -0,0 +1 @@
+PASS did not crash.
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html (0 => 214798)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html 2017-04-03 16:10:45 UTC (rev 214798)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+ testRunner.waitUntilDone();
+}
+</script>
+<iframe id="iframe"></iframe>
+<script>
+var frame = document.getElementById("iframe");
+frame.contentWindow._onunload_ = () => {
+ var link = document.createElement('a');
+ link.href = ""
+ link.click();
+
+ window.setTimeout(() => {
+ // Do nothing.
+ }, 0);
+};
+window.location = '_javascript_:"PASS did not crash.<script>window.testRunner && window.testRunner.notifyDone();</' + 'script>"';
+</script>
+</body>
+</html>
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog (214797 => 214798)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-04-03 16:00:04 UTC (rev 214797)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-04-03 16:10:45 UTC (rev 214798)
@@ -1,3 +1,40 @@
+2017-03-24 Daniel Bates <daba...@apple.com>
+
+ Prevent new navigations during document unload
+ https://bugs.webkit.org/show_bug.cgi?id=169934
+ <rdar://problem/31247584>
+
+ Reviewed by Chris Dumez.
+
+ Similar to our policy of preventing new navigations from onbeforeunload handlers
+ we should prevent new navigations that are initiated during the document unload
+ process.
+
+ The significant part of this change is the instantiation of the RAII object NavigationDisabler
+ in Document::prepareForDestruction(). The rest of this change just renames class
+ NavigationDisablerForBeforeUnload to NavigationDisabler now that this RAII class is
+ used to prevent navigation from both onbeforeunload event handlers and when unloading
+ a document.
+
+ Test: fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::prepareForDestruction): Disable new navigations when disconnecting
+ subframes. Also assert that the document is not in the page cache before we fall off
+ the end of the function.
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::isNavigationAllowed): Update for renaming below.
+ (WebCore::FrameLoader::shouldClose): Ditto.
+ * loader/NavigationScheduler.cpp:
+ (WebCore::NavigationScheduler::shouldScheduleNavigation): Ditto.
+ * loader/NavigationScheduler.h:
+ (WebCore::NavigationDisabler::NavigationDisabler): Renamed class; formerly named NavigationDisablerForBeforeUnload.
+ (WebCore::NavigationDisabler::~NavigationDisabler): Ditto.
+ (WebCore::NavigationDisabler::isNavigationAllowed): Ditto.
+ (WebCore::NavigationDisablerForBeforeUnload::NavigationDisablerForBeforeUnload): Deleted.
+ (WebCore::NavigationDisablerForBeforeUnload::~NavigationDisablerForBeforeUnload): Deleted.
+ (WebCore::NavigationDisablerForBeforeUnload::isNavigationAllowed): Deleted.
+
2017-03-24 Per Arne Vollan <pvol...@apple.com>
Text stroke is sometimes clipped on video captions.
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.cpp (214797 => 214798)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.cpp 2017-04-03 16:00:04 UTC (rev 214797)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.cpp 2017-04-03 16:10:45 UTC (rev 214798)
@@ -2272,8 +2272,12 @@
cache->clearTextMarkerNodesInUse(this);
}
#endif
-
- disconnectDescendantFrames();
+
+ {
+ NavigationDisabler navigationDisabler;
+ disconnectDescendantFrames();
+ }
+
if (m_domWindow && m_frame)
m_domWindow->willDetachDocumentFromFrame();
@@ -2327,6 +2331,11 @@
detachFromFrame();
m_hasPreparedForDestruction = true;
+
+ // Note that m_pageCacheState can be Document::AboutToEnterPageCache if our frame
+ // was removed in an onpagehide event handler fired when the top-level frame is
+ // about to enter the page cache.
+ ASSERT_WITH_SECURITY_IMPLICATION(m_pageCacheState != Document::InPageCache);
}
void Document::removeAllEventListeners()
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp (214797 => 214798)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp 2017-04-03 16:00:04 UTC (rev 214797)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp 2017-04-03 16:10:45 UTC (rev 214798)
@@ -1187,7 +1187,7 @@
bool FrameLoader::isNavigationAllowed() const
{
- return m_pageDismissalEventBeingDispatched == PageDismissalType::None && NavigationDisablerForBeforeUnload::isNavigationAllowed();
+ return m_pageDismissalEventBeingDispatched == PageDismissalType::None && NavigationDisabler::isNavigationAllowed();
}
void FrameLoader::loadURL(const FrameLoadRequest& frameLoadRequest, const String& referrer, FrameLoadType newLoadType, Event* event, FormState* formState)
@@ -2908,7 +2908,7 @@
bool shouldClose = false;
{
- NavigationDisablerForBeforeUnload navigationDisabler;
+ NavigationDisabler navigationDisabler;
size_t i;
for (i = 0; i < targetFrames.size(); i++) {
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/NavigationScheduler.cpp (214797 => 214798)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/NavigationScheduler.cpp 2017-04-03 16:00:04 UTC (rev 214797)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/NavigationScheduler.cpp 2017-04-03 16:10:45 UTC (rev 214798)
@@ -55,7 +55,7 @@
namespace WebCore {
-unsigned NavigationDisablerForBeforeUnload::s_navigationDisableCount = 0;
+unsigned NavigationDisabler::s_navigationDisableCount = 0;
class ScheduledNavigation {
WTF_MAKE_NONCOPYABLE(ScheduledNavigation); WTF_MAKE_FAST_ALLOCATED;
@@ -364,7 +364,7 @@
return false;
if (protocolIsJavaScript(url))
return true;
- return NavigationDisablerForBeforeUnload::isNavigationAllowed();
+ return NavigationDisabler::isNavigationAllowed();
}
void NavigationScheduler::scheduleRedirect(Document& initiatingDocument, double delay, const URL& url)
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/NavigationScheduler.h (214797 => 214798)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/NavigationScheduler.h 2017-04-03 16:00:04 UTC (rev 214797)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/NavigationScheduler.h 2017-04-03 16:10:45 UTC (rev 214798)
@@ -43,13 +43,13 @@
class SecurityOrigin;
class URL;
-class NavigationDisablerForBeforeUnload {
+class NavigationDisabler {
public:
- NavigationDisablerForBeforeUnload()
+ NavigationDisabler()
{
s_navigationDisableCount++;
}
- ~NavigationDisablerForBeforeUnload()
+ ~NavigationDisabler()
{
ASSERT(s_navigationDisableCount);
s_navigationDisableCount--;
