WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
233789
Introduce a MainThreadBridge class inside WebLockManager
https://bugs.webkit.org/show_bug.cgi?id=233789
Summary
Introduce a MainThreadBridge class inside WebLockManager
Chris Dumez
Reported
2021-12-02 16:23:20 PST
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.
Attachments
Patch
(19.11 KB, patch)
2021-12-02 16:26 PST
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2021-12-02 16:26:12 PST
Created
attachment 445786
[details]
Patch
Darin Adler
Comment 2
2021-12-02 16:54:24 PST
Comment on
attachment 445786
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=445786&action=review
> Source/WebCore/Modules/web-locks/WebLockManager.cpp:68 > +class WebLockManager::MainThreadBridge : public ThreadSafeRefCounted<MainThreadBridge, WTF::DestructionThread::Main> {
Are we reference counting this just for the "destroy on main thread" bit? Because if so, I suggest we instead use unique_ptr and null it out on the main thread in ~WebLockManager. I don’t see any ref/deref of this going on.
Chris Dumez
Comment 3
2021-12-02 16:55:39 PST
Comment on
attachment 445786
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=445786&action=review
We need ref-counting to keep ourselves alive while dispatching to the main thread as well.
> Source/WebCore/Modules/web-locks/WebLockManager.cpp:161 > + Ref protectedThis { *this };
We Ref here.
> Source/WebCore/Modules/web-locks/WebLockManager.cpp:165 > + downcast<WorkerGlobalScope>(*m_context).thread().workerLoaderProxy().postTaskToLoader([task = WTFMove(task), protectedThis = Ref { *this }](auto& context) {
and here.
EWS
Comment 4
2021-12-02 18:54:46 PST
Committed
r286474
(
244813@main
): <
https://commits.webkit.org/244813@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 445786
[details]
.
Radar WebKit Bug Importer
Comment 5
2021-12-02 18:55:25 PST
<
rdar://problem/85999405
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug