- Revision
- 207080
- Author
- carlo...@webkit.org
- Date
- 2016-10-11 03:38:37 -0700 (Tue, 11 Oct 2016)
Log Message
Merge r206062 - Cancelling one frame's load cancels load in other frames that have the same URL as well
https://bugs.webkit.org/show_bug.cgi?id=162094
Reviewed by Antti Koivisto.
Source/WebCore:
Cancelling one frame's load cancels load in other frames that have the same URL as well.
So if you have several frames that are loading URL X and you navigate one of the frames
to Y, then the load of X will be cancelled and this frame will navigate to Y. All other
frames will not load URL X even though they should.
The issue is that all the DocumentLoaders share the same CachedResource because of the
memoryCache. When we call DocumentLoader::stopLoading(), it will cancel the
CachedResource's load even though there are several clients for this CachedResource
and other clients still want the load.
The approach chosen in this patch is to not reuse CachedResources that are still
loading when trying to load a main resource. This is not the most efficient approach.
I still chose this approach because:
- It is very unlikely to introduce new bugs.
- The change is very simple.
- This is a corner case (several iframes having the same URL and cancelling the load in
one of them).
Test: http/tests/navigation/frames-same-url-cancel-load.html
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::determineRevalidationPolicy):
LayoutTests:
Add layout test coverage.
* http/tests/cache/iframe-detach-expected.txt: Added.
* http/tests/cache/iframe-detach.html: Added.
* http/tests/cache/resources/slow-iframe.php: Added.
Import Alex Christensen's test from Bug 157563.
* http/tests/navigation/frames-same-url-cancel-load-expected.txt: Added.
* http/tests/navigation/frames-same-url-cancel-load.html: Added.
* http/tests/navigation/resources/success.html: Added.
* http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients-expected.txt:
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.14/LayoutTests/ChangeLog (207079 => 207080)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/ChangeLog 2016-10-11 10:36:43 UTC (rev 207079)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/ChangeLog 2016-10-11 10:38:37 UTC (rev 207080)
@@ -1,3 +1,22 @@
+2016-09-16 Chris Dumez <cdu...@apple.com>
+
+ Cancelling one frame's load cancels load in other frames that have the same URL as well
+ https://bugs.webkit.org/show_bug.cgi?id=162094
+
+ Reviewed by Antti Koivisto.
+
+ Add layout test coverage.
+
+ * http/tests/cache/iframe-detach-expected.txt: Added.
+ * http/tests/cache/iframe-detach.html: Added.
+ * http/tests/cache/resources/slow-iframe.php: Added.
+ Import Alex Christensen's test from Bug 157563.
+
+ * http/tests/navigation/frames-same-url-cancel-load-expected.txt: Added.
+ * http/tests/navigation/frames-same-url-cancel-load.html: Added.
+ * http/tests/navigation/resources/success.html: Added.
+ * http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients-expected.txt:
+
2016-09-15 Zalan Bujtas <za...@apple.com>
ASSERTION FAILED: willBeComposited == needsToBeComposited(layer) in WebCore::RenderLayerCompositor::computeCompositingRequirements
Added: releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/cache/iframe-detach-expected.txt (0 => 207080)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/cache/iframe-detach-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/cache/iframe-detach-expected.txt 2016-10-11 10:38:37 UTC (rev 207080)
@@ -0,0 +1,2 @@
+ALERT: PASS - iframe loaded
+
Added: releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/cache/iframe-detach.html (0 => 207080)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/cache/iframe-detach.html (rev 0)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/cache/iframe-detach.html 2016-10-11 10:38:37 UTC (rev 207080)
@@ -0,0 +1,27 @@
+<html>
+<body>
+<div id="shadow">
+<iframe src=""
+</div>
+
+<div id="empty"></div>
+</body>
+
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+function set2() {
+ document.getElementById("shadow").innerHTML = "";
+}
+
+function set1() {
+ document.getElementById("empty").innerHTML = document.getElementById("shadow").innerHTML;
+ setTimeout(set2, 250);
+}
+
+setTimeout(set1, 250);
+</script>
+</html>
Added: releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/cache/resources/slow-iframe.php (0 => 207080)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/cache/resources/slow-iframe.php (rev 0)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/cache/resources/slow-iframe.php 2016-10-11 10:38:37 UTC (rev 207080)
@@ -0,0 +1,5 @@
+<?php
+sleep(1);
+header("Content-Type: text/html");
+echo("<script>alert('PASS - iframe loaded'); if (window.testRunner) testRunner.notifyDone(); </script>");
+?>
Added: releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/navigation/frames-same-url-cancel-load-expected.txt (0 => 207080)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/navigation/frames-same-url-cancel-load-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/navigation/frames-same-url-cancel-load-expected.txt 2016-10-11 10:38:37 UTC (rev 207080)
@@ -0,0 +1,10 @@
+Test that when several frames are loading the same URL, and you navigate one, it does not cancel the load for the other frames.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS frame2.contentDocument.URL is "http://127.0.0.1:8000/navigation/resources/success.html"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/navigation/frames-same-url-cancel-load.html (0 => 207080)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/navigation/frames-same-url-cancel-load.html (rev 0)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/navigation/frames-same-url-cancel-load.html 2016-10-11 10:38:37 UTC (rev 207080)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<iframe id="frame1" src=""
+<iframe id="frame2" src=""
+<script>
+description("Test that when several frames are loading the same URL, and you navigate one, it does not cancel the load for the other frames.");
+jsTestIsAsync = true;
+
+document.getElementById("frame1").src = ""
+
+_onload_ = function() {
+ frame2 = document.getElementById("frame2");
+ shouldBeEqualToString("frame2.contentDocument.URL", "http://127.0.0.1:8000/navigation/resources/success.html");
+
+ finishJSTest();
+}
+</script>
+<script src=""
+</body>
+</html>
Added: releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/navigation/resources/success.html (0 => 207080)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/navigation/resources/success.html (rev 0)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/navigation/resources/success.html 2016-10-11 10:38:37 UTC (rev 207080)
@@ -0,0 +1 @@
+SUCCESS
Modified: releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients-expected.txt (207079 => 207080)
--- releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients-expected.txt 2016-10-11 10:36:43 UTC (rev 207079)
+++ releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients-expected.txt 2016-10-11 10:38:37 UTC (rev 207080)
@@ -1,3 +1,5 @@
CONSOLE MESSAGE: Refused to display 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi' in a frame because it set 'X-Frame-Options' to 'deny'.
ALERT: PASS: onload fired.
+CONSOLE MESSAGE: Refused to display 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi' in a frame because it set 'X-Frame-Options' to 'deny'.
+ALERT: PASS: onload fired.
Test that two main resources pointing to the same url that are canceled within didReceiveResponse() don't cause us to crash.
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog (207079 => 207080)
--- releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog 2016-10-11 10:36:43 UTC (rev 207079)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog 2016-10-11 10:38:37 UTC (rev 207080)
@@ -1,3 +1,34 @@
+2016-09-16 Chris Dumez <cdu...@apple.com>
+
+ Cancelling one frame's load cancels load in other frames that have the same URL as well
+ https://bugs.webkit.org/show_bug.cgi?id=162094
+
+ Reviewed by Antti Koivisto.
+
+ Cancelling one frame's load cancels load in other frames that have the same URL as well.
+
+ So if you have several frames that are loading URL X and you navigate one of the frames
+ to Y, then the load of X will be cancelled and this frame will navigate to Y. All other
+ frames will not load URL X even though they should.
+
+ The issue is that all the DocumentLoaders share the same CachedResource because of the
+ memoryCache. When we call DocumentLoader::stopLoading(), it will cancel the
+ CachedResource's load even though there are several clients for this CachedResource
+ and other clients still want the load.
+
+ The approach chosen in this patch is to not reuse CachedResources that are still
+ loading when trying to load a main resource. This is not the most efficient approach.
+ I still chose this approach because:
+ - It is very unlikely to introduce new bugs.
+ - The change is very simple.
+ - This is a corner case (several iframes having the same URL and cancelling the load in
+ one of them).
+
+ Test: http/tests/navigation/frames-same-url-cancel-load.html
+
+ * loader/cache/CachedResourceLoader.cpp:
+ (WebCore::CachedResourceLoader::determineRevalidationPolicy):
+
2016-09-16 Michael Catanzaro <mcatanz...@igalia.com>
ASSERTION FAILED: The string being removed is atomic in the string table of an other thread! iterator != atomicStringTable.end() at Source/WTF/wtf/text/AtomicStringImpl.cpp(453)
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/cache/CachedResourceLoader.cpp (207079 => 207080)
--- releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2016-10-11 10:36:43 UTC (rev 207079)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2016-10-11 10:38:37 UTC (rev 207080)
@@ -864,10 +864,17 @@
logMemoryCacheResourceRequest(frame(), DiagnosticLoggingKeys::inMemoryCacheKey(), DiagnosticLoggingKeys::unusedReasonErrorKey());
return Reload;
}
-
- // For resources that are not yet loaded we ignore the cache policy.
- if (existingResource->isLoading())
+
+ if (existingResource->isLoading()) {
+ // Do not use cached main resources that are still loading because sharing
+ // loading CachedResources in this case causes issues with regards to cancellation.
+ // If one of the DocumentLoader clients decides to cancel the load, then the load
+ // would be cancelled for all other DocumentLoaders as well.
+ if (type == CachedResource::Type::MainResource)
+ return Reload;
+ // For cached subresources that are still loading we ignore the cache policy.
return Use;
+ }
auto revalidationDecision = existingResource->makeRevalidationDecision(cachePolicy(type));
logResourceRevalidationDecision(revalidationDecision, frame());
