Changeset 194888 in webkit


Ignore:
Timestamp:
Jan 12, 2016 12:23:36 AM (8 years ago)
Author:
Carlos Garcia Campos
Message:

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:
Location:
trunk
Files:
3 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r194887 r194888  
     12016-01-11  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        Inconsistencies in main resource load delegates when loading from history
     4        https://bugs.webkit.org/show_bug.cgi?id=150927
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        Add test to check that main resource load delegates are emitted in
     9        the same order before the load is committed when loading a page
     10        from history with the page cache enabled and disabled.
     11
     12        * http/tests/loading/main-resource-delegates-on-back-navigation-expected.txt: Added.
     13        * http/tests/loading/main-resource-delegates-on-back-navigation.html: Added.
     14        * http/tests/loading/resources/page-go-back-onload.html: Added.
     15        * loader/go-back-cached-main-resource-expected.txt:
     16
    1172016-01-11  Johan K. Jensen  <jj@johanjensen.dk>
    218

  • trunk/LayoutTests/loader/go-back-cached-main-resource-expected.txt

    r153903 r194888  
    1212resources/other-page.html - didReceiveResponse <NSURLResponse resources/other-page.html, http status code 0>
    1313resources/other-page.html - didFinishLoading
    14 resources/first-page.html - willSendRequest <NSURLRequest URL resources/first-page.html, main document URL (null), http method GET> redirectResponse (null)
     14resources/first-page.html - willSendRequest <NSURLRequest URL resources/first-page.html, main document URL resources/first-page.html, http method GET> redirectResponse (null)
    1515resources/first-page.html - didReceiveResponse <NSURLResponse resources/first-page.html, http status code 0>
    1616resources/first-page.html - didFinishLoading

  • trunk/Source/WebCore/ChangeLog

    r194880 r194888  
     12016-01-11  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        Inconsistencies in main resource load delegates when loading from history
     4        https://bugs.webkit.org/show_bug.cgi?id=150927
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        When restoring a page from the page cache, even though there
     9        isn't an actual load of resources, we are still emitting the load
     10        delegates to let the API layer know there are contents being
     11        loaded in the web view. This makes the page cache restoring
     12        transparent for the API layer. However, when restoring a page from
     13        the cache, all the delegates are emitted after the load is
     14        committed. This is not consistent with real loads, where we first
     15        load the main resource and once we get a response we commit the
     16        load. This inconsistency is problematic if the API layer expects
     17        to always have a main resource with a response when the load is
     18        committed. This is the case of the GTK+ port, for example. So,
     19        this patch ensures that when a page is restored from the page
     20        cache, the main resource load delegates that are emitted until a
     21        response is received in normal loads, are emitted before the load
     22        is committed.
     23
     24        Test: http/tests/loading/main-resource-delegates-on-back-navigation.html
     25
     26        * loader/FrameLoader.cpp:
     27        (WebCore::FrameLoader::commitProvisionalLoad): When loading from
     28        the page cache, send delegate messages up to didReceiveResponse
     29        for the main resource before the load is committed, and the
     30        remaining messages afterwards.
     31
    1322016-01-09  Andy Estes  <aestes@apple.com>
    233

  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r194666 r194888  
    18011801        prepareForCachedPageRestore();
    18021802
     1803        // Start request for the main resource and dispatch didReceiveResponse before the load is committed for
     1804        // consistency with all other loads. See https://bugs.webkit.org/show_bug.cgi?id=150927.
     1805        ResourceError mainResouceError;
     1806        unsigned long mainResourceIdentifier;
     1807        ResourceRequest mainResourceRequest(cachedPage->documentLoader()->request());
     1808        requestFromDelegate(mainResourceRequest, mainResourceIdentifier, mainResouceError);
     1809        notifier().dispatchDidReceiveResponse(cachedPage->documentLoader(), mainResourceIdentifier, cachedPage->documentLoader()->response());
     1810
    18031811        // FIXME: This API should be turned around so that we ground CachedPage into the Page.
    18041812        cachedPage->restore(*m_frame.page());
    … …  
    18141822            m_client.dispatchDidReceiveTitle(title);
    18151823
     1824        // Send remaining notifications for the main resource.
     1825        notifier().sendRemainingDelegateMessages(m_documentLoader.get(), mainResourceIdentifier, mainResourceRequest, ResourceResponse(),
     1826            nullptr, static_cast<int>(m_documentLoader->response().expectedContentLength()), 0, mainResouceError);
     1827
    18161828        checkCompleted();
    18171829    } else
    … …  
    18401852#endif
    18411853
    1842         for (auto& response : m_documentLoader->responses()) {
     1854        // Main resource delegates were already sent, so we skip the first response here.
     1855        for (unsigned i = 1; i < m_documentLoader->responses().size(); ++i) {
     1856            const auto& response = m_documentLoader->responses()[i];
    18431857            // FIXME: If the WebKit client changes or cancels the request, this is not respected.
    18441858            ResourceError error;
Note: See TracChangeset for help on using the changeset viewer.