Don't start service worker fetch when there is substitute data
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Jun 2018 14:59:03 +0000 (14:59 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Jun 2018 14:59:03 +0000 (14:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186349
<rdar://problem/38881568>

Reviewed by Youenn Fablet.

Loading content via WKWebView.loadData may also end up starting a main resource service worker fetch.
This breaks DocumentWriter assumptions.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::tryLoadingRequestFromApplicationCache):
(WebCore::DocumentLoader::tryLoadingSubstituteData):

Factor substitute resource loading out from tryLoadingRequestFromApplicationCache.

(WebCore::DocumentLoader::startLoadingMainResource):

If we have substitute data already (typically from WKWebView.loadData), allow service worker registration
but load the main resource using the substitute data.

(WebCore::DocumentLoader::handleSubstituteDataLoadSoon): Deleted.

Merge to tryLoadingSubstituteData.

* loader/DocumentLoader.h:

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

Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/DocumentLoader.h
Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm

index 2929856..2eb7e10 100644 (file)
@@ -1,3 +1,31 @@
+2018-06-07  Antti Koivisto  <antti@apple.com>
+
+        Don't start service worker fetch when there is substitute data
+        https://bugs.webkit.org/show_bug.cgi?id=186349
+        <rdar://problem/38881568>
+
+        Reviewed by Youenn Fablet.
+
+        Loading content via WKWebView.loadData may also end up starting a main resource service worker fetch.
+        This breaks DocumentWriter assumptions.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::tryLoadingRequestFromApplicationCache):
+        (WebCore::DocumentLoader::tryLoadingSubstituteData):
+
+        Factor substitute resource loading out from tryLoadingRequestFromApplicationCache.
+
+        (WebCore::DocumentLoader::startLoadingMainResource):
+
+        If we have substitute data already (typically from WKWebView.loadData), allow service worker registration
+        but load the main resource using the substitute data.
+
+        (WebCore::DocumentLoader::handleSubstituteDataLoadSoon): Deleted.
+
+        Merge to tryLoadingSubstituteData.
+
+        * loader/DocumentLoader.h:
+
 2018-06-07  Thibault Saunier  <tsaunier@igalia.com>
 
         [GStreamer] Fix the way GstStreamCollection is handled
index a7ee202..fb07ee5 100644 (file)
@@ -478,14 +478,6 @@ void DocumentLoader::startDataLoadTimer()
 #endif
 }
 
-void DocumentLoader::handleSubstituteDataLoadSoon()
-{
-    if (!m_deferMainResourceDataLoad || frameLoader()->loadsSynchronously())
-        handleSubstituteDataLoadNow();
-    else
-        startDataLoadTimer();
-}
-
 #if ENABLE(SERVICE_WORKER)
 void DocumentLoader::matchRegistration(const URL& url, SWClientConnection::RegistrationCallback&& callback)
 {
@@ -671,15 +663,24 @@ void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const Resourc
 bool DocumentLoader::tryLoadingRequestFromApplicationCache()
 {
     m_applicationCacheHost->maybeLoadMainResource(m_request, m_substituteData);
+    return tryLoadingSubstituteData();
+}
 
+bool DocumentLoader::tryLoadingSubstituteData()
+{
     if (!m_substituteData.isValid() || !m_frame->page())
         return false;
 
-    RELEASE_LOG_IF_ALLOWED("startLoadingMainResource: Returning cached main resource (frame = %p, main = %d)", m_frame, m_frame->isMainFrame());
+    RELEASE_LOG_IF_ALLOWED("startLoadingMainResource: Returning substitute data (frame = %p, main = %d)", m_frame, m_frame->isMainFrame());
     m_identifierForLoadWithoutResourceLoader = m_frame->page()->progress().createUniqueIdentifier();
     frameLoader()->notifier().assignIdentifierToInitialRequest(m_identifierForLoadWithoutResourceLoader, this, m_request);
     frameLoader()->notifier().dispatchWillSendRequest(this, m_identifierForLoadWithoutResourceLoader, m_request, ResourceResponse());
-    handleSubstituteDataLoadSoon();
+
+    if (!m_deferMainResourceDataLoad || frameLoader()->loadsSynchronously())
+        handleSubstituteDataLoadNow();
+    else
+        startDataLoadTimer();
+
     return true;
 }
 
@@ -1728,6 +1729,11 @@ void DocumentLoader::startLoadingMainResource(ShouldContinue shouldContinue)
                 return;
 
             m_serviceWorkerRegistrationData = WTFMove(registrationData);
+
+            // Prefer existing substitute data (from WKWebView.loadData etc) over service worker fetch.
+            if (this->tryLoadingSubstituteData())
+                return;
+            // Try app cache only if there is no service worker.
             if (!m_serviceWorkerRegistrationData && this->tryLoadingRequestFromApplicationCache())
                 return;
             this->loadMainResource(WTFMove(request));
index 6be8e41..9f9a822 100644 (file)
@@ -377,6 +377,7 @@ private:
     bool isPostOrRedirectAfterPost(const ResourceRequest&, const ResourceResponse&);
 
     bool tryLoadingRequestFromApplicationCache();
+    bool tryLoadingSubstituteData();
     bool tryLoadingRedirectRequestFromApplicationCache(const ResourceRequest&);
 #if ENABLE(SERVICE_WORKER)
     void restartLoadingDueToServiceWorkerRegistrationChange(ResourceRequest&&, std::optional<ServiceWorkerRegistrationData>&&);
@@ -391,7 +392,6 @@ private:
 #else
     typedef Timer DocumentLoaderTimer;
 #endif
-    void handleSubstituteDataLoadSoon();
     void handleSubstituteDataLoadNow();
     void startDataLoadTimer();
 
index 91668b9..09d1a4d 100644 (file)
@@ -1370,4 +1370,52 @@ TEST(ServiceWorkers, ProcessPerOrigin)
     EXPECT_EQ(0U, processPool._serviceWorkerProcessCount);
 }
 
+TEST(ServiceWorkers, LoadData)
+{
+    ASSERT(mainBytes);
+    ASSERT(scriptBytes);
+
+    [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<SWMessageHandler> messageHandler = adoptNS([[SWMessageHandler 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", mainBytes });
+    handler->resources.set("sw://host/sw.js", ResourceInfo { @"application/javascript", scriptBytes });
+    [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"];
+
+    auto delegate = adoptNS([[TestSWAsyncNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+    [webView setUIDelegate:delegate.get()];
+
+    done = false;
+
+    // Normal load to get SW registered.
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"sw://host/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    // Now try a data load.
+    NSData *data = [NSData dataWithBytes:mainBytes length:strlen(mainBytes)];
+    [webView loadData:data MIMEType:@"text/html" characterEncodingName:@"UTF-8" baseURL:[NSURL URLWithString:@"sw://host/main.html"]];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+}
+
 #endif // WK_API_ENABLED