Title: [286474] trunk/Source/WebCore
Revision
286474
Author
cdu...@apple.com
Date
2021-12-02 18:54:36 -0800 (Thu, 02 Dec 2021)

Log Message

Introduce a MainThreadBridge class inside WebLockManager
https://bugs.webkit.org/show_bug.cgi?id=233789

Reviewed by Darin Adler.

Extract all the logic to hop to and back from the main thread out of the WebLockManager and into a MainThreadBridge
internal class. This is similar to what was done for BroadcastChannel already. It provides better structuring and
slightly better performance by calling isolatedCopy() on the ClientOrigin only once instead of on every call.

No new tests, no web-facing behavior change.

* Modules/web-locks/WebLockManager.cpp:
(WebCore::WebLockManager::MainThreadBridge::create):
(WebCore::WebLockManager::MainThreadBridge::MainThreadBridge):
(WebCore::WebLockManager::MainThreadBridge::requestLock):
(WebCore::WebLockManager::MainThreadBridge::releaseLock):
(WebCore::WebLockManager::MainThreadBridge::abortLockRequest):
(WebCore::WebLockManager::MainThreadBridge::query):
(WebCore::WebLockManager::MainThreadBridge::clientIsGoingAway):
(WebCore::WebLockManager::MainThreadBridge::ensureOnMainThread):
(WebCore::WebLockManager::WebLockManager):
(WebCore::WebLockManager::request):
(WebCore::WebLockManager::didCompleteLockRequest):
(WebCore::WebLockManager::query):
(WebCore::WebLockManager::signalToAbortTheRequest):
(WebCore::WebLockManager::clientIsGoingAway):
(WebCore::WebLockManager::requestLockOnMainThread): Deleted.
(WebCore::WebLockManager::releaseLockOnMainThread): Deleted.
(WebCore::WebLockManager::abortLockRequestOnMainThread): Deleted.
(WebCore::WebLockManager::queryOnMainThread): Deleted.
(WebCore::WebLockManager::ensureOnMainThread): Deleted.
* Modules/web-locks/WebLockManager.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (286473 => 286474)


--- trunk/Source/WebCore/ChangeLog	2021-12-03 02:35:49 UTC (rev 286473)
+++ trunk/Source/WebCore/ChangeLog	2021-12-03 02:54:36 UTC (rev 286474)
@@ -1,3 +1,38 @@
+2021-12-02  Chris Dumez  <cdu...@apple.com>
+
+        Introduce a MainThreadBridge class inside WebLockManager
+        https://bugs.webkit.org/show_bug.cgi?id=233789
+
+        Reviewed by Darin Adler.
+
+        Extract all the logic to hop to and back from the main thread out of the WebLockManager and into a MainThreadBridge
+        internal class. This is similar to what was done for BroadcastChannel already. It provides better structuring and
+        slightly better performance by calling isolatedCopy() on the ClientOrigin only once instead of on every call.
+
+        No new tests, no web-facing behavior change.
+
+        * Modules/web-locks/WebLockManager.cpp:
+        (WebCore::WebLockManager::MainThreadBridge::create):
+        (WebCore::WebLockManager::MainThreadBridge::MainThreadBridge):
+        (WebCore::WebLockManager::MainThreadBridge::requestLock):
+        (WebCore::WebLockManager::MainThreadBridge::releaseLock):
+        (WebCore::WebLockManager::MainThreadBridge::abortLockRequest):
+        (WebCore::WebLockManager::MainThreadBridge::query):
+        (WebCore::WebLockManager::MainThreadBridge::clientIsGoingAway):
+        (WebCore::WebLockManager::MainThreadBridge::ensureOnMainThread):
+        (WebCore::WebLockManager::WebLockManager):
+        (WebCore::WebLockManager::request):
+        (WebCore::WebLockManager::didCompleteLockRequest):
+        (WebCore::WebLockManager::query):
+        (WebCore::WebLockManager::signalToAbortTheRequest):
+        (WebCore::WebLockManager::clientIsGoingAway):
+        (WebCore::WebLockManager::requestLockOnMainThread): Deleted.
+        (WebCore::WebLockManager::releaseLockOnMainThread): Deleted.
+        (WebCore::WebLockManager::abortLockRequestOnMainThread): Deleted.
+        (WebCore::WebLockManager::queryOnMainThread): Deleted.
+        (WebCore::WebLockManager::ensureOnMainThread): Deleted.
+        * Modules/web-locks/WebLockManager.h:
+
 2021-12-02  Said Abou-Hallawa  <s...@apple.com>
 
         [GPU Process] Move the FilterEffect boundaries to a new class FilterEffectGeometry

Modified: trunk/Source/WebCore/Modules/web-locks/WebLockManager.cpp (286473 => 286474)


--- trunk/Source/WebCore/Modules/web-locks/WebLockManager.cpp	2021-12-03 02:35:49 UTC (rev 286473)
+++ trunk/Source/WebCore/Modules/web-locks/WebLockManager.cpp	2021-12-03 02:54:36 UTC (rev 286474)
@@ -57,7 +57,6 @@
 
 struct WebLockManager::LockRequest {
     WebLockIdentifier lockIdentifier;
-    ScriptExecutionContextIdentifier clientID;
     String name;
     WebLockMode mode { WebLockMode::Exclusive };
     RefPtr<WebLockGrantedCallback> grantedCallback;
@@ -66,6 +65,110 @@
     bool isValid() const { return !!lockIdentifier; }
 };
 
+class WebLockManager::MainThreadBridge : public ThreadSafeRefCounted<MainThreadBridge, WTF::DestructionThread::Main> {
+public:
+    static RefPtr<MainThreadBridge> create(ScriptExecutionContext* context)
+    {
+        auto clientOrigin = clientOriginFromContext(context);
+        if (!clientOrigin)
+            return nullptr;
+
+        return adoptRef(*new MainThreadBridge(*context, WTFMove(*clientOrigin)));
+    }
+
+    void requestLock(WebLockIdentifier, const String& name, const Options&, Function<void(bool)>&&, Function<void()>&& lockStolenHandler);
+    void releaseLock(WebLockIdentifier, const String& name);
+    void abortLockRequest(WebLockIdentifier, const String& name, CompletionHandler<void(bool)>&&);
+    void query(CompletionHandler<void(Snapshot&&)>&&);
+    void clientIsGoingAway();
+
+private:
+    MainThreadBridge(ScriptExecutionContext&, ClientOrigin&&);
+
+    void ensureOnMainThread(Function<void(WebLockRegistry&)>&& task);
+
+    WeakPtr<ScriptExecutionContext> m_context;
+    const ScriptExecutionContextIdentifier m_clientID;
+    const ClientOrigin m_clientOrigin; // Main thread only.
+};
+
+WebLockManager::MainThreadBridge::MainThreadBridge(ScriptExecutionContext& context, ClientOrigin&& clientOrigin)
+    : m_context(context)
+    , m_clientID(context.identifier())
+    , m_clientOrigin(WTFMove(clientOrigin).isolatedCopy())
+{
+}
+
+void WebLockManager::MainThreadBridge::requestLock(WebLockIdentifier lockIdentifier, const String& name, const Options& options, Function<void(bool)>&& grantedHandler, Function<void()>&& lockStolenHandler)
+{
+    ensureOnMainThread([this, name = crossThreadCopy(name), mode = options.mode, steal = options.steal, ifAvailable = options.ifAvailable, lockIdentifier, grantedHandler = WTFMove(grantedHandler), lockStolenHandler = WTFMove(lockStolenHandler)](auto& registry) mutable {
+        registry.requestLock(m_clientOrigin, lockIdentifier, m_clientID, name, mode, steal, ifAvailable, [clientID = m_clientID, grantedHandler = WTFMove(grantedHandler)] (bool success) mutable {
+            ScriptExecutionContext::ensureOnContextThread(clientID, [grantedHandler = WTFMove(grantedHandler), success](auto&) mutable {
+                grantedHandler(success);
+            });
+        }, [clientID = m_clientID, lockStolenHandler = WTFMove(lockStolenHandler)]() mutable {
+            ScriptExecutionContext::ensureOnContextThread(clientID, [lockStolenHandler = WTFMove(lockStolenHandler)](auto&) mutable {
+                lockStolenHandler();
+            });
+        });
+    });
+}
+
+void WebLockManager::MainThreadBridge::releaseLock(WebLockIdentifier lockIdentifier, const String& name)
+{
+    ensureOnMainThread([this, lockIdentifier, name = crossThreadCopy(name)](auto& registry) {
+        registry.releaseLock(m_clientOrigin, lockIdentifier, m_clientID, name);
+    });
+}
+
+void WebLockManager::MainThreadBridge::abortLockRequest(WebLockIdentifier lockIdentifier, const String& name, CompletionHandler<void(bool)>&& completionHandler)
+{
+    ensureOnMainThread([this, lockIdentifier, name = crossThreadCopy(name), completionHandler = WTFMove(completionHandler)](auto& registry) mutable {
+        registry.abortLockRequest(m_clientOrigin, lockIdentifier, m_clientID, name, [clientID = m_clientID, completionHandler = WTFMove(completionHandler)](bool wasAborted) mutable {
+            ScriptExecutionContext::ensureOnContextThread(clientID, [completionHandler = WTFMove(completionHandler), wasAborted](auto&) mutable {
+                completionHandler(wasAborted);
+            });
+        });
+    });
+}
+
+void WebLockManager::MainThreadBridge::query(CompletionHandler<void(Snapshot&&)>&& completionHandler)
+{
+    ensureOnMainThread([this, completionHandler = WTFMove(completionHandler)](auto& registry) mutable {
+        registry.snapshot(m_clientOrigin, [clientID = m_clientID, completionHandler = WTFMove(completionHandler)](Snapshot&& snapshot) mutable {
+            ScriptExecutionContext::ensureOnContextThread(clientID, [completionHandler = WTFMove(completionHandler), snapshot = crossThreadCopy(snapshot)](auto&) mutable {
+                completionHandler(WTFMove(snapshot));
+            });
+        });
+    });
+}
+
+void WebLockManager::MainThreadBridge::clientIsGoingAway()
+{
+    ensureOnMainThread([this](auto& registry) {
+        registry.clientIsGoingAway(m_clientOrigin, m_clientID);
+    });
+}
+
+void WebLockManager::MainThreadBridge::ensureOnMainThread(Function<void(WebLockRegistry&)>&& task)
+{
+    if (!m_context)
+        return;
+    ASSERT(m_context->isContextThread());
+
+    if (is<Document>(*m_context)) {
+        if (auto page = downcast<Document>(*m_context).page()) {
+            Ref protectedThis { *this };
+            task(page->webLockRegistry());
+        }
+    } else {
+        downcast<WorkerGlobalScope>(*m_context).thread().workerLoaderProxy().postTaskToLoader([task = WTFMove(task), protectedThis = Ref { *this }](auto& context) {
+            if (auto page = downcast<Document>(context).page())
+                task(page->webLockRegistry());
+        });
+    }
+}
+
 Ref<WebLockManager> WebLockManager::create(NavigatorBase& navigator)
 {
     auto manager = adoptRef(*new WebLockManager(navigator));
@@ -75,7 +178,7 @@
 
 WebLockManager::WebLockManager(NavigatorBase& navigator)
     : ActiveDOMObject(navigator.scriptExecutionContext())
-    , m_clientOrigin(clientOriginFromContext(navigator.scriptExecutionContext()))
+    , m_mainThreadBridge(MainThreadBridge::create(navigator.scriptExecutionContext()))
 {
 }
 
@@ -102,7 +205,7 @@
         return;
     }
 
-    if (!m_clientOrigin) {
+    if (!m_mainThreadBridge) {
         releasePromise->reject(SecurityError, "Context's origin is opaque"_s);
         return;
     }
@@ -142,10 +245,9 @@
         });
     }
 
-    auto clientID = scriptExecutionContext()->identifier();
-    m_pendingRequests.add(lockIdentifier, LockRequest { lockIdentifier, clientID, name, options.mode, WTFMove(grantedCallback), WTFMove(options.signal) });
+    m_pendingRequests.add(lockIdentifier, LockRequest { lockIdentifier, name, options.mode, WTFMove(grantedCallback), WTFMove(options.signal) });
 
-    requestLockOnMainThread(lockIdentifier, clientID, name, options, [weakThis = WeakPtr { *this }, lockIdentifier](bool success) mutable {
+    m_mainThreadBridge->requestLock(lockIdentifier, name, options, [weakThis = WeakPtr { *this }, lockIdentifier](bool success) mutable {
         if (weakThis)
             weakThis->didCompleteLockRequest(lockIdentifier, success);
     }, [weakThis = WeakPtr { *this }, lockIdentifier]() mutable {
@@ -163,7 +265,7 @@
 
         if (success) {
             if (request.signal && request.signal->aborted()) {
-                releaseLockOnMainThread(request.lockIdentifier, request.clientID, request.name);
+                m_mainThreadBridge->releaseLock(request.lockIdentifier, request.name);
                 return;
             }
 
@@ -171,15 +273,15 @@
             auto result = request.grantedCallback->handleEvent(lock.ptr());
             RefPtr<DOMPromise> waitingPromise = result.type() == CallbackResultType::Success ? result.releaseReturnValue() : nullptr;
             if (!waitingPromise) {
-                releaseLockOnMainThread(request.lockIdentifier, request.clientID, request.name);
+                m_mainThreadBridge->releaseLock(request.lockIdentifier, request.name);
                 settleReleasePromise(request.lockIdentifier, Exception { ExistingExceptionError });
                 return;
             }
 
-            DOMPromise::whenPromiseIsSettled(waitingPromise->globalObject(), waitingPromise->promise(), [this, weakThis = WTFMove(weakThis), lockIdentifier = request.lockIdentifier, clientID = request.clientID, name = request.name, waitingPromise] {
+            DOMPromise::whenPromiseIsSettled(waitingPromise->globalObject(), waitingPromise->promise(), [this, weakThis = WTFMove(weakThis), lockIdentifier = request.lockIdentifier, name = request.name, waitingPromise] {
                 if (!weakThis)
                     return;
-                releaseLockOnMainThread(lockIdentifier, clientID, name);
+                m_mainThreadBridge->releaseLock(lockIdentifier, name);
                 settleReleasePromise(lockIdentifier, static_cast<JSC::JSValue>(waitingPromise->promise()));
             });
         } else {
@@ -194,50 +296,6 @@
     });
 }
 
-void WebLockManager::requestLockOnMainThread(WebLockIdentifier lockIdentifier, ScriptExecutionContextIdentifier clientID, const String& name, const Options& options, Function<void(bool)>&& grantedHandler, Function<void()>&& lockStolenHandler)
-{
-    ensureOnMainThread([clientID, clientOrigin = crossThreadCopy(*m_clientOrigin), name = crossThreadCopy(name), mode = options.mode, steal = options.steal, ifAvailable = options.ifAvailable, lockIdentifier, grantedHandler = WTFMove(grantedHandler), lockStolenHandler = WTFMove(lockStolenHandler)](auto& registry) mutable {
-        registry.requestLock(clientOrigin, lockIdentifier, clientID, name, mode, steal, ifAvailable, [clientID, grantedHandler = WTFMove(grantedHandler)] (bool success) mutable {
-            ScriptExecutionContext::ensureOnContextThread(clientID, [grantedHandler = WTFMove(grantedHandler), success](auto&) mutable {
-                grantedHandler(success);
-            });
-        }, [clientID, lockStolenHandler = WTFMove(lockStolenHandler)]() mutable {
-            ScriptExecutionContext::ensureOnContextThread(clientID, [lockStolenHandler = WTFMove(lockStolenHandler)](auto&) mutable {
-                lockStolenHandler();
-            });
-        });
-    });
-}
-
-void WebLockManager::releaseLockOnMainThread(WebLockIdentifier lockIdentifier, ScriptExecutionContextIdentifier clientID, const String& name)
-{
-    ensureOnMainThread([clientOrigin = crossThreadCopy(*m_clientOrigin), lockIdentifier, clientID, name = crossThreadCopy(name)](auto& registry) {
-        registry.releaseLock(clientOrigin, lockIdentifier, clientID, name);
-    });
-}
-
-void WebLockManager::abortLockRequestOnMainThread(WebLockIdentifier lockIdentifier, const String& name, CompletionHandler<void(bool)>&& completionHandler)
-{
-    ensureOnMainThread([clientID = scriptExecutionContext()->identifier(), clientOrigin = crossThreadCopy(*m_clientOrigin), lockIdentifier, name = crossThreadCopy(name), completionHandler = WTFMove(completionHandler)](auto& registry) mutable {
-        registry.abortLockRequest(clientOrigin, lockIdentifier, clientID, name, [clientID, completionHandler = WTFMove(completionHandler)](bool wasAborted) mutable {
-            ScriptExecutionContext::ensureOnContextThread(clientID, [completionHandler = WTFMove(completionHandler), wasAborted](auto&) mutable {
-                completionHandler(wasAborted);
-            });
-        });
-    });
-}
-
-void WebLockManager::queryOnMainThread(CompletionHandler<void(Snapshot&&)>&& completionHandler)
-{
-    ensureOnMainThread([contextIdentifier = scriptExecutionContext()->identifier(), clientOrigin = crossThreadCopy(*m_clientOrigin), completionHandler = WTFMove(completionHandler)](auto& registry) mutable {
-        registry.snapshot(clientOrigin, [contextIdentifier, completionHandler = WTFMove(completionHandler)](Snapshot&& snapshot) mutable {
-            ScriptExecutionContext::ensureOnContextThread(contextIdentifier, [completionHandler = WTFMove(completionHandler), snapshot = crossThreadCopy(snapshot)](auto&) mutable {
-                completionHandler(WTFMove(snapshot));
-            });
-        });
-    });
-}
-
 void WebLockManager::query(Ref<DeferredPromise>&& promise)
 {
     if (!scriptExecutionContext()) {
@@ -250,12 +308,12 @@
         return;
     }
 
-    if (!m_clientOrigin) {
+    if (!m_mainThreadBridge) {
         promise->reject(SecurityError, "Context's origin is opaque"_s);
         return;
     }
 
-    queryOnMainThread([weakThis = WeakPtr { *this }, promise = WTFMove(promise)](Snapshot&& snapshot) mutable {
+    m_mainThreadBridge->query([weakThis = WeakPtr { *this }, promise = WTFMove(promise)](Snapshot&& snapshot) mutable {
         if (!weakThis)
             return;
 
@@ -268,7 +326,7 @@
 // https://wicg.github.io/web-locks/#signal-to-abort-the-request
 void WebLockManager::signalToAbortTheRequest(WebLockIdentifier lockIdentifier)
 {
-    if (!scriptExecutionContext())
+    if (!scriptExecutionContext() || !m_mainThreadBridge)
         return;
 
     auto requestsIterator = m_pendingRequests.find(lockIdentifier);
@@ -276,7 +334,7 @@
         return;
     auto& request = requestsIterator->value;
 
-    abortLockRequestOnMainThread(request.lockIdentifier, request.name, [weakThis = WeakPtr { *this }, lockIdentifier](bool wasAborted) {
+    m_mainThreadBridge->abortLockRequest(request.lockIdentifier, request.name, [weakThis = WeakPtr { *this }, lockIdentifier](bool wasAborted) {
         if (wasAborted && weakThis)
             weakThis->m_pendingRequests.remove(lockIdentifier);
     });
@@ -308,29 +366,10 @@
     m_pendingRequests.clear();
     m_releasePromises.clear();
 
-    ensureOnMainThread([clientOrigin = crossThreadCopy(*m_clientOrigin), contextIdentifier = scriptExecutionContext()->identifier()](auto& registry) {
-        registry.clientIsGoingAway(clientOrigin, contextIdentifier);
-    });
+    if (m_mainThreadBridge)
+        m_mainThreadBridge->clientIsGoingAway();
 }
 
-void WebLockManager::ensureOnMainThread(Function<void(WebLockRegistry&)>&& task)
-{
-    auto* context = scriptExecutionContext();
-    if (!context)
-        return;
-    ASSERT(context->isContextThread());
-
-    if (is<Document>(*context)) {
-        if (auto page = downcast<Document>(*context).page())
-            task(page->webLockRegistry());
-    } else {
-        downcast<WorkerGlobalScope>(*context).thread().workerLoaderProxy().postTaskToLoader([task = WTFMove(task)](auto& context) {
-            if (auto page = downcast<Document>(context).page())
-                task(page->webLockRegistry());
-        });
-    }
-}
-
 bool WebLockManager::virtualHasPendingActivity() const
 {
     return !m_pendingRequests.isEmpty() || !m_releasePromises.isEmpty();

Modified: trunk/Source/WebCore/Modules/web-locks/WebLockManager.h (286473 => 286474)


--- trunk/Source/WebCore/Modules/web-locks/WebLockManager.h	2021-12-03 02:35:49 UTC (rev 286473)
+++ trunk/Source/WebCore/Modules/web-locks/WebLockManager.h	2021-12-03 02:54:36 UTC (rev 286474)
@@ -63,24 +63,19 @@
 private:
     explicit WebLockManager(NavigatorBase&);
 
-    void requestLockOnMainThread(WebLockIdentifier, ScriptExecutionContextIdentifier, const String& name, const Options&, Function<void(bool)>&&, Function<void()>&& lockStolenHandler);
-    void releaseLockOnMainThread(WebLockIdentifier, ScriptExecutionContextIdentifier, const String& name);
-    void abortLockRequestOnMainThread(WebLockIdentifier, const String& name, CompletionHandler<void(bool)>&&);
-    void queryOnMainThread(CompletionHandler<void(Snapshot&&)>&&);
-
     void didCompleteLockRequest(WebLockIdentifier, bool success);
     void settleReleasePromise(WebLockIdentifier, ExceptionOr<JSC::JSValue>&&);
     void signalToAbortTheRequest(WebLockIdentifier);
     void clientIsGoingAway();
 
-    void ensureOnMainThread(Function<void(WebLockRegistry&)>&& task);
-
     // ActiveDOMObject.
     void stop() final;
     const char* activeDOMObjectName() const final;
     bool virtualHasPendingActivity() const final;
 
-    const std::optional<ClientOrigin> m_clientOrigin;
+    class MainThreadBridge;
+    RefPtr<MainThreadBridge> m_mainThreadBridge;
+
     HashMap<WebLockIdentifier, RefPtr<DeferredPromise>> m_releasePromises;
 
     struct LockRequest;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to