Bug 234883

Summary: WorkerMessagePortChannelProvider::takeAllMessagesForPort should guarantee execution of the takeMessagePort callback
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, esprehn+autocc, ews-watchlist, kangil.han, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description youenn fablet 2022-01-05 08:11:18 PST 
WorkerMessagePortChannelProvider::takeAllMessagesForPort should guarantee execution of the takeMessagePort callback
Comment 1 youenn fablet 2022-01-05 08:11:30 PST 
<rdar://86708232>
Comment 2 youenn fablet 2022-01-05 08:14:28 PST 
Created attachment 448388 [details]
Patch
Comment 3 Alex Christensen 2022-01-05 11:42:08 PST 
Comment on attachment 448388 [details]
Patch

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

> Source/WebCore/dom/messageports/WorkerMessagePortChannelProvider.cpp:110
> +    Function<void()> m_completionHandler;

Can this be a CompletionHandler?

> Source/WebCore/dom/messageports/WorkerMessagePortChannelProvider.cpp:119
>          MessagePortChannelProvider::singleton().takeAllMessagesForPort(identifier, [this, workerThread = WTFMove(workerThread), callbackIdentifier](Vector<MessageWithMessagePorts>&& messages, Function<void()>&& completionHandler) {

ditto
Comment 4 youenn fablet 2022-01-06 01:33:43 PST 
(In reply to Alex Christensen from comment #3)
> Comment on attachment 448388 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=448388&action=review
> 
> > Source/WebCore/dom/messageports/WorkerMessagePortChannelProvider.cpp:110
> > +    Function<void()> m_completionHandler;
> 
> Can this be a CompletionHandler?

Yes, will change it.

> > Source/WebCore/dom/messageports/WorkerMessagePortChannelProvider.cpp:119
> >          MessagePortChannelProvider::singleton().takeAllMessagesForPort(identifier, [this, workerThread = WTFMove(workerThread), callbackIdentifier](Vector<MessageWithMessagePorts>&& messages, Function<void()>&& completionHandler) {
> 
> ditto

It seems so, and this might be good given that probably makes sure we do not accumulate data in the networking process.
We should do that in a separate patch though.
Comment 5 youenn fablet 2022-01-06 02:04:14 PST 
Created attachment 448473 [details]
Patch for landing
Comment 6 EWS 2022-01-06 02:51:10 PST 
Committed r287679 (245776@main): <https://commits.webkit.org/245776@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448473 [details].