Bug 230262 – Migrate to libwebrtc non-racy setRemoteDescription/setLocalDescription variants

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

Migrate to libwebrtc non-racy setRemoteDescription/setLocalDescription variants

[youenn fablet]

Created attachment 438129 [details]
Patch

[youenn fablet]

Created attachment 438150 [details]
Patch

[Darin Adler]

Comment on attachment 438150 [details]
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.

[youenn fablet]

(In reply to Darin Adler from comment #3)
> Comment on attachment 438150 [details]
> 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.

We indeed can do it.
Thinking more about it, given the typed parameters, we can probably just pass a pointer to the methods and the scoped_refptr will be created for us.

> > 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.

OK.

> > Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCObservers.h:93
> > +    void OnSetRemoteDescriptionComplete(webrtc::RTCError error) final
> 
> Not important, but seems like this can be private.

OK.

[youenn fablet]

Created attachment 438219 [details]
Patch for landing

[EWS]

Committed r282444 (241697@main): <https://commits.webkit.org/241697@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/83135937>