Closed
Bug 1018928
Opened 10 years ago
Closed 10 years ago
Regression: Selecting "No video" option from the permission prompt still shows the content of the device camera.
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(firefox29 unaffected, firefox30 unaffected, firefox31 unaffected, firefox32+ verified, firefox33 verified, fennec32+)
VERIFIED
FIXED
Firefox 33
Tracking | Status | |
---|---|---|
firefox29 | --- | unaffected |
firefox30 | --- | unaffected |
firefox31 | --- | unaffected |
firefox32 | + | verified |
firefox33 | --- | verified |
fennec | 32+ | --- |
People
(Reporter: CristinaM, Assigned: gcp)
References
Details
(Keywords: regression, reproducible)
Attachments
(3 files, 1 obsolete file)
(deleted),
patch
|
blassey
:
review+
lmandel
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
blassey
:
review+
|
Details | Diff | Splinter Review |
Environment:
Build: 32.0a1 (2014-06-02)
Device: Motorola Razr (Android 4.0.4)
Steps to reproduce:
1. Go to http://mozilla.github.com/webrtc-landing/gum_test.html;
2. Tap on "Audio & Video";
3. After the permission prompt appears, at video source select "No video" and tap on "Share".
Expected result:
No video should be displayed.
Actual result:
A video appears showing the content from the device camera.
Reporter | ||
Comment 1•10 years ago
|
||
Regression window:
Last good revision: cbe4f69c2e9c (2014-05-27)
First bad revision: e017c15325ae (2014-05-28)
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cbe4f69c2e9c&tochange=e017c15325ae
status-firefox29:
--- → unaffected
status-firefox30:
--- → unaffected
status-firefox31:
--- → unaffected
status-firefox32:
--- → affected
Reporter | ||
Comment 2•10 years ago
|
||
Possible a regression of Bug 995777?
Updated•10 years ago
|
tracking-fennec: --- → ?
Updated•10 years ago
|
tracking-firefox32:
--- → ?
Updated•10 years ago
|
Keywords: regression,
reproducible
Summary: Selecting "No video" option from the permission prompt, still shows the content of the device camera. → Regression: Selecting "No video" option from the permission prompt still shows the content of the device camera.
Comment 3•10 years ago
|
||
Backing out Bug 995777 prevents the webrtc dialog from showing when visiting webrtc sites. So this is caused by some other bug. Though 995777 may have prevented this bug from being discovered. For right now I am going to mark this blocking the webrtc upgrade bug.
Blocks: 987979
Comment 4•10 years ago
|
||
If someone on Android could look at this that'd be super-helpful; I doubt it's the 3.50 upgrade, though certainly it's possible. Perhaps some of the other tabvideo source changesets?
Assignee | ||
Comment 5•10 years ago
|
||
The 3.50 upgrade has all-new code to detect the number and name of the cameras. It's possible that a subtle or not-so-subtle change in the names it generates trips up the JS code that deals with displaying the doorhanger.
Updated•10 years ago
|
Updated•10 years ago
|
Assignee: nobody → gpascutto
tracking-fennec: ? → 32+
Reporter | ||
Updated•10 years ago
|
status-firefox33:
--- → affected
Updated•10 years ago
|
Status: NEW → ASSIGNED
Comment 6•10 years ago
|
||
Any update here?
Assignee | ||
Comment 7•10 years ago
|
||
I'll tackle it as soon as I finish my review queue.
Comment 8•10 years ago
|
||
Looks like whatever option I choose to share form the contextmenu ('back facing camera', 'front facing camera', 'choose a tab to stream' or 'no video') the 'Back facing camera' will be shared.
Assignee | ||
Comment 9•10 years ago
|
||
Looks like the enumeration of the cameras is fine, and their naming doesn't cause any problems or hasn't changed in a relevant way.
We seem to be arriving in this callback:
http://dxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/WebrtcUI.js#100
but with a mismatch between videoSource and videoDevice:
D/GeckoBrowser(16648): inputs: {"videoSource":0}
Assignee | ||
Comment 10•10 years ago
|
||
So, this was indeed regressed by bug 995777.
That bug, for some reason, decided to change the naming of things from audioDevice and videoDevice to audioDevice and videoSource...but only in some places, and not in others.
I think I'd prefer to undo that entirely, because the source right now has it inconsistent all over, but those strings are localized, so maybe that generates additional l10n churn.
This patch is the simplest fix but leaves the inconsistence.
Oh, and let me check if I can add some more tests for this.
Attachment #8442868 -
Flags: review?(blassey.bugs)
Comment 11•10 years ago
|
||
Comment on attachment 8442868 [details] [diff] [review]
Patch 1. Fix inconsistent naming
Review of attachment 8442868 [details] [diff] [review]:
-----------------------------------------------------------------
The reason for the entity change is that the string changed, and that's how our l10n system works
Attachment #8442868 -
Flags: review?(blassey.bugs) → review+
Assignee | ||
Comment 13•10 years ago
|
||
I'm trying to expand the test coverage of gUM in Robocop, but alas:
08:16:26 INFO - 06-23 08:16:16.414 D/****CameraHAL( 1293): 148: camera_start_preview() ENTER
08:16:26 INFO - 06-23 08:16:16.414 I/CameraHardware( 1293): startPreview: in startpreview
08:16:26 INFO - 06-23 08:16:16.414 I/CameraHardware( 1293): trying the node /dev/video0 width=640 height=480
08:16:26 INFO - 06-23 08:16:16.445 E/V4L2Camera( 1293): StartStreaming: Unable to start capture: Invalid argument
08:16:26 INFO - 06-23 08:16:16.445 I/CameraHardware( 1293): startPreview: Camera.StartStreaming failed
08:16:26 INFO - 06-23 08:16:16.445 E/V4L2Camera( 1293): Uninit: VIDIOC_DQBUF Failed
08:16:26 INFO - 06-23 08:16:16.445 E/V4L2Camera( 1293): Uninit: VIDIOC_DQBUF Failed
08:16:26 INFO - 06-23 08:16:16.445 E/V4L2Camera( 1293): Uninit: VIDIOC_DQBUF Failed
08:16:26 INFO - 06-23 08:16:16.500 E/WEBRTC-JC( 3122): startCapture failed
08:16:26 INFO - 06-23 08:16:16.500 E/WEBRTC-JC( 3122): java.lang.RuntimeException: startPreview failed
08:16:26 INFO - 06-23 08:16:16.500 E/WEBRTC-JC( 3122): at android.hardware.Camera.startPreview(Native Method)
08:16:26 INFO - 06-23 08:16:16.500 E/WEBRTC-JC( 3122): at org.webrtc.videoengine.VideoCaptureAndroid.startCapture(VideoCaptureAndroid.java:207)
08:16:26 INFO - 06-23 08:16:16.500 E/WEBRTC-JC( 3122): at dalvik.system.NativeStart.run(Native Method)
Assignee | ||
Comment 14•10 years ago
|
||
Attachment #8444592 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 15•10 years ago
|
||
That function isn't supposed to fail, but bugs in Android and bugs in the Camera driver can cause it to fail anyway. Protecting against it won't hurt.
Attachment #8444594 -
Flags: review?(blassey.bugs)
Updated•10 years ago
|
Attachment #8444594 -
Flags: review?(blassey.bugs) → review+
Updated•10 years ago
|
Attachment #8444592 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 16•10 years ago
|
||
Unfortunately making the tests use Tab Streaming seems to expose another bug. I'll land the first patch with the fix for this bug and will look at this one so we can enable more tests.
16:20:57 INFO - 0 libxul.so!mozilla::MediaEngineTabVideoSource::StopRunnable::Run() [MediaEngineTabVideoSource.cpp:3522953e529c : 64 + 0x2]
16:20:57 INFO - r4 = 0x52005d90 r5 = 0x00000000 r6 = 0x4752683c r7 = 0x4752686f
16:20:57 INFO - r8 = 0x472022e0 r9 = 0x00000001 r10 = 0x00000000 fp = 0x0000000f
16:20:57 INFO - sp = 0x47526808 lr = 0x4db4c593 pc = 0x4e399fe0
16:20:57 INFO - Found by: given as instruction pointer in context
Assignee | ||
Comment 17•10 years ago
|
||
Keywords: leave-open
Comment 18•10 years ago
|
||
Updated•10 years ago
|
Priority: -- → P1
Target Milestone: --- → Firefox 33
Assignee | ||
Comment 19•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f0d227e054d
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a03d24b11e8
Keywords: leave-open
Comment 20•10 years ago
|
||
backed out for test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=42620970&tree=Mozilla-Inbound , sorry
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 22•10 years ago
|
||
Relanding test part with tests disabled on 2.3:
https://hg.mozilla.org/integration/mozilla-inbound/rev/57e490053263
Blocks: 1032991
Comment 24•10 years ago
|
||
Backed out for causing bug 1032991:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ff73cfaac13
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 33 → ---
Updated•10 years ago
|
Comment 25•10 years ago
|
||
Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/2ff73cfaac13
Updated•10 years ago
|
Status: REOPENED → ASSIGNED
Comment 26•10 years ago
|
||
The bug seems to be fixed on latest Nightly (2014-07-14);
Can you please uplift to Aurora before beta?
Flags: needinfo?(gpascutto)
Assignee | ||
Comment 27•10 years ago
|
||
Comment on attachment 8442868 [details] [diff] [review]
Patch 1. Fix inconsistent naming
Approval Request Comment
[Feature/regressing bug #]: Bug 995777?
[User impact if declined]: Sharing of video camera even if user denies.
[Describe test coverage new/current, TBPL]: Test coverage backed out of m-c due to yellows.
[Risks and why]: No real risk of breaking it further.
[String/UUID change made/needed]: N/A
Attachment #8442868 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(gpascutto)
Assignee | ||
Comment 28•10 years ago
|
||
Changing flags to reflect only the tests were backed out.
Comment 29•10 years ago
|
||
Verified as fixed in build 33.0a1 (2014-07-14);
Device: Google Nexus 7 (Android 4.4.4).
Comment 30•10 years ago
|
||
Comment on attachment 8442868 [details] [diff] [review]
Patch 1. Fix inconsistent naming
Aurora+
Attachment #8442868 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Updated•10 years ago
|
Whiteboard: [checkin-needed-aurora]
Comment 31•10 years ago
|
||
Whiteboard: [checkin-needed-aurora]
Comment 32•10 years ago
|
||
Tested with:
Device: Samsung Galaxy Nexus
OS: Android 4.2.1
No video is displayed, so verified fixed on:
Build: Firefox for Android 32 Beta 1
Assignee | ||
Comment 33•10 years ago
|
||
Let's close this and deal with the tests later in a split bug.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Target Milestone: --- → Firefox 33
Assignee | ||
Comment 34•10 years ago
|
||
Assignee | ||
Comment 35•10 years ago
|
||
Comment on attachment 8506977 [details] [diff] [review]
Expand Android gUM + gUM Doorhanger tests. r=
Wrong bug.
Attachment #8506977 -
Attachment is obsolete: true
Reporter | ||
Comment 36•10 years ago
|
||
I will mark this as Verified fixed based on comment 29 and comment 32.
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 37•10 years ago
|
||
We also have in-tree tests for this now after bug 1042795 landed.
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•