Changeset 229675 in webkit


Ignore:
Timestamp:
Mar 16, 2018 11:33:48 AM (6 years ago)
Author:
Chris Dumez
Message:

WebKit.RestoreSessionStateContainingScrollRestorationDefault API test is failing with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183679

Reviewed by Alex Christensen.

Source/WebCore:

Update CachedRawResource::didAddClient() to not send data until we've received
the policy decision for the response.

No new tests, covered by new API test.

  • loader/DocumentLoader.cpp:

(WebCore::DocumentLoader::handleSubstituteDataLoadNow):
(WebCore::DocumentLoader::responseReceived):

  • loader/DocumentLoader.h:
  • loader/DocumentThreadableLoader.cpp:

(WebCore::DocumentThreadableLoader::responseReceived):

  • loader/DocumentThreadableLoader.h:
  • loader/MediaResourceLoader.cpp:

(WebCore::MediaResource::responseReceived):

  • loader/MediaResourceLoader.h:
  • loader/appcache/ApplicationCacheResourceLoader.cpp:

(WebCore::ApplicationCacheResourceLoader::responseReceived):

  • loader/appcache/ApplicationCacheResourceLoader.h:
  • loader/cache/CachedRawResource.cpp:

(WebCore::CachedRawResource::didAddClient):
(WebCore::CachedRawResource::responseReceived):

  • loader/cache/CachedRawResourceClient.h:

(WebCore::CachedRawResourceClient::responseReceived):

  • loader/cache/KeepaliveRequestTracker.cpp:

(WebCore::KeepaliveRequestTracker::responseReceived):

  • loader/cache/KeepaliveRequestTracker.h:
  • platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.h:
  • platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.mm:

(WebCore::WebCoreAVFResourceLoader::responseReceived):

Tools:

Add API test coverage.

  • TestWebKitAPI/Tests/WebKit/RestoreSessionState.cpp:

(TestWebKitAPI::decidePolicyForNavigationAction):
(TestWebKitAPI::decidePolicyForResponse):
(TestWebKitAPI::TEST):

Location:
trunk
Files:
19 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r229674 r229675  
     12018-03-16  Chris Dumez  <cdumez@apple.com>
     2
     3        WebKit.RestoreSessionStateContainingScrollRestorationDefault API test is failing with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183679
     5
     6        Reviewed by Alex Christensen.
     7
     8        Update CachedRawResource::didAddClient() to not send data until we've received
     9        the policy decision for the response.
     10
     11        No new tests, covered by new API test.
     12
     13        * loader/DocumentLoader.cpp:
     14        (WebCore::DocumentLoader::handleSubstituteDataLoadNow):
     15        (WebCore::DocumentLoader::responseReceived):
     16        * loader/DocumentLoader.h:
     17        * loader/DocumentThreadableLoader.cpp:
     18        (WebCore::DocumentThreadableLoader::responseReceived):
     19        * loader/DocumentThreadableLoader.h:
     20        * loader/MediaResourceLoader.cpp:
     21        (WebCore::MediaResource::responseReceived):
     22        * loader/MediaResourceLoader.h:
     23        * loader/appcache/ApplicationCacheResourceLoader.cpp:
     24        (WebCore::ApplicationCacheResourceLoader::responseReceived):
     25        * loader/appcache/ApplicationCacheResourceLoader.h:
     26        * loader/cache/CachedRawResource.cpp:
     27        (WebCore::CachedRawResource::didAddClient):
     28        (WebCore::CachedRawResource::responseReceived):
     29        * loader/cache/CachedRawResourceClient.h:
     30        (WebCore::CachedRawResourceClient::responseReceived):
     31        * loader/cache/KeepaliveRequestTracker.cpp:
     32        (WebCore::KeepaliveRequestTracker::responseReceived):
     33        * loader/cache/KeepaliveRequestTracker.h:
     34        * platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.h:
     35        * platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.mm:
     36        (WebCore::WebCoreAVFResourceLoader::responseReceived):
     37
    1382018-03-16  Youenn Fablet  <youenn@apple.com>
    239
  • trunk/Source/WebCore/loader/DocumentLoader.cpp

    r229596 r229675  
    462462        response = ResourceResponse(m_request.url(), m_substituteData.mimeType(), m_substituteData.content()->size(), m_substituteData.textEncoding());
    463463
    464     responseReceived(response);
     464    responseReceived(response, nullptr);
    465465}
    466466
     
    717717}
    718718
    719 void DocumentLoader::responseReceived(CachedResource& resource, const ResourceResponse& response)
     719void DocumentLoader::responseReceived(CachedResource& resource, const ResourceResponse& response, CompletionHandler<void()>&& completionHandler)
    720720{
    721721    ASSERT_UNUSED(resource, m_mainResource == &resource);
    722     responseReceived(response);
    723 }
    724 
    725 void DocumentLoader::responseReceived(const ResourceResponse& response)
    726 {
     722    responseReceived(response, WTFMove(completionHandler));
     723}
     724
     725void DocumentLoader::responseReceived(const ResourceResponse& response, CompletionHandler<void()>&& completionHandler)
     726{
     727    CompletionHandlerCallingScope completionHandlerCaller(WTFMove(completionHandler));
     728
    727729#if ENABLE(CONTENT_FILTERING)
    728730    if (m_contentFilter && !m_contentFilter->continueAfterResponseReceived(response))
     
    810812    if (mainResourceLoader)
    811813        mainResourceLoader->markInAsyncResponsePolicyCheck();
    812     frameLoader()->checkContentPolicy(m_response, [this, protectedThis = makeRef(*this), mainResourceLoader = WTFMove(mainResourceLoader)](PolicyAction policy) {
     814    frameLoader()->checkContentPolicy(m_response, [this, protectedThis = makeRef(*this), mainResourceLoader = WTFMove(mainResourceLoader), completionHandler = completionHandlerCaller.release()](PolicyAction policy) {
    813815        continueAfterContentPolicy(policy);
    814816        if (mainResourceLoader)
    815817            mainResourceLoader->didReceiveResponsePolicy();
     818        if (completionHandler)
     819            completionHandler();
    816820    });
    817821}
  • trunk/Source/WebCore/loader/DocumentLoader.h

    r229181 r229675  
    356356    void mainReceivedError(const ResourceError&);
    357357    WEBCORE_EXPORT void redirectReceived(CachedResource&, ResourceRequest&&, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&&) override;
    358     WEBCORE_EXPORT void responseReceived(CachedResource&, const ResourceResponse&) override;
     358    WEBCORE_EXPORT void responseReceived(CachedResource&, const ResourceResponse&, CompletionHandler<void()>&&) override;
    359359    WEBCORE_EXPORT void dataReceived(CachedResource&, const char* data, int length) override;
    360360    WEBCORE_EXPORT void notifyFinished(CachedResource&) override;
    361361
    362     void responseReceived(const ResourceResponse&);
     362    void responseReceived(const ResourceResponse&, CompletionHandler<void()>&&);
    363363    void dataReceived(const char* data, int length);
    364364
  • trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp

    r227626 r229675  
    309309}
    310310
    311 void DocumentThreadableLoader::responseReceived(CachedResource& resource, const ResourceResponse& response)
     311void DocumentThreadableLoader::responseReceived(CachedResource& resource, const ResourceResponse& response, CompletionHandler<void()>&& completionHandler)
    312312{
    313313    ASSERT_UNUSED(resource, &resource == m_resource);
    314314    didReceiveResponse(m_resource->identifier(), response);
     315
     316    if (completionHandler)
     317        completionHandler();
    315318}
    316319
  • trunk/Source/WebCore/loader/DocumentThreadableLoader.h

    r224699 r229675  
    8282        // CachedRawResourceClient
    8383        void dataSent(CachedResource&, unsigned long long bytesSent, unsigned long long totalBytesToBeSent) override;
    84         void responseReceived(CachedResource&, const ResourceResponse&) override;
     84        void responseReceived(CachedResource&, const ResourceResponse&, CompletionHandler<void()>&&) override;
    8585        void dataReceived(CachedResource&, const char* data, int dataLength) override;
    8686        void redirectReceived(CachedResource&, ResourceRequest&&, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&&) override;
  • trunk/Source/WebCore/loader/MediaResourceLoader.cpp

    r226508 r229675  
    140140}
    141141
    142 void MediaResource::responseReceived(CachedResource& resource, const ResourceResponse& response)
    143 {
    144     ASSERT_UNUSED(resource, &resource == m_resource);
     142void MediaResource::responseReceived(CachedResource& resource, const ResourceResponse& response, CompletionHandler<void()>&& completionHandler)
     143{
     144    ASSERT_UNUSED(resource, &resource == m_resource);
     145    CompletionHandlerCallingScope completionHandlerCaller(WTFMove(completionHandler));
    145146
    146147    if (!m_loader->document())
  • trunk/Source/WebCore/loader/MediaResourceLoader.h

    r226508 r229675  
    8383
    8484    // CachedRawResourceClient
    85     void responseReceived(CachedResource&, const ResourceResponse&) override;
     85    void responseReceived(CachedResource&, const ResourceResponse&, CompletionHandler<void()>&&) override;
    8686    void redirectReceived(CachedResource&, ResourceRequest&&, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&&) override;
    8787    bool shouldCacheResponse(CachedResource&, const ResourceResponse&) override;
  • trunk/Source/WebCore/loader/appcache/ApplicationCacheResourceLoader.cpp

    r228901 r229675  
    7676}
    7777
    78 void ApplicationCacheResourceLoader::responseReceived(CachedResource& resource, const ResourceResponse& response)
     78void ApplicationCacheResourceLoader::responseReceived(CachedResource& resource, const ResourceResponse& response, CompletionHandler<void()>&& completionHandler)
    7979{
    8080    ASSERT_UNUSED(resource, &resource == m_resource);
     81    CompletionHandlerCallingScope completionHandlerCaller(WTFMove(completionHandler));
    8182
    8283    if (response.httpStatusCode() == 404 || response.httpStatusCode() == 410) {
  • trunk/Source/WebCore/loader/appcache/ApplicationCacheResourceLoader.h

    r228901 r229675  
    5555
    5656    // CachedRawResourceClient
    57     void responseReceived(CachedResource&, const ResourceResponse&) final;
     57    void responseReceived(CachedResource&, const ResourceResponse&, CompletionHandler<void()>&&) final;
    5858    void dataReceived(CachedResource&, const char* data, int dataLength) final;
    5959    void redirectReceived(CachedResource&, ResourceRequest&&, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&&) final;
  • trunk/Source/WebCore/loader/cache/CachedRawResource.cpp

    r228435 r229675  
    159159        if (!hasClient(*client))
    160160            return;
     161        auto responseProcessedHandler = [this, protectedThis = WTFMove(protectedThis), client] {
     162            if (!hasClient(*client))
     163                return;
     164            if (m_data)
     165                client->dataReceived(*this, m_data->data(), m_data->size());
     166            if (!hasClient(*client))
     167                return;
     168            CachedResource::didAddClient(*client);
     169        };
     170
    161171        if (!m_response.isNull()) {
    162172            ResourceResponse response(m_response);
     
    167177                response.setSource(ResourceResponse::Source::MemoryCache);
    168178            }
    169             client->responseReceived(*this, response);
    170         }
    171         if (!hasClient(*client))
    172             return;
    173         if (m_data)
    174             client->dataReceived(*this, m_data->data(), m_data->size());
    175         if (!hasClient(*client))
    176             return;
    177         CachedResource::didAddClient(*client);
     179            client->responseReceived(*this, response, WTFMove(responseProcessedHandler));
     180        } else
     181            responseProcessedHandler();
    178182    });
    179183}
     
    216220    CachedResourceClientWalker<CachedRawResourceClient> w(m_clients);
    217221    while (CachedRawResourceClient* c = w.next())
    218         c->responseReceived(*this, m_response);
     222        c->responseReceived(*this, m_response, nullptr);
    219223}
    220224
  • trunk/Source/WebCore/loader/cache/CachedRawResourceClient.h

    r226508 r229675  
    4040
    4141    virtual void dataSent(CachedResource&, unsigned long long /* bytesSent */, unsigned long long /* totalBytesToBeSent */) { }
    42     virtual void responseReceived(CachedResource&, const ResourceResponse&) { }
     42    virtual void responseReceived(CachedResource&, const ResourceResponse&, CompletionHandler<void()>&& completionHandler)
     43    {
     44        if (completionHandler)
     45            completionHandler();
     46    }
     47
    4348    virtual bool shouldCacheResponse(CachedResource&, const ResourceResponse&) { return true; }
    4449    virtual void dataReceived(CachedResource&, const char* /* data */, int /* length */) { }
  • trunk/Source/WebCore/loader/cache/KeepaliveRequestTracker.cpp

    r220751 r229675  
    6767}
    6868
    69 void KeepaliveRequestTracker::responseReceived(CachedResource& resource, const ResourceResponse&)
     69void KeepaliveRequestTracker::responseReceived(CachedResource& resource, const ResourceResponse&, CompletionHandler<void()>&& completionHandler)
    7070{
    7171    // Per Fetch specification, allocated quota should be returned before the promise is resolved,
    7272    // which is when the response is received.
    7373    unregisterRequest(resource);
     74
     75    if (completionHandler)
     76        completionHandler();
    7477}
    7578
  • trunk/Source/WebCore/loader/cache/KeepaliveRequestTracker.h

    r220751 r229675  
    3939
    4040    // CachedRawResourceClient.
    41     void responseReceived(CachedResource&, const ResourceResponse&) final;
     41    void responseReceived(CachedResource&, const ResourceResponse&, CompletionHandler<void()>&&) final;
    4242    void notifyFinished(CachedResource&) final;
    4343
  • trunk/Source/WebCore/platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp

    r225499 r229675  
    112112}
    113113
    114 void WebCoreAVCFResourceLoader::responseReceived(CachedResource& resource, const ResourceResponse& response)
     114void WebCoreAVCFResourceLoader::responseReceived(CachedResource& resource, const ResourceResponse& response, CompletionHandler<void()>&& completionHandler)
    115115{
    116116    ASSERT_UNUSED(resource, &resource == m_resource);
     117    CompletionHandlerCallingScope completionHandlerCaller(WTFMove(completionHandler));
    117118
    118119    int status = response.httpStatusCode();
  • trunk/Source/WebCore/platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.h

    r216702 r229675  
    5757private:
    5858    // CachedRawResourceClient
    59     void responseReceived(CachedResource&, const ResourceResponse&) override;
     59    void responseReceived(CachedResource&, const ResourceResponse&, CompletionHandler<void()>&&) override;
    6060    void dataReceived(CachedResource&, const char*, int) override;
    6161    void notifyFinished(CachedResource&) override;
  • trunk/Source/WebCore/platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.h

    r216702 r229675  
    5757private:
    5858    // CachedResourceClient
    59     void responseReceived(CachedResource&, const ResourceResponse&) override;
     59    void responseReceived(CachedResource&, const ResourceResponse&, CompletionHandler<void()>&&) override;
    6060    void dataReceived(CachedResource&, const char*, int) override;
    6161    void notifyFinished(CachedResource&) override;
  • trunk/Source/WebCore/platform/graphics/avfoundation/objc/WebCoreAVFResourceLoader.mm

    r225499 r229675  
    109109}
    110110
    111 void WebCoreAVFResourceLoader::responseReceived(CachedResource& resource, const ResourceResponse& response)
     111void WebCoreAVFResourceLoader::responseReceived(CachedResource& resource, const ResourceResponse& response, CompletionHandler<void()>&& completionHandler)
    112112{
    113113    ASSERT_UNUSED(resource, &resource == m_resource);
     114    CompletionHandlerCallingScope completionHandlerCaller(WTFMove(completionHandler));
    114115
    115116    int status = response.httpStatusCode();
  • trunk/Tools/ChangeLog

    r229671 r229675  
     12018-03-16  Chris Dumez  <cdumez@apple.com>
     2
     3        WebKit.RestoreSessionStateContainingScrollRestorationDefault API test is failing with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183679
     5
     6        Reviewed by Alex Christensen.
     7
     8        Add API test coverage.
     9
     10        * TestWebKitAPI/Tests/WebKit/RestoreSessionState.cpp:
     11        (TestWebKitAPI::decidePolicyForNavigationAction):
     12        (TestWebKitAPI::decidePolicyForResponse):
     13        (TestWebKitAPI::TEST):
     14
    1152018-03-16  Zalan Bujtas  <zalan@apple.com>
    216
  • trunk/Tools/TestWebKitAPI/Tests/WebKit/RestoreSessionState.cpp

    r221505 r229675  
    3333#include "Test.h"
    3434#include <WebKit/WKSessionStateRef.h>
     35#include <wtf/RunLoop.h>
    3536
    3637namespace TestWebKitAPI {
     
    4142{
    4243    didFinishLoad = true;
     44}
     45
     46static void decidePolicyForNavigationAction(WKPageRef page, WKFrameRef frame, WKFrameNavigationType navigationType, WKEventModifiers modifiers, WKEventMouseButton mouseButton, WKFrameRef originatingFrame, WKURLRequestRef request, WKFramePolicyListenerRef listener, WKTypeRef userData, const void* clientInfo)
     47{
     48    WKRetainPtr<WKFramePolicyListenerRef> retainedListener(listener);
     49    RunLoop::main().dispatch([retainedListener = WTFMove(retainedListener)] {
     50        WKFramePolicyListenerUse(retainedListener.get());
     51    });
     52}
     53
     54static void decidePolicyForResponse(WKPageRef page, WKFrameRef frame, WKURLResponseRef response, WKURLRequestRef request, bool canShowMIMEType, WKFramePolicyListenerRef listener, WKTypeRef userData, const void* clientInfo)
     55{
     56    WKRetainPtr<WKFramePolicyListenerRef> retainedListener(listener);
     57    RunLoop::main().dispatch([retainedListener = WTFMove(retainedListener)] {
     58        WKFramePolicyListenerUse(retainedListener.get());
     59    });
    4360}
    4461
     
    85102}
    86103
     104TEST(WebKit, RestoreSessionStateContainingScrollRestorationDefaultWithAsyncPolicyDelegates)
     105{
     106    WKRetainPtr<WKContextRef> context(AdoptWK, WKContextCreate());
     107
     108    PlatformWebView webView(context.get());
     109    setPageLoaderClient(webView.page());
     110
     111    WKPagePolicyClientV1 policyClient;
     112    memset(&policyClient, 0, sizeof(policyClient));
     113    policyClient.base.version = 1;
     114    policyClient.decidePolicyForNavigationAction = decidePolicyForNavigationAction;
     115    policyClient.decidePolicyForResponse = decidePolicyForResponse;
     116    WKPageSetPagePolicyClient(webView.page(), &policyClient.base);
     117
     118    WKRetainPtr<WKDataRef> data = createSessionStateData(context.get());
     119    EXPECT_NOT_NULL(data);
     120
     121    auto sessionState = adoptWK(WKSessionStateCreateFromData(data.get()));
     122    WKPageRestoreFromSessionState(webView.page(), sessionState.get());
     123
     124    Util::run(&didFinishLoad);
     125
     126    EXPECT_JS_EQ(webView.page(), "history.scrollRestoration", "auto");
     127}
     128
    87129} // namespace TestWebKitAPI
    88130
Note: See TracChangeset for help on using the changeset viewer.