Bug 236038

Summary: Move connect event dispatch logic to SharedWorkerGlobalScope
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, ryuan.choi, sam, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 236028    
Bug Blocks: 235958    
Attachments:
Description Flags
Patch
none
Patch ews-feeder: commit-queue-

Description Chris Dumez 2022-02-02 13:58:53 PST
Move connect event dispatch logic to SharedWorkerGlobalScope and add logging.
Comment 1 Chris Dumez 2022-02-02 14:01:50 PST
Created attachment 450696 [details]
Patch
Comment 2 Darin Adler 2022-02-02 15:06:33 PST
Comment on attachment 450696 [details]
Patch

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

> Source/WebCore/workers/shared/SharedWorkerGlobalScope.cpp:68
> +    auto serializedScriptValue = SerializedScriptValue::create("");

emptyString()?

Shorter variable name? Maybe serializedValue or even just value?

> Source/WebCore/workers/shared/SharedWorkerGlobalScope.h:28
> +#include "MessageWithMessagePorts.h"

Do we really need this header? No way to forward declare TransferredMessagePort instead?
Comment 3 Chris Dumez 2022-02-02 15:11:03 PST
(In reply to Darin Adler from comment #2)
> Comment on attachment 450696 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=450696&action=review
> 
> > Source/WebCore/workers/shared/SharedWorkerGlobalScope.cpp:68
> > +    auto serializedScriptValue = SerializedScriptValue::create("");
> 
> emptyString()?
> 
> Shorter variable name? Maybe serializedValue or even just value?
> 
> > Source/WebCore/workers/shared/SharedWorkerGlobalScope.h:28
> > +#include "MessageWithMessagePorts.h"
> 
> Do we really need this header? No way to forward declare
> TransferredMessagePort instead?


Once Bug 236028 lands, I'll be able to include TransferredMessagePort.h instead which will be an improvement. I don't think I can conveniently forward-declare though since this is basically a typedef to a std::pair<MessagePortIdentifier, MessagePortIdentifier>.

I will wait for Bug 236028 before landing this one, so I can fix the include.
Comment 4 Chris Dumez 2022-02-02 15:33:46 PST
Created attachment 450706 [details]
Patch
Comment 5 Chris Dumez 2022-02-02 18:19:20 PST
Committed r289023 (246731@trunk): <https://commits.webkit.org/246731@trunk>
Comment 6 Radar WebKit Bug Importer 2022-02-02 18:20:20 PST
<rdar://problem/88412313>