Attachment 342150 Details for Bug 186349 – patch

[patch] patch

substitute-data-sw-3.patch (text/plain), 7.23 KB, created by Antti Koivisto on 2018-06-07 05:17:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2018-06-07 05:17:13 PDT

Size: 7.23 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 232539)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        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-06  Antti Koivisto  <antti@apple.com>
> 
>         Use minimal coverage rect for tiled layers when under memory pressure
>Index: Source/WebCore/loader/DocumentLoader.cpp
>===================================================================
>--- Source/WebCore/loader/DocumentLoader.cpp	(revision 232538)
>+++ Source/WebCore/loader/DocumentLoader.cpp	(working copy)
>@@ -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(Res
> 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::startLoadingMainRes
>                 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: Source/WebCore/loader/DocumentLoader.h
>===================================================================
>--- Source/WebCore/loader/DocumentLoader.h	(revision 232538)
>+++ Source/WebCore/loader/DocumentLoader.h	(working copy)
>@@ -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: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm
>===================================================================
>--- Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm	(revision 232538)
>+++ Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm	(working copy)
>@@ -1370,4 +1370,56 @@ 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);
>+
>+    EXPECT_TRUE(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);
>+
>+    EXPECT_TRUE(done);
>+}
>+
> #endif // WK_API_ENABLED

Actions: View | Formatted Diff | Diff

Attachments on bug 186349: 342046 | 342049 | 342136 | 342148 | 342150 | 342151 | 342157