[webkit-changes] [195252] releases/WebKitGTK/webkit-2.10

Mon, 18 Jan 2016 23:58:10 -0800

Title: [195252] releases/WebKitGTK/webkit-2.10
Revision
195252
Author
[email protected]
Date
2016-01-18 23:57:57 -0800 (Mon, 18 Jan 2016)

Log Message

Merge r194888 - Inconsistencies in main resource load delegates when loading from history
https://bugs.webkit.org/show_bug.cgi?id=150927

Reviewed by Michael Catanzaro.

Source/WebCore:

When restoring a page from the page cache, even though there
isn't an actual load of resources, we are still emitting the load
delegates to let the API layer know there are contents being
loaded in the web view. This makes the page cache restoring
transparent for the API layer. However, when restoring a page from
the cache, all the delegates are emitted after the load is
committed. This is not consistent with real loads, where we first
load the main resource and once we get a response we commit the
load. This inconsistency is problematic if the API layer expects
to always have a main resource with a response when the load is
committed. This is the case of the GTK+ port, for example. So,
this patch ensures that when a page is restored from the page
cache, the main resource load delegates that are emitted until a
response is received in normal loads, are emitted before the load
is committed.

Test: http/tests/loading/main-resource-delegates-on-back-navigation.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::commitProvisionalLoad): When loading from
the page cache, send delegate messages up to didReceiveResponse
for the main resource before the load is committed, and the
remaining messages afterwards.

LayoutTests:

Add test to check that main resource load delegates are emitted in
the same order before the load is committed when loading a page
from history with the page cache enabled and disabled.

* http/tests/loading/main-resource-delegates-on-back-navigation-expected.txt: Added.
* http/tests/loading/main-resource-delegates-on-back-navigation.html: Added.
* http/tests/loading/resources/page-go-back-onload.html: Added.
* loader/go-back-cached-main-resource-expected.txt:

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog (195251 => 195252)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog	2016-01-19 07:51:45 UTC (rev 195251)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog	2016-01-19 07:57:57 UTC (rev 195252)
@@ -1,3 +1,19 @@
+2016-01-11  Carlos Garcia Campos  <[email protected]>
+
+        Inconsistencies in main resource load delegates when loading from history
+        https://bugs.webkit.org/show_bug.cgi?id=150927
+
+        Reviewed by Michael Catanzaro.
+
+        Add test to check that main resource load delegates are emitted in
+        the same order before the load is committed when loading a page
+        from history with the page cache enabled and disabled.
+
+        * http/tests/loading/main-resource-delegates-on-back-navigation-expected.txt: Added.
+        * http/tests/loading/main-resource-delegates-on-back-navigation.html: Added.
+        * http/tests/loading/resources/page-go-back-onload.html: Added.
+        * loader/go-back-cached-main-resource-expected.txt:
+
 2016-01-11  Zalan Bujtas  <[email protected]>
 
         Padding added to table-cell element after font-size change.

Added: releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/loading/main-resource-delegates-on-back-navigation-expected.txt (0 => 195252)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/loading/main-resource-delegates-on-back-navigation-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/loading/main-resource-delegates-on-back-navigation-expected.txt	2016-01-19 07:57:57 UTC (rev 195252)
@@ -0,0 +1,41 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+main frame - didFinishLoadForFrame
+http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html - didFinishLoading
+main frame - willPerformClientRedirectToURL: http://127.0.0.1:8000/loading/resources/page-go-back-onload.html 
+main frame - didStartProvisionalLoadForFrame
+http://127.0.0.1:8000/loading/resources/page-go-back-onload.html - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/loading/resources/page-go-back-onload.html, main document URL http://127.0.0.1:8000/loading/resources/page-go-back-onload.html, http method GET> redirectResponse (null)
+http://127.0.0.1:8000/loading/resources/page-go-back-onload.html - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/loading/resources/page-go-back-onload.html, http status code 200>
+main frame - didCancelClientRedirectForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+main frame - didFinishLoadForFrame
+http://127.0.0.1:8000/loading/resources/page-go-back-onload.html - didFinishLoading
+main frame - didStartProvisionalLoadForFrame
+http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html, main document URL http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html, http method GET> redirectResponse (null)
+http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html, http status code 200>
+main frame - didCommitLoadForFrame
+http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html - didFinishLoading
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+main frame - didFinishLoadForFrame
+main frame - willPerformClientRedirectToURL: http://127.0.0.1:8000/loading/resources/page-go-back-onload.html 
+main frame - didStartProvisionalLoadForFrame
+http://127.0.0.1:8000/loading/resources/page-go-back-onload.html - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/loading/resources/page-go-back-onload.html, main document URL http://127.0.0.1:8000/loading/resources/page-go-back-onload.html, http method GET> redirectResponse (null)
+http://127.0.0.1:8000/loading/resources/page-go-back-onload.html - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/loading/resources/page-go-back-onload.html, http status code 200>
+main frame - didCancelClientRedirectForFrame
+main frame - didCommitLoadForFrame
+http://127.0.0.1:8000/loading/resources/page-go-back-onload.html - didFinishLoading
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+main frame - didFinishLoadForFrame
+main frame - didStartProvisionalLoadForFrame
+http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html, main document URL http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html, http method GET> redirectResponse (null)
+http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html, http status code 200>
+main frame - didCommitLoadForFrame
+http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html - didFinishLoading
+main frame - didFinishLoadForFrame
+Test that main resource load delegates are the same when page cache is enabled and disabled

Added: releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/loading/main-resource-delegates-on-back-navigation.html (0 => 195252)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/loading/main-resource-delegates-on-back-navigation.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/loading/main-resource-delegates-on-back-navigation.html	2016-01-19 07:57:57 UTC (rev 195252)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<body>
+Test that main resource load delegates are the same when page cache is enabled and disabled
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.dumpResourceLoadCallbacks();
+    testRunner.waitUntilDone();
+}
+
+window.addEventListener('pageshow', function() {
+    if (!sessionStorage.testLoadCount)
+        sessionStorage.testLoadCount = 1;
+    else
+        sessionStorage.testLoadCount = parseInt(sessionStorage.testLoadCount, 10) + 1
+    var pageCacheEnabled;
+    switch(parseInt(sessionStorage.testLoadCount, 10)) {
+    case 1:
+        pageCacheEnabled = 0;
+        break;
+    case 2:
+        pageCacheEnabled = 1;
+        break;
+    case 3:
+        setTimeout(function() {
+            delete sessionStorage.testLoadCount;
+            testRunner.notifyDone();
+        }, 0);
+        break;
+    }
+
+    if (window.testRunner)
+        testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", pageCacheEnabled);
+
+    setTimeout(function() {
+        window.location.href = ""
+    }, 0);
+}, false);
+</script>
+</body>
+</html>

Added: releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/loading/resources/page-go-back-onload.html (0 => 195252)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/loading/resources/page-go-back-onload.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/http/tests/loading/resources/page-go-back-onload.html	2016-01-19 07:57:57 UTC (rev 195252)
@@ -0,0 +1,7 @@
+<script>
+window.addEventListener("load", function() {
+    setTimeout(function() {
+        history.back();
+    }, 0);
+}, false);
+</script>

Modified: releases/WebKitGTK/webkit-2.10/LayoutTests/loader/go-back-cached-main-resource-expected.txt (195251 => 195252)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/loader/go-back-cached-main-resource-expected.txt	2016-01-19 07:51:45 UTC (rev 195251)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/loader/go-back-cached-main-resource-expected.txt	2016-01-19 07:57:57 UTC (rev 195252)
@@ -11,7 +11,7 @@
 resources/other-page.html - willSendRequest <NSURLRequest URL resources/other-page.html, main document URL resources/other-page.html, http method GET> redirectResponse (null)
 resources/other-page.html - didReceiveResponse <NSURLResponse resources/other-page.html, http status code 0>
 resources/other-page.html - didFinishLoading
-resources/first-page.html - willSendRequest <NSURLRequest URL resources/first-page.html, main document URL (null), http method GET> redirectResponse (null)
+resources/first-page.html - willSendRequest <NSURLRequest URL resources/first-page.html, main document URL resources/first-page.html, http method GET> redirectResponse (null)
 resources/first-page.html - didReceiveResponse <NSURLResponse resources/first-page.html, http status code 0>
 resources/first-page.html - didFinishLoading
 This test check the following situation:

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog (195251 => 195252)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog	2016-01-19 07:51:45 UTC (rev 195251)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog	2016-01-19 07:57:57 UTC (rev 195252)
@@ -1,3 +1,34 @@
+2016-01-11  Carlos Garcia Campos  <[email protected]>
+
+        Inconsistencies in main resource load delegates when loading from history
+        https://bugs.webkit.org/show_bug.cgi?id=150927
+
+        Reviewed by Michael Catanzaro.
+
+        When restoring a page from the page cache, even though there
+        isn't an actual load of resources, we are still emitting the load
+        delegates to let the API layer know there are contents being
+        loaded in the web view. This makes the page cache restoring
+        transparent for the API layer. However, when restoring a page from
+        the cache, all the delegates are emitted after the load is
+        committed. This is not consistent with real loads, where we first
+        load the main resource and once we get a response we commit the
+        load. This inconsistency is problematic if the API layer expects
+        to always have a main resource with a response when the load is
+        committed. This is the case of the GTK+ port, for example. So,
+        this patch ensures that when a page is restored from the page
+        cache, the main resource load delegates that are emitted until a
+        response is received in normal loads, are emitted before the load
+        is committed.
+
+        Test: http/tests/loading/main-resource-delegates-on-back-navigation.html
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::commitProvisionalLoad): When loading from
+        the page cache, send delegate messages up to didReceiveResponse
+        for the main resource before the load is committed, and the
+        remaining messages afterwards.
+
 2016-01-11  Zalan Bujtas  <[email protected]>
 
         Padding added to table-cell element after font-size change.

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/loader/FrameLoader.cpp (195251 => 195252)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/loader/FrameLoader.cpp	2016-01-19 07:51:45 UTC (rev 195251)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/loader/FrameLoader.cpp	2016-01-19 07:57:57 UTC (rev 195252)
@@ -1799,6 +1799,14 @@
 #endif
         prepareForCachedPageRestore();
 
+        // Start request for the main resource and dispatch didReceiveResponse before the load is committed for
+        // consistency with all other loads. See https://bugs.webkit.org/show_bug.cgi?id=150927.
+        ResourceError mainResouceError;
+        unsigned long mainResourceIdentifier;
+        ResourceRequest mainResourceRequest(cachedPage->documentLoader()->request());
+        requestFromDelegate(mainResourceRequest, mainResourceIdentifier, mainResouceError);
+        notifier().dispatchDidReceiveResponse(cachedPage->documentLoader(), mainResourceIdentifier, cachedPage->documentLoader()->response());
+
         // FIXME: This API should be turned around so that we ground CachedPage into the Page.
         cachedPage->restore(*m_frame.page());
 
@@ -1812,6 +1820,10 @@
         if (!title.isNull())
             m_client.dispatchDidReceiveTitle(title);
 
+        // Send remaining notifications for the main resource.
+        notifier().sendRemainingDelegateMessages(m_documentLoader.get(), mainResourceIdentifier, mainResourceRequest, ResourceResponse(),
+            nullptr, static_cast<int>(m_documentLoader->response().expectedContentLength()), 0, mainResouceError);
+
         checkCompleted();
     } else
         didOpenURL();
@@ -1838,10 +1850,9 @@
         m_frame.view()->forceLayout();
 #endif
 
-        const ResponseVector& responses = m_documentLoader->responses();
-        size_t count = responses.size();
-        for (size_t i = 0; i < count; i++) {
-            const ResourceResponse& response = responses[i];
+        // Main resource delegates were already sent, so we skip the first response here.
+        for (unsigned i = 1; i < m_documentLoader->responses().size(); ++i) {
+            const auto& response = m_documentLoader->responses()[i];
             // FIXME: If the WebKit client changes or cancels the request, this is not respected.
             ResourceError error;
             unsigned long identifier;

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to