1093457 - [Camera][Gecko] abort setConfiguration or startRecording call while another call is in progress

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Camera, defect)

Description

[shinuk]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.111 Safari/537.36

Steps to reproduce:

1. start Camera
2. touch front/back camera switch button & video recording button at the same time


Actual results:

StartRecording error occurred;

DOM OnUserError aContext=6, error='HardwareClosed'

File descriptor was not closed.
And cannot be connected as USB storage.


Expected results:

Aborting startRecording or setConfiguration call while previous setConfiguration or startRecording call is in progress

Comment 1

[shinuk]

Attachment: abort_setconfiguration_startrecording_call

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 #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8516468 [details] [diff] [review]
abort_setconfiguration_startrecording_call

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

I will r+ if you make the corrections below:

::: dom/camera/DOMCameraControl.cpp
@@ +1596,5 @@
> +  if (mDSFileDescriptor->mFileDescriptor.IsValid()) {
> +    nsRefPtr<CloseFileRunnable> closer =
> +      new CloseFileRunnable(mDSFileDescriptor->mFileDescriptor);
> +    closer->Dispatch();
> +  }

We should only need to check this for the start recording failures, rather than every error. You should also remove the equivalent code (but copy the comments over) at nsDOMCameraControl::OnCreatedFileDescriptor since we don't need to do this twice.

Comment 3

[shinuk]

Attachment: abort_setconfiguration_startrecording_call

I don't think we need below code block inside OnCreatedFileDescriptor, because CameraControlImpl::StartRecording returns no error even if 
nsGonkCameraControl::StartRecordingImpl returns an error.

>  OnUserError(CameraControlListener::kInStartRecording, rv);

>  if (mDSFileDescriptor->mFileDescriptor.IsValid()) {
>    // An error occured. We need to manually close the file associated with the
>    // FileDescriptor, and we shouldn't do this on the main thread, so we
>    // use a little helper.
>    nsRefPtr<CloseFileRunnable> closer =
>      new CloseFileRunnable(mDSFileDescriptor->mFileDescriptor);
>    closer->Dispatch();
>  }

but I only moved mDSFileDescriptorclose condition to OnUserError.

Please find the attachment.

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8517148 [details] [diff] [review]
abort_setconfiguration_startrecording_call

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

(In reply to shinuk from comment #3)
> Created attachment 8517148 [details] [diff] [review]
> abort_setconfiguration_startrecording_call
> 
> I don't think we need below code block inside OnCreatedFileDescriptor,
> because CameraControlImpl::StartRecording returns no error even if 
> nsGonkCameraControl::StartRecordingImpl returns an error.
> 
> but I only moved mDSFileDescriptorclose condition to OnUserError.
> 
> Please find the attachment.

For the CameraControlImpl::StartRecording call, the dispatch to camera thread could fail in theory, so we check :).

Comment 5

[BMO Automation]

Firefox OS is not being worked on