1144211 - [B2G][Camera] Increase code coverage of mochitests

Status: RESOLVED FIXED

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

Description

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

Increase code coverage of camera mochitests, including
- Configuration, particular video mode
- Start / stop recording
- Camera capabilities
- Certain failure paths of APIs

Comment 1

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

Attachment: bug1144211.patch

Comment 2

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

Comment on attachment 8578737 [details] [diff] [review]
bug1144211.patch

Feel free to review the individual tests but I am mainly looking for the non-test case bits.

Comment 3

[Mike Habicher [:mikeh] (high bugzilla latency)]

Comment on attachment 8578737 [details] [diff] [review]
bug1144211.patch

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

This looks good, with the question and issue noted below addresses. Hopefully your Gecko changes don't break my patch too much. :)

::: dom/camera/GonkCameraControl.cpp
@@ +1269,5 @@
>    mRecorder->stop();
>    mRecorder = nullptr;
> +#else
> +  if (!mVideoFile) {
> +    return NS_OK;

Can we ever have !!mVideoFile?

@@ +1286,5 @@
>      }
>    }
>  
>    // notify DeviceStorage that the new video file is closed and ready
> +  nsresult rv = NS_DispatchToMainThread(new RecordingComplete(mVideoFile.forget()));

And 'rv' is used where...?

Comment 4

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

Attachment: bug1144211.patch, v2

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=47299624c007

(In reply to Mike Habicher [:mikeh] from comment #3)
> Comment on attachment 8578737 [details] [diff] [review]
> bug1144211.patch
> 
> Review of attachment 8578737 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks good, with the question and issue noted below addresses.
> Hopefully your Gecko changes don't break my patch too much. :)
> 
> ::: dom/camera/GonkCameraControl.cpp
> @@ +1269,5 @@
> >    mRecorder->stop();
> >    mRecorder = nullptr;
> > +#else
> > +  if (!mVideoFile) {
> > +    return NS_OK;
> 
> Can we ever have !!mVideoFile?
> 

I changed the dispatch further down to use mVideoFile.forget() so that it is cleared when the recording is stopped. So it should *only* be set if there was a pending recording, where mRecorder is also set. On B2G desktop, where recordering isn't supported fully, mRecorder doesn't exist.

> @@ +1286,5 @@
> >      }
> >    }
> >  
> >    // notify DeviceStorage that the new video file is closed and ready
> > +  nsresult rv = NS_DispatchToMainThread(new RecordingComplete(mVideoFile.forget()));
> 
> And 'rv' is used where...?

Why didn't the build break?!?! Fixed. It should have returned from NS_Dispatch... here.

Comment 5

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

Attachment: bug1144211.patch, v2.1 [carries r=mikeh]

Tests pass on B2G desktop, but a few failed on the emulator. Round two!

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a060f4bf783

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/be44c0a5cf9e

Comment 7

[Phil Ringnalda (:philor)]

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