- Revision
- 214365
- Author
- [email protected]
- Date
- 2017-03-24 12:34:11 -0700 (Fri, 24 Mar 2017)
Log Message
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: trunk/LayoutTests/ChangeLog (214364 => 214365)
--- trunk/LayoutTests/ChangeLog 2017-03-24 19:01:54 UTC (rev 214364)
+++ trunk/LayoutTests/ChangeLog 2017-03-24 19:34:11 UTC (rev 214365)
@@ -1,3 +1,17 @@
+2017-03-24 Daniel Bates <[email protected]>
+
+ 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 Myles C. Maxfield <[email protected]>
Implement font-optical-sizing
Added: trunk/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail-expected.txt (0 => 214365)
--- trunk/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail-expected.txt 2017-03-24 19:34:11 UTC (rev 214365)
@@ -0,0 +1 @@
+PASS did not crash.
Added: trunk/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html (0 => 214365)
--- trunk/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html (rev 0)
+++ trunk/LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html 2017-03-24 19:34:11 UTC (rev 214365)
@@ -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: trunk/Source/WebCore/ChangeLog (214364 => 214365)
--- trunk/Source/WebCore/ChangeLog 2017-03-24 19:01:54 UTC (rev 214364)
+++ trunk/Source/WebCore/ChangeLog 2017-03-24 19:34:11 UTC (rev 214365)
@@ -1,3 +1,40 @@
+2017-03-24 Daniel Bates <[email protected]>
+
+ 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 Myles C. Maxfield <[email protected]>
Implement font-optical-sizing
Modified: trunk/Source/WebCore/dom/Document.cpp (214364 => 214365)
--- trunk/Source/WebCore/dom/Document.cpp 2017-03-24 19:01:54 UTC (rev 214364)
+++ trunk/Source/WebCore/dom/Document.cpp 2017-03-24 19:34:11 UTC (rev 214365)
@@ -2224,8 +2224,12 @@
cache->clearTextMarkerNodesInUse(this);
}
#endif
-
- disconnectDescendantFrames();
+
+ {
+ NavigationDisabler navigationDisabler;
+ disconnectDescendantFrames();
+ }
+
if (m_domWindow && m_frame)
m_domWindow->willDetachDocumentFromFrame();
@@ -2281,6 +2285,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: trunk/Source/WebCore/loader/FrameLoader.cpp (214364 => 214365)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2017-03-24 19:01:54 UTC (rev 214364)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2017-03-24 19:34:11 UTC (rev 214365)
@@ -1212,7 +1212,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)
@@ -2990,7 +2990,7 @@
bool shouldClose = false;
{
- NavigationDisablerForBeforeUnload navigationDisabler;
+ NavigationDisabler navigationDisabler;
size_t i;
for (i = 0; i < targetFrames.size(); i++) {
Modified: trunk/Source/WebCore/loader/NavigationScheduler.cpp (214364 => 214365)
--- trunk/Source/WebCore/loader/NavigationScheduler.cpp 2017-03-24 19:01:54 UTC (rev 214364)
+++ trunk/Source/WebCore/loader/NavigationScheduler.cpp 2017-03-24 19:34:11 UTC (rev 214365)
@@ -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: trunk/Source/WebCore/loader/NavigationScheduler.h (214364 => 214365)
--- trunk/Source/WebCore/loader/NavigationScheduler.h 2017-03-24 19:01:54 UTC (rev 214364)
+++ trunk/Source/WebCore/loader/NavigationScheduler.h 2017-03-24 19:34:11 UTC (rev 214365)
@@ -43,13 +43,13 @@
class SecurityOrigin;
class URL;
-class NavigationDisablerForBeforeUnload {
+class NavigationDisabler {
public:
- NavigationDisablerForBeforeUnload()
+ NavigationDisabler()
{
s_navigationDisableCount++;
}
- ~NavigationDisablerForBeforeUnload()
+ ~NavigationDisabler()
{
ASSERT(s_navigationDisableCount);
s_navigationDisableCount--;
