Bug 191611 – [WebRTC] Provide default implementation of LibWebRTCProvider

• Component: WebRTC
• Status: RESOLVED FIXED

[Don Olmstead]

If !USE(LIBWEBRTC) there should be a default disabled LibWebRTCProvider. Currently Windows has one since it has no support but other platforms should be taken into account.

[Ross Kirsling]

Created attachment 354739 [details]
Patch

[Ross Kirsling]

Created attachment 354740 [details]
Patch

[Don Olmstead]

Comment on attachment 354740 [details]
Patch

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

> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:71
> +#if !USE(LIBWEBRTC)
> +bool LibWebRTCProvider::webRTCAvailable()
> +{
> +#if PLATFORM(COCOA)
> +    return true;
> +#else
> +    return false;
> +#endif
> +}
> +#endif

I know this was in the original code for cocoa returning true but I have a feeling this is a bug on their end. If so you can just collapse this into one big #if

[Michael Catanzaro]

Comment on attachment 354740 [details]
Patch

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

> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:55
> -#if !PLATFORM(COCOA) && !PLATFORM(GTK) && !PLATFORM(WPE)
> +#if !USE(LIBWEBRTC) || (!PLATFORM(COCOA) && !USE(GSTREAMER))

Confusing... why not just:

#if !USE(LIBWEBRTC)

>> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:71
>> +#endif
> 
> I know this was in the original code for cocoa returning true but I have a feeling this is a bug on their end. If so you can just collapse this into one big #if

Yeah Don is right, there's no way this is supposed to return true. We could do archaeology to figure out where it broke... or just fix it now. :)

[Don Olmstead]

(In reply to Michael Catanzaro from comment #4)
> Comment on attachment 354740 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354740&action=review
> 
> > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:55
> > -#if !PLATFORM(COCOA) && !PLATFORM(GTK) && !PLATFORM(WPE)
> > +#if !USE(LIBWEBRTC) || (!PLATFORM(COCOA) && !USE(GSTREAMER))
> 
> Confusing... why not just:
> 
> #if !USE(LIBWEBRTC)

Technically it seemed like the Glib ports wanted USE(LIBWEBRTC) && USE(GSTREAMER) to be true.

> >> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:71
> >> +#endif
> > 
> > I know this was in the original code for cocoa returning true but I have a feeling this is a bug on their end. If so you can just collapse this into one big #if
> 
> Yeah Don is right, there's no way this is supposed to return true. We could
> do archaeology to figure out where it broke... or just fix it now. :)

Yea its really weird not sure if we should wait for youenn to chime in.

[youenn fablet]

Comment on attachment 354740 [details]
Patch

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

>>>> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:71
>>>> +#endif
>>> 
>>> I know this was in the original code for cocoa returning true but I have a feeling this is a bug on their end. If so you can just collapse this into one big #if
>> 
>> Yeah Don is right, there's no way this is supposed to return true. We could do archaeology to figure out where it broke... or just fix it now. :)
> 
> Yea its really weird not sure if we should wait for youenn to chime in.

Yep, sounds good to simplify it.

[Ross Kirsling]

Created attachment 354756 [details]
Patch for landing

[Ross Kirsling]

Created attachment 354757 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 354757 [details]
Patch for landing

Clearing flags on attachment: 354757

Committed r238159: <https://trac.webkit.org/changeset/238159>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46051555>

[Ross Kirsling]

Argh, this is what I get for being impatient -- GTK/WPE build fix incoming.

[Ross Kirsling]

(In reply to Ross Kirsling from comment #12)
> Argh, this is what I get for being impatient -- GTK/WPE build fix incoming.

Committed r238160: <https://trac.webkit.org/changeset/238160>

[Don Olmstead]

Your unreviewed build fix should be doing USE(GSTREAMER) && USE(LIBWEBRTC). Of course check the file to see what’s there.

[Ross Kirsling]

(In reply to Don Olmstead from comment #14)
> Your unreviewed build fix should be doing USE(GSTREAMER) && USE(LIBWEBRTC).
> Of course check the file to see what’s there.

Sorry about that.

Committed r238161: <https://trac.webkit.org/changeset/238161>