[webkit-reviews] review granted: [Bug 230262] Migrate to libwebrtc non-racy setRemoteDescription/setLocalDescription variants : [Attachment 438150] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 230262: Migrate to libwebrtc non-racy
setRemoteDescription/setLocalDescription variants
https://bugs.webkit.org/show_bug.cgi?id=230262

Attachment 438150: Patch

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 438150
  --> https://bugs.webkit.org/attachment.cgi?id=438150
Patch

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

> Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:148
> +    rtc::scoped_refptr<webrtc::SetLocalDescriptionObserverInterface>
observer(&m_setLocalSessionDescriptionObserver);

I wonder if we can leave out the <> here. If it was std::unique_ptr or
WTF::RefPtr we could, but not sure about rtc::scoped_refptr.

> Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:182
> +    rtc::scoped_refptr<webrtc::SetRemoteDescriptionObserverInterface>
observer(&m_setRemoteSessionDescriptionObserver);

Ditto.

> Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCObservers.h:69
> +    void OnSetLocalDescriptionComplete(webrtc::RTCError error) final

Not important, but seems like this can be private.

> Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCObservers.h:93
> +    void OnSetRemoteDescriptionComplete(webrtc::RTCError error) final

Not important, but seems like this can be private.