1313758 - WebRTC getUserMedia mediaSource 'browser' broken: Cause: webrtcUI.jsm (listScreenShareDevices) ==> getString() NS_ERROR_FAILURE

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[Matthew Jacobsen]

Attachment: Screenshare security chooser.

Attempt to start a WebRTC screen share using mediaSource: 'browser' causes NS_ERROR_FAILURE when looking up browser window String in bundle.

Firefox 49.0.2
Build identifier: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Target: x86_64-pc-linux-gnu (Ubuntu 14.04 64 bit).

Test scenario:

1) In about:config, add current https domain to media.getusermedia.screensharing.allowed_domains preference. Note that media.getusermedia.browser.enabled is true (by default).
2) Get the 'browser' media device via WebRTC API:
navigator.mediaDevices.getUserMedia({
  audio: false,
  video: { mediaSource: 'browser' }
}).then(
  function(stream) { alert('got stream!'); }
).catch(
  function(e) { alert('getUserMedia() error: ' + e.name); }
);
3) Wait for the screenshare security window chooser to display.


Screenshare security window chooser displays empty label in drop down selection (see attached screenshot) and selecting the only option and clicking "Share Selected Items" results in no callbacks to onSuccess or onFailure functions. The call to getUserMedia neither succeeds nor fails.

In the JS console the following describes the error:

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: XStringBundle :: getString :: line 21"  data: no]

getString()                                                                   XStringBundle
listScreenShareDevices()                                               webrtcUI.jsm:451
prompt/options.eventCallback()                                     webrtcUI.jsm:515
PopupNotifications_fireCallback()                                   PopupNotifications.jsm:1040
PopupNotifications_showPanel/notificationsToShow<()     PopupNotifications.jsm:720
filter()                                                                         self-hosted
PopupNotifications_showPanel()                                     PopupNotifications.jsm:719
PopupNotifications_update()                                          PopupNotifications.jsm:844
PopupNotifications_show()                                             PopupNotifications.jsm:394
prompt()                                                                      webrtcUI.jsm:582
this.webrtcUI.receiveMessage()                                      webrtcUI.jsm:216


Repeated tests will occasionally display the actual browser title bar text, instead of an empty String, in the screenshare security chooser. But the error is the same in the console and the behavior is otherwise identical insofar as the call to getUserMedia does not callback.


The call to getUserMedia should have succeeded or failed, calling back one of the onSuccess or onFailure functions.

NOTE: disabling the screenshare security chooser by setting about:config preference media.navigator.permission.disabled to true, avoids the issue and correctly provides the browser stream to the onSuccess callback. So it appears browser based WebRTC screensharing does work if the screenshare security chooser is avoided.

Comment 1

[Matthew Jacobsen]

Line 451 in webrtcUI.jsm corresponds to the marked line below:

      function listScreenShareDevices(menupopup, devices) {
        while (menupopup.lastChild)
          menupopup.removeChild(menupopup.lastChild);

        let type = devices[0].mediaSource;
        let typeName = type.charAt(0).toUpperCase() + type.substr(1);

        let label = chromeDoc.getElementById("webRTC-selectWindow-label");
        let stringId = "getUserMedia.select" + typeName;
        label.setAttribute("value",
--->                       stringBundle.getString(stringId + ".label"));
        label.setAttribute("accesskey",
                           stringBundle.getString(stringId + ".accesskey"));

        // "No <type>" is the default because we can't pick a
        // 'default' window to share.
        addDeviceToList(menupopup,
                        stringBundle.getString("getUserMedia.no" + typeName + ".label"),
                        "-1");
        menupopup.appendChild(chromeDoc.createElement("menuseparator"));

Comment 2

[Florian Quèze [:florian]]

Are we supporting tab sharing? I thought this was dropped when we removed Hello support (and that's most likely when we removed the strings) ?

Comment 3

[Matthew Jacobsen]

I didn't realize the discontinuation of Hello also meant the discontinuation of mediaSource: browser (MediaEngineTabVideoSource). Can someone confirm?

If it is supported still, can we add these strings back?

If it's not supported anymore, we should at least throw an error (call the on onFailure callback) when mediaSource: 'browser' is specified. Especially, given that it seems to still be fully supported in all other respects.

Comment 4

[Matthew Jacobsen]

Based on my testing, using mediaSource: 'browser' seems equivalent to using 'window' where the screenshare security chooser is simply limited to just the current browser window. This is really why I was using 'browser' to begin with. To avoid the confusion of selecting the current browser window in the chooser when using mediaSource: 'window'.

Can one pass in the browser's window id in the constraints using the experimental flag browserWindow to pre-select the correct window? Something like this seems like A) it'd be pretty helpful since there's no thumbnails like in Chrome, B) obviate the need for mediaSource: 'browser', and C) not too much work, maybe?

Comment 5

[Maire Reavy [:mreavy]]

(In reply to Florian Quèze [:florian] [:flo] from comment #2)
> Are we supporting tab sharing? I thought this was dropped when we removed
> Hello support (and that's most likely when we removed the strings) ?

My reading of the spec for screen sharing (https://www.w3.org/TR/screen-capture) says for Firefox to be compliant we would need to support tab capturing.  If I'm reading the spec correctly, 'browser' can be a single tab (document) or a full browser window.  We can verify this with Martin Thomson, who is one of the draft authors.

Firefox does not have and never has had UI (other than in Hello) for tab sharing.  To comply with the spec we will need to have it at some point, but it does not have to happen and will not happen in the current release.  To be extra clear: for this release (Firefox 52) we should not support a mediasource of 'browser'.

We should file a follow up meta bug for screen sharing compliance.  I believe we will need additional work, beyond the UI, to be fully spec compliant, and I believe it makes sense to plan to do that work in H1 of 2017.

Comment 6

[Florian Quèze [:florian]]

(In reply to Maire Reavy [:mreavy] from comment #5)
> To
> be extra clear: for this release (Firefox 52) we should not support a
> mediasource of 'browser'.

If I understand you correctly, we need a platform fix in 52 to call the failure callback immediately in this case without triggering the UI.

Comment 7

[Florian Quèze [:florian]]

Maybe we just want to set media.getusermedia.browser.enabled to false by default for now.

Comment 8

[Back Tuesday [:jib]]

Wfm, provided we keep the tests alive [1].

Note we currently implement an outdated draft [2] of the spec, so there's more work to do before we're spec compliant, but functionality wise it's the same.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/tests/mochitest/test_getUserMedia_basicTabshare.html
[2] http://fluffy.github.io/w3c-screen-share/#screen-based-video-constraints

Comment 9

[Back Tuesday [:jib]]

To be clear, I'm agreeing with Florian we should set media.getusermedia.browser.enabled to false (and modify the tests to set it to true so they still work and test the code).

Comment 10

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: Bug 1313758 - set media.getusermedia.browser.enabled to default false;

Review commit: https://reviewboard.mozilla.org/r/96484/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96484/

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8815612 [details]
Bug 1313758 - set media.getusermedia.browser.enabled to default false;

https://reviewboard.mozilla.org/r/96484/#review96978

Looks good

Comment 12

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ee0b377299f

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/617cd94bd7c5
set media.getusermedia.browser.enabled to default false; r=jesup

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/617cd94bd7c5