[webkit-reviews] review granted: [Bug 201333] Introduce WorkerMessagePortChannelRegistry : [Attachment 377690] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 201333: Introduce WorkerMessagePortChannelRegistry
https://bugs.webkit.org/show_bug.cgi?id=201333

Attachment 377690: Patch

https://bugs.webkit.org/attachment.cgi?id=377690&action=review




--- Comment #3 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 377690
  --> https://bugs.webkit.org/attachment.cgi?id=377690
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*