Title: [227819] branches/safari-605-branch

Diff

Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (227818 => 227819)


--- branches/safari-605-branch/Source/WebCore/ChangeLog	2018-01-30 18:50:56 UTC (rev 227818)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog	2018-01-30 18:51:01 UTC (rev 227819)
@@ -1,5 +1,42 @@
 2018-01-30  Jason Marcell  <[email protected]>
 
+        Cherry-pick r227696. rdar://problem/37019435
+
+    2018-01-26  Chris Dumez  <[email protected]>
+
+            Offlined content does not work for apps on home screen
+            https://bugs.webkit.org/show_bug.cgi?id=182070
+            <rdar://problem/36843906>
+
+            Reviewed by Youenn Fablet.
+
+            Already registered service workers were unable to intercept the very first
+            load because registration matching was happening after the registration
+            was loaded from disk, but *before* its active worker was populated.
+
+            We now initialize the registrations' active worker as soon as we load
+            them from disk. We do not necessarily have a SW Context process connection
+            identifier yet at this point so I made it optional on the SWServerWorker.
+            This identifier gets set on the SWServerWorker when the worker is actually
+            launched and gets cleared when the SWServerWorker gets terminated.
+
+            Covered by new API test.
+
+            * workers/service/server/SWServer.cpp:
+            (WebCore::SWServer::addRegistrationFromStore):
+            (WebCore::SWServer::installContextData):
+            (WebCore::SWServer::terminateWorkerInternal):
+            (WebCore::SWServer::workerContextTerminated):
+            (WebCore::SWServer::fireInstallEvent):
+            (WebCore::SWServer::fireActivateEvent):
+            * workers/service/server/SWServerWorker.cpp:
+            (WebCore::SWServerWorker::SWServerWorker):
+            * workers/service/server/SWServerWorker.h:
+            (WebCore::SWServerWorker::contextConnectionIdentifier const):
+            (WebCore::SWServerWorker::setContextConnectionIdentifier):
+
+2018-01-30  Jason Marcell  <[email protected]>
+
         Cherry-pick r227686. rdar://problem/37019446
 
     2018-01-26  Antoine Quint  <[email protected]>

Modified: branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.cpp (227818 => 227819)


--- branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.cpp	2018-01-30 18:50:56 UTC (rev 227818)
+++ branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.cpp	2018-01-30 18:51:01 UTC (rev 227819)
@@ -139,8 +139,13 @@
 
     auto registration = std::make_unique<SWServerRegistration>(*this, data.registration.key, data.registration.updateViaCache, data.registration.scopeURL, data.scriptURL);
     registration->setLastUpdateTime(data.registration.lastUpdateTime);
+    auto registrationPtr = registration.get();
     addRegistration(WTFMove(registration));
-    tryInstallContextData(WTFMove(data));
+
+    auto* connection = SWServerToContextConnection::globalServerToContextConnection();
+    auto worker = SWServerWorker::create(*this, *registrationPtr, connection ? connection->identifier() : SWServerToContextConnectionIdentifier(), data.scriptURL, data.script, data.contentSecurityPolicy, data.workerType, data.serviceWorkerIdentifier);
+    registrationPtr->updateRegistrationState(ServiceWorkerRegistrationState::Active, worker.ptr());
+    worker->setState(ServiceWorkerState::Activated);
 }
 
 void SWServer::addRegistration(std::unique_ptr<SWServerRegistration>&& registration)
@@ -500,9 +505,10 @@
 
 void SWServer::installContextData(const ServiceWorkerContextData& data)
 {
-    if (!data.loadedFromDisk)
-        m_registrationStore.updateRegistration(data);
+    ASSERT_WITH_MESSAGE(!data.loadedFromDisk, "Workers we just read from disk should only be launched as needed");
 
+    m_registrationStore.updateRegistration(data);
+
     auto* connection = SWServerToContextConnection::globalServerToContextConnection();
     ASSERT(connection);
 
@@ -511,14 +517,6 @@
 
     auto worker = SWServerWorker::create(*this, *registration, connection->identifier(), data.scriptURL, data.script, data.contentSecurityPolicy, data.workerType, data.serviceWorkerIdentifier);
 
-    // We don't immediately launch all workers that were just read in from disk,
-    // as it is unlikely they will be needed immediately.
-    if (data.loadedFromDisk) {
-        registration->updateRegistrationState(ServiceWorkerRegistrationState::Active, worker.ptr());
-        worker->setState(ServiceWorkerState::Activated);
-        return;
-    }
-
     registration->setPreInstallationWorker(worker.ptr());
     worker->setState(SWServerWorker::State::Running);
     auto result = m_runningOrTerminatingWorkers.add(data.serviceWorkerIdentifier, WTFMove(worker));
@@ -594,7 +592,15 @@
 
     worker.setState(SWServerWorker::State::Terminating);
 
-    auto* connection = SWServerToContextConnection::connectionForIdentifier(worker.contextConnectionIdentifier());
+    auto contextConnectionIdentifier = worker.contextConnectionIdentifier();
+    ASSERT(contextConnectionIdentifier);
+    if (!contextConnectionIdentifier) {
+        LOG_ERROR("Request to terminate a worker whose contextConnectionIdentifier is invalid");
+        workerContextTerminated(worker);
+        return;
+    }
+
+    auto* connection = SWServerToContextConnection::connectionForIdentifier(*contextConnectionIdentifier);
     ASSERT(connection);
     if (!connection) {
         LOG_ERROR("Request to terminate a worker whose context connection does not exist");
@@ -621,6 +627,7 @@
 void SWServer::workerContextTerminated(SWServerWorker& worker)
 {
     worker.setState(SWServerWorker::State::NotRunning);
+    worker.setContextConnectionIdentifier(std::nullopt);
 
     // At this point if no registrations are referencing the worker then it will be destroyed,
     // removing itself from the m_workersByID map.
@@ -630,7 +637,12 @@
 
 void SWServer::fireInstallEvent(SWServerWorker& worker)
 {
-    auto* connection = SWServerToContextConnection::connectionForIdentifier(worker.contextConnectionIdentifier());
+    auto contextConnectionIdentifier = worker.contextConnectionIdentifier();
+    if (!contextConnectionIdentifier) {
+        LOG_ERROR("Request to fire install event on a worker whose contextConnectionIdentifier is invalid");
+        return;
+    }
+    auto* connection = SWServerToContextConnection::connectionForIdentifier(*contextConnectionIdentifier);
     if (!connection) {
         LOG_ERROR("Request to fire install event on a worker whose context connection does not exist");
         return;
@@ -641,7 +653,13 @@
 
 void SWServer::fireActivateEvent(SWServerWorker& worker)
 {
-    auto* connection = SWServerToContextConnection::connectionForIdentifier(worker.contextConnectionIdentifier());
+    auto contextConnectionIdentifier = worker.contextConnectionIdentifier();
+    if (!contextConnectionIdentifier) {
+        LOG_ERROR("Request to fire install event on a worker whose contextConnectionIdentifier is invalid");
+        return;
+    }
+
+    auto* connection = SWServerToContextConnection::connectionForIdentifier(*contextConnectionIdentifier);
     if (!connection) {
         LOG_ERROR("Request to fire install event on a worker whose context connection does not exist");
         return;

Modified: branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerWorker.cpp (227818 => 227819)


--- branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerWorker.cpp	2018-01-30 18:50:56 UTC (rev 227818)
+++ branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerWorker.cpp	2018-01-30 18:51:01 UTC (rev 227819)
@@ -44,7 +44,7 @@
 }
 
 // FIXME: Use r-value references for script and contentSecurityPolicy
-SWServerWorker::SWServerWorker(SWServer& server, SWServerRegistration& registration, SWServerToContextConnectionIdentifier contextConnectionIdentifier, const URL& scriptURL, const String& script, const ContentSecurityPolicyResponseHeaders& contentSecurityPolicy, WorkerType type, ServiceWorkerIdentifier identifier)
+SWServerWorker::SWServerWorker(SWServer& server, SWServerRegistration& registration, std::optional<SWServerToContextConnectionIdentifier> contextConnectionIdentifier, const URL& scriptURL, const String& script, const ContentSecurityPolicyResponseHeaders& contentSecurityPolicy, WorkerType type, ServiceWorkerIdentifier identifier)
     : m_server(server)
     , m_registrationKey(registration.key())
     , m_contextConnectionIdentifier(contextConnectionIdentifier)

Modified: branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerWorker.h (227818 => 227819)


--- branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerWorker.h	2018-01-30 18:50:56 UTC (rev 227818)
+++ branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerWorker.h	2018-01-30 18:51:01 UTC (rev 227819)
@@ -78,8 +78,8 @@
 
     ServiceWorkerIdentifier identifier() const { return m_data.identifier; }
 
-    SWServerToContextConnectionIdentifier contextConnectionIdentifier() const { return m_contextConnectionIdentifier; }
-    void setContextConnectionIdentifier(SWServerToContextConnectionIdentifier identifier) { m_contextConnectionIdentifier = identifier; }
+    std::optional<SWServerToContextConnectionIdentifier> contextConnectionIdentifier() const { return m_contextConnectionIdentifier; }
+    void setContextConnectionIdentifier(std::optional<SWServerToContextConnectionIdentifier> identifier) { m_contextConnectionIdentifier = identifier; }
 
     ServiceWorkerState state() const { return m_data.state; }
     void setState(ServiceWorkerState);
@@ -106,13 +106,13 @@
     const ClientOrigin& origin() const;
 
 private:
-    SWServerWorker(SWServer&, SWServerRegistration&, SWServerToContextConnectionIdentifier, const URL&, const String& script, const ContentSecurityPolicyResponseHeaders&,  WorkerType, ServiceWorkerIdentifier);
+    SWServerWorker(SWServer&, SWServerRegistration&, std::optional<SWServerToContextConnectionIdentifier>, const URL&, const String& script, const ContentSecurityPolicyResponseHeaders&,  WorkerType, ServiceWorkerIdentifier);
 
     void callWhenActivatedHandler(bool success);
 
     SWServer& m_server;
     ServiceWorkerRegistrationKey m_registrationKey;
-    SWServerToContextConnectionIdentifier m_contextConnectionIdentifier;
+    std::optional<SWServerToContextConnectionIdentifier> m_contextConnectionIdentifier;
     ServiceWorkerData m_data;
     String m_script;
     ContentSecurityPolicyResponseHeaders m_contentSecurityPolicy;

Modified: branches/safari-605-branch/Tools/ChangeLog (227818 => 227819)


--- branches/safari-605-branch/Tools/ChangeLog	2018-01-30 18:50:56 UTC (rev 227818)
+++ branches/safari-605-branch/Tools/ChangeLog	2018-01-30 18:51:01 UTC (rev 227819)
@@ -1,5 +1,23 @@
 2018-01-30  Jason Marcell  <[email protected]>
 
+        Cherry-pick r227696. rdar://problem/37019435
+
+    2018-01-26  Chris Dumez  <[email protected]>
+
+            Offlined content does not work for apps on home screen
+            https://bugs.webkit.org/show_bug.cgi?id=182070
+            <rdar://problem/36843906>
+
+            Reviewed by Youenn Fablet.
+
+            Add API test coverage to make sure an already registered service worker is able to intercept
+            the very first load.
+
+            * TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
+            (-[SWMessageHandlerWithExpectedMessage userContentController:didReceiveScriptMessage:]):
+
+2018-01-30  Jason Marcell  <[email protected]>
+
         Cherry-pick r227647. rdar://problem/37019494
 
     2018-01-25  Alex Christensen  <[email protected]>

Modified: branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm (227818 => 227819)


--- branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm	2018-01-30 18:50:56 UTC (rev 227818)
+++ branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm	2018-01-30 18:51:01 UTC (rev 227819)
@@ -51,7 +51,7 @@
 
 static bool done;
 
-static Deque<String> expectedMessages;
+static String expectedMessage;
 
 @interface SWMessageHandler : NSObject <WKScriptMessageHandler>
 @end
@@ -81,6 +81,17 @@
 - (instancetype)initWithExpectedMessage:(NSString *)expectedMessage;
 @end
 
+@interface SWMessageHandlerWithExpectedMessage : NSObject <WKScriptMessageHandler>
+@end
+
+@implementation SWMessageHandlerWithExpectedMessage
+- (void)userContentController:(WKUserContentController *)userContentController didReceiveScriptMessage:(WKScriptMessage *)message
+{
+    EXPECT_TRUE([[message body] isEqualToString:expectedMessage]);
+    done = true;
+}
+@end
+
 @implementation SWMessageHandlerForRestoreFromDiskTest
 
 - (instancetype)initWithExpectedMessage:(NSString *)expectedMessage
@@ -177,6 +188,11 @@
 <html>
 <body>
 <script>
+function log(msg)
+{
+    window.webkit.messageHandlers.sw.postMessage(msg);
+}
+
 try {
 
 function addFrame()
@@ -219,6 +235,50 @@
 
 )SWRESOURCE";
 
+static const char* scriptInterceptingFirstLoadBytes = R"SWRESOURCE(
+
+self.addEventListener("fetch", (event) => {
+    if (event.request.url.indexOf("main.html") !== -1) {
+        event.respondWith(new Response(new Blob(['Intercepted by worker <script>window.webkit.messageHandlers.sw.postMessage(\'Intercepted by worker\');</script>'], {type: 'text/html'})));
+    }
+});
+
+)SWRESOURCE";
+
+static const char* mainForFirstLoadInterceptTestBytes = R"SWRESOURCE(
+ <html>
+<body>
+<script>
+function log(msg)
+{
+    window.webkit.messageHandlers.sw.postMessage(msg);
+}
+
+try {
+
+navigator.serviceWorker.register('/sw.js').then(function(reg) {
+    if (reg.active) {
+        window.webkit.messageHandlers.sw.postMessage('Service Worker activated');
+        return;
+    }
+
+    worker = reg.installing;
+    worker.addEventListener('statechange', function() {
+        if (worker.state == 'activated')
+            window.webkit.messageHandlers.sw.postMessage('Service Worker activated');
+    });
+}).catch(function(error) {
+    log("Registration failed with: " + error);
+});
+} catch(e) {
+    log("Exception: " + e);
+}
+
+</script>
+</body>
+</html>
+)SWRESOURCE";
+
 static const char* testBytes = R"SWRESOURCE(
 <body>
 NOT intercepted by worker
@@ -446,6 +506,66 @@
     done = false;
 }
 
+TEST(ServiceWorkers, InterceptFirstLoadAfterRestoreFromDisk)
+{
+    ASSERT(mainForFirstLoadInterceptTestBytes);
+    ASSERT(scriptHandlingFetchBytes);
+
+    [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"];
+
+    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"];
+
+    expectedMessage = "Intercepted by worker";
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"sw://host/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+}
+
 #if WK_HAVE_C_SPI
 
 void setConfigurationInjectedBundlePath(WKWebViewConfiguration* configuration)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to