Bug 165251 – Annotate MediaStream and WebRTC idl with EnabledAtRuntime flag

• Component: Bindings
• Status: RESOLVED FIXED

[Alex. Gouaillard]

It has been brought to my attention that apparently Safari Tech Preview Release 18 (Safari 10.1, WebKit 12603.1.12) is exposing RTCPeerConnection and navigator.mediaDevices.getUserMedia.

Since a check for
   navigator.mediaDevices.getUserMedia && window.RTCPeerConnection
and redirecting browsers that support neither is pretty common this is rather unfortunate.

The getUserMedia implementation does not seems to work on one of the most basic GUM samples at
    https://webrtc.github.io/samples/src/content/getusermedia/gum/
I am actually not seeing GUM resolve or reject the promise at all.

And RTCPeerConnection did not work either in
    https://webrtc.github.io/samples/src/content/peerconnection/pc1/

What is the plan here?

Philipp

[Alex. Gouaillard]

I think we should add runtime flags for getUserMedia and RTCPeerConnection and set them to off in Safari Tech Preview until stable enough.

Then we might just need to make the related IDLs exposed according that flag through EnabledAtRuntime keyword.

[Philippe Normand]

There's a Setting already for gUM, AFAIK.

[Philippe Normand]

(In reply to comment #2)
> There's a Setting already for gUM, AFAIK.

For the GTK port only though :)

[youenn fablet]

You might want to consider the following IDLs:

Source/WebCore/Modules/mediastream/MediaStreamEvent.idl
Source/WebCore/Modules/mediastream/NavigatorMediaDevices.idl
Source/WebCore/Modules/mediastream/NavigatorUserMedia.idl
Source/WebCore/Modules/mediastream/RTCDTMFToneChangeEvent.idl
Source/WebCore/Modules/mediastream/RTCIceCandidate.idl
Source/WebCore/Modules/mediastream/RTCPeerConnection.idl
Source/WebCore/Modules/mediastream/RTCRtpReceiver.idl
Source/WebCore/Modules/mediastream/RTCRtpSender.idl
Source/WebCore/Modules/mediastream/RTCRtpTransceiver.idl
Source/WebCore/Modules/mediastream/RTCSessionDescription.idl
Source/WebCore/Modules/mediastream/RTCTrackEvent.idl

You might also need to update CommonIdentifiers.h

[Alex. Gouaillard]

and not

https://trac.webkit.org/browser/trunk/Source/WebCore/Modules/mediastream/NavigatorMediaDevices.idl

?

[Alex. Gouaillard]

my bad, was in the list ....
waiting for webkit checkout ... i will never complain about chromium checkout time again ....

[Alex. Gouaillard]

Created attachment 295935 [details]
Patch

[youenn fablet]

Updating CommonIdentifiers.h should fix the build issue

[youenn fablet]

Comment on attachment 295935 [details]
Patch

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

> Source/WebCore/Modules/mediastream/RTCIceCandidate.idl:37
>      ImplementationLacksVTable,

You probable need to do something like EnabledAtRuntime=PeerConnection (or MediaStream) depending on the IDL.

[Alex. Gouaillard]

(In reply to comment #8)
> Updating CommonIdentifiers.h should fix the build issue

thanks for the comment. I took a look at the file (eventually the one in JavaScriptCore), however, I'm not sure what I am supposed to do there. Please educate me.

[Alex. Gouaillard]

Should I add some entries in generic/RuntimeEnabledFeatures.h instead?

[youenn fablet]

If you look at the generated code, it is searching for property names that are not defined (like XxEvent). You need to add them directly by hand in CommonIdentifiers.h. please have a look at other IDLs having EnabledAtRuntime.
You will see the name of these interfaces in CommonIdentifiers.h
We probably should have a better way to handle that in the future.

[Alex. Gouaillard]

ok, looked at IDBTransaction and I see the corresponding entry in the CommonIdentifiers.h.

I do not understand what the =InterfaceName is doing there though, and if I should add it for the RTC objects.

I will take care of the CommonIdentifiers additions first.

[Chris Dumez]

Comment on attachment 295935 [details]
Patch

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

> Source/WebCore/Modules/mediastream/MediaStreamEvent.idl:28
> +    EnabledAtRuntime,

It needs to be EnabledAtRuntime=Something

And Something needs to be defined in RuntimeEnabledFeatures.h

[Chris Dumez]

Comment on attachment 295935 [details]
Patch

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

>> Source/WebCore/Modules/mediastream/MediaStreamEvent.idl:28
>> +    EnabledAtRuntime,
> 
> It needs to be EnabledAtRuntime=Something
> 
> And Something needs to be defined in RuntimeEnabledFeatures.h

There is already one for MediaStream:
EnabledAtRuntime=MediaStream

[Alex. Gouaillard]

Created attachment 295964 [details]
Patch

[Alex. Gouaillard]

(In reply to comment #15)
> Comment on attachment 295935 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295935&action=review
> 
> >> Source/WebCore/Modules/mediastream/MediaStreamEvent.idl:28
> >> +    EnabledAtRuntime,
> > 
> > It needs to be EnabledAtRuntime=Something
> > 
> > And Something needs to be defined in RuntimeEnabledFeatures.h
> 
> There is already one for MediaStream:
> EnabledAtRuntime=MediaStream

thanks.
understood. changes in the way.

[Alex. Gouaillard]

Created attachment 295965 [details]
Patch

[Build Bot]

Comment on attachment 295965 [details]
Patch

Attachment 295965 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2608563

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl.html
imported/w3c/web-platform-tests/webrtc/datachannel-emptystring.html
imported/w3c/web-platform-tests/webrtc/no-media-call.html
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html
imported/w3c/web-platform-tests/webrtc/promises-call.html

[Build Bot]

Created attachment 295977 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 295965 [details]
Patch

Attachment 295965 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2608607

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl.html
imported/w3c/web-platform-tests/webrtc/datachannel-emptystring.html
imported/w3c/web-platform-tests/webrtc/no-media-call.html
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html
imported/w3c/web-platform-tests/webrtc/promises-call.html

[Build Bot]

Created attachment 295978 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 295965 [details]
Patch

Attachment 295965 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2608610

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl.html
imported/w3c/web-platform-tests/webrtc/datachannel-emptystring.html
imported/w3c/web-platform-tests/webrtc/no-media-call.html
imported/w3c/web-platform-tests/webrtc/promises-call.html
http/tests/ssl/media-stream/get-user-media-nested.html
http/tests/ssl/media-stream/get-user-media-secure-connection.html
http/tests/ssl/media-stream/get-user-media-different-host.html
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html

[Build Bot]

Created attachment 295983 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 295965 [details]
Patch

Attachment 295965 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2608650

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 295984 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Philippe Normand]

If this runtime flag is off by default then the test harness needs to turn it on, to avoid the test failures reported by the EWS.

[Philippe Normand]

See how it was done for SubtleCrypto, for instance, in bug #164982

[Alex. Gouaillard]

Thanks phil. I will address in 12 hours.

[Eric Carlson]

Created attachment 296718 [details]
Updated patch.

[Alex. Gouaillard]

Comment on attachment 296718 [details]
Updated patch.

thanks eric for pushing this through the finish line. I'm not sure I would have been able to find all the remaining needed modifications this week-end.

[Dean Jackson]

Comment on attachment 296718 [details]
Updated patch.

To make it easier for testing you might want to add support for the comment lines in DRT and WKTR.

[Eric Carlson]

Created attachment 296759 [details]
Patch for landing.

[Eric Carlson]

(In reply to comment #31)
> Comment on attachment 296718 [details]
> Updated patch.
> 
> thanks eric for pushing this through the finish line. I'm not sure I would
> have been able to find all the remaining needed modifications this week-end.

Thank you for getting things started!

[WebKit Commit Bot]

Comment on attachment 296759 [details]
Patch for landing.

Clearing flags on attachment: 296759

Committed r209643: <http://trac.webkit.org/changeset/209643>

[Alexey Proskuryakov]

This change appears to have made WebKit2.EnumerateDevices API test always time out.

[Eric Carlson]

Created attachment 297013 [details]
Updated to fix broken API test.

[Eric Carlson]

Committed rr209757: https://trac.webkit.org/r209757