Media manager needs to accept an array of devices to approve when receiving the getUserMedia:response:allow notification

RESOLVED FIXED in mozilla20

Status

()

P1
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dao, Assigned: jesup)

Tracking

Trunk
mozilla20
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [getUserMedia][blocking-gum+][qa-])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
See bug 802421 comment 7.

Who can take this? Randell?
Assignee: nobody → rjesup
Priority: -- → P1
(Assignee)

Comment 1

6 years ago
Created attachment 694399 [details] [diff] [review]
Change gUM permissions response to be an array
(Assignee)

Comment 2

6 years ago
Comment on attachment 694399 [details] [diff] [review]
Change gUM permissions response to be an array

Gold star to the first reviewer to step up!
Dao, if you're comfortable reviewing this feel free to step in and do so.
Attachment #694399 - Flags: review?(tterribe)
Attachment #694399 - Flags: review?(anant)
(Assignee)

Comment 3

6 years ago
FYI, tested with the patch from bug 802421 and appears happy
(Reporter)

Comment 4

6 years ago
Comment on attachment 694399 [details] [diff] [review]
Change gUM permissions response to be an array

>     if (aSubject) {
>-      // A particular device was chosen by the user.
>-      // NOTE: does not allow setting a device to null; assumes nullptr
>-      nsCOMPtr<nsIMediaDevice> device = do_QueryInterface(aSubject);
>-      if (device) {
>+      nsCOMPtr<nsISupportsArray> array(do_QueryInterface(aSubject));
>+      if (array) {
>+        // A particular device or devices were chosen by the user.

The array should always be there -- if it's missing, you can treat that as a failure.

I'm curious what happens when the array is empty, which can happen if we add the "No Video"/"No Audio" options.
(Assignee)

Comment 5

6 years ago
If No Audio and No Video are selected, that's denying access.  We can do that on the UI side, or in the backend code - or both.  Right now, count of 0 will select default devices instead of no devices, so we need one or the other to consider it a Deny.
(Assignee)

Comment 6

6 years ago
Created attachment 694423 [details] [diff] [review]
Change gUM permissions response to be an array

treating empty array as Deny here; could be done as well in UI.  Can't test easily since 'No audio' and 'No video' don't appear in the dropdown - they should be added
(Assignee)

Updated

6 years ago
Attachment #694399 - Attachment is obsolete: true
Attachment #694399 - Flags: review?(tterribe)
Attachment #694399 - Flags: review?(anant)
(Assignee)

Comment 7

6 years ago
Comment on attachment 694423 [details] [diff] [review]
Change gUM permissions response to be an array

Depending on what we do in the UI we could lose the MOZ_ASSERT(len) as well
Attachment #694423 - Flags: review?(tterribe)
Attachment #694423 - Flags: review?(anant)
Comment on attachment 694423 [details] [diff] [review]
Change gUM permissions response to be an array

Review of attachment 694423 [details] [diff] [review]:
-----------------------------------------------------------------

Look good to me!
Attachment #694423 - Flags: review?(anant) → review+
(Assignee)

Updated

6 years ago
Attachment #694423 - Flags: review?(tterribe)
(Assignee)

Comment 9

6 years ago
Note: needs to land with bug 802421
https://hg.mozilla.org/mozilla-central/rev/faa37c5e3508
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20

Updated

6 years ago
Whiteboard: [getUserMedia][blocking-gum+] → [getUserMedia][blocking-gum+][qa-]

Updated

6 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.