Closed Bug 826072 Opened 9 years ago Closed 9 years ago

[Camera] Camera throwing an exception in mochitests

Categories

(Firefox OS Graveyard :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(blocking-b2g:tef+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

RESOLVED FIXED
B2G C4 (2jan on)
blocking-b2g tef+
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: onecyrenus, Assigned: sotaro)

Details

(Whiteboard: QARegressExclude [qa-])

Attachments

(5 files, 2 obsolete files)

Attached file error log for camera (obsolete) —
I am unsure if this would happen if an app developer was attempting to write an app. But I thought since the bug seems so basic we could investigate. 

Basic mochitest test case: 

  navigator.mozCameras.getCamera(function() {alert('onsucces')}, function() {alert('onerror')});;
  ok(true, "navigator.mozCameras.getCamera should not throw");
  cameras = navigator.mozCameras.getListOfCameras();

abbreviated logcat output
--------------

D/EmulatedCamera_QemuClient(  606): Emulated camera list: 
D/EmulatedCamera_FakeCamera(  606): Initialize: Fake camera is facing back
V/EmulatedCamera_Factory(  606): 1 cameras are being emulated. Fake camera ID is 0
...

E/GeckoConsole(  565): [JavaScript Error: "this._permissionList is null" {file: "jar:file:///system/b2g/omni.ja!/components/RadioInterfaceLayer.js" line: 690}]
That looks like an exception in the RIL, not the camera.
Mike, the camera emulator on android works fine doing this same basic operation. 
basically the emulator camera has the ability to take and save pictures. 

What would you recommend we do here ? As it would be nice to be able to test the camera api somewhat.
(In reply to dclarke@mozilla.com  [:onecyrenus] from comment #2)
> 
> What would you recommend we do here ? As it would be nice to be able to test
> the camera api somewhat.

It would be nice to exercise the camera API.  There's a fallback implementation that doesn't do anything right now except throw a NOT_AVAILABLE exception--it needs to be plumbed into the camera drivers.
mike just to follow up on this the ffos (android ) emulator also does not allow you to take a picture, whereas the android emulator does. 

attached is that logcat.
I've started looking into this, but I'm running into a bunch of (unrelated?) issues.
I can't reproduce the crash, but the camera is definitely not working in the emulator.  Looks like a deadlock between the camera library preview thread and the media thread.  The latter isn't picking up the preview frames for some reason, so the camera just stops in dequeueBuffer().  I've created bug 839628 to track this.
(In reply to dclarke@mozilla.com  [:onecyrenus] from comment #6)
> Created attachment 711595 [details]
> a repeatable crash on emulator

dclarke, there should be a second occurrence of "SIGSEGV" in these logs; can you post a log showing that one (and say +/-10 lines of context) as well?  It will be more useful for narrowing down where the error is occurring.
Flags: needinfo?(dclarke)
Attached file camera debug log
Mike, here is the extended debug log.
Attachment #697220 - Attachment is obsolete: true
Flags: needinfo?(dclarke)
dclarke, I still can't reproduce this crash.  Which emulator are you running?  'emulator' or 'emulator-x86' (i.e. what are you running $B2G/config.sh with)?

I'm building with |./config.sh emulator|, and although the preview isn't working, you can successfully take fake photos using the patch in bug 839628.
Flags: needinfo?(dclarke)
Mike, I am not using the patch in bug 839628.  I am answering Comment #9.
Flags: needinfo?(dclarke)
Mike the STR: 

#1) Attempt to take a photo, open the camera app, take photo
#2) Switch to homescreen 
#3) switch back to camera app, and click the button to take another photo. 
#4) The app should crash

A crash should be reported in logcat.
(In reply to dclarke@mozilla.com  [:onecyrenus] from comment #13)
> Mike the STR: 
> 
> #1) Attempt to take a photo, open the camera app, take photo
> #2) Switch to homescreen 
> #3) switch back to camera app, and click the button to take another photo. 
> #4) The app should crash
> 
> A crash should be reported in logcat.

This is being built from source using

./config.sh emulator
./build.sh
With bug 839628, a crasher is still happening when we switch out of the camera app, and switch back to take another picture.
I manually added log outs to some function's enter and exit. Then got a log of the crash.

From the log, I found that nsGonkCameraControl::Init() is completed before nsGonkCameraControl::nsGonkCameraControl() exit. And nsGonkCameraControl class's constructor and destructor are called within nsDOMCameraControl::nsDOMCameraControl().

It seems that nsGonkCameraControl is deleted by refcount from InitGonkCameraControl, before nsGonkCameraControl is refcounted by nsDOMCameraControl.
By applying the patch, I confirmed that the crash do not happens on emulator.
Comment on attachment 714095 [details] [diff] [review]
patch: dispatch Init() out of nsGonkCameraControl's constructor

mikeh, can you review the patch?
Attachment #714095 - Flags: review?(mhabicher)
Comment on attachment 714095 [details] [diff] [review]
patch: dispatch Init() out of nsGonkCameraControl's constructor

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

r+ with that style fix.

::: dom/camera/GonkCameraControl.cpp
@@ +220,2 @@
>  
> +void nsGonkCameraControl::DispatchInit(nsDOMCameraControl* aDOMCameraControl, nsICameraGetCameraCallback* onSuccess, nsICameraErrorCallback* onError, uint64_t aWindowId) {

Style nit:

void
nsGonkCameraControl::DispatchInit(nsDOMCameraControl* aDOMCameraControl, nsICameraGetCameraCallback* onSuccess, nsICameraErrorCallback* onError, uint64_t aWindowId)
{
Attachment #714095 - Flags: review?(mhabicher) → review+
carry "mhabicher: review+ "

apply the comment.
Attachment #714095 - Attachment is obsolete: true
Attachment #714234 - Flags: review+
(In reply to Sotaro Ikeda [:sotaro] from comment #21)
> https://tbpl.mozilla.org/?tree=Try&rev=d0b74cc3cb82

The tryserver's result is strange. almost all "B2G Arm opt" tests fails. It seems that tests fails before start. The patch can not make such result. It seems infrastructures or base source's problem.
https://tbpl.mozilla.org/?tree=Try&rev=6dc96cefc231

Re-push the patch to tryserver. Tests progress as normal.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/62619edc3226
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 714234 [details] [diff] [review]
patch v2: dispatch Init() out of nsGonkCameraControl's constructor

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Camera app always throwing an exception in mochitests in emulator. This could happen also in FirefoxOS devices, because the exception happens just because of thread scheduling timing.
Testing completed: Tested in Emulator and Unagui phone.
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Attachment #714234 - Flags: approval-mozilla-b2g18?
We should get this into b2g18.  It's a good (and relatively simple) stability fix.
blocking-b2g: --- → tef?
Go ahead with uplift to v1-train and v1.0.1 since this is a stability fix with low risk.
blocking-b2g: tef? → tef+
Attachment #714234 - Flags: approval-mozilla-b2g18?
Mike, 
  one thing i am noticing on the emulator is that in order for me to be able to take a second photo i need to exit the app, and then return. 

V/EmulatedCamera_Device(  172): Stopping emulated camera device's worker thread...

when i exit the app logcat outputs

I/IdleService(   43): Register idle observer 469741a0 for 1 seconds
I/IdleService(   43): Register: adjusting next switch from -1 to 1 seconds
I/IdleService(   43): next timeout -653 msec from now
I/IdleService(   43): SetTimerExpiryIfBefore: next timeout -653 msec from now
I/IdleService(   43): reset timer expiry to 10 msec from now
I/IdleService(   43): Get idle time: time since reset 1654 msec
I/IdleService(   43): Get idle time: time since reset 1665 msec
I/IdleService(   43): Idle timer callback: current idle time 1665 msec
I/IdleService(   43): next timeout 4294967293335 msec from now
I/IdleService(   43): SetTimerExpiryIfBefore: next timeout 4294967293335 msec from now
I/IdleService(   43): reset timer expiry to 4294967293345 msec from now
I/IdleService(   43): Idle timer callback: tell observer 469741a0 user is idle
I/IdleService(   43): Get idle time: time since reset 1665 msec
V/EmulatedCamera_Device(  172): Emulated camera device's worker thread has been stopped.
V/EmulatedCamera_FakeDevice(  172): stopDevice
V/EmulatedCamera_Preview(  172): stopPreview
D/EmulatedCamera_Camera(  172): Starting camera for picture: NV21(jpeg)[640x480]

do you notice the same ?
Target Milestone: --- → B2G C4 (2jan on)
(In reply to dclarke@mozilla.com  [:onecyrenus] from comment #30)
>
>   one thing i am noticing on the emulator is that in order for me to be able
> to take a second photo i need to exit the app, and then return. 

New issue, new bug 843616.
Whiteboard: QARegressExclude
No Test case creation is needed in moztrap for this issue.
Flags: in-moztrap-
Cannot Verify, need steps to blackbox test this issue.
Darren, this was a simulator issue.
Tools bug, marking qa-, no need to verify.
Whiteboard: QARegressExclude → QARegressExclude [qa-]
You need to log in before you can comment on or make changes to this bug.