WebSWClientConnection should do IPC to StorageProcess if its WebSWOriginTable is not yet initialized

https://bugs.webkit.org/show_bug.cgi?id=179668

Patch by Youenn Fablet <[email protected]> on 2017-11-14
Reviewed by Chris Dumez.

Source/WebCore:

Covered by existing updated tests.

Removing hasServiceWorkerRegisteredForOrigin and using hasServiceWorkerRegistration instead.
The former is only checking the shared map which might not be initialized at the time the function is called.
The latter is going to the StorageProcess if the map is not yet initialized.

* testing/Internals.cpp:
(WebCore::Internals::hasServiceWorkerRegisteredForOrigin): Deleted.
* testing/Internals.h:
* testing/Internals.idl:
* workers/service/server/SWClientConnection.h:

Source/WebKit:

There may be cases where the origin table is not initialized and we would think there is no service worker registration.
In such a case, we should go to the StorageProcess.
StorageProcess is now sending an IPC message back to each registered SW connection so that WebProcess will know whether its map
is correctly initialized or not.

Renaming hasServiceWorkerRegisteredForOrigin in mayHaveServiceWorkerRegisteredForOrigin.

* WebProcess/Storage/WebSWClientConnection.cpp:
(WebKit::WebSWClientConnection::mayHaveServiceWorkerRegisteredForOrigin const):
(WebKit::WebSWClientConnection::matchRegistration):
(WebKit::WebSWClientConnection::hasServiceWorkerRegisteredForOrigin const): Deleted.
* WebProcess/Storage/WebSWClientConnection.h:
* WebProcess/Storage/WebSWOriginTable.h:
(WebKit::WebSWOriginTable::isInitialized const):
* WebProcess/Storage/WebServiceWorkerProvider.cpp:
(WebKit::shouldHandleFetch):

LayoutTests:

Updated tests to use hasServiceWorkerRegistration instead of hasServiceWorkerRegisteredForOrigin.
Since the latter is trying to match a registration and compares scopes, we need the scopes to be set right on the tests.

* http/tests/workers/service/basic-unregister.https-expected.txt:
* http/tests/workers/service/resources/basic-register.js:
* http/tests/workers/service/resources/basic-unregister.js:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@224832 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

LayoutTests/ChangeLog

@@ -1,3 +1,17 @@
+2017-11-14  Youenn Fablet  <[email protected]>
+
+        WebSWClientConnection should do IPC to StorageProcess if its WebSWOriginTable is not yet initialized
+        https://bugs.webkit.org/show_bug.cgi?id=179668
+
+        Reviewed by Chris Dumez.
+
+        Updated tests to use hasServiceWorkerRegistration instead of hasServiceWorkerRegisteredForOrigin.
+        Since the latter is trying to match a registration and compares scopes, we need the scopes to be set right on the tests.
+
+        * http/tests/workers/service/basic-unregister.https-expected.txt:
+        * http/tests/workers/service/resources/basic-register.js:
+        * http/tests/workers/service/resources/basic-unregister.js:
+
 2017-11-14  Ms2ger  <[email protected]>
 
         Add some bug numbers for failing XHR tests

LayoutTests/http/tests/workers/service/basic-unregister.https-expected.txt

@@ -1,5 +1,5 @@
 PASS: There is initially no service worker registered for the origin
-PASS: registration scope is https://127.0.0.1:8443/workers/service/
+PASS: registration scope is https://127.0.0.1:8443/
 PASS: There is a service worker registered for the origin
 PASS: Unregistration was successful
 PASS: There is no service worker registered for the origin

LayoutTests/http/tests/workers/service/resources/basic-register.js

@@ -5,14 +5,14 @@ function done()
 
 async function test()
 {
-    if (!internals.hasServiceWorkerRegisteredForOrigin(self.origin))
+    if (!await internals.hasServiceWorkerRegistration(self.origin))
         log("PASS: No service worker is initially registered for this origin");
     else
         log("FAIL: A service worker is initially registered for this origin");
 
     testRunner.setPrivateBrowsingEnabled(true);
 
-    if (!internals.hasServiceWorkerRegisteredForOrigin(self.origin))
+    if (!await internals.hasServiceWorkerRegistration(self.origin))
         log("PASS: No service worker is initially registered for this origin in private session");
     else
         log("FAIL: A service worker is initially registered for this origin in private session");
@@ -32,14 +32,14 @@ async function test()
         else
             log("FAIL: registration object's updateViaCache is invalid, got: " + r.updateViaCache);
 
-        if (internals.hasServiceWorkerRegisteredForOrigin(self.origin))
+        if (await internals.hasServiceWorkerRegistration("/test"))
             log("PASS: A service worker is now registered for this origin");
         else
             log("FAIL: No service worker is registered for this origin");
 
         testRunner.setPrivateBrowsingEnabled(true);
 
-        if (!internals.hasServiceWorkerRegisteredForOrigin(self.origin))
+        if (!await internals.hasServiceWorkerRegistration("/test"))
             log("PASS: No service worker is registered for this origin in private session");
         else
             log("FAIL: A service worker is registered for this origin in private session");

LayoutTests/http/tests/workers/service/resources/basic-unregister.js

@@ -6,18 +6,18 @@ function done()
 async function test()
 {
     try {
-        if (!internals.hasServiceWorkerRegisteredForOrigin(self.origin))
+        if (!await internals.hasServiceWorkerRegistration(self.origin))
             log("PASS: There is initially no service worker registered for the origin");
         else
             log("FAIL: There is initially a service worker registered for the origin");
 
-        let registration = await navigator.serviceWorker.register("resources/basic-fetch-worker.js", { });
-        if (registration.scope === "https://127.0.0.1:8443/%3Cspan%20class="x x-first x-last">workers/service/")
+        let registration = await navigator.serviceWorker.register("resources/basic-fetch-worker.js", { scope: "/" });
+        if (registration.scope === "https://127.0.0.1:8443/")
             log("PASS: registration scope is " + registration.scope);
         else
             log("FAIL: registration scope is " + registration.scope);
 
-        if (internals.hasServiceWorkerRegisteredForOrigin(self.origin))
+        if (await internals.hasServiceWorkerRegistration(self.origin))
             log("PASS: There is a service worker registered for the origin");
         else
             log("FAIL: There is no service worker registered for the origin");
@@ -28,7 +28,7 @@ async function test()
         else
             log("FAIL: Unregistration failed");
 
-        if (!internals.hasServiceWorkerRegisteredForOrigin(self.origin))
+        if (!await internals.hasServiceWorkerRegistration(self.origin))
             log("PASS: There is no service worker registered for the origin");
         else
             log("FAIL: There is a service worker registered for the origin");
@@ -39,7 +39,7 @@ async function test()
         else
             log("FAIL: Unregistration succeeded unexpectedly");
 
-        if (!internals.hasServiceWorkerRegisteredForOrigin(self.origin))
+        if (!await internals.hasServiceWorkerRegistration(self.origin))
             log("PASS: There is no service worker registered for the origin");
         else
             log("FAIL: There is a service worker registered for the origin");
@@ -50,7 +50,7 @@ async function test()
         else
             log("FAIL: registration scope is " + registration.scope);
 
-        if (internals.hasServiceWorkerRegisteredForOrigin(self.origin))
+        if (await internals.hasServiceWorkerRegistration("/workers/service/resources/test"))
             log("PASS: There is a service worker registered for the origin");
         else
             log("FAIL: There is no service worker registered for the origin");

Source/WebCore/ChangeLog

@@ -1,3 +1,22 @@
+2017-11-14  Youenn Fablet  <[email protected]>
+
+        WebSWClientConnection should do IPC to StorageProcess if its WebSWOriginTable is not yet initialized
+        https://bugs.webkit.org/show_bug.cgi?id=179668
+
+        Reviewed by Chris Dumez.
+
+        Covered by existing updated tests.
+
+        Removing hasServiceWorkerRegisteredForOrigin and using hasServiceWorkerRegistration instead.
+        The former is only checking the shared map which might not be initialized at the time the function is called.
+        The latter is going to the StorageProcess if the map is not yet initialized.
+
+        * testing/Internals.cpp:
+        (WebCore::Internals::hasServiceWorkerRegisteredForOrigin): Deleted.
+        * testing/Internals.h:
+        * testing/Internals.idl:
+        * workers/service/server/SWClientConnection.h:
+
 2017-11-14  Joseph Pecoraro  <[email protected]>
 
         Web Inspector: Add a ServiceWorker domain to get information about an inspected ServiceWorker

Source/WebCore/testing/Internals.cpp

@@ -4232,19 +4232,6 @@ void Internals::setConsoleMessageListener(RefPtr<StringCallback>&& listener)
     contextDocument()->setConsoleMessageListener(WTFMove(listener));
 }
 
-bool Internals::hasServiceWorkerRegisteredForOrigin(const String& origin)
-{
-#if ENABLE(SERVICE_WORKER)
-    if (!contextDocument())
-        return false;
-
-    return ServiceWorkerProvider::singleton().serviceWorkerConnectionForSession(contextDocument()->sessionID()).hasServiceWorkerRegisteredForOrigin(SecurityOrigin::createFromString(origin));
-#else
-    UNUSED_PARAM(origin);
-    return false;
-#endif
-}
-
 void Internals::setResponseSizeWithPadding(FetchResponse& response, uint64_t size)
 {
     response.setBodySizeWithPadding(size);

Source/WebCore/testing/Internals.h

@@ -625,8 +625,6 @@ class Internals final : public RefCounted<Internals>,  private ContextDestructio
     void hasServiceWorkerRegistration(const String& clientURL, HasRegistrationPromise&&);
 #endif
 
-    bool hasServiceWorkerRegisteredForOrigin(const String&);
-
 #if ENABLE(APPLE_PAY)
     MockPaymentCoordinator& mockPaymentCoordinator() const;
 #endif

Source/WebCore/testing/Internals.idl

@@ -568,8 +568,6 @@ enum EventThrottlingBehavior {
     [Conditional=SERVICE_WORKER, CallWith=ScriptExecutionContext] FetchEvent createBeingDispatchedFetchEvent();
     [Conditional=SERVICE_WORKER] Promise<boolean> hasServiceWorkerRegistration(DOMString scopeURL);
 
-    boolean hasServiceWorkerRegisteredForOrigin(DOMString origin);
-
     [EnabledAtRuntime=WebAnimations] DOMString timelineDescription(AnimationTimeline timeline);
     [EnabledAtRuntime=WebAnimations] void pauseTimeline(AnimationTimeline timeline);
     [EnabledAtRuntime=WebAnimations] void setTimelineCurrentTime(AnimationTimeline timeline, double currentTime);

Source/WebCore/workers/service/server/SWClientConnection.h

@@ -66,7 +66,7 @@ class SWClientConnection : public ThreadSafeRefCounted<SWClientConnection> {
 
     virtual void postMessageToServiceWorkerGlobalScope(ServiceWorkerIdentifier destinationIdentifier, Ref<SerializedScriptValue>&&, ScriptExecutionContext& source) = 0;
     virtual uint64_t identifier() const = 0;
-    virtual bool hasServiceWorkerRegisteredForOrigin(const SecurityOrigin&) const = 0;
+    virtual bool mayHaveServiceWorkerRegisteredForOrigin(const SecurityOrigin&) const = 0;
 
 protected:
     WEBCORE_EXPORT SWClientConnection();

Source/WebKit/ChangeLog

@@ -1,3 +1,27 @@
+2017-11-14  Youenn Fablet  <[email protected]>
+
+        WebSWClientConnection should do IPC to StorageProcess if its WebSWOriginTable is not yet initialized
+        https://bugs.webkit.org/show_bug.cgi?id=179668
+
+        Reviewed by Chris Dumez.
+
+        There may be cases where the origin table is not initialized and we would think there is no service worker registration.
+        In such a case, we should go to the StorageProcess.
+        StorageProcess is now sending an IPC message back to each registered SW connection so that WebProcess will know whether its map
+        is correctly initialized or not.
+
+        Renaming hasServiceWorkerRegisteredForOrigin in mayHaveServiceWorkerRegisteredForOrigin.
+
+        * WebProcess/Storage/WebSWClientConnection.cpp:
+        (WebKit::WebSWClientConnection::mayHaveServiceWorkerRegisteredForOrigin const):
+        (WebKit::WebSWClientConnection::matchRegistration):
+        (WebKit::WebSWClientConnection::hasServiceWorkerRegisteredForOrigin const): Deleted.
+        * WebProcess/Storage/WebSWClientConnection.h:
+        * WebProcess/Storage/WebSWOriginTable.h:
+        (WebKit::WebSWOriginTable::isInitialized const):
+        * WebProcess/Storage/WebServiceWorkerProvider.cpp:
+        (WebKit::shouldHandleFetch):
+
 2017-11-14  Brent Fulgham  <[email protected]>
 
         Consolidate sysctl-read rules in WebProcess sandbox

Source/WebKit/StorageProcess/ServiceWorker/WebSWOriginStore.cpp

@@ -62,8 +62,10 @@ void WebSWOriginStore::registerSWServerConnection(WebSWServerConnection& connect
 {
     m_webSWServerConnections.add(&connection);
 
-    if (m_store.isEmpty())
+    if (m_store.isEmpty()) {
+        connection.send(Messages::WebSWClientConnection::InitializeSWOriginTableAsEmpty());
         return;
+    }
 
     sendStoreHandle(connection);
 }

Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp

@@ -94,8 +94,11 @@ void WebSWClientConnection::didResolveRegistrationPromise(const ServiceWorkerReg
     send(Messages::WebSWServerConnection::DidResolveRegistrationPromise(key));
 }
 
-bool WebSWClientConnection::hasServiceWorkerRegisteredForOrigin(const SecurityOrigin& origin) const
+bool WebSWClientConnection::mayHaveServiceWorkerRegisteredForOrigin(const SecurityOrigin& origin) const
 {
+    if (!m_swOriginTable->isInitialized())
+        return true;
+
     return m_swOriginTable->contains(origin);
 }
 
@@ -104,6 +107,11 @@ void WebSWClientConnection::setSWOriginTableSharedMemory(const SharedMemory::Han
     m_swOriginTable->setSharedMemory(handle);
 }
 
+void WebSWClientConnection::initializeSWOriginTableAsEmpty()
+{
+    m_swOriginTable->initializeAsEmpty();
+}
+
 void WebSWClientConnection::didMatchRegistration(uint64_t matchingRequest, std::optional<ServiceWorkerRegistrationData>&& result)
 {
     if (auto completionHandler = m_ongoingMatchRegistrationTasks.take(matchingRequest))
@@ -112,7 +120,7 @@ void WebSWClientConnection::didMatchRegistration(uint64_t matchingRequest, std::
 
 void WebSWClientConnection::matchRegistration(const SecurityOrigin& topOrigin, const URL& clientURL, RegistrationCallback&& callback)
 {
-    if (!hasServiceWorkerRegisteredForOrigin(topOrigin)) {
+    if (!mayHaveServiceWorkerRegisteredForOrigin(topOrigin)) {
         callback(std::nullopt);
         return;
     }

Source/WebKit/WebProcess/Storage/WebSWClientConnection.h

@@ -60,7 +60,7 @@ class WebSWClientConnection : public WebCore::SWClientConnection, public IPC::Me
     void disconnectedFromWebProcess();
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final;
 
-    bool hasServiceWorkerRegisteredForOrigin(const WebCore::SecurityOrigin&) const final;
+    bool mayHaveServiceWorkerRegisteredForOrigin(const WebCore::SecurityOrigin&) const final;
     Ref<ServiceWorkerClientFetch> startFetch(WebServiceWorkerProvider&, Ref<WebCore::ResourceLoader>&&, uint64_t identifier, ServiceWorkerClientFetch::Callback&&);
 
     void postMessageToServiceWorkerClient(uint64_t destinationScriptExecutionContextIdentifier, const IPC::DataReference& message, WebCore::ServiceWorkerData&& source, const String& sourceOrigin);
@@ -81,6 +81,7 @@ class WebSWClientConnection : public WebCore::SWClientConnection, public IPC::Me
     uint64_t messageSenderDestinationID() final { return m_identifier; }
 
     void setSWOriginTableSharedMemory(const SharedMemory::Handle&);
+    void initializeSWOriginTableAsEmpty();
 
     PAL::SessionID m_sessionID;
     uint64_t m_identifier;

Source/WebKit/WebProcess/Storage/WebSWClientConnection.messages.in

@@ -32,6 +32,7 @@ messages -> WebSWClientConnection {
     UpdateWorkerState(WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, enum WebCore::ServiceWorkerState state)
     FireUpdateFoundEvent(WebCore::ServiceWorkerRegistrationIdentifier identifier)
 
+    InitializeSWOriginTableAsEmpty()
     SetSWOriginTableSharedMemory(WebKit::SharedMemory::Handle handle)
     PostMessageToServiceWorkerClient(uint64_t destinationScriptExecutionContextIdentifier, IPC::DataReference message, struct WebCore::ServiceWorkerData source, String sourceOrigin)
 

Source/WebKit/WebProcess/Storage/WebSWOriginTable.cpp

@@ -44,6 +44,8 @@ bool WebSWOriginTable::contains(const SecurityOrigin& origin) const
 
 void WebSWOriginTable::setSharedMemory(const SharedMemory::Handle& handle)
 {
+    m_isInitialized = true;
+
     auto sharedMemory = SharedMemory::map(handle, SharedMemory::Protection::ReadOnly);
     if (!sharedMemory)
         return;

Source/WebKit/WebProcess/Storage/WebSWOriginTable.h

@@ -40,11 +40,14 @@ class WebSWOriginTable {
 public:
     WebSWOriginTable() = default;
 
+    bool isInitialized() const { return m_isInitialized; }
     bool contains(const WebCore::SecurityOrigin&) const;
     void setSharedMemory(const SharedMemory::Handle&);
+    void initializeAsEmpty() { m_isInitialized = true; }
 
 private:
     SharedStringHashTableReadOnly m_serviceWorkerOriginTable;
+    bool m_isInitialized { false };
 };
 
 } // namespace WebKit

Source/WebKit/WebProcess/Storage/WebServiceWorkerProvider.cpp

@@ -74,7 +74,7 @@ static inline bool shouldHandleFetch(const WebSWClientConnection& connection, Ca
     if (!resource)
         return false;
 
-    return connection.hasServiceWorkerRegisteredForOrigin(*resource->origin());
+    return connection.mayHaveServiceWorkerRegisteredForOrigin(*resource->origin());
 }
 
 void WebServiceWorkerProvider::handleFetch(ResourceLoader& loader, CachedResource* resource, PAL::SessionID sessionID, ServiceWorkerClientFetch::Callback&& callback)