- Revision
- 227220
- Author
- [email protected]
- Date
- 2018-01-19 11:13:24 -0800 (Fri, 19 Jan 2018)
Log Message
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.
Source/WebCore:
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:
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
(-[SWMessageHandlerForRestoreFromDiskTest initWithExpectedMessage:]):
(-[SWMessageHandlerForRestoreFromDiskTest userContentController:didReceiveScriptMessage:]):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (227219 => 227220)
--- trunk/Source/WebCore/ChangeLog 2018-01-19 19:12:35 UTC (rev 227219)
+++ trunk/Source/WebCore/ChangeLog 2018-01-19 19:13:24 UTC (rev 227220)
@@ -1,3 +1,32 @@
+2018-01-19 Chris Dumez <[email protected]>
+
+ 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-19 James Craig <[email protected]>
AX: when invert colors is on, double-invert image and picture elements in UserAgentStyleSheet
Modified: trunk/Source/WebCore/workers/service/server/SWServer.cpp (227219 => 227220)
--- trunk/Source/WebCore/workers/service/server/SWServer.cpp 2018-01-19 19:12:35 UTC (rev 227219)
+++ trunk/Source/WebCore/workers/service/server/SWServer.cpp 2018-01-19 19:13:24 UTC (rev 227220)
@@ -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: trunk/Source/WebCore/workers/service/server/SWServer.h (227219 => 227220)
--- trunk/Source/WebCore/workers/service/server/SWServer.h 2018-01-19 19:12:35 UTC (rev 227219)
+++ trunk/Source/WebCore/workers/service/server/SWServer.h 2018-01-19 19:13:24 UTC (rev 227220)
@@ -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: trunk/Tools/ChangeLog (227219 => 227220)
--- trunk/Tools/ChangeLog 2018-01-19 19:12:35 UTC (rev 227219)
+++ trunk/Tools/ChangeLog 2018-01-19 19:13:24 UTC (rev 227220)
@@ -1,3 +1,17 @@
+2018-01-19 Chris Dumez <[email protected]>
+
+ 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-19 Ryan Haddad <[email protected]>
Remove El Capitan queues from flakiness dashboard
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm (227219 => 227220)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm 2018-01-19 19:12:35 UTC (rev 227219)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm 2018-01-19 19:13:24 UTC (rev 227220)
@@ -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);
