ServiceWorkerClientFetch should send data to its resource loader once the didReceiveR...
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Mar 2018 20:47:11 +0000 (20:47 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Mar 2018 20:47:11 +0000 (20:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183110

Reviewed by Chris Dumez.

Buffering data/finish event/fail event until the response completion handler is called.

* WebProcess/Storage/ServiceWorkerClientFetch.cpp:
(WebKit::ServiceWorkerClientFetch::didReceiveResponse):
(WebKit::ServiceWorkerClientFetch::didReceiveData):
(WebKit::ServiceWorkerClientFetch::didFinish):
(WebKit::ServiceWorkerClientFetch::didFail):
(WebKit::ServiceWorkerClientFetch::didNotHandle):
(WebKit::ServiceWorkerClientFetch::cancel):
(WebKit::ServiceWorkerClientFetch::continueLoadingAfterCheckingResponse):
* WebProcess/Storage/ServiceWorkerClientFetch.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229477 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp
Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h
Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm

index 034a963..bc4113f 100644 (file)
@@ -1,3 +1,22 @@
+2018-03-09  Youenn Fablet  <youenn@apple.com>
+
+        ServiceWorkerClientFetch should send data to its resource loader once the didReceiveResponse completion handler is called
+        https://bugs.webkit.org/show_bug.cgi?id=183110
+
+        Reviewed by Chris Dumez.
+
+        Buffering data/finish event/fail event until the response completion handler is called.
+
+        * WebProcess/Storage/ServiceWorkerClientFetch.cpp:
+        (WebKit::ServiceWorkerClientFetch::didReceiveResponse):
+        (WebKit::ServiceWorkerClientFetch::didReceiveData):
+        (WebKit::ServiceWorkerClientFetch::didFinish):
+        (WebKit::ServiceWorkerClientFetch::didFail):
+        (WebKit::ServiceWorkerClientFetch::didNotHandle):
+        (WebKit::ServiceWorkerClientFetch::cancel):
+        (WebKit::ServiceWorkerClientFetch::continueLoadingAfterCheckingResponse):
+        * WebProcess/Storage/ServiceWorkerClientFetch.h:
+
 2018-03-09  Jer Noble  <jer.noble@apple.com>
 
         Add new CSS env constants for use with fullscreen
index 1f72a30..2808f87 100644 (file)
@@ -106,6 +106,7 @@ void ServiceWorkerClientFetch::didReceiveResponse(ResourceResponse&& response)
 
         if (auto error = validateResponse(response)) {
             m_loader->didFail(error.value());
+            ASSERT(!m_loader);
             if (auto callback = WTFMove(m_callback))
                 callback(Result::Succeeded);
             return;
@@ -141,20 +142,36 @@ void ServiceWorkerClientFetch::didReceiveResponse(ResourceResponse&& response)
         if (response.url().isNull())
             response.setURL(m_loader->request().url());
 
+        m_isCheckingResponse = true;
         m_loader->didReceiveResponse(response, [this, protectedThis = WTFMove(protectedThis)] {
+            m_isCheckingResponse = false;
+            continueLoadingAfterCheckingResponse();
             if (auto callback = WTFMove(m_callback))
                 callback(Result::Succeeded);
         });
     });
 }
 
-void ServiceWorkerClientFetch::didReceiveData(const IPC::DataReference& data, int64_t encodedDataLength)
+void ServiceWorkerClientFetch::didReceiveData(const IPC::DataReference& dataReference, int64_t encodedDataLength)
 {
-    callOnMainThread([this, protectedThis = makeRef(*this), data = data.vector(), encodedDataLength] {
+    auto* data = reinterpret_cast<const char*>(dataReference.data());
+    if (!m_buffer) {
+        m_buffer = SharedBuffer::create(data, dataReference.size());
+        m_encodedDataLength = encodedDataLength;
+    } else {
+        m_buffer->append(data, dataReference.size());
+        m_encodedDataLength += encodedDataLength;
+    }
+
+    if (m_isCheckingResponse)
+        return;
+
+    callOnMainThread([this, protectedThis = makeRef(*this)] {
         if (!m_loader)
             return;
 
-        m_loader->didReceiveData(reinterpret_cast<const char*>(data.data()), data.size(), encodedDataLength, DataPayloadBytes);
+        m_loader->didReceiveBuffer(m_buffer.releaseNonNull(), m_encodedDataLength, DataPayloadBytes);
+        m_encodedDataLength = 0;
     });
 }
 
@@ -165,6 +182,11 @@ void ServiceWorkerClientFetch::didReceiveFormData(const IPC::FormDataReference&)
 
 void ServiceWorkerClientFetch::didFinish()
 {
+    m_didFinish = true;
+
+    if (m_isCheckingResponse)
+        return;
+
     callOnMainThread([this, protectedThis = makeRef(*this)] {
         if (!m_loader)
             return;
@@ -192,6 +214,11 @@ void ServiceWorkerClientFetch::didFinish()
 
 void ServiceWorkerClientFetch::didFail()
 {
+    m_didFail = true;
+
+    if (m_isCheckingResponse)
+        return;
+
     callOnMainThread([this, protectedThis = makeRef(*this)] {
         if (!m_loader)
             return;
@@ -207,6 +234,8 @@ void ServiceWorkerClientFetch::didFail()
 
 void ServiceWorkerClientFetch::didNotHandle()
 {
+    ASSERT(!m_isCheckingResponse);
+
     callOnMainThread([this, protectedThis = makeRef(*this)] {
         if (!m_loader)
             return;
@@ -223,6 +252,31 @@ void ServiceWorkerClientFetch::cancel()
     if (auto callback = WTFMove(m_callback))
         callback(Result::Cancelled);
     m_loader = nullptr;
+    m_buffer = nullptr;
+}
+
+void ServiceWorkerClientFetch::continueLoadingAfterCheckingResponse()
+{
+    ASSERT(!m_isCheckingResponse);
+    if (!m_loader)
+        return;
+
+    if (m_encodedDataLength) {
+        callOnMainThread([this, protectedThis = makeRef(*this)] {
+            if (!m_loader || !m_encodedDataLength)
+                return;
+            m_loader->didReceiveBuffer(m_buffer.releaseNonNull(), m_encodedDataLength, DataPayloadBytes);
+            m_encodedDataLength = 0;
+        });
+    }
+
+    if (m_didFail) {
+        didFail();
+        return;
+    }
+
+    if (m_didFinish)
+        didFinish();
 }
 
 } // namespace WebKit
index 5e31038..8da1419 100644 (file)
@@ -66,6 +66,8 @@ private:
     void didFail();
     void didNotHandle();
 
+    void continueLoadingAfterCheckingResponse();
+
     WebServiceWorkerProvider& m_serviceWorkerProvider;
     RefPtr<WebCore::ResourceLoader> m_loader;
     uint64_t m_identifier { 0 };
@@ -74,6 +76,11 @@ private:
     enum class RedirectionStatus { None, Receiving, Following, Received };
     RedirectionStatus m_redirectionStatus { RedirectionStatus::None };
     bool m_shouldClearReferrerOnHTTPSToHTTPRedirect { true };
+    RefPtr<WebCore::SharedBuffer> m_buffer;
+    int64_t m_encodedDataLength { 0 };
+    bool m_isCheckingResponse { false };
+    bool m_didFinish { false };
+    bool m_didFail { false };
 };
 
 } // namespace WebKit
index 42f4426..43f0be7 100644 (file)
@@ -153,6 +153,48 @@ static String retrievedString;
 
 @end
 
+static bool shouldAccept = true;
+static bool navigationComplete = false;
+static bool navigationFailed = false;
+
+@interface TestSWAsyncNavigationDelegate : NSObject <WKNavigationDelegate, WKUIDelegate>
+@end
+
+@implementation TestSWAsyncNavigationDelegate
+
+- (void)webView:(WKWebView *)webView didFinishNavigation:(null_unspecified WKNavigation *)navigation
+{
+    navigationComplete = true;
+}
+
+- (void)webView:(WKWebView *)webView didFailNavigation:(null_unspecified WKNavigation *)navigation withError:(NSError *)error
+{
+    navigationFailed = true;
+    navigationComplete = true;
+}
+
+- (void)webView:(WKWebView *)webView didFailProvisionalNavigation:(null_unspecified WKNavigation *)navigation withError:(NSError *)error
+{
+    navigationFailed = true;
+    navigationComplete = true;
+}
+
+- (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
+{
+    decisionHandler(WKNavigationActionPolicyAllow);
+}
+
+- (void)webView:(WKWebView *)webView decidePolicyForNavigationResponse:(WKNavigationResponse *)navigationResponse decisionHandler:(void (^)(WKNavigationResponsePolicy))decisionHandler
+{
+    int64_t deferredWaitTime = 100 * NSEC_PER_MSEC;
+    dispatch_time_t when = dispatch_time(DISPATCH_TIME_NOW, deferredWaitTime);
+    dispatch_after(when, dispatch_get_main_queue(), ^{
+        decisionHandler(shouldAccept ? WKNavigationResponsePolicyAllow : WKNavigationResponsePolicyCancel);
+    });
+}
+@end
+
+
 static const char* mainBytes = R"SWRESOURCE(
 <script>
 
@@ -618,6 +660,97 @@ TEST(ServiceWorkers, InterceptFirstLoadAfterRestoreFromDisk)
     done = false;
 }
 
+TEST(ServiceWorkers, WaitForPolicyDelegate)
+{
+    [WKWebsiteDataStore _allowWebsiteDataRecordsForAllOrigins];
+
+    // Start with a clean slate data store
+    [[WKWebsiteDataStore defaultDataStore] removeDataOfTypes:[WKWebsiteDataStore allWebsiteDataTypes] modifiedSince:[NSDate distantPast] completionHandler:^() {
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+
+    RetainPtr<SWMessageHandlerWithExpectedMessage> messageHandler = adoptNS([[SWMessageHandlerWithExpectedMessage alloc] init]);
+    [[configuration userContentController] addScriptMessageHandler:messageHandler.get() name:@"sw"];
+
+    RetainPtr<SWSchemes> handler = adoptNS([[SWSchemes alloc] init]);
+    handler->resources.set("sw://host/main.html", ResourceInfo { @"text/html", mainForFirstLoadInterceptTestBytes });
+    handler->resources.set("sw://host/sw.js", ResourceInfo { @"application/javascript", scriptInterceptingFirstLoadBytes });
+    [configuration setURLSchemeHandler:handler.get() forURLScheme:@"SW"];
+
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+    [webView.get().configuration.processPool _registerURLSchemeServiceWorkersCanHandle:@"sw"];
+
+    // Register a service worker and activate it.
+    expectedMessage = "Service Worker activated";
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"sw://host/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+
+    webView = nullptr;
+    configuration = nullptr;
+    messageHandler = nullptr;
+    handler = nullptr;
+
+    done = false;
+
+    configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    messageHandler = adoptNS([[SWMessageHandlerWithExpectedMessage alloc] init]);
+    [[configuration userContentController] addScriptMessageHandler:messageHandler.get() name:@"sw"];
+
+    handler = adoptNS([[SWSchemes alloc] init]);
+    handler->resources.set("sw://host/main.html", ResourceInfo { @"text/html", mainForFirstLoadInterceptTestBytes });
+    handler->resources.set("sw://host/sw.js", ResourceInfo { @"application/javascript", scriptInterceptingFirstLoadBytes });
+    [configuration setURLSchemeHandler:handler.get() forURLScheme:@"SW"];
+
+    webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+    [webView.get().configuration.processPool _registerURLSchemeServiceWorkersCanHandle:@"sw"];
+
+    // Verify service worker is intercepting load.
+    expectedMessage = "Intercepted by worker";
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"sw://host/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+    [webView.get().configuration.processPool _registerURLSchemeServiceWorkersCanHandle:@"sw"];
+    auto delegate = adoptNS([[TestSWAsyncNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+    [webView setUIDelegate:delegate.get()];
+
+    shouldAccept = true;
+    navigationFailed = false;
+    navigationComplete = false;
+
+    // Verify service worker load goes well when policy delegate is ok.
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"sw://host/main.html"]];
+    [webView loadRequest:request];
+    TestWebKitAPI::Util::run(&navigationComplete);
+
+    EXPECT_FALSE(navigationFailed);
+
+    webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+    [webView.get().configuration.processPool _registerURLSchemeServiceWorkersCanHandle:@"sw"];
+    [webView setNavigationDelegate:delegate.get()];
+    [webView setUIDelegate:delegate.get()];
+
+    shouldAccept = false;
+    navigationFailed = false;
+    navigationComplete = false;
+
+    // Verify service worker load fails well when policy delegate is not ok.
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"sw://host/main.html"]];
+    [webView loadRequest:request];
+    TestWebKitAPI::Util::run(&navigationComplete);
+
+    EXPECT_TRUE(navigationFailed);
+}
 #if WK_HAVE_C_SPI
 
 void setConfigurationInjectedBundlePath(WKWebViewConfiguration* configuration)