Make sure nested worker get controlled if matching a service worker r…

…egistration

https://bugs.webkit.org/show_bug.cgi?id=247619
rdar://problem/102090425

Reviewed by Chris Dumez.

Allow to register WorkerScriptLoader living of the main thread (nested workers do create them).
Instead of registering a WorkerScriptLoader*, we now register a callback which is called when we need to set a worker as controlled.
Covered by unskipped test.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/nested-blob-url-workers.https-expected.txt: Added.
* Source/WebCore/workers/WorkerScriptLoader.cpp:
(WebCore::workerScriptLoaderControlledCallbackMap):
(WebCore::accessWorkerScriptLoaderMap):
(WebCore::WorkerScriptLoader::~WorkerScriptLoader):
(WebCore::WorkerScriptLoader::loadAsynchronously):
(WebCore::WorkerScriptLoader::getWorkerClientControlledCallback):
(): Deleted.
(WebCore::WorkerScriptLoader::fromScriptExecutionContextIdentifier): Deleted.
* Source/WebCore/workers/WorkerScriptLoader.h:
* Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp:
(WebKit::WebSWClientConnection::setServiceWorkerClientIsControlled):

Canonical link: https://commits.webkit.org/256637@main

LayoutTests/TestExpectations

@@ -364,7 +364,6 @@ imported/w3c/web-platform-tests/service-workers/service-worker/worker-in-sandbox
 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-worker-timing-frame.tentative.https.html [ Skip ]
 imported/w3c/web-platform-tests/service-workers/service-worker/claim-worker-fetch.https.html [ Skip ]
 imported/w3c/web-platform-tests/service-workers/service-worker/client-navigate.https.html [ Skip ]
-imported/w3c/web-platform-tests/service-workers/service-worker/nested-blob-url-workers.https.html [ Skip ]
 imported/w3c/web-platform-tests/service-workers/service-worker/update-bytecheck-cors-import.https.html [ Skip ]
 
 # This test is a flaky timeout.

LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/nested-blob-url-workers.https-expected.txt

@@ -0,0 +1,5 @@
+
+PASS Nested blob URL workers should be intercepted by a service worker.
+PASS Nested worker created from a blob URL worker should be intercepted by a service worker.
+PASS Nested blob URL worker created from a worker should be intercepted by a service worker.
+

Source/WebCore/workers/WorkerScriptLoader.cpp

@@ -41,17 +41,22 @@
 #include "TextResourceDecoder.h"
 #include "WorkerFetchResult.h"
 #include "WorkerGlobalScope.h"
+#include "WorkerSWClientConnection.h"
 #include "WorkerScriptLoaderClient.h"
 #include "WorkerThreadableLoader.h"
 #include <wtf/Ref.h>
 
 namespace WebCore {
 
-static HashMap<ScriptExecutionContextIdentifier, WorkerScriptLoader*>& scriptExecutionContextIdentifierToWorkerScriptLoaderMap()
+#if ENABLE(SERVICE_WORKER)
+static Lock workerScriptLoaderControlledCallbackMapLock;
+static void accessWorkerScriptLoaderMap(CompletionHandler<void(HashMap<ScriptExecutionContextIdentifier, Ref<WorkerScriptLoader::ServiceWorkerDataManager>>&)>&& callback)
 {
-    static MainThreadNeverDestroyed<HashMap<ScriptExecutionContextIdentifier, WorkerScriptLoader*>> map;
-    return map.get();
+    Locker locker { workerScriptLoaderControlledCallbackMapLock };
+    static NeverDestroyed<HashMap<ScriptExecutionContextIdentifier, Ref<WorkerScriptLoader::ServiceWorkerDataManager>>> map;
+    callback(map.get());
 }
+#endif
 
 WorkerScriptLoader::WorkerScriptLoader()
     : m_script(ScriptBuffer::empty())
@@ -60,14 +65,9 @@ WorkerScriptLoader::WorkerScriptLoader()
 
 WorkerScriptLoader::~WorkerScriptLoader()
 {
-    if (m_didAddToWorkerScriptLoaderMap)
-        scriptExecutionContextIdentifierToWorkerScriptLoaderMap().remove(m_clientIdentifier);
-
 #if ENABLE(SERVICE_WORKER)
-    if (m_activeServiceWorkerData) {
-        if (auto* serviceWorkerConnection = ServiceWorkerProvider::singleton().existingServiceWorkerConnection())
-            serviceWorkerConnection->unregisterServiceWorkerClient(m_clientIdentifier);
-    }
+    if (m_didAddToWorkerScriptLoaderMap)
+        accessWorkerScriptLoaderMap([clientIdentifier = m_clientIdentifier](auto& map) { map.remove(clientIdentifier); });
 #endif
 }
 
@@ -158,15 +158,20 @@ void WorkerScriptLoader::loadAsynchronously(ScriptExecutionContext& scriptExecut
     // A service worker job can be executed from a worker context or a document context.
     options.serviceWorkersMode = serviceWorkerMode;
 #if ENABLE(SERVICE_WORKER)
-    if ((m_destination == FetchOptions::Destination::Worker || m_destination == FetchOptions::Destination::Sharedworker) && is<Document>(scriptExecutionContext) && downcast<Document>(scriptExecutionContext).settings().serviceWorkersEnabled()) {
+    if (scriptExecutionContext.settingsValues().serviceWorkersEnabled && clientIdentifier) {
+        ASSERT(m_destination == FetchOptions::Destination::Worker || m_destination == FetchOptions::Destination::Sharedworker);
         m_topOriginForServiceWorkerRegistration = SecurityOriginData { scriptExecutionContext.topOrigin().data() };
-        ASSERT(clientIdentifier);
         options.clientIdentifier = clientIdentifier;
-        // In case of blob URLs, we reuse the document controlling service worker.
+        m_serviceWorkerDataManager = ServiceWorkerDataManager::create(clientIdentifier);
+        m_context = scriptExecutionContext;
+
+        // In case of blob URLs, we reuse the context controlling service worker.
         if (request->url().protocolIsBlob() && scriptExecutionContext.activeServiceWorker())
             setControllingServiceWorker(ServiceWorkerData { scriptExecutionContext.activeServiceWorker()->data() });
         else {
-            scriptExecutionContextIdentifierToWorkerScriptLoaderMap().add(m_clientIdentifier, this);
+            accessWorkerScriptLoaderMap([this](auto& map) mutable {
+                map.add(m_clientIdentifier, *m_serviceWorkerDataManager);
+            });
             m_didAddToWorkerScriptLoaderMap = true;
         }
     } else if (auto* activeServiceWorker = scriptExecutionContext.activeServiceWorker())
@@ -253,9 +258,10 @@ void WorkerScriptLoader::didReceiveResponse(ResourceLoaderIdentifier identifier,
     m_referrerPolicy = response.httpHeaderField(HTTPHeaderName::ReferrerPolicy);
 
 #if ENABLE(SERVICE_WORKER)
-    if (m_topOriginForServiceWorkerRegistration && response.source() == ResourceResponse::Source::MemoryCache) {
+    if (m_topOriginForServiceWorkerRegistration && response.source() == ResourceResponse::Source::MemoryCache && m_context) {
         m_isMatchingServiceWorkerRegistration = true;
-        ServiceWorkerProvider::singleton().serviceWorkerConnection().matchRegistration(WTFMove(*m_topOriginForServiceWorkerRegistration), response.url(), [this, protectedThis = Ref { *this }, response, identifier](auto&& registrationData) mutable {
+        auto& swConnection = is<WorkerGlobalScope>(m_context) ? static_cast<SWClientConnection&>(downcast<WorkerGlobalScope>(*m_context).swClientConnection()) : ServiceWorkerProvider::singleton().serviceWorkerConnection();
+        swConnection.matchRegistration(WTFMove(*m_topOriginForServiceWorkerRegistration), response.url(), [this, protectedThis = Ref { *this }, response, identifier](auto&& registrationData) mutable {
             m_isMatchingServiceWorkerRegistration = false;
             if (registrationData && registrationData->activeWorker)
                 setControllingServiceWorker(WTFMove(*registrationData->activeWorker));
@@ -356,20 +362,49 @@ WorkerFetchResult WorkerScriptLoader::fetchResult() const
     return { script(), responseURL(), certificateInfo(), contentSecurityPolicy(), crossOriginEmbedderPolicy(), referrerPolicy(), { } };
 }
 
-WorkerScriptLoader* WorkerScriptLoader::fromScriptExecutionContextIdentifier(ScriptExecutionContextIdentifier identifier)
+#if ENABLE(SERVICE_WORKER)
+std::optional<ServiceWorkerData> WorkerScriptLoader::takeServiceWorkerData()
 {
-    return scriptExecutionContextIdentifierToWorkerScriptLoaderMap().get(identifier);
+    if (!m_serviceWorkerDataManager)
+        return { };
+    return m_serviceWorkerDataManager->takeData();
 }
 
-#if ENABLE(SERVICE_WORKER)
-bool WorkerScriptLoader::setControllingServiceWorker(ServiceWorkerData&& activeServiceWorkerData)
+RefPtr<WorkerScriptLoader::ServiceWorkerDataManager> WorkerScriptLoader::serviceWorkerDataManagerFromIdentifier(ScriptExecutionContextIdentifier identifier)
+{
+    RefPtr<ServiceWorkerDataManager> result;
+    accessWorkerScriptLoaderMap([identifier, &result](auto& map) {
+        result = map.get(identifier);
+    });
+    return result;
+}
+
+void WorkerScriptLoader::setControllingServiceWorker(ServiceWorkerData&& activeServiceWorkerData)
 {
-    if (!m_client)
-        return false;
+    m_serviceWorkerDataManager->setData(WTFMove(activeServiceWorkerData));
+}
 
-    m_activeServiceWorkerData = WTFMove(activeServiceWorkerData);
-    return true;
+WorkerScriptLoader::ServiceWorkerDataManager::~ServiceWorkerDataManager()
+{
+    if (!m_activeServiceWorkerData)
+        return;
+    if (auto* serviceWorkerConnection = ServiceWorkerProvider::singleton().existingServiceWorkerConnection())
+        serviceWorkerConnection->unregisterServiceWorkerClient(m_clientIdentifier);
 }
+
+void WorkerScriptLoader::ServiceWorkerDataManager::setData(ServiceWorkerData&& data)
+{
+    Locker lock(m_activeServiceWorkerDataLock);
+    m_activeServiceWorkerData = WTFMove(data).isolatedCopy();
+}
+
+std::optional<ServiceWorkerData> WorkerScriptLoader::ServiceWorkerDataManager::takeData()
+{
+    Locker lock(m_activeServiceWorkerDataLock);
+    std::optional<ServiceWorkerData> data = std::exchange(data, m_activeServiceWorkerData);
+    return data;
+}
+
 #endif
 
 } // namespace WebCore

Source/WebCore/workers/WorkerScriptLoader.h

@@ -96,11 +96,29 @@ class WorkerScriptLoader : public RefCounted<WorkerScriptLoader>, public Threada
 
     WEBCORE_EXPORT static ResourceError validateWorkerResponse(const ResourceResponse&, Source, FetchOptions::Destination);
 
-    WEBCORE_EXPORT static WorkerScriptLoader* fromScriptExecutionContextIdentifier(ScriptExecutionContextIdentifier);
-
 #if ENABLE(SERVICE_WORKER)
-    WEBCORE_EXPORT bool setControllingServiceWorker(ServiceWorkerData&&);
-    std::optional<ServiceWorkerData> takeServiceWorkerData() { return std::exchange(m_activeServiceWorkerData, { }); }
+    class ServiceWorkerDataManager : public ThreadSafeRefCounted<ServiceWorkerDataManager, WTF::DestructionThread::Main> {
+    public:
+        static Ref<ServiceWorkerDataManager> create(ScriptExecutionContextIdentifier identifier) { return adoptRef(*new ServiceWorkerDataManager(identifier)); }
+        WEBCORE_EXPORT ~ServiceWorkerDataManager();
+
+        WEBCORE_EXPORT void setData(ServiceWorkerData&&);
+        std::optional<ServiceWorkerData> takeData();
+
+    private:
+        explicit ServiceWorkerDataManager(ScriptExecutionContextIdentifier identifier)
+            : m_clientIdentifier(identifier)
+        {
+        }
+
+        ScriptExecutionContextIdentifier m_clientIdentifier;
+        Lock m_activeServiceWorkerDataLock;
+        std::optional<ServiceWorkerData> m_activeServiceWorkerData WTF_GUARDED_BY_LOCK(m_activeServiceWorkerDataLock);
+    };
+
+    void setControllingServiceWorker(ServiceWorkerData&&);
+    std::optional<ServiceWorkerData> takeServiceWorkerData();
+    WEBCORE_EXPORT static RefPtr<ServiceWorkerDataManager> serviceWorkerDataManagerFromIdentifier(ScriptExecutionContextIdentifier);
 #endif
 
     ScriptExecutionContextIdentifier clientIdentifier() const { return m_clientIdentifier; }
@@ -134,15 +152,16 @@ class WorkerScriptLoader : public RefCounted<WorkerScriptLoader>, public Threada
     bool m_finishing { false };
     bool m_isRedirected { false };
     bool m_isCOEPEnabled { false };
-    bool m_didAddToWorkerScriptLoaderMap { false };
     ResourceResponse::Source m_responseSource { ResourceResponse::Source::Unknown };
     ResourceResponse::Tainting m_responseTainting { ResourceResponse::Tainting::Basic };
     ResourceError m_error;
     ScriptExecutionContextIdentifier m_clientIdentifier;
 #if ENABLE(SERVICE_WORKER)
+    bool m_didAddToWorkerScriptLoaderMap { false };
     bool m_isMatchingServiceWorkerRegistration { false };
     std::optional<SecurityOriginData> m_topOriginForServiceWorkerRegistration;
-    std::optional<ServiceWorkerData> m_activeServiceWorkerData;
+    RefPtr<ServiceWorkerDataManager> m_serviceWorkerDataManager;
+    WeakPtr<ScriptExecutionContext> m_context;
 #endif
     String m_userAgentForSharedWorker;
 };

Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp

@@ -189,9 +189,12 @@ void WebSWClientConnection::setServiceWorkerClientIsControlled(ScriptExecutionCo
         return;
     }
 
-    if (auto* loader = WorkerScriptLoader::fromScriptExecutionContextIdentifier(identifier)) {
-        completionHandler(data.activeWorker ? loader->setControllingServiceWorker(WTFMove(*data.activeWorker)) : false);
-        return;
+    if (auto manager = WorkerScriptLoader::serviceWorkerDataManagerFromIdentifier(identifier)) {
+        if (data.activeWorker) {
+            manager->setData(WTFMove(*data.activeWorker));
+            completionHandler(true);
+            return;
+        }
     }
 
     completionHandler(false);