Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge r185337 - 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. 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.
- Loading branch information
1 parent
db81d7f
commit 1491b79
Showing
22 changed files
with
367 additions
and
57 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 changes: 3 additions & 0 deletions
3
LayoutTests/http/tests/navigation/image-load-in-pagehide-handler-expected.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
30 changes: 30 additions & 0 deletions
30
LayoutTests/http/tests/navigation/image-load-in-pagehide-handler.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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> |
13 changes: 13 additions & 0 deletions
13
LayoutTests/http/tests/navigation/resources/frame-do-load.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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> |
13 changes: 13 additions & 0 deletions
13
LayoutTests/http/tests/navigation/resources/frame-pagehide-starts-load-in-subframe.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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> |
12 changes: 12 additions & 0 deletions
12
LayoutTests/http/tests/navigation/resources/frame-pagehide-starts-load.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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> |
3 changes: 3 additions & 0 deletions
3
LayoutTests/http/tests/navigation/resources/image-load-in-pagehide-handler-2.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| <script> | ||
| location.href = 'check-ping.php?test=/navigation/image-load-in-pagehide-handler.html'; | ||
| </script> |
13 changes: 13 additions & 0 deletions
13
LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load-expected.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
|
47 changes: 47 additions & 0 deletions
47
LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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> |
13 changes: 13 additions & 0 deletions
13
LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load2-expected.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
|
47 changes: 47 additions & 0 deletions
47
LayoutTests/http/tests/navigation/subframe-pagehide-handler-starts-load2.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.