Diff
Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (227316 => 227317)
--- branches/safari-605-branch/Source/WebCore/ChangeLog 2018-01-22 17:57:48 UTC (rev 227316)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog 2018-01-22 17:57:50 UTC (rev 227317)
@@ -1,5 +1,38 @@
2018-01-22 Jason Marcell <jmarc...@apple.com>
+ Cherry-pick r227220. rdar://problem/36722596
+
+ 2018-01-19 Chris Dumez <cdu...@apple.com>
+
+ Service worker registrations restored from disk may not be reused when the JS calls register() again
+ https://bugs.webkit.org/show_bug.cgi?id=181810
+ <rdar://problem/36591711>
+
+ Reviewed by Youenn Fablet.
+
+ The issue was that when restoring a registration from disk, we would not set its active worker right
+ away. We only set it later in installContextData(). installContextData() is only called after we’ve
+ launched the service worker process and established a connection to it.
+
+ However, we would start processing jobs (such as registrations) before we’ve established the connection
+ to the service worker process. SWServerJobQueue::runRegisterJob(), in order to reuse an existing
+ registration checks the registration’s active worker has the right script URL. The issue was that when
+ this code would execute, we may not have set the registration’s active service worker yet, in which case,
+ we would update the existing registration instead of reusing it as-is.
+
+ To address the issue, we now delay the processing of jobs until the connection to the service worker
+ process has been established and we've installed all pending contexts via installContextData().
+
+ Changed is covered by new API test.
+
+ * workers/service/server/SWServer.cpp:
+ (WebCore::SWServer::Connection::scheduleJobInServer):
+ (WebCore::SWServer::scheduleJob):
+ (WebCore::SWServer::serverToContextConnectionCreated):
+ * workers/service/server/SWServer.h:
+
+2018-01-22 Jason Marcell <jmarc...@apple.com>
+
Cherry-pick r227219. rdar://problem/36722501
2018-01-19 James Craig <jcr...@apple.com>
Modified: branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.cpp (227316 => 227317)
--- branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.cpp 2018-01-22 17:57:48 UTC (rev 227316)
+++ branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.cpp 2018-01-22 17:57:50 UTC (rev 227317)
@@ -209,12 +209,12 @@
m_registrationStore.flushChanges(WTFMove(completionHandler));
}
-void SWServer::Connection::scheduleJobInServer(const ServiceWorkerJobData& jobData)
+void SWServer::Connection::scheduleJobInServer(ServiceWorkerJobData&& jobData)
{
LOG(ServiceWorker, "Scheduling ServiceWorker job %s in server", jobData.identifier().loggingString().utf8().data());
ASSERT(identifier() == jobData.connectionIdentifier());
- m_server.scheduleJob(jobData);
+ m_server.scheduleJob(WTFMove(jobData));
}
void SWServer::Connection::finishFetchingScriptInServer(const ServiceWorkerFetchResult& result)
@@ -256,10 +256,15 @@
}
// https://w3c.github.io/ServiceWorker/#schedule-job-algorithm
-void SWServer::scheduleJob(const ServiceWorkerJobData& jobData)
+void SWServer::scheduleJob(ServiceWorkerJobData&& jobData)
{
ASSERT(m_connections.contains(jobData.connectionIdentifier()));
+ if (!SWServerToContextConnection::globalServerToContextConnection()) {
+ m_pendingJobs.append(WTFMove(jobData));
+ return;
+ }
+
// FIXME: Per the spec, check if this job is equivalent to the last job on the queue.
// If it is, stack it along with that job.
@@ -471,6 +476,10 @@
for (auto& callback : item.value)
callback(success, *connection);
}
+
+ auto pendingJobs = WTFMove(m_pendingJobs);
+ for (auto& jobData : pendingJobs)
+ scheduleJob(WTFMove(jobData));
}
void SWServer::installContextData(const ServiceWorkerContextData& data)
Modified: branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.h (227316 => 227317)
--- branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.h 2018-01-22 17:57:48 UTC (rev 227316)
+++ branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.h 2018-01-22 17:57:50 UTC (rev 227317)
@@ -94,7 +94,7 @@
WEBCORE_EXPORT explicit Connection(SWServer&);
SWServer& server() { return m_server; }
- WEBCORE_EXPORT void scheduleJobInServer(const ServiceWorkerJobData&);
+ WEBCORE_EXPORT void scheduleJobInServer(ServiceWorkerJobData&&);
WEBCORE_EXPORT void finishFetchingScriptInServer(const ServiceWorkerFetchResult&);
WEBCORE_EXPORT void addServiceWorkerRegistrationInServer(ServiceWorkerRegistrationIdentifier);
WEBCORE_EXPORT void removeServiceWorkerRegistrationInServer(ServiceWorkerRegistrationIdentifier);
@@ -131,7 +131,7 @@
void removeRegistration(const ServiceWorkerRegistrationKey&);
WEBCORE_EXPORT Vector<ServiceWorkerRegistrationData> getRegistrations(const SecurityOriginData& topOrigin, const URL& clientURL);
- void scheduleJob(const ServiceWorkerJobData&);
+ void scheduleJob(ServiceWorkerJobData&&);
void rejectJob(const ServiceWorkerJobData&, const ExceptionData&);
void resolveRegistrationJob(const ServiceWorkerJobData&, const ServiceWorkerRegistrationData&, ShouldNotifyWhenResolved);
void resolveUnregistrationJob(const ServiceWorkerJobData&, const ServiceWorkerRegistrationKey&, bool unregistrationResult);
@@ -238,7 +238,8 @@
bool m_mainThreadReplyScheduled { false };
UniqueRef<SWOriginStore> m_originStore;
RegistrationStore m_registrationStore;
- Deque<ServiceWorkerContextData> m_pendingContextDatas;
+ Vector<ServiceWorkerContextData> m_pendingContextDatas;
+ Vector<ServiceWorkerJobData> m_pendingJobs;
HashMap<ServiceWorkerIdentifier, Vector<RunServiceWorkerCallback>> m_serviceWorkerRunRequests;
PAL::SessionID m_sessionID;
bool m_importCompleted { false };
Modified: branches/safari-605-branch/Tools/ChangeLog (227316 => 227317)
--- branches/safari-605-branch/Tools/ChangeLog 2018-01-22 17:57:48 UTC (rev 227316)
+++ branches/safari-605-branch/Tools/ChangeLog 2018-01-22 17:57:50 UTC (rev 227317)
@@ -1,5 +1,23 @@
2018-01-22 Jason Marcell <jmarc...@apple.com>
+ Cherry-pick r227220. rdar://problem/36722596
+
+ 2018-01-19 Chris Dumez <cdu...@apple.com>
+
+ Service worker registrations restored from disk may not be reused when the JS calls register() again
+ https://bugs.webkit.org/show_bug.cgi?id=181810
+ <rdar://problem/36591711>
+
+ Reviewed by Youenn Fablet.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
+ (-[SWMessageHandlerForRestoreFromDiskTest initWithExpectedMessage:]):
+ (-[SWMessageHandlerForRestoreFromDiskTest userContentController:didReceiveScriptMessage:]):
+
+2018-01-22 Jason Marcell <jmarc...@apple.com>
+
Cherry-pick r227153. rdar://problem/36722558
2018-01-18 Chris Dumez <cdu...@apple.com>
Modified: branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm (227316 => 227317)
--- branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm 2018-01-22 17:57:48 UTC (rev 227316)
+++ branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm 2018-01-22 17:57:50 UTC (rev 227317)
@@ -75,6 +75,31 @@
}
@end
+@interface SWMessageHandlerForRestoreFromDiskTest : NSObject <WKScriptMessageHandler> {
+ NSString *_expectedMessage;
+}
+- (instancetype)initWithExpectedMessage:(NSString *)expectedMessage;
+@end
+
+@implementation SWMessageHandlerForRestoreFromDiskTest
+
+- (instancetype)initWithExpectedMessage:(NSString *)expectedMessage
+{
+ if (!(self = [super init]))
+ return nil;
+
+ _expectedMessage = expectedMessage;
+
+ return self;
+}
+
+- (void)userContentController:(WKUserContentController *)userContentController didReceiveScriptMessage:(WKScriptMessage *)message
+{
+ EXPECT_TRUE([[message body] isEqualToString:_expectedMessage]);
+ done = true;
+}
+@end
+
@interface SWSchemes : NSObject <WKURLSchemeHandler> {
@public
HashMap<String, ResourceInfo> resources;
@@ -200,6 +225,62 @@
</body>
)SWRESOURCE";
+static const char* mainRegisteringWorkerBytes = R"SWRESOURCE(
+<script>
+try {
+function log(msg)
+{
+ window.webkit.messageHandlers.sw.postMessage(msg);
+}
+
+navigator.serviceWorker.register('/sw.js').then(function(reg) {
+ if (reg.active) {
+ log("FAIL: Registration already has an active worker");
+ return;
+ }
+ worker = reg.installing;
+ worker.addEventListener('statechange', function() {
+ if (worker.state == 'activated')
+ log("PASS: Registration was successful and service worker was activated");
+ });
+}).catch(function(error) {
+ log("Registration failed with: " + error);
+});
+} catch(e) {
+ log("Exception: " + e);
+}
+</script>
+)SWRESOURCE";
+
+static const char* mainRegisteringAlreadyExistingWorkerBytes = R"SWRESOURCE(
+<script>
+try {
+function log(msg)
+{
+ window.webkit.messageHandlers.sw.postMessage(msg);
+}
+
+navigator.serviceWorker.register('/sw.js').then(function(reg) {
+ if (reg.installing) {
+ log("FAIL: Registration had an installing worker");
+ return;
+ }
+ if (reg.active) {
+ if (reg.active.state == "activated")
+ log("PASS: Registration already has an active worker");
+ else
+ log("FAIL: Registration has an active worker but its state is not activated");
+ } else
+ log("FAIL: Registration does not have an active worker");
+}).catch(function(error) {
+ log("Registration failed with: " + error);
+});
+} catch(e) {
+ log("Exception: " + e);
+}
+</script>
+)SWRESOURCE";
+
TEST(ServiceWorkers, Basic)
{
ASSERT(mainBytes);
@@ -246,6 +327,65 @@
done = false;
}
+TEST(ServiceWorkers, RestoreFromDisk)
+{
+ ASSERT(mainRegisteringWorkerBytes);
+ ASSERT(scriptBytes);
+ ASSERT(mainRegisteringAlreadyExistingWorkerBytes);
+
+ [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<SWMessageHandlerForRestoreFromDiskTest> messageHandler = adoptNS([[SWMessageHandlerForRestoreFromDiskTest alloc] initWithExpectedMessage:@"PASS: Registration was successful and service worker was activated"]);
+ [[configuration userContentController] addScriptMessageHandler:messageHandler.get() name:@"sw"];
+
+ RetainPtr<SWSchemes> handler = adoptNS([[SWSchemes alloc] init]);
+ handler->resources.set("sw://host/main.html", ResourceInfo { @"text/html", mainRegisteringWorkerBytes });
+ 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"];
+
+ 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([[SWMessageHandlerForRestoreFromDiskTest alloc] initWithExpectedMessage:@"PASS: Registration already has an active worker"]);
+ [[configuration userContentController] addScriptMessageHandler:messageHandler.get() name:@"sw"];
+
+ handler = adoptNS([[SWSchemes alloc] init]);
+ handler->resources.set("sw://host/main.html", ResourceInfo { @"text/html", mainRegisteringAlreadyExistingWorkerBytes });
+ handler->resources.set("sw://host/sw.js", ResourceInfo { @"application/_javascript_", scriptBytes });
+ [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"];
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"sw://host/main.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+}
+
TEST(ServiceWorkers, FetchAfterRestoreFromDisk)
{
ASSERT(mainForFetchTestBytes);
