Diff
Modified: trunk/Source/WebCore/ChangeLog (227695 => 227696)
--- trunk/Source/WebCore/ChangeLog 2018-01-26 21:41:19 UTC (rev 227695)
+++ trunk/Source/WebCore/ChangeLog 2018-01-26 22:11:06 UTC (rev 227696)
@@ -1,3 +1,36 @@
+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-26 Chris Nardi <[email protected]>
Addressing post-review comments after r226614
Modified: trunk/Source/WebCore/workers/service/server/SWServer.cpp (227695 => 227696)
--- trunk/Source/WebCore/workers/service/server/SWServer.cpp 2018-01-26 21:41:19 UTC (rev 227695)
+++ trunk/Source/WebCore/workers/service/server/SWServer.cpp 2018-01-26 22:11:06 UTC (rev 227696)
@@ -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)
@@ -509,9 +514,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);
@@ -520,14 +526,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));
@@ -604,7 +602,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");
@@ -631,6 +637,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.
@@ -640,7 +647,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;
@@ -651,7 +663,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: trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp (227695 => 227696)
--- trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp 2018-01-26 21:41:19 UTC (rev 227695)
+++ trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp 2018-01-26 22:11:06 UTC (rev 227696)
@@ -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: trunk/Source/WebCore/workers/service/server/SWServerWorker.h (227695 => 227696)
--- trunk/Source/WebCore/workers/service/server/SWServerWorker.h 2018-01-26 21:41:19 UTC (rev 227695)
+++ trunk/Source/WebCore/workers/service/server/SWServerWorker.h 2018-01-26 22:11:06 UTC (rev 227696)
@@ -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: trunk/Tools/ChangeLog (227695 => 227696)
--- trunk/Tools/ChangeLog 2018-01-26 21:41:19 UTC (rev 227695)
+++ trunk/Tools/ChangeLog 2018-01-26 22:11:06 UTC (rev 227696)
@@ -1,3 +1,17 @@
+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-26 Chris Nardi <[email protected]>
Addressing post-review comments after r226614
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm (227695 => 227696)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm 2018-01-26 21:41:19 UTC (rev 227695)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm 2018-01-26 22:11:06 UTC (rev 227696)
@@ -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)
