Closed Bug 885768 Opened 6 years ago Closed 6 years ago

Cannot make a getUserMedia request within an iframe

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 25
Tracking Status
firefox24 --- verified
firefox25 --- verified

People

(Reporter: jsmith, Assigned: mfinkle)

References

(Blocks 1 open bug)

Details

(Keywords: compat, Whiteboard: [getUserMedia][android-gum-])

Attachments

(2 files)

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.
tracking-fennec: --- → ?
Whiteboard: [getUserMedia][android-gum+]
"You are not authorized to access bug #876044."

Is this specific to mobile?
(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.
need-info to jason for getting gcp access
Flags: needinfo?(jsmith)
I have access to the testcase in the other bug.
Flags: needinfo?(jsmith)
OK, so can you make some recommendation about how we should track this? Is this expected behavior? and is it cross-platform?
Flags: needinfo?(gpascutto)
If it works on desktop I see no reason why we shouldn't get it working for mobile.
Flags: needinfo?(gpascutto)
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.
Whiteboard: [getUserMedia][android-gum+] → [getUserMedia][android-gum-]
See Also: → 868095
tracking-fennec: ? → -
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.
tracking-fennec: - → ?
(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.
Keywords: compat
(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.
tracking-fennec: ? → ---
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
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 ()
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.
Assignee: nobody → mark.finkle
Attachment #775646 - Flags: review?(wjohnston)
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 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?
Attachment #775646 - Flags: review?(wjohnston) → review+
(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.
Depends on: 894938
Patch based on desktop code.
Attachment #779293 - Flags: review?(mark.finkle)
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.
Attachment #779293 - Flags: review?(mark.finkle) → review+
>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).
navigator.mozGetUserMediaDevices does the same in any chrome window, so what chrome window you use there doesn't matter in practice.
(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.
https://hg.mozilla.org/mozilla-central/rev/f242b23cc687
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
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.
Attachment #779293 - Flags: approval-mozilla-aurora?
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.
Attachment #779293 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: verifyme
Whiteboard: [getUserMedia][android-gum-] → [getUserMedia][android-gum-][hold of uplift on aurora due to 894938]
Whiteboard: [getUserMedia][android-gum-][hold of uplift on aurora due to 894938] → [getUserMedia][android-gum-][hold-off uplift on aurora due to 894938]
Whiteboard: [getUserMedia][android-gum-][hold-off uplift on aurora due to 894938] → [getUserMedia][android-gum-]
Blocks: 902453
No longer blocks: 902453
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)
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.