[webkit-reviews] review granted: [Bug 221891] ThreadMessageReceiverRefCounted subclasses are accessed in non-thread-safe way : [Attachment 420291] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Kimmo Kinnunen
<kkinnunen at apple.com>'s request for review:
Bug 221891: ThreadMessageReceiverRefCounted subclasses are accessed in
non-thread-safe way
https://bugs.webkit.org/show_bug.cgi?id=221891

Attachment 420291: Patch

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




--- Comment #3 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 420291
  --> https://bugs.webkit.org/attachment.cgi?id=420291
Patch

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

r=me with nits.

>
Source/WebKit/GPUProcess/webrtc/RemoteAudioMediaStreamTrackRendererManager.cpp:
46
> +void RemoteAudioMediaStreamTrackRendererManager::connectGPUProcess()

I don't understand why this is named connectGPUProcess(). This code runs in the
GPUProcess and if anything, it starts listening for IPC from the WebProcess. I
think we should find a better name. Maybe startListeningForIPC()?

>
Source/WebKit/GPUProcess/webrtc/RemoteAudioMediaStreamTrackRendererManager.h:66
> +    void disconnectGPUProcess();

What's this? Can't find the definition.

> Source/WebKit/GPUProcess/webrtc/RemoteSampleBufferDisplayLayerManager.cpp:48
> +void RemoteSampleBufferDisplayLayerManager::connectGPUProcess()

Same comment about naming.

> Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp:98
> +void NetworkRTCProvider::connectWebProcess()

Same comment about naming.

> Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:171
> +void LibWebRTCCodecs::connectGPUProcess()

Same comment about naming.

> Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.cpp:52
> +void RemoteCaptureSampleManager::disconnectGPUProcess()

Same comment about naming.