navigator.storage.getDirectory() fails in nested workers with an `I…

…nvalidStateError` https://bugs.webkit.org/show_bug.cgi?id=255458 rdar://108069396 Reviewed by Chris Dumez. WorkerStorageConnection assumes its Worker's loader is a Document, and thus posts tasks to main thread and asks for StorageConnection from its loader (WorkerLoaderProxy) directly. This is no longer true after nested worker is enabled -- the loader might be another Worker. Therefore it should get StorageConnection from the real main thread loader (via WorkerLoaderProxy::postTaskToLoader). Test: storage/storagemanager/nested-dedicated-workers.html * LayoutTests/TestExpectations: * LayoutTests/platform/wk2/TestExpectations: * LayoutTests/storage/storagemanager/nested-dedicated-workers-expected.txt: Added. * LayoutTests/storage/storagemanager/nested-dedicated-workers.html: Added. * LayoutTests/storage/storagemanager/resources/nested-dedicated-workers.js: Added. (nestedWorker.onmessage): (nestedWorker.onerror): * LayoutTests/storage/storagemanager/resources/nested-dedicated-workers2.js: Added. (navigator.storage.navigator.storage.estimate.then): (navigator.storage.catch): * Source/WebCore/Modules/storage/WorkerStorageConnection.cpp: (WebCore::WorkerStorageConnection::getPersisted): (WebCore::WorkerStorageConnection::getEstimate): (WebCore::WorkerStorageConnection::fileSystemGetDirectory): * Source/WebCore/workers/WorkerLoaderProxy.h: (WebCore::WorkerLoaderProxy::storageConnection): Deleted. * Source/WebCore/workers/WorkerMessagingProxy.cpp: (WebCore::WorkerMessagingProxy::storageConnection): Deleted. * Source/WebCore/workers/WorkerMessagingProxy.h: Canonical link: https://commits.webkit.org/263075@main
WebKit · Apr 18, 2023 · 808b1c7 · 808b1c7
1 parent 99f7ecf
commit 808b1c7
Show file tree

Hide file tree

Showing 10 changed files with 78 additions and 29 deletions.
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -214,6 +214,7 @@ imported/w3c/web-platform-tests/storage/ [ Skip ]
 imported/w3c/web-platform-tests/file-system-access/ [ Skip ]
 imported/w3c/web-platform-tests/fs/ [ Skip ]
 storage/filesystemaccess/ [ Skip ]
+storage/storagemanager/ [ Skip ]
 
 # app-privacy-report tests are iOS only.
 http/tests/app-privacy-report/ [ Skip ]

diff --git a/LayoutTests/platform/wk2/TestExpectations b/LayoutTests/platform/wk2/TestExpectations
@@ -855,7 +855,7 @@ imported/w3c/web-platform-tests/permissions/ [ Pass ]
 http/tests/notifications/permission/ [ Pass ]
 http/tests/permissions/ [ Pass ]
 imported/w3c/web-platform-tests/storage/ [ Pass ]
-
+storage/storagemanager/ [ Pass ]
 fast/canvas/large-getImageData.html [ Pass ]
 
 webkit.org/b/253533 imported/w3c/web-platform-tests/fetch/api/basic/status.h2.any.html [ Pass Failure ]

diff --git a/LayoutTests/storage/storagemanager/nested-dedicated-workers-expected.txt b/LayoutTests/storage/storagemanager/nested-dedicated-workers-expected.txt
@@ -0,0 +1,12 @@
+[Worker] This test ensures StorageManager API works in nested dedicated workers.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Starting worker: resources/nested-dedicated-workers.js
+PASS [Worker] estimate.quota is non-null.
+PASS [Worker] estimate.usage is non-null.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/storagemanager/nested-dedicated-workers.html b/LayoutTests/storage/storagemanager/nested-dedicated-workers.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+<script>
+jsTestIsAsync = true;
+worker = startWorker('resources/nested-dedicated-workers.js');
+</script>
+</body>
+</html>
diff --git a/LayoutTests/storage/storagemanager/resources/nested-dedicated-workers.js b/LayoutTests/storage/storagemanager/resources/nested-dedicated-workers.js
@@ -0,0 +1,19 @@
+if (this.importScripts) {
+    importScripts('../../../resources/js-test.js');
+}
+
+description("This test ensures StorageManager API works in nested dedicated workers.");
+
+var estimate = null;
+var nestedWorker = new Worker('nested-dedicated-workers2.js');
+nestedWorker.onmessage = (event) => {
+    estimate = event.data;
+    shouldBeNonNull("estimate.quota");
+    shouldBeNonNull("estimate.usage");
+    finishJSTest();
+};
+
+nestedWorker.onerror = (event) => {
+    testFailed('nestedWorker has error ' + event.message());
+    finishJSTest();
+}
diff --git a/LayoutTests/storage/storagemanager/resources/nested-dedicated-workers2.js b/LayoutTests/storage/storagemanager/resources/nested-dedicated-workers2.js
@@ -0,0 +1,9 @@
+if (navigator.storage) {
+	navigator.storage.estimate().then((estimate) => {
+		self.postMessage(estimate);
+	}).catch((error) => {
+		self.postMessage('fail');
+	}); 
+} else {
+	self.postMessage('navigator.storage is not available');
+}
diff --git a/Source/WebCore/Modules/storage/WorkerStorageConnection.cpp b/Source/WebCore/Modules/storage/WorkerStorageConnection.cpp
@@ -66,12 +66,15 @@ void WorkerStorageConnection::getPersisted(ClientOrigin&& origin, StorageConnect
     auto callbackIdentifier = ++m_lastCallbackIdentifier;
     m_getPersistedCallbacks.add(callbackIdentifier, WTFMove(completionHandler));
 
-    callOnMainThread([callbackIdentifier, workerThread = Ref { m_scope->thread() }, origin = WTFMove(origin).isolatedCopy()]() mutable {
-        auto mainThreadConnection = workerThread->workerLoaderProxy().storageConnection();
-        auto mainThreadCallback = [callbackIdentifier, workerThread = WTFMove(workerThread)](bool result) mutable {
-            workerThread->runLoop().postTaskForMode([callbackIdentifier, result] (auto& scope) mutable {
+    m_scope->thread().workerLoaderProxy().postTaskToLoader([callbackIdentifier, contextIdentifier = m_scope->identifier(), origin = WTFMove(origin).isolatedCopy()](auto& context) mutable {
+        ASSERT(isMainThread());
+
+        auto& document = downcast<Document>(context);
+        auto mainThreadConnection = document.storageConnection();
+        auto mainThreadCallback = [callbackIdentifier, contextIdentifier](bool result) mutable {
+            ScriptExecutionContext::postTaskTo(contextIdentifier, [callbackIdentifier, result] (auto& scope) mutable {
                 downcast<WorkerGlobalScope>(scope).storageConnection().didGetPersisted(callbackIdentifier, result);
-            }, WorkerRunLoop::defaultMode());
+            });
         };
         if (!mainThreadConnection)
             return mainThreadCallback(false);
@@ -93,12 +96,15 @@ void WorkerStorageConnection::getEstimate(ClientOrigin&& origin, StorageConnecti
     auto callbackIdentifier = ++m_lastCallbackIdentifier;
     m_getEstimateCallbacks.add(callbackIdentifier, WTFMove(completionHandler));
 
-    callOnMainThread([callbackIdentifier, workerThread = Ref { m_scope->thread() }, origin = WTFMove(origin).isolatedCopy()]() mutable {
-        auto mainThreadConnection = workerThread->workerLoaderProxy().storageConnection();
-        auto mainThreadCallback = [callbackIdentifier, workerThread = WTFMove(workerThread)](auto&& result) mutable {
-            workerThread->runLoop().postTaskForMode([callbackIdentifier, result = crossThreadCopy(WTFMove(result))] (auto& scope) mutable {
+    m_scope->thread().workerLoaderProxy().postTaskToLoader([callbackIdentifier, contextIdentifier = m_scope->identifier(), origin = WTFMove(origin).isolatedCopy()](auto& context) mutable {
+        ASSERT(isMainThread());
+
+        auto& document = downcast<Document>(context);
+        auto mainThreadConnection = document.storageConnection();
+        auto mainThreadCallback = [callbackIdentifier, contextIdentifier](auto&& result) mutable {
+            ScriptExecutionContext::postTaskTo(contextIdentifier, [callbackIdentifier, result = crossThreadCopy(WTFMove(result))] (auto& scope) mutable {
                 downcast<WorkerGlobalScope>(scope).storageConnection().didGetEstimate(callbackIdentifier, WTFMove(result));
-            }, WorkerRunLoop::defaultMode());
+            });
         };
         if (!mainThreadConnection)
             return mainThreadCallback(Exception { InvalidStateError });
@@ -120,12 +126,15 @@ void WorkerStorageConnection::fileSystemGetDirectory(ClientOrigin&& origin, Stor
     auto callbackIdentifier = ++m_lastCallbackIdentifier;
     m_getDirectoryCallbacks.add(callbackIdentifier, WTFMove(completionHandler));
 
-    callOnMainThread([callbackIdentifier, workerThread = Ref { m_scope->thread() }, origin = WTFMove(origin).isolatedCopy()]() mutable {
-        auto mainThreadConnection = workerThread->workerLoaderProxy().storageConnection();
-        auto mainThreadCallback = [callbackIdentifier, workerThread = WTFMove(workerThread)](auto&& result) mutable {
-            workerThread->runLoop().postTaskForMode([callbackIdentifier, result = crossThreadCopy(WTFMove(result))] (auto& scope) mutable {
+    m_scope->thread().workerLoaderProxy().postTaskToLoader([callbackIdentifier, contextIdentifier = m_scope->identifier(), origin = WTFMove(origin).isolatedCopy()](auto& context) mutable {
+        ASSERT(isMainThread());
+
+        auto& document = downcast<Document>(context);
+        auto mainThreadConnection = document.storageConnection();
+        auto mainThreadCallback = [callbackIdentifier, contextIdentifier](auto&& result) mutable {
+            ScriptExecutionContext::postTaskTo(contextIdentifier, [callbackIdentifier, result = crossThreadCopy(WTFMove(result))] (auto& scope) mutable {
                 downcast<WorkerGlobalScope>(scope).storageConnection().didGetDirectory(callbackIdentifier, WTFMove(result));
-            }, WorkerRunLoop::defaultMode());
+            });
         };
         if (!mainThreadConnection)
             return mainThreadCallback(Exception { InvalidStateError });

diff --git a/Source/WebCore/workers/WorkerLoaderProxy.h b/Source/WebCore/workers/WorkerLoaderProxy.h
@@ -50,9 +50,6 @@ class WorkerLoaderProxy {
     // Creates a cache storage connection to be used on the main thread. Method must be called on the main thread.
     virtual RefPtr<CacheStorageConnection> createCacheStorageConnection() = 0;
 
-    // Get the storage connection to be used on the main thread.
-    virtual StorageConnection* storageConnection() { return nullptr; };
-
     virtual RefPtr<RTCDataChannelRemoteHandlerConnection> createRTCDataChannelRemoteHandlerConnection() = 0;
 
     virtual ScriptExecutionContextIdentifier loaderContextIdentifier() const = 0;

diff --git a/Source/WebCore/workers/WorkerMessagingProxy.cpp b/Source/WebCore/workers/WorkerMessagingProxy.cpp
@@ -289,15 +289,6 @@ RefPtr<CacheStorageConnection> WorkerMessagingProxy::createCacheStorageConnectio
     return document->page()->cacheStorageProvider().createCacheStorageConnection();
 }
 
-StorageConnection* WorkerMessagingProxy::storageConnection()
-{
-    ASSERT(isMainThread());
-    auto* document = dynamicDowncast<Document>(*m_scriptExecutionContext);
-    ASSERT(document);
-
-    return document ? document->storageConnection() : nullptr;
-}
-
 RefPtr<RTCDataChannelRemoteHandlerConnection> WorkerMessagingProxy::createRTCDataChannelRemoteHandlerConnection()
 {
     ASSERT(isMainThread());

diff --git a/Source/WebCore/workers/WorkerMessagingProxy.h b/Source/WebCore/workers/WorkerMessagingProxy.h
@@ -77,7 +77,6 @@ class WorkerMessagingProxy final : public ThreadSafeRefCounted<WorkerMessagingPr
     void postTaskToLoader(ScriptExecutionContext::Task&&) final;
     ScriptExecutionContextIdentifier loaderContextIdentifier() const final;
     RefPtr<CacheStorageConnection> createCacheStorageConnection() final;
-    StorageConnection* storageConnection() final;
     RefPtr<RTCDataChannelRemoteHandlerConnection> createRTCDataChannelRemoteHandlerConnection() final;
 
     void workerThreadCreated(DedicatedWorkerThread&);