Closed Bug 1144211 Opened 5 years ago Closed 5 years ago

[B2G][Camera] Increase code coverage of mochitests

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(firefox40 fixed)

RESOLVED FIXED
Tracking Status
firefox40 --- fixed

People

(Reporter: aosmond, Assigned: aosmond)

References

Details

Attachments

(1 file, 2 obsolete files)

Increase code coverage of camera mochitests, including
- Configuration, particular video mode
- Start / stop recording
- Camera capabilities
- Certain failure paths of APIs
Attached patch bug1144211.patch (obsolete) — Splinter Review
Assignee: nobody → aosmond
Status: NEW → ASSIGNED
Flags: in-testsuite+
Depends on: 1062387
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.
Attachment #8578737 - Flags: review?(mhabicher)
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...?
Attachment #8578737 - Flags: review?(mhabicher) → review+
Attached patch bug1144211.patch, v2 (obsolete) — Splinter Review
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.
Attachment #8578737 - Attachment is obsolete: true
Attachment #8594249 - Flags: review+
Tests pass on B2G desktop, but a few failed on the emulator. Round two!

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a060f4bf783
Attachment #8594249 - Attachment is obsolete: true
Attachment #8594281 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/be44c0a5cf9e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.