Bug 236038 - Move connect event dispatch logic to SharedWorkerGlobalScope
Summary: Move connect event dispatch logic to SharedWorkerGlobalScope
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 236028
Blocks: 235958
  Show dependency treegraph
 
Reported: 2022-02-02 13:58 PST by Chris Dumez
Modified: 2022-02-02 18:28 PST (History)
10 users (show)

See Also:


Attachments
Patch (21.73 KB, patch)
2022-02-02 14:01 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (21.64 KB, patch)
2022-02-02 15:33 PST, Chris Dumez
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>