WebContent crash in WebCore::Page::sessionID() const + 0 (Page.cpp:1660)
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Jun 2015 23:44:29 +0000 (23:44 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Jun 2015 23:44:29 +0000 (23:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145748
<rdar://problem/21226577>

Reviewed by Brady Eidson.

Source/WebCore:

We would sometimes crash when pruning the PageCache because it was
possible for frames to still be loading while in the PageCache and
we would try to stop the load when the CachedFrame is destroyed. This
code path was not supposed to be exercised as we were not supposed to
have pages still loading inside the PageCache.

r185017 made sure we don't insert into the PageCache pages that are
still loading. However, nothing was preventing content from starting
new loads in their 'pagehide' event handlers, *after* the decision
to put the page in the PageCache was made.

This patch prevents content from starting loads from a 'pagehide'
event handler so that we can no longer have content that is loading
inside the PageCache. 'ping' image loads still go through though as
these are specially handled and use PingLoaders.

Tests: http/tests/navigation/image-load-in-pagehide-handler.html
       http/tests/navigation/subframe-pagehide-handler-starts-load.html
       http/tests/navigation/subframe-pagehide-handler-starts-load2.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::FrameLoader):
(WebCore::FrameLoader::stopLoading):
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::loadWithDocumentLoader):
(WebCore::FrameLoader::stopAllLoaders):
(WebCore::FrameLoader::handleBeforeUnloadEvent):
* loader/FrameLoader.h:
(WebCore::FrameLoader::pageDismissalEventBeingDispatched):
(WebCore::FrameLoader::PageDismissalEventType::PageDismissalEventType):
(WebCore::FrameLoader::PageDismissalEventType::operator Page::DismissalType):

Add wrapper class for m_pageDismissalEventBeingDispatched member type.
The wrapper takes care of updating the m_dismissalEventBeingDispatched
member on the Page every time the member on FrameLoader is updated. We
now cache this information on the Page so that clients can cheaply
query if a dismissal event is being dispatched in any of the Page's
frame, without having to traverse the frame tree.

* loader/ImageLoader.cpp:
(WebCore::pageIsBeingDismissed):

* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::load):

Abort the load early if we are currently dispatching a 'pagehide'
event. We don't allow new loads at such point because we've already
made the decision to add the Page to the PageCache.

* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::requestImage):

* page/Chrome.cpp:
(WebCore::Chrome::runModal): Deleted.
(WebCore::Chrome::setToolbarsVisible): Deleted.
(WebCore::Chrome::toolbarsVisible): Deleted.
(WebCore::Chrome::runJavaScriptConfirm): Deleted.
(WebCore::Chrome::runJavaScriptPrompt): Deleted.
(WebCore::Chrome::shouldInterruptJavaScript): Deleted.
* page/Chrome.h:
* page/ChromeClient.h:
* page/DOMWindow.cpp:
(WebCore::DOMWindow::canShowModalDialogNow):

Drop ChromeClient::shouldRunModalDialogDuringPageDismissal() and code
using it as it is unused and I did not think it was worth updating
this code.

* page/Page.h:
(WebCore::Page::dismissalEventBeingDispatched):
(WebCore::Page::setDismissalEventBeingDispatched):

Add a m_dismissalEventBeingDispatched member to the Page so that we can
easily query if a dismissal event is being dispatched in any of the
frames, without having to traverse the frame tree. I suspect more call
sites of FrameLoader::pageDismissalEventBeingDispatched() may actually
want this but I did not make such change in this patch. It is important
to check all the frames and not simply the current one because a frame's
pagehide event handler may trigger a load in another frame.

LayoutTests:

* http/tests/navigation/image-load-in-pagehide-handler-expected.txt: Added.
* http/tests/navigation/image-load-in-pagehide-handler.html: Added.
* http/tests/navigation/resources/image-load-in-pagehide-handler-2.html: Added.

Add layout test to make sure that ping loads in 'pagehide' handlers are
still going through after this change.

* http/tests/navigation/resources/frame-do-load.html: Added.
* http/tests/navigation/resources/frame-pagehide-starts-load-in-subframe.html: Added.
* http/tests/navigation/resources/frame-pagehide-starts-load.html: Added.
* http/tests/navigation/subframe-pagehide-handler-starts-load-expected.txt: Added.
* http/tests/navigation/subframe-pagehide-handler-starts-load.html: Added.
* http/tests/navigation/subframe-pagehide-handler-starts-load2-expected.txt: Added.
* http/tests/navigation/subframe-pagehide-handler-starts-load2.html: Added.

Add layout tests to make sure we don't crash if a frame starts an XHR load
from the 'pagehide' event handler. One of the tests covers the case where a
frame's pagehide handler starts a load in a subframe as this case is
requires a bit more handling.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@185337 268f45cc-cd09-0410-ab3c-d52691b4dbfc

22 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/navigation/image-load-in-pagehide-handler-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/navigation/image-load-in-pagehide-handler.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/resources/frame-do-load.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/resources/frame-pagehide-starts-load-in-subframe.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/resources/frame-pagehide-starts-load.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/resources/image-load-in-pagehide-handler-2.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load2-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load2.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoader.h
Source/WebCore/loader/ImageLoader.cpp
Source/WebCore/loader/cache/CachedResource.cpp
Source/WebCore/loader/cache/CachedResourceLoader.cpp
Source/WebCore/page/Chrome.cpp
Source/WebCore/page/Chrome.h
Source/WebCore/page/ChromeClient.h
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/Page.h

index a761c74..3b749c3 100644 (file)
@@ -1,3 +1,31 @@
+2015-06-08  Chris Dumez  <cdumez@apple.com>
+
+        WebContent crash in WebCore::Page::sessionID() const + 0 (Page.cpp:1660)
+        https://bugs.webkit.org/show_bug.cgi?id=145748
+        <rdar://problem/21226577>
+
+        Reviewed by Brady Eidson.
+
+        * http/tests/navigation/image-load-in-pagehide-handler-expected.txt: Added.
+        * http/tests/navigation/image-load-in-pagehide-handler.html: Added.
+        * http/tests/navigation/resources/image-load-in-pagehide-handler-2.html: Added.
+
+        Add layout test to make sure that ping loads in 'pagehide' handlers are
+        still going through after this change.
+
+        * http/tests/navigation/resources/frame-do-load.html: Added.
+        * http/tests/navigation/resources/frame-pagehide-starts-load-in-subframe.html: Added.
+        * http/tests/navigation/resources/frame-pagehide-starts-load.html: Added.
+        * http/tests/navigation/subframe-pagehide-handler-starts-load-expected.txt: Added.
+        * http/tests/navigation/subframe-pagehide-handler-starts-load.html: Added.
+        * http/tests/navigation/subframe-pagehide-handler-starts-load2-expected.txt: Added.
+        * http/tests/navigation/subframe-pagehide-handler-starts-load2.html: Added.
+
+        Add layout tests to make sure we don't crash if a frame starts an XHR load
+        from the 'pagehide' event handler. One of the tests covers the case where a
+        frame's pagehide handler starts a load in a subframe as this case is
+        requires a bit more handling.
+
 2015-06-08  Chris Fleizach  <cfleizach@apple.com>
 
         AX: improve list heuristics (presentational use versus actual lists)
diff --git a/LayoutTests/http/tests/navigation/image-load-in-pagehide-handler-expected.txt b/LayoutTests/http/tests/navigation/image-load-in-pagehide-handler-expected.txt
new file mode 100644 (file)
index 0000000..71c4b3e
--- /dev/null
@@ -0,0 +1,3 @@
+Ping sent successfully
+HTTP_REFERER: http://127.0.0.1:8000/navigation/image-load-in-pagehide-handler.html
+REQUEST_METHOD: GET
diff --git a/LayoutTests/http/tests/navigation/image-load-in-pagehide-handler.html b/LayoutTests/http/tests/navigation/image-load-in-pagehide-handler.html
new file mode 100644 (file)
index 0000000..8abe211
--- /dev/null
@@ -0,0 +1,30 @@
+<html><head>
+<title>Image load in pagehide handler</title>
+<script>
+
+var testCalled = false;
+
+function test() {
+    if (!testCalled) {
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+            testRunner.waitUntilDone();
+        }
+        testCalled = true;
+        return;
+    }
+    location.assign("resources/image-load-in-pagehide-handler-2.html");
+}
+
+function ping() {
+    var img = new Image(1, 1);
+    img.src = "resources/save-Ping.php";
+}
+
+</script>
+</head>
+<body onload="test();" onpagehide="ping();">
+<img src="resources/delete-ping.php" onload="test();" onerror="test();"></img>
+<p>Tests that ping loads in 'pagehide' handlers go through.</p>
+</body></html>
diff --git a/LayoutTests/http/tests/navigation/resources/frame-do-load.html b/LayoutTests/http/tests/navigation/resources/frame-do-load.html
new file mode 100644 (file)
index 0000000..d666cb5
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+function doLoad()
+{
+    var xhr = new XMLHttpRequest();
+    xhr.open("GET", "resources/slow-resource.pl?delay=3000", true);
+    xhr.send();
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/navigation/resources/frame-pagehide-starts-load-in-subframe.html b/LayoutTests/http/tests/navigation/resources/frame-pagehide-starts-load-in-subframe.html
new file mode 100644 (file)
index 0000000..074fa19
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<body>
+<iframe src="frame-do-load.html" id="doLoadSubframe"></iframe>
+<script>
+window.addEventListener("pagehide", function(event) {
+    // Start load in subframe.
+    var subframe = document.getElementById("doLoadSubframe");
+    subframe.contentWindow.doLoad();
+}, false);
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/navigation/resources/frame-pagehide-starts-load.html b/LayoutTests/http/tests/navigation/resources/frame-pagehide-starts-load.html
new file mode 100644 (file)
index 0000000..8302e80
--- /dev/null
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+window.addEventListener("pagehide", function(event) {
+    var xhr = new XMLHttpRequest();
+    xhr.open("GET", "resources/slow-resource.pl?delay=3000", true);
+    xhr.send();
+}, false);
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/navigation/resources/image-load-in-pagehide-handler-2.html b/LayoutTests/http/tests/navigation/resources/image-load-in-pagehide-handler-2.html
new file mode 100644 (file)
index 0000000..8fb334e
--- /dev/null
@@ -0,0 +1,3 @@
+<script>
+location.href = 'check-ping.php?test=/navigation/image-load-in-pagehide-handler.html';
+</script>
diff --git a/LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load-expected.txt b/LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load-expected.txt
new file mode 100644 (file)
index 0000000..c0389c2
--- /dev/null
@@ -0,0 +1,13 @@
+Tests that we don't crash when a load is started in a subframe on 'pagehide' handling
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+pageshow - not from cache
+pagehide - entering cache
+pageshow - from cache
+PASS Page did enter and was restored from the page cache
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load.html b/LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load.html
new file mode 100644 (file)
index 0000000..436df06
--- /dev/null
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<body onload="runTest()">
+<script src="/resources/js-test-pre.js"></script>
+<script>
+description("Tests that we don't crash when a load is started in a subframe on 'pagehide' handling");
+window.jsTestIsAsync = true;
+var totalLoaded = 0;
+
+if (window.testRunner)
+    testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+
+window.addEventListener("pageshow", function(event) {
+    debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
+
+    if (event.persisted) {
+        testPassed("Page did enter and was restored from the page cache");
+        finishJSTest();
+    }
+}, false);
+
+window.addEventListener("pagehide", function(event) {
+    debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
+    if (!event.persisted) {
+        testFailed("Page did not enter the page cache.");
+        finishJSTest();
+    }
+}, false);
+
+function runTest() {
+    totalLoaded++;
+    if (totalLoaded < 2)
+      return;
+
+    // This needs to happen in a setTimeout because a navigation inside the onload handler would
+    // not create a history entry.
+    setTimeout(function() {
+      // Force a back navigation back to this page.
+      window.location.href = "resources/page-cache-helper.html";
+    }, 4000);
+}
+
+</script>
+<iframe id="testFrame" src="resources/frame-pagehide-starts-load.html" onload="runTest()"></iframe>
+<script src="/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load2-expected.txt b/LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load2-expected.txt
new file mode 100644 (file)
index 0000000..c0389c2
--- /dev/null
@@ -0,0 +1,13 @@
+Tests that we don't crash when a load is started in a subframe on 'pagehide' handling
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+pageshow - not from cache
+pagehide - entering cache
+pageshow - from cache
+PASS Page did enter and was restored from the page cache
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load2.html b/LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load2.html
new file mode 100644 (file)
index 0000000..9fd5dc5
--- /dev/null
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<body onload="runTest()">
+<script src="/resources/js-test-pre.js"></script>
+<script>
+description("Tests that we don't crash when a load is started in a subframe on 'pagehide' handling");
+window.jsTestIsAsync = true;
+var totalLoaded = 0;
+
+if (window.testRunner)
+    testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+
+window.addEventListener("pageshow", function(event) {
+    debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
+
+    if (event.persisted) {
+        testPassed("Page did enter and was restored from the page cache");
+        finishJSTest();
+    }
+}, false);
+
+window.addEventListener("pagehide", function(event) {
+    debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
+    if (!event.persisted) {
+        testFailed("Page did not enter the page cache.");
+        finishJSTest();
+    }
+}, false);
+
+function runTest() {
+    totalLoaded++;
+    if (totalLoaded < 2)
+      return;
+
+    // This needs to happen in a setTimeout because a navigation inside the onload handler would
+    // not create a history entry.
+    setTimeout(function() {
+      // Force a back navigation back to this page.
+      window.location.href = "resources/page-cache-helper.html";
+    }, 4000);
+}
+
+</script>
+<iframe id="testFrame" src="resources/frame-pagehide-starts-load-in-subframe.html" onload="runTest()"></iframe>
+<script src="/resources/js-test-post.js"></script>
+</body>
+</html>
index d201056..696ce83 100644 (file)
@@ -1,3 +1,91 @@
+2015-06-08  Chris Dumez  <cdumez@apple.com>
+
+        WebContent crash in WebCore::Page::sessionID() const + 0 (Page.cpp:1660)
+        https://bugs.webkit.org/show_bug.cgi?id=145748
+        <rdar://problem/21226577>
+
+        Reviewed by Brady Eidson.
+
+        We would sometimes crash when pruning the PageCache because it was
+        possible for frames to still be loading while in the PageCache and
+        we would try to stop the load when the CachedFrame is destroyed. This
+        code path was not supposed to be exercised as we were not supposed to
+        have pages still loading inside the PageCache.
+
+        r185017 made sure we don't insert into the PageCache pages that are
+        still loading. However, nothing was preventing content from starting
+        new loads in their 'pagehide' event handlers, *after* the decision
+        to put the page in the PageCache was made.
+
+        This patch prevents content from starting loads from a 'pagehide'
+        event handler so that we can no longer have content that is loading
+        inside the PageCache. 'ping' image loads still go through though as
+        these are specially handled and use PingLoaders.
+
+        Tests: http/tests/navigation/image-load-in-pagehide-handler.html
+               http/tests/navigation/subframe-pagehide-handler-starts-load.html
+               http/tests/navigation/subframe-pagehide-handler-starts-load2.html
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::FrameLoader):
+        (WebCore::FrameLoader::stopLoading):
+        (WebCore::FrameLoader::loadURL):
+        (WebCore::FrameLoader::loadWithDocumentLoader):
+        (WebCore::FrameLoader::stopAllLoaders):
+        (WebCore::FrameLoader::handleBeforeUnloadEvent):
+        * loader/FrameLoader.h:
+        (WebCore::FrameLoader::pageDismissalEventBeingDispatched):
+        (WebCore::FrameLoader::PageDismissalEventType::PageDismissalEventType):
+        (WebCore::FrameLoader::PageDismissalEventType::operator Page::DismissalType):
+
+        Add wrapper class for m_pageDismissalEventBeingDispatched member type.
+        The wrapper takes care of updating the m_dismissalEventBeingDispatched
+        member on the Page every time the member on FrameLoader is updated. We
+        now cache this information on the Page so that clients can cheaply
+        query if a dismissal event is being dispatched in any of the Page's
+        frame, without having to traverse the frame tree.
+
+        * loader/ImageLoader.cpp:
+        (WebCore::pageIsBeingDismissed):
+
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::load):
+
+        Abort the load early if we are currently dispatching a 'pagehide'
+        event. We don't allow new loads at such point because we've already
+        made the decision to add the Page to the PageCache.
+
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::requestImage):
+
+        * page/Chrome.cpp:
+        (WebCore::Chrome::runModal): Deleted.
+        (WebCore::Chrome::setToolbarsVisible): Deleted.
+        (WebCore::Chrome::toolbarsVisible): Deleted.
+        (WebCore::Chrome::runJavaScriptConfirm): Deleted.
+        (WebCore::Chrome::runJavaScriptPrompt): Deleted.
+        (WebCore::Chrome::shouldInterruptJavaScript): Deleted.
+        * page/Chrome.h:
+        * page/ChromeClient.h:
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::canShowModalDialogNow):
+
+        Drop ChromeClient::shouldRunModalDialogDuringPageDismissal() and code
+        using it as it is unused and I did not think it was worth updating
+        this code.
+
+        * page/Page.h:
+        (WebCore::Page::dismissalEventBeingDispatched):
+        (WebCore::Page::setDismissalEventBeingDispatched):
+
+        Add a m_dismissalEventBeingDispatched member to the Page so that we can
+        easily query if a dismissal event is being dispatched in any of the
+        frames, without having to traverse the frame tree. I suspect more call
+        sites of FrameLoader::pageDismissalEventBeingDispatched() may actually
+        want this but I did not make such change in this patch. It is important
+        to check all the frames and not simply the current one because a frame's
+        pagehide event handler may trigger a load in another frame.
+
 2015-06-08  Hunseop Jeong  <hs85.jeong@samsung.com>
 
         Replaced 0 with nullptr in WebCore/Modules.
index c9c5913..628e67d 100644 (file)
@@ -84,7 +84,6 @@
 #include "MIMETypeRegistry.h"
 #include "MainFrame.h"
 #include "MemoryCache.h"
-#include "Page.h"
 #include "PageCache.h"
 #include "PageThrottler.h"
 #include "PageTransitionEvent.h"
@@ -226,7 +225,7 @@ FrameLoader::FrameLoader(Frame& frame, FrameLoaderClient& client)
     , m_isExecutingJavaScriptFormAction(false)
     , m_didCallImplicitClose(true)
     , m_wasUnloadEventEmitted(false)
-    , m_pageDismissalEventBeingDispatched(NoDismissal)
+    , m_pageDismissalEventBeingDispatched(frame)
     , m_isComplete(false)
     , m_needsClear(false)
     , m_checkTimer(*this, &FrameLoader::checkTimerFired)
@@ -425,9 +424,9 @@ void FrameLoader::stopLoading(UnloadEventPolicy unloadEventPolicy)
                 Element* currentFocusedElement = m_frame.document()->focusedElement();
                 if (currentFocusedElement && currentFocusedElement->toInputElement())
                     currentFocusedElement->toInputElement()->endEditing();
-                if (m_pageDismissalEventBeingDispatched == NoDismissal) {
+                if (m_pageDismissalEventBeingDispatched == Page::DismissalType::None) {
                     if (unloadEventPolicy == UnloadEventPolicyUnloadAndPageHide) {
-                        m_pageDismissalEventBeingDispatched = PageHideDismissal;
+                        m_pageDismissalEventBeingDispatched = Page::DismissalType::PageHide;
                         m_frame.document()->domWindow()->dispatchEvent(PageTransitionEvent::create(eventNames().pagehideEvent, m_frame.document()->inPageCache()), m_frame.document());
                     }
 
@@ -440,7 +439,7 @@ void FrameLoader::stopLoading(UnloadEventPolicy unloadEventPolicy)
                         // while dispatching the event, so protect it to prevent writing the end
                         // time into freed memory.
                         RefPtr<DocumentLoader> documentLoader = m_provisionalDocumentLoader;
-                        m_pageDismissalEventBeingDispatched = UnloadDismissal;
+                        m_pageDismissalEventBeingDispatched = Page::DismissalType::Unload;
                         if (documentLoader && !documentLoader->timing().unloadEventStart() && !documentLoader->timing().unloadEventEnd()) {
                             DocumentLoadTiming& timing = documentLoader->timing();
                             ASSERT(timing.navigationStart());
@@ -451,7 +450,7 @@ void FrameLoader::stopLoading(UnloadEventPolicy unloadEventPolicy)
                             m_frame.document()->domWindow()->dispatchEvent(unloadEvent, m_frame.document());
                     }
                 }
-                m_pageDismissalEventBeingDispatched = NoDismissal;
+                m_pageDismissalEventBeingDispatched = Page::DismissalType::None;
                 if (m_frame.document())
                     m_frame.document()->updateStyleIfNeeded();
                 m_wasUnloadEventEmitted = true;
@@ -1227,7 +1226,7 @@ void FrameLoader::loadURL(const FrameLoadRequest& frameLoadRequest, const String
         return;
     }
 
-    if (m_pageDismissalEventBeingDispatched != NoDismissal)
+    if (m_pageDismissalEventBeingDispatched != Page::DismissalType::None)
         return;
 
     NavigationAction action(request, newLoadType, isFormSubmission, event, frameLoadRequest.shouldOpenExternalURLsPolicy());
@@ -1418,7 +1417,7 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
 
     ASSERT(m_frame.view());
 
-    if (m_pageDismissalEventBeingDispatched != NoDismissal)
+    if (m_pageDismissalEventBeingDispatched != Page::DismissalType::None)
         return;
 
     if (m_frame.document())
@@ -1592,7 +1591,7 @@ void FrameLoader::reload(bool endToEndReload)
 void FrameLoader::stopAllLoaders(ClearProvisionalItemPolicy clearProvisionalItemPolicy)
 {
     ASSERT(!m_frame.document() || !m_frame.document()->inPageCache());
-    if (m_pageDismissalEventBeingDispatched != NoDismissal)
+    if (m_pageDismissalEventBeingDispatched != Page::DismissalType::None)
         return;
 
     // If this method is called from within this method, infinite recursion can occur (3442218). Avoid this.
@@ -2848,7 +2847,7 @@ bool FrameLoader::handleBeforeUnloadEvent(Chrome& chrome, FrameLoader* frameLoad
         return true;
     
     RefPtr<BeforeUnloadEvent> beforeUnloadEvent = BeforeUnloadEvent::create();
-    m_pageDismissalEventBeingDispatched = BeforeUnloadDismissal;
+    m_pageDismissalEventBeingDispatched = Page::DismissalType::BeforeUnload;
 
     // We store the frame's page in a local variable because the frame might get detached inside dispatchEvent.
     Page* page = m_frame.page();
@@ -2856,7 +2855,7 @@ bool FrameLoader::handleBeforeUnloadEvent(Chrome& chrome, FrameLoader* frameLoad
     domWindow->dispatchEvent(beforeUnloadEvent.get(), domWindow->document());
     page->decrementFrameHandlingBeforeUnloadEventCount();
 
-    m_pageDismissalEventBeingDispatched = NoDismissal;
+    m_pageDismissalEventBeingDispatched = Page::DismissalType::None;
 
     if (!beforeUnloadEvent->defaultPrevented())
         document->defaultEventHandler(beforeUnloadEvent.get());
@@ -3553,4 +3552,14 @@ RefPtr<Frame> createWindow(Frame& openerFrame, Frame& lookupFrame, const FrameLo
     return WTF::move(frame);
 }
 
+auto FrameLoader::PageDismissalEventType::operator=(Page::DismissalType dismissalType) -> PageDismissalEventType&
+{
+    m_dismissalEventBeingDispatched = dismissalType;
+
+    if (auto* page = m_frame.page())
+        page->setDismissalEventBeingDispatched(dismissalType);
+
+    return *this;
+}
+
 } // namespace WebCore
index 3eab3f9..e7bfe17 100644 (file)
@@ -38,6 +38,7 @@
 #include "IconURL.h"
 #include "LayoutMilestones.h"
 #include "MixedContentChecker.h"
+#include "Page.h"
 #include "PageThrottler.h"
 #include "ResourceHandleTypes.h"
 #include "ResourceLoadNotifier.h"
@@ -271,13 +272,7 @@ public:
     
     void started();
 
-    enum PageDismissalType {
-        NoDismissal = 0,
-        BeforeUnloadDismissal = 1,
-        PageHideDismissal = 2,
-        UnloadDismissal = 3
-    };
-    PageDismissalType pageDismissalEventBeingDispatched() const { return m_pageDismissalEventBeingDispatched; }
+    Page::DismissalType pageDismissalEventBeingDispatched() const { return m_pageDismissalEventBeingDispatched; }
 
     WEBCORE_EXPORT NetworkingContext* networkingContext() const;
 
@@ -417,7 +412,22 @@ private:
 
     bool m_didCallImplicitClose;
     bool m_wasUnloadEventEmitted;
-    PageDismissalType m_pageDismissalEventBeingDispatched;
+
+    class PageDismissalEventType {
+    public:
+        PageDismissalEventType(Frame& frame)
+            : m_frame(frame)
+        { }
+
+        PageDismissalEventType& operator=(Page::DismissalType);
+        operator Page::DismissalType() const { return m_dismissalEventBeingDispatched; }
+
+    private:
+        Frame& m_frame;
+        Page::DismissalType m_dismissalEventBeingDispatched { Page::DismissalType::None };
+    };
+
+    PageDismissalEventType m_pageDismissalEventBeingDispatched;
     bool m_isComplete;
 
     RefPtr<SerializedScriptValue> m_pendingStateObject;
index 1a46972..20394d1 100644 (file)
@@ -84,7 +84,7 @@ static ImageEventSender& errorEventSender()
 static inline bool pageIsBeingDismissed(Document& document)
 {
     Frame* frame = document.frame();
-    return frame && frame->loader().pageDismissalEventBeingDispatched() != FrameLoader::NoDismissal;
+    return frame && frame->loader().pageDismissalEventBeingDispatched() != Page::DismissalType::None;
 }
 
 ImageLoader::ImageLoader(Element& element)
index 888ffc0..5378d9b 100644 (file)
@@ -214,6 +214,12 @@ void CachedResource::load(CachedResourceLoader& cachedResourceLoader, const Reso
         return;
     }
 
+    // Prevent 'pagehide' event handlers from starting new loads as we are in the PageCache.
+    if (cachedResourceLoader.frame()->page() && cachedResourceLoader.frame()->page()->dismissalEventBeingDispatched() == Page::DismissalType::PageHide) {
+        failBeforeStarting();
+        return;
+    }
+
     FrameLoader& frameLoader = cachedResourceLoader.frame()->loader();
     if (options.securityCheck() == DoSecurityCheck && (frameLoader.state() == FrameStateProvisional || !frameLoader.activeDocumentLoader() || frameLoader.activeDocumentLoader()->isStopping())) {
         failBeforeStarting();
index 88e39a7..ef4c7b2 100644 (file)
@@ -176,7 +176,7 @@ SessionID CachedResourceLoader::sessionID() const
 CachedResourceHandle<CachedImage> CachedResourceLoader::requestImage(CachedResourceRequest& request)
 {
     if (Frame* frame = this->frame()) {
-        if (frame->loader().pageDismissalEventBeingDispatched() != FrameLoader::NoDismissal) {
+        if (frame->loader().pageDismissalEventBeingDispatched() != Page::DismissalType::None) {
             URL requestURL = request.resourceRequest().url();
             if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request.options(), request.forPreload()))
                 PingLoader::loadImage(*frame, requestURL);
index 5b58961..0a19914 100644 (file)
@@ -213,23 +213,6 @@ bool Chrome::canRunModal() const
     return m_client.canRunModal();
 }
 
-static bool canRunModalIfDuringPageDismissal(Page& page, ChromeClient::DialogType dialog, const String& message)
-{
-    for (Frame* frame = &page.mainFrame(); frame; frame = frame->tree().traverseNext()) {
-        FrameLoader::PageDismissalType dismissal = frame->loader().pageDismissalEventBeingDispatched();
-        if (dismissal != FrameLoader::NoDismissal)
-            return page.chrome().client().shouldRunModalDialogDuringPageDismissal(dialog, message, dismissal);
-    }
-    return true;
-}
-
-bool Chrome::canRunModalNow() const
-{
-    // If loads are blocked, we can't run modal because the contents
-    // of the modal dialog will never show up!
-    return canRunModal() && canRunModalIfDuringPageDismissal(m_page, ChromeClient::HTMLDialog, String());
-}
-
 void Chrome::runModal() const
 {
     // Defer callbacks in all the other pages in this group, so we don't try to run JavaScript
@@ -309,9 +292,6 @@ void Chrome::closeWindowSoon()
 
 void Chrome::runJavaScriptAlert(Frame* frame, const String& message)
 {
-    if (!canRunModalIfDuringPageDismissal(m_page, ChromeClient::AlertDialog, message))
-        return;
-
     // Defer loads in case the client method runs a new event loop that would
     // otherwise cause the load to continue while we're in the middle of executing JavaScript.
     PageGroupLoadDeferrer deferrer(m_page, true);
@@ -327,9 +307,6 @@ void Chrome::runJavaScriptAlert(Frame* frame, const String& message)
 
 bool Chrome::runJavaScriptConfirm(Frame* frame, const String& message)
 {
-    if (!canRunModalIfDuringPageDismissal(m_page, ChromeClient::ConfirmDialog, message))
-        return false;
-
     // Defer loads in case the client method runs a new event loop that would
     // otherwise cause the load to continue while we're in the middle of executing JavaScript.
     PageGroupLoadDeferrer deferrer(m_page, true);
@@ -346,9 +323,6 @@ bool Chrome::runJavaScriptConfirm(Frame* frame, const String& message)
 
 bool Chrome::runJavaScriptPrompt(Frame* frame, const String& prompt, const String& defaultValue, String& result)
 {
-    if (!canRunModalIfDuringPageDismissal(m_page, ChromeClient::PromptDialog, prompt))
-        return false;
-
     // Defer loads in case the client method runs a new event loop that would
     // otherwise cause the load to continue while we're in the middle of executing JavaScript.
     PageGroupLoadDeferrer deferrer(m_page, true);
index b157262..842316e 100644 (file)
@@ -120,7 +120,6 @@ public:
     WEBCORE_EXPORT void show() const;
 
     bool canRunModal() const;
-    bool canRunModalNow() const;
     void runModal() const;
 
     void setToolbarsVisible(bool) const;
index a56cdc2..81a9692 100644 (file)
@@ -382,14 +382,6 @@ public:
 
     virtual WTF::Optional<ScrollbarOverlayStyle> preferredScrollbarOverlayStyle() { return ScrollbarOverlayStyleDefault; }
 
-    enum DialogType {
-        AlertDialog = 0,
-        ConfirmDialog = 1,
-        PromptDialog = 2,
-        HTMLDialog = 3
-    };
-    virtual bool shouldRunModalDialogDuringPageDismissal(const DialogType&, const String& dialogMessage, FrameLoader::PageDismissalType) const { UNUSED_PARAM(dialogMessage); return true; }
-
     virtual void wheelEventHandlersChanged(bool hasHandlers) = 0;
         
     virtual bool isSVGImageChromeClient() const { return false; }
index 175d94a..9805946 100644 (file)
@@ -387,7 +387,7 @@ bool DOMWindow::canShowModalDialogNow(const Frame* frame)
     Page* page = frame->page();
     if (!page)
         return false;
-    return page->chrome().canRunModalNow();
+    return page->chrome().canRunModal();
 }
 
 DOMWindow::DOMWindow(Document* document)
index cfbef66..44685f1 100644 (file)
@@ -152,6 +152,15 @@ public:
     MainFrame& mainFrame() { ASSERT(m_mainFrame); return *m_mainFrame; }
     const MainFrame& mainFrame() const { ASSERT(m_mainFrame); return *m_mainFrame; }
 
+    enum class DismissalType {
+        None,
+        BeforeUnload,
+        PageHide,
+        Unload
+    };
+    DismissalType dismissalEventBeingDispatched() const { return m_dismissalEventBeingDispatched; }
+    void setDismissalEventBeingDispatched(DismissalType dismissalType) { m_dismissalEventBeingDispatched = dismissalType; }
+
     bool openedByDOM() const;
     void setOpenedByDOM();
 
@@ -607,6 +616,7 @@ private:
     bool m_isClosing;
 
     MediaProducer::MediaStateFlags m_mediaState { MediaProducer::IsNotPlaying };
+    DismissalType m_dismissalEventBeingDispatched { DismissalType::None };
 };
 
 inline PageGroup& Page::group()