1720643 - "media.navigator.permission.disabled" pref should not bypass developer permissions policies (only user prompts)

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, task)

Description

[Karl Tomlinson (:karlt)]

MediaManager::GetUserMedia() uses PermissionDelegateHandler::GetPermission() to see whether the action should be denied without a prompt. This has a check on developer permissions policies via IsFeatureAllowed(), but also checks user permissions.

When the toplevel context is out of process, WindowContext fields are used for user permissions. Due to multiple async hops across processes, these are not necessarily up-to-date as hinted in code comments.

The permission checks in GetUserMedia() were added when webrtcUI.jsm merely added and removed permissions, without its own checks to short-circuit the prompt.

prompt() in WebRTCParent.jsm now has its own early check for BLOCKed user permissions, making redundant most of the checks from GetUserMedia().
Similarly GeckoView tells clients they should expect requests even when they have previously been denied.

Changing the GetUserMedia() code to use IsFeatureAllowed() directly would remove the duplication, clarify the purpose of the code, and also mean the developer policy check can be performed regardless of "media.navigator.permission.disabled" while still allowing prompts to be bypassed, facilitating testing.

A comment in options.eventCallback() in WebRTCParent.jsm can be updated for the BLOCK handling in prompt().

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: Bug 1720643 be consistent about using origin to separate subtest descriptions r?johannh

The sandbox attributes recognizes a specific set of tokens.
https://html.spec.whatwg.org/multipage/iframe-embed-object.html#attr-iframe-sandbox

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: Bug 1720643 clarify which subtest is using same origin iframe r?jib

Depends on D120063

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: Bug 1720643 perform getUserMedia Permissions Policy checks even when "media.navigator.permission.disabled" is set r?jib

"media.getusermedia.microphone.deny" and "media.getusermedia.camera.deny" now
override "media.navigator.permission.disabled".

User permission checks are removed because they are repeated in the app.

Depends on D120064

Comment 4

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/jobs?repo=try&revision=72d4d8b384f8cff1e06b6657389196493c2392c3

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/67c1702f50c3
be consistent about using origin to separate subtest descriptions r=johannh DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/ce3d9656ed7e
clarify which subtest is using same origin iframe r=jib DONTBUILD

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c77f9f68fe8
perform getUserMedia Permissions Policy checks even when "media.navigator.permission.disabled" is set r=jib

Comment 7

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/67c1702f50c3
https://hg.mozilla.org/mozilla-central/rev/ce3d9656ed7e
https://hg.mozilla.org/mozilla-central/rev/3c77f9f68fe8

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: Bug 1720643 update comment now that BLOCK is no longer handled in MediaManager r?jib

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de27d0b3862f
update comment now that BLOCK is no longer handled in MediaManager r=jib DONTBUILD

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/de27d0b3862f