Bug 236017

Summary: Move SharedWorkerThread & SharedWorkerProxy to workers/shared/context/
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, darin, ews-watchlist, ggaren, gyuyoung.kim, mkwst, ryuan.choi, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 235958    
Attachments:
Description Flags
Patch
none
Patch none

Description Chris Dumez 2022-02-02 08:09:12 PST
Move SharedWorkerThread & SharedWorkerProxy to workers/shared/context/, for consistency with service workers. Also rename SharedWorkerProxy to SharedWorkerThreadProxy.
Comment 1 Chris Dumez 2022-02-02 08:35:18 PST
Created attachment 450646 [details]
Patch
Comment 2 youenn fablet 2022-02-02 08:58:44 PST
Comment on attachment 450646 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=450646&action=review

> Source/WebCore/workers/shared/context/SharedWorkerThreadProxy.cpp:112
> +    m_scriptExecutionContext->postTask([this] (ScriptExecutionContext&) {

s/ScriptExecutionContext/auto/.
Are we sure this is valid?
Maybe add a comment stating why.

> Source/WebCore/workers/shared/context/SharedWorkerThreadProxy.h:40
> +class SharedWorkerThreadProxy : public ThreadSafeRefCounted<SharedWorkerThreadProxy>, public WorkerGlobalScopeProxy, public WorkerObjectProxy, public WorkerLoaderProxy, public WorkerDebuggerProxy {

final?

> Source/WebCore/workers/shared/context/SharedWorkerThreadProxy.h:65
> +    void setResourceCachingDisabledByWebInspector(bool) final;

Can they be made private?

> Source/WebCore/workers/shared/context/SharedWorkerThreadProxy.h:68
> +    SharedWorkerThreadProxy(SharedWorker&);

explicit.
Comment 3 Chris Dumez 2022-02-02 09:00:57 PST
(In reply to youenn fablet from comment #2)
> Comment on attachment 450646 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=450646&action=review
> 
> > Source/WebCore/workers/shared/context/SharedWorkerThreadProxy.cpp:112
> > +    m_scriptExecutionContext->postTask([this] (ScriptExecutionContext&) {
> 
> s/ScriptExecutionContext/auto/.
> Are we sure this is valid?
> Maybe add a comment stating why.
> 
> > Source/WebCore/workers/shared/context/SharedWorkerThreadProxy.h:40
> > +class SharedWorkerThreadProxy : public ThreadSafeRefCounted<SharedWorkerThreadProxy>, public WorkerGlobalScopeProxy, public WorkerObjectProxy, public WorkerLoaderProxy, public WorkerDebuggerProxy {
> 
> final?
> 
> > Source/WebCore/workers/shared/context/SharedWorkerThreadProxy.h:65
> > +    void setResourceCachingDisabledByWebInspector(bool) final;
> 
> Can they be made private?
> 
> > Source/WebCore/workers/shared/context/SharedWorkerThreadProxy.h:68
> > +    SharedWorkerThreadProxy(SharedWorker&);
> 
> explicit.

These classes change *significantly* in the next patch at Bug 235958 but OK.
Comment 4 Chris Dumez 2022-02-02 09:13:35 PST
Created attachment 450657 [details]
Patch
Comment 5 Chris Dumez 2022-02-02 11:57:00 PST
Comment on attachment 450657 [details]
Patch

Clearing flags on attachment: 450657

Committed r288975 (246704@trunk): <https://commits.webkit.org/246704@trunk>
Comment 6 Chris Dumez 2022-02-02 11:57:04 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2022-02-02 11:58:18 PST
<rdar://problem/88393944>