885768 - Cannot make a getUserMedia request within an iframe

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Jason Smith [:jsmith]]

Build: Nightly 6/21/2013
Device: Galaxy Nexus
OS: Android 4.2

STR

1. Go to https://bug876044.bugzilla.mozilla.org/attachment.cgi?id=755705&t=vShSZ1wJ1O
2. Select get microphone on either test case provided (same or different domain iframe)

Expected

A prompt should appear allowing the user to accept permissions for your microphone.

Actual

No prompt appears.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

"You are not authorized to access bug #876044."

Is this specific to mobile?

Comment 2

[Jason Smith [:jsmith]]

(In reply to Gian-Carlo Pascutto (:gcp) from comment #1)
> "You are not authorized to access bug #876044."
> 
> Is this specific to mobile?

The test case is generic to all platforms, although the URL is locked down as a security bug. Let me get you access.

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

need-info to jason for getting gcp access

Comment 4

[Gian-Carlo Pascutto [:gcp]]

I have access to the testcase in the other bug.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

OK, so can you make some recommendation about how we should track this? Is this expected behavior? and is it cross-platform?

Comment 6

[Gian-Carlo Pascutto [:gcp]]

If it works on desktop I see no reason why we shouldn't get it working for mobile.

Comment 7

[Jason Smith [:jsmith]]

Going to knock this down to non-blocking only because this is a valid use case that is supported on desktop, but no webrtc apps are currently using this. This is okay to have in the short-term, but does need to be resolved eventually to be in alignment for compatibility with desktop.

Comment 8

[Jason Smith [:jsmith]]

Disagreeing with the minus per comment 7. Although this isn't needed now, this is a compatibility need in alignment with desktop. WebAPI usage in iframes is a valid use case. The fact that FxAndroid supported will cause us compatibility problems.

Comment 9

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #8)
> Disagreeing with the minus per comment 7. Although this isn't needed now,
> this is a compatibility need in alignment with desktop. WebAPI usage in
> iframes is a valid use case. The fact that FxAndroid supported will cause us
> compatibility problems.

Doesn't support this is what I meant to say.

Comment 10

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #9)
> (In reply to Jason Smith [:jsmith] from comment #8)
> > Disagreeing with the minus per comment 7. Although this isn't needed now,
> > this is a compatibility need in alignment with desktop. WebAPI usage in
> > iframes is a valid use case. The fact that FxAndroid supported will cause us
> > compatibility problems.
> 
> Doesn't support this is what I meant to say.

I talked this over Brad. Although iframes are a compatibility issue, WebAPI usage is iframes is rare. So I guess this doesn't need to be tracked then.

Comment 11

[Gian-Carlo Pascutto [:gcp]]

Looking at this with all WebRTC logging enabled, no WebRTC related function is even called, ever.

adb logcat shows this error:

I/Gecko   ( 3082): WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed: file /home/morbo/hg/mozilla-central/docshell/base/nsDocShell.cpp, line 8392

Comment 12

[Gian-Carlo Pascutto [:gcp]]

Backtrace for failing assert:

#0  nsDocShell::CheckLoadingPermissions (this=0x849bf000)
    at /home/morbo/hg/mozilla-central/docshell/base/nsDocShell.cpp:8392
#1  0x7c7be3be in nsDocShell::InternalLoad (this=<optimized out>, aURI=0x7504bc90, aReferrer=0x8521b5c0, 
    aOwner=0x7ea4df70, aFlags=0, aWindowTarget=0x753ac668, aTypeHint=0x7e243d54 <gNullChar> "", 
    aFileName=..., aPostData=0x0, aHeadersData=0x0, aLoadType=2097153, aSHEntry=0x0, aFirstParty=true, 
    aSrcdoc=..., aDocShell=0x0, aRequest=0x0)
    at /home/morbo/hg/mozilla-central/docshell/base/nsDocShell.cpp:8867
#2  0x7c7a81dc in nsDocShell::OnLinkClickSync (this=0x849bf000, aContent=0x74fa8400, 
    aURI=<optimized out>, aTargetSpec=<optimized out>, aFileName=..., aPostDataStream=0x0, 
    aHeadersDataStream=0x0, aDocShell=0x0, aRequest=0x0)
    at /home/morbo/hg/mozilla-central/docshell/base/nsDocShell.cpp:12334
#3  0x7c79dce0 in OnLinkClickEvent::Run (this=<optimized out>)
    at /home/morbo/hg/mozilla-central/docshell/base/nsDocShell.cpp:12133
#4  0x7b9110ce in nsThread::ProcessNextEvent (this=0x74702160, mayWait=<optimized out>, 
    result=0x753ac8c7) at /home/morbo/hg/mozilla-central/xpcom/threads/nsThread.cpp:621
#5  0x7b8c127c in NS_ProcessNextEvent (thread=<optimized out>, mayWait=<optimized out>)
    at /home/morbo/hg/mozilla-central/objdir-android/xpcom/build/nsThreadUtils.cpp:238
#6  0x7b37b902 in mozilla::ipc::MessagePump::Run (this=0x74701be0, aDelegate=0x747460c0)
    at /home/morbo/hg/mozilla-central/ipc/glue/MessagePump.cpp:116
#7  0x7b94cc46 in MessageLoop::RunInternal (this=0x747460c0)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:219
#8  0x7b94cc56 in MessageLoop::RunHandler (this=<optimized out>)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:212
#9  0x7b94cd90 in MessageLoop::Run (this=0x747460c0)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:186
#10 0x7c9cc634 in nsBaseAppShell::Run (this=0x74710ca0)
    at /home/morbo/hg/mozilla-central/widget/xpwidgets/nsBaseAppShell.cpp:163
#11 0x7c84ef6e in nsAppStartup::Run (this=0x7ea4dd90)
    at /home/morbo/hg/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:269
#12 0x7b285de4 in XREMain::XRE_mainRun (this=0x753acadc)
    at /home/morbo/hg/mozilla-central/toolkit/xre/nsAppRunner.cpp:3853
#13 0x7b2861f4 in XREMain::XRE_main (this=0x753acadc, argc=9, argv=0x74735188, 
    aAppData=0x70065cb0 <sAppData>) at /home/morbo/hg/mozilla-central/toolkit/xre/nsAppRunner.cpp:3921
#14 0x7b2863ec in XRE_main (argc=9, argv=0x74735188, aAppData=0x70065cb0 <sAppData>, 
    aFlags=<optimized out>) at /home/morbo/hg/mozilla-central/toolkit/xre/nsAppRunner.cpp:4123
#15 0x7b27af40 in GeckoStart (data=0x74742280, appData=0x70065cb0 <sAppData>)
    at /home/morbo/hg/mozilla-central/toolkit/xre/nsAndroidStartup.cpp:73
#16 0x70025e02 in Java_org_mozilla_gecko_mozglue_GeckoLoader_nativeRun (jenv=0x70200660, 
    jc=<optimized out>, jargs=0x20700005)
    at /home/morbo/hg/mozilla-central/mozglue/android/APKOpen.cpp:379
#17 0x40850294 in dvmPlatformInvoke ()
   from /home/morbo/jimdb/moz-gdb/lib/00658c727e962a43/system/lib/libdvm.so
#18 0x4087f414 in dvmCallJNIMethod(unsigned int const*, JValue*, Method const*, Thread*) ()
   from /home/morbo/jimdb/moz-gdb/lib/00658c727e962a43/system/lib/libdvm.so
#19 0x40881570 in dvmResolveNativeMethod(unsigned int const*, JValue*, Method const*, Thread*) ()
   from /home/morbo/jimdb/moz-gdb/lib/00658c727e962a43/system/lib/libdvm.so
#20 0x408596a4 in dvmJitToInterpNoChain ()
   from /home/morbo/jimdb/moz-gdb/lib/00658c727e962a43/system/lib/libdvm.so
#21 0x408596a4 in dvmJitToInterpNoChain ()

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: use the right host

This patch just makes Fennec use the iframe (the DOM window making the request) host in the prompt instead of always picking the top-level DOM window.

I used the same pattern as desktop does, by passing the windowID to the prompt function, but I did not use the getMostRecentWindow call to get the right navigator. I kept our code that uses the active browser.

Comment 14

[Jason Smith [:jsmith]]

Comment on attachment 775646 [details] [diff] [review]
use the right host

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

::: mobile/android/chrome/content/WebrtcUI.js
@@ +183,5 @@
>      else
>        return;
>  
> +    let contentWindow = Services.wm.getOuterWindowWithId(aWindowID);
> +    let host = contentWindow.document.documentURIObject.asciiHost;

Note - This will fail if the host is ever using UTF-8 characters.

See https://bugzilla.mozilla.org/show_bug.cgi?id=878939 where we had a bug that resulted from this.

If we did what desktop did here, then you would want:

contentWindow.document.documentURIObject.host

Comment 15

[Wesley Johnston (:wesj)]

Comment on attachment 775646 [details] [diff] [review]
use the right host

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

Host seems fine to me

::: mobile/android/chrome/content/WebrtcUI.js
@@ +184,5 @@
>        return;
>  
> +    let contentWindow = Services.wm.getOuterWindowWithId(aWindowID);
> +    let host = contentWindow.document.documentURIObject.asciiHost;
> +    let browser = BrowserApp.getBrowserForWindow(contentWindow);

What's this for?

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Jason Smith [:jsmith] from comment #14)

> > +    let host = contentWindow.document.documentURIObject.asciiHost;
> 
> Note - This will fail if the host is ever using UTF-8 characters.
> 
> See https://bugzilla.mozilla.org/show_bug.cgi?id=878939 where we had a bug
> that resulted from this.
> 
> If we did what desktop did here, then you would want:
> 
> contentWindow.document.documentURIObject.host

I'll switch from asciiHost to host, to keep in-sync with desktop

(In reply to Wesley Johnston (:wesj) from comment #15)

> > +    let browser = BrowserApp.getBrowserForWindow(contentWindow);
> 
> What's this for?

Unneeded and will remove.

You know, this patch addresses some issues around the displayed host, but does not fix the bug about nothing happening in an iframe. I think I'll file a new bug and move this patch over. This bug can remain open for fixing the iframe issue.

Comment 17

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Fix window lookup

Patch based on desktop code.

Comment 18

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 779293 [details] [diff] [review]
Patch 1. Fix window lookup

This patch makes me sad. Services.wm.getMostRecentWindow(null) is basically saying "I have no fucking clue what window I need" and we are actually passed a window ID.

This will hurt if/when we have multiple windows which could happen with GeckoView.

GCP assures me he has tried his damnedest to utilize the the existing window ID to no avail.

Comment 19

[Gian-Carlo Pascutto [:gcp]]

>GCP assures me he has tried his damnedest to utilize the the existing window ID to no avail.

Imagine the "I have no idea what I'm doing" meme with that though.

This code does match what desktop does, so if this is really bad, file a generic bug (also against desktop UI).

Comment 20

[Dão Gottwald [:dao]]

navigator.mozGetUserMediaDevices does the same in any chrome window, so what chrome window you use there doesn't matter in practice.

Comment 21

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Dão Gottwald [:dao] from comment #20)
> navigator.mozGetUserMediaDevices does the same in any chrome window, so what
> chrome window you use there doesn't matter in practice.

Thanks. I feel better.

Comment 22

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f242b23cc687

Comment 23

[Ed Morley [:emorley]]

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

Comment 24

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 779293 [details] [diff] [review]
Patch 1. Fix window lookup

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Android WebRTC
User impact if declined: no WebRTC support in iframes
Testing completed (on m-c, etc.): landed on m-c few days ago
Risk to taking this patch (and alternatives if risky): Very low, Android WebRTC only. Can also ignore it for now as use case isn't all that common.

Comment 25

[bhavana bajaj [:bajaj]]

Comment on attachment 779293 [details] [diff] [review]
Patch 1. Fix window lookup

Although edge-casey, given its low risk patch for a new feature in Fx24 and we have some more time on aurora, approving the patch.

If this causes any regression's we'll backout as needed.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/53cf02a9bf32

Comment 27

[u421692]

Verified as fixed on:
Device: LG Nexus 4 (Android 4.2.2)
Build: Firefox 24 Beta 6 (2013-08-26) / Firefox 25 Aurora (2013-08-27)