1732409 - Prepare for dispatching devicechange events even without an actively capturing MediaStreamTrack

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, enhancement, P1)

Description

[Karl Tomlinson (:karlt)]

Gecko currently dispatches "devicechange" events only when

1. there is an actively capturing MediaStreamTrack, or
2. the origin has a permission for permanent access to a camera or microphone.

"fully active and has focus" has subsequently been added to the spec as a sufficient condition. This provides for "audiooutput" devices even when getUserMedia() is not used.

The permissions check was replaced in the spec with a [[canExposeDeviceInfo]] MediaDevices slot, set on getUserMedia() grant.
This makes the actively capturing test redundant as a track cannot become active without a getUserMedia() grant.

Removing the actively capturing test will resolve a defect where actively capturing tracks can be prematurely GC'd causing loss of "devicechange" events.

With provision of "devicechange" events in more circumstances, we should only add events for devices that should be exposed.

Comment 1

[Karl Tomlinson (:karlt)]

"User Agents MAY add fuzzing on the timing of events to avoid cross-origin activity correlation."
Waiting for focus before dispatch would provide the best protection against cross-origin activity correlation. With the plan to wait for focus in enumerateDevices(), there would be little value in dispatching "devicechange" before enumerateDevices() could proceed.

Comment 2

[Karl Tomlinson (:karlt)]

"devicechange" events should be dispatched only when devices are physically added or removed rather then when they become exposed to content through getUserMedia() or selectAudioOutput(). To provide this, a record of the ordered set of system physical devices at the time when devicechange event decisions were last performed will be kept on each MediaDevices object. This record can be compared with subsequent sets to determine whether the changes should be exposed in a "devicechange" event.

This approach overcomes a number of issues with the spec's approach of keeping a [[storedDeviceList]] that includes only exposed devices and is reset on exposure decision changes.

To reduce to memory footprint of these device set records, the sets will be shared across Window/MediaDevices objects where appropriate.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 use is(a,b) instead of ok(a===b) for more info in failure message r?jib

Depends on D132893

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 check test assumptions about device info labels r?pehrsons

to aid in debugging.

Depends on D132894

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 let fake:true getUserMedia() parameter override loopback prefs r?jib

and ignore fake:false so that it behaves like a missing/default fake parameter
and does not override "media.navigator.streams.fake".

Neither of these combinations are required in tests, but the fake:true
override provides simpler testing of real-world scenarios, such as bug
1743524, even when loopback prefs are set, as is the preferred default test
configuration.

Also, adjusting the logic in this way so that fake: in getUserMedia()
behaves similarly to ShouldResistFingerprinting() in enumerateDevices() will
allow simplification of EnumerateDevicesImpl() parameters and moving the logic
for the effect of preferences on enumeration to one place.

Depends on D132895

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 make getUserMedia() permission prompt requirement independent of "media.navigator.streams.fake" r?jib

"media.navigator.permission.disabled" exists for controlling the permission
prompt separately.

This will allow consolidation of camera and microphone preference handling
into EnumerateRawDevices(). Although auto-disabling permission with
"media.navigator.streams.fake" is sometimes useful for tests, it is actually
easier to consider them independently because sometimes the auto-disabling
needs to be overridden.

The logic to disable the permission prompt with content-exposed fake:true
parameter is maintained as is.

The CamerasParent usage of "media.navigator.permission.fake" seems to have
been a misunderstanding in
https://hg.mozilla.org/mozilla-central/rev/c5d6c3e00c91dd0595a12b145c66cf4f2a890591#l6.129

Depends on D132896

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 consolidate camera and microphone preference handling into EnumerateRawDevices r?jib

This is performed for
"media.navigator.streams.fake"
"media.video_loopback_dev"
"media.audio_loopback_dev"

Depends on D132897

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 move GetPrincipalKey() call to AnonymizeDevices() r?jib

reducing the complexity in EnumerateDevicesImpl() and facilitating a future
call to perform a similar operation in an enumerateDevices() refactor for
device-change event analysis.

Depends on D132898

Comment 9

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 remove unnecessary additional SetupDeviceChangeListener() call r?jib

EventTarget::SetEventHandler() will cause EventListenerAdded() to be called.

Depends on D132899

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 support main thread enumeration of devices from MediaEngineDefault r?pehrsons

MediaDevices are suitable for use on any thread.
This will support a simple override of devices from enumerateDevices() when
resistFingerprinting is set.

Depends on D132900

Comment 11

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 use an enumerator template for adding and removing pref observation r?jib

Provides that the list added must match the list removed.

Depends on D132901

Comment 12

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 provide a method to return a cached list of physical devices r?jib

The same list of physical devices can, in a subsequent patch, be returned to
multiple Windows, so that their MediaDevices objects can efficiently keep (and
share) a record of the set of devices when the last "devicechange" event was
queued.

Depends on D132902

Comment 13

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 emulate focus for webrtc mochitests r?jib

enumerateDevices() and devicechange events both require focus.
Focus emulation facilitates running tests on developer machines.

Depends on D132903

Comment 14

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 change fake camera device id when name changes r?jib

The device MUST be uniquely identified by its identifier and its kind.
https://w3c.github.io/mediacapture-main/#dom-mediadeviceinfo-deviceid

A change in name now behaves like a change in device, which is useful for
devicechange event tests.

Depends on D132904

Comment 15

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 use fake device name changes to test devicechange events r?jib

"media.ondevicechange.fakeDeviceChangeEvent.enabled" does not change any
devices and so will not generate any events if/when no-op devicechange events
are filtered out.

The test is rearranged to reduce the wait for no event and run some steps in
parallel, so that the test runs in half the time. This benefit will
accumulate as more scenarios are added to the test.

Depends on D132905

Comment 16

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 wait for focus before dispatching devicechange events r?jib

There was little value in dispatching devicechange before enumerateDevices
could proceed.

Depends on D132906

Comment 17

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 Dispatch devicechange events even without an actively capturing MediaStreamTrack r?jib

"fully active and has focus" is now a sufficient condition for dispatching "devicechange" events if the change in devices should be visible from enumerateDevices().
https://github.com/w3c/mediacapture-main/pull/574/files#diff-1217ca1c44ff30a33dd50c49d03b5cadc9633c789df8ff9370ed4a42859e1211R3146

Permissions checks are replaced with [[canExposeCameraInfo]] and [[canExposeMicrophoneInfo]] slots set by getUserMedia().
https://github.com/w3c/mediacapture-main/pull/641
https://github.com/w3c/mediacapture-main/pull/773

The "media.navigator.permission.disabled" pref is no longer involved in "devicechange" dispatch decisions.

Depends on D132907

Comment 18

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 test devicechange events when first device is added or last is removed r?jib

Depends on D132908

Comment 19

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 test "devicechange" event with different exposure timing r?jib

Depends on D132910

Comment 20

[Karl Tomlinson (:karlt)]

Attachment: Bug 1732409 remove ignored "fake" dictionary member passed when screensharing r?jib

The fake MediaStreamConstraints dictionary member has no effect on screen and
window sharing because it affects only microphones and cameras since
https://hg.mozilla.org/mozilla-central/rev/39c9062db9d9

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21564d1c7057
remove ignored "fake" dictionary member passed when screensharing r=jib
https://hg.mozilla.org/integration/autoland/rev/3f3fe6a97855
use is(a,b) instead of ok(a===b) for more info in failure message r=jib
https://hg.mozilla.org/integration/autoland/rev/748715ddf3c0
check test assumptions about device info labels r=jib
https://hg.mozilla.org/integration/autoland/rev/a82289f4ec60
let fake:true getUserMedia() parameter override loopback prefs r=jib
https://hg.mozilla.org/integration/autoland/rev/0a6943f27392
make getUserMedia() permission prompt requirement independent of "media.navigator.streams.fake" r=jib
https://hg.mozilla.org/integration/autoland/rev/39bcb53f30ba
consolidate camera and microphone preference handling into EnumerateRawDevices r=jib

Comment 22

[Atila Butkovits]

Backed out for causing failures at test_bug386782.html.

Backout link: https://hg.mozilla.org/integration/autoland/rev/f9aa1b75c0bf4ca99838fb1b90e510a0ee087d01

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=HkjK3lmISyefV1cvWIu3dQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=39bcb53f30bade3b4063088650205067e2d8e4ab

Failure log: https://treeherder.mozilla.org/logviewer?job_id=362023394&repo=autoland&lineNumber=2563

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b6419c9cc70
remove ignored "fake" dictionary member passed when screensharing r=jib
https://hg.mozilla.org/integration/autoland/rev/ae9a00796731
use is(a,b) instead of ok(a===b) for more info in failure message r=jib
https://hg.mozilla.org/integration/autoland/rev/482eff672f41
check test assumptions about device info labels r=jib

Comment 24

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/7b6419c9cc70
https://hg.mozilla.org/mozilla-central/rev/ae9a00796731
https://hg.mozilla.org/mozilla-central/rev/482eff672f41

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53ee4bd9dd9e
remove unnecessary additional SetupDeviceChangeListener() call r=jib
https://hg.mozilla.org/integration/autoland/rev/6ace5bb48f4b
support main thread enumeration of devices from MediaEngineDefault r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/e4b27e4b78aa
use an enumerator template for adding and removing pref observation r=jib
https://hg.mozilla.org/integration/autoland/rev/214b3fd5a76a
emulate focus for webrtc mochitests r=jib
https://hg.mozilla.org/integration/autoland/rev/9e5cbf574e40
change fake camera device id when name changes r=jib

Comment 26

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/53ee4bd9dd9e
https://hg.mozilla.org/mozilla-central/rev/6ace5bb48f4b
https://hg.mozilla.org/mozilla-central/rev/e4b27e4b78aa
https://hg.mozilla.org/mozilla-central/rev/214b3fd5a76a
https://hg.mozilla.org/mozilla-central/rev/9e5cbf574e40

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a1358132987f
let fake:true getUserMedia() parameter override loopback prefs r=jib
https://hg.mozilla.org/integration/autoland/rev/bbe19c0105fb
make getUserMedia() permission prompt requirement independent of "media.navigator.streams.fake" r=jib
https://hg.mozilla.org/integration/autoland/rev/e9a430d93229
consolidate camera and microphone preference handling into EnumerateRawDevices r=jib

Comment 28

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a1358132987f
https://hg.mozilla.org/mozilla-central/rev/bbe19c0105fb
https://hg.mozilla.org/mozilla-central/rev/e9a430d93229

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2dcdc75fdf55
move GetPrincipalKey() call to AnonymizeDevices() r=jib

Comment 30

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/2dcdc75fdf55

Comment 31

[Karl Tomlinson (:karlt)]

I'll move the remaining patches to bug 1753131.

Comment 32

[Phabricator Automation]

Comment on attachment 9253816 [details]
Bug 1732409 provide a method to return a cached list of physical devices r?jib

Revision D132903 was moved to bug 1753131. Setting attachment 9253816 [details] to obsolete.

Comment 33

[Phabricator Automation]

Comment on attachment 9253819 [details]
Bug 1732409 use fake device name changes to test devicechange events r?jib

Revision D132906 was moved to bug 1753131. Setting attachment 9253819 [details] to obsolete.

Comment 34

[Phabricator Automation]

Comment on attachment 9253820 [details]
Bug 1732409 wait for focus before dispatching devicechange events r?jib

Revision D132907 was moved to bug 1753131. Setting attachment 9253820 [details] to obsolete.

Comment 35

[Phabricator Automation]

Comment on attachment 9253821 [details]
Bug 1732409 Dispatch devicechange events even without an actively capturing MediaStreamTrack r?jib

Revision D132908 was moved to bug 1753131. Setting attachment 9253821 [details] to obsolete.

Comment 36

[Phabricator Automation]

Comment on attachment 9253822 [details]
Bug 1732409 test devicechange events when first device is added or last is removed r?jib

Revision D132909 was moved to bug 1753131. Setting attachment 9253822 [details] to obsolete.

Comment 37

[Phabricator Automation]

Comment on attachment 9254991 [details]
Bug 1732409 test "devicechange" event with different exposure timing r?jib

Revision D133604 was moved to bug 1753131. Setting attachment 9254991 [details] to obsolete.