Closed Bug 918054 Opened 7 years ago Closed 7 years ago

getUserMedia audio can be requested while a call is active, but shouldn't be

Categories

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

26 Branch
ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla27
blocking-b2g koi+
Tracking Status
firefox25 --- wontfix
firefox26 --- fixed
firefox27 --- fixed
b2g-v1.2 --- fixed

People

(Reporter: jsmith, Assigned: schien)

References

Details

Attachments

(1 file, 3 obsolete files)

Build: 9/17/2013 Master
Device: Buri

STR

1. Start a call with a different person
2. Hit the home button & go to the browser
3. Go to http://mozilla.github.io/webrtc-landing/gum_test.html
4. Select Audio

Expected

A HARDWARE_UNAVAILABLE error should fire, as a higher priority application has access to the microphone.

Actual

A permission prompt appears allowing access to the microphone. Granting access turns on microphone use for that site while the call is active. This means talking into your microphone at this point sends audio to the media stream at that website and the other person in the call at the same time.
Blocks: 894848
blocking-b2g: --- → koi?
I assume the "call" referred to in STR #1 is a phone call to the other person, not a webrtc call.
(In reply to Randell Jesup [:jesup] from comment #1)
> I assume the "call" referred to in STR #1 is a phone call to the other
> person, not a webrtc call.

Yup - calling via the dialer app to another person on a different phone.
Are we sure web and phone should be mutually exclusive here? What's the precedent?
(In reply to Jan-Ivar Bruaroey [:jib] from comment #3)
> Are we sure web and phone should be mutually exclusive here? What's the
> precedent?

FWIW - I filed this bug per a discussion I had with Paul, so let me needinfo Paul for his views on the impact of this bug.
Flags: needinfo?(ptheriault)
Allowing recording of phone calls sounds like both a useful feature, and a privacy risk. I can't to either partner requirements or privacy guidelines (or even law?). A quick skim of android and iphone apps and there are many that provide this functionality though, so maybe this is a valuable feature.

But after talking with SC last week, I thought that regardless of what we want to do here, that currently it isn't actually possible for both webrtc and phone to use the microphone at the same time. So at the very least this wont do what the user is expecting.

If we are planning to allow recording of phone calls, please outline the privacy controls planned to prevent bugging of calls. Could an app start recording a call without user interaction? 

A visual notification doesn't seem very strong here if you have the phone up to your ear. But maybe the user always needs to click a prompt to start audio recording. I have read the spec but the current implementation doesn't match that.
Flags: needinfo?(ptheriault)
Blocks: b2g-getusermedia
No longer blocks: 894848
Need to further discuss with UX and legal team on the actual behavior. If the time is short for fixing it. We probably can just not allow to get user media when there is a call
Assignee: nobody → schien
blocking-b2g: koi? → koi+
Jason asked me to provide further guidance here - my opinion is that if our indicators were stronger, then I can see this being a safe and valid use case (assuming we completely mitigate the risk of accidentally recording your own call). 

There are probably legal concerns here too - according to my extensive legal knowledge [1] call recording is considering wire-tapping in some jurisdictions (rly australia!?!). That said, answering machines record phone calls and this could be a valid legitimate use case for this API. 

So my opinion here is that this needs to be blocked until we completely the risk of accidentally being recorded. Other bugs (e.g. bug 917359, bug 917544) may address this risk partially, but last time I checked, there was no notification which shows the user which app is doing the recording. I can't see a bug for that so I will raise one since to me that is probably the most important control.




[1] http://en.wikipedia.org/wiki/Telephone_recording_laws
Just to clarify by "that this needs to be blocked" I mean we should not allow GUM and calls simultaneously until we address the risk of accidental recording.
I did a experiment of doing recording during a phone call. The media recorder can only record the voice from microphone but not the remote side.
After discussing with @slee, we can monitor the phone state in MediaManager like what we did in bug 904025. However, I would like to move the audio input handling from MediaManager to AudioManager on Firefox OS in the long run.
We can reject GetUserMedia by checking the call status in MediaManager. However, we'll need a way to query the call status while initializing MediaManger.
Attachment #812419 - Flags: feedback?(rlin)
Attachment #812419 - Flags: feedback?(rjesup)
Right now there is no API for conent process to query the phone state immediately.
We may have another way to achieve thing....
According to comment 11, we cannot obtain the correct call state while initializing MediaManager in content process. Therefore, I move the check logic to ContentPermissionPrompt.js, which is running in chrome process. However, taking this approach means the error message will be "Permission Denied" instead of "HARDWARE_UNAVAILABLE".
Attachment #812595 - Flags: feedback?(rjesup)
Attachment #812595 - Flags: feedback?(jsmith)
Attachment #812595 - Flags: feedback?(fabrice)
(In reply to Shih-Chiang Chien [:schien] from comment #12)
> Created attachment 812595 [details] [diff] [review]
> WIP2- prevent-gum-while-in-call.patch
> 
> According to comment 11, we cannot obtain the correct call state while
> initializing MediaManager in content process. Therefore, I move the check
> logic to ContentPermissionPrompt.js, which is running in chrome process.
> However, taking this approach means the error message will be "Permission
> Denied" instead of "HARDWARE_UNAVAILABLE".

If you can get me a try build, then I can provide feedback here.
Comment on attachment 812595 [details] [diff] [review]
WIP2- prevent-gum-while-in-call.patch

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

I can live with the error - not ideal, but ok
Attachment #812595 - Flags: feedback?(rjesup) → feedback+
Comment on attachment 812595 [details] [diff] [review]
WIP2- prevent-gum-while-in-call.patch

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

::: b2g/components/ContentPermissionPrompt.js
@@ +28,5 @@
>  
>  var permissionManager = Cc["@mozilla.org/permissionmanager;1"].getService(Ci.nsIPermissionManager);
>  var secMan = Cc["@mozilla.org/scriptsecuritymanager;1"].getService(Ci.nsIScriptSecurityManager);
>  
> +var permissionSpecificChecker = {};

s/var/let

@@ +121,5 @@
>      request.cancel();
>      return true;
>    },
>  
> +  handledSpecific: function handledSpecific(request) {

s/request/aRequest

Also we should find a better name for this function...

@@ +146,5 @@
>  
> +    if (this.handledSpecific(request)) {
> +      return;
> +    }
> +

merge these two as:
 if (this.handleExistingPermission(request) ||
     this.handledSpecific(request))
Attachment #812595 - Flags: feedback?(fabrice) → feedback+
Attached patch prevent-gum-while-in-call.patch (obsolete) — Splinter Review
Update according to feedback comment.

@jsmith, please use the following build for feedback. Thanks.
https://tbpl.mozilla.org/?tree=Try&rev=6f49da1901bd
Attachment #812419 - Attachment is obsolete: true
Attachment #812595 - Attachment is obsolete: true
Attachment #812419 - Flags: feedback?(rlin)
Attachment #812419 - Flags: feedback?(rjesup)
Attachment #812595 - Flags: feedback?(jsmith)
Attachment #812947 - Flags: review?(rjesup)
Attachment #812947 - Flags: review?(fabrice)
Attachment #812947 - Flags: feedback?(jsmith)
Comment on attachment 812947 [details] [diff] [review]
prevent-gum-while-in-call.patch

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

r=me with comments addressed

::: b2g/components/ContentPermissionPrompt.js
@@ +126,5 @@
> +    if (permissionSpecificChecker.hasOwnProperty(request.type)) {
> +      return permissionSpecificChecker[request.type](request);
> +    } else {
> +      return false;
> +    }

return permissionSpecificChecker.hasOwnProperty(request.type) 
         ? permissionSpecificChecker[request.type](request)
         : false;

@@ +136,5 @@
>        request.allow();
>        return true;
>      }
>  
> +    if (this.handledByApp(request) || 

nit: trailing whitespace
Attachment #812947 - Flags: review?(fabrice) → review+
Attachment #812947 - Flags: review?(rjesup) → review+
Update patch according to review comments, carry r+.

@jsmith, you may use the try build for feedback.
https://tbpl.mozilla.org/?tree=Try&rev=a4c299d4e981
Attachment #812947 - Attachment is obsolete: true
Attachment #812947 - Flags: feedback?(jsmith)
Attachment #813077 - Flags: review+
Attachment #813077 - Flags: feedback?(jsmith)
Comment on attachment 813077 [details] [diff] [review]
prevent-gum-while-in-call.patch

Tested on your most recent try build - lgtm. I got permission denied when I try to access gUM audio while within a call. gUM audio worked after the call was complete, so it didn't cause any immediate regressions. Also checked to make sure that making a call to the phone with gUM audio active releases the stream - that still works as well.
Attachment #813077 - Flags: feedback?(jsmith) → feedback+
https://hg.mozilla.org/mozilla-central/rev/1bf11c407d09
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Already verified pre-landing, so marking as verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.