Bug 201333

Summary: Introduce WorkerMessagePortChannelRegistry
Product: WebKit Reporter: youenn fablet <youennf>
Component: Page LoadingAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, cdumez, commit-queue, dbates, esprehn+autocc, ews-watchlist, kangil.han, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=205357
https://bugs.webkit.org/show_bug.cgi?id=205414
Bug Depends on:    
Bug Blocks: 201299    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

youenn fablet
Reported 2019-08-30 00:41:16 PDT
Introduce WorkerMessagePortChannelRegistry
Attachments
Patch (55.23 KB, patch)
2019-08-30 01:48 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch (55.27 KB, patch)
2019-08-30 02:04 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch (55.23 KB, patch)
2019-09-01 13:21 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (55.26 KB, patch)
2019-09-01 22:51 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
youenn fablet
Comment 1 2019-08-30 01:48:17 PDT
Created attachment 377689 [details] Patch
youenn fablet
Comment 2 2019-08-30 02:04:28 PDT
Created attachment 377690 [details] Patch
Alex Christensen
Comment 3 2019-08-30 08:53:23 PDT
Comment on attachment 377690 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377690&action=review > Source/WebCore/dom/MessagePort.cpp:252 > + auto scopeExit = makeScopeExit([&completionCallback] { > + completionCallback(); > + }); Why make a lambda that calls the Function instead of just using the Function itself? Why no WTFMove? Also, this seems like it should be a CompletionHandler. > Source/WebCore/dom/MessagePort.cpp:315 > + callOnMainThread([remoteIdentifier = m_remoteIdentifier, weakThis = makeWeakPtr(const_cast<MessagePort*>(this)), workerThread = WTFMove(workerThread)]() mutable { makeWeakPtr should take const T& instead of non-const T& and this const_cast shouldn't be necessary. Same with const T*
youenn fablet
Comment 4 2019-09-01 13:21:38 PDT
Created attachment 377829 [details] Patch
youenn fablet
Comment 5 2019-09-01 22:51:08 PDT
Created attachment 377839 [details] Patch for landing
youenn fablet
Comment 6 2019-09-01 22:53:55 PDT
(In reply to Alex Christensen from comment #3) > Comment on attachment 377690 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=377690&action=review > > > Source/WebCore/dom/MessagePort.cpp:252 > > + auto scopeExit = makeScopeExit([&completionCallback] { > > + completionCallback(); > > + }); > > Why make a lambda that calls the Function instead of just using the Function > itself? Why no WTFMove? Also, this seems like it should be a > CompletionHandler. Right. > > Source/WebCore/dom/MessagePort.cpp:315 > > + callOnMainThread([remoteIdentifier = m_remoteIdentifier, weakThis = makeWeakPtr(const_cast<MessagePort*>(this)), workerThread = WTFMove(workerThread)]() mutable { > > makeWeakPtr should take const T& instead of non-const T& and this const_cast > shouldn't be necessary. Same with const T* makeWeakPtr can take a const pointer as input but it will create a WeakPtr of a const pointer. In that case, we want to have a non const weakThis to call updateActivity.
WebKit Commit Bot
Comment 7 2019-09-02 02:31:21 PDT
Comment on attachment 377839 [details] Patch for landing Clearing flags on attachment: 377839 Committed r249378: <https://trac.webkit.org/changeset/249378>
WebKit Commit Bot
Comment 8 2019-09-02 02:31:23 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2019-09-02 02:32:18 PDT
<rdar://problem/54945252>
Note You need to log in before you can comment on or make changes to this bug.