Changeset 214365 in webkit

Timestamp:

Mar 24, 2017 12:34:11 PM (7 years ago)

Author:

dbates@webkit.org

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail-expected.txt (added)
• LayoutTests/fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (2 diffs)
• Source/WebCore/loader/FrameLoader.cpp (modified) (2 diffs)
• Source/WebCore/loader/NavigationScheduler.cpp (modified) (2 diffs)
• Source/WebCore/loader/NavigationScheduler.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-03-24  Daniel Bates  <dabates@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  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-03-24  Daniel Bates  <dabates@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  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -2225 +2225 @@
     }
 #endif
-    
-    disconnectDescendantFrames();
+
+    {
+        NavigationDisabler navigationDisabler;
+        disconnectDescendantFrames();
+    }
+
     if (m_domWindow && m_frame)
         m_domWindow->willDetachDocumentFromFrame();
@@ -2282 +2286 @@
 
     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);
 }
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -1213 +1213 @@
 bool FrameLoader::isNavigationAllowed() const
 {
-    return m_pageDismissalEventBeingDispatched == PageDismissalType::None && NavigationDisablerForBeforeUnload::isNavigationAllowed();
+    return m_pageDismissalEventBeingDispatched == PageDismissalType::None && NavigationDisabler::isNavigationAllowed();
 }
 
@@ -2991 +2991 @@
     bool shouldClose = false;
     {
-        NavigationDisablerForBeforeUnload navigationDisabler;
+        NavigationDisabler navigationDisabler;
         size_t i;
 

trunk/Source/WebCore/loader/NavigationScheduler.cpp

@@ -56 +56 @@
 namespace WebCore {
 
-unsigned NavigationDisablerForBeforeUnload::s_navigationDisableCount = 0;
+unsigned NavigationDisabler::s_navigationDisableCount = 0;
 
 class ScheduledNavigation {
@@ -365 +365 @@
     if (protocolIsJavaScript(url))
         return true;
-    return NavigationDisablerForBeforeUnload::isNavigationAllowed();
+    return NavigationDisabler::isNavigationAllowed();
 }
 

trunk/Source/WebCore/loader/NavigationScheduler.h

@@ -44 +44 @@
 class URL;
 
-class NavigationDisablerForBeforeUnload {
+class NavigationDisabler {
 public:
-    NavigationDisablerForBeforeUnload()
+    NavigationDisabler()
     {
         s_navigationDisableCount++;
     }
-    ~NavigationDisablerForBeforeUnload()
+    ~NavigationDisabler()
     {
         ASSERT(s_navigationDisableCount);