[Camera][Gecko] Allow autoFocus() calls to be interrupted

RESOLVED FIXED in 2.0 S4 (20june)

Status

Firefox OS
Gaia::Camera
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mikeh, Assigned: mikeh)

Tracking

unspecified
2.0 S4 (20june)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
Created attachment 8437067 [details] [diff] [review]
allow autoFocus() calls to be interrupted, v1

try-server push: https://tbpl.mozilla.org/?tree=Try&rev=74e8b52f063b
Attachment #8437067 - Flags: feedback?(jdarcangelo)
Attachment #8437067 - Flags: feedback?(dmarcos)
Blocks: 1022880
Comment on attachment 8437067 [details] [diff] [review]
allow autoFocus() calls to be interrupted, v1

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

Patch works great for me! No more NS_ERROR_FAILURE messages in the console and I was unable to crash the app by rapidly tapping-to-focus. The focus feature seemed to be more responsive as well (probably because it is no longer waiting for previous focus operations to complete).
Attachment #8437067 - Flags: feedback?(jdarcangelo) → feedback+
Comment on attachment 8437067 [details] [diff] [review]
allow autoFocus() calls to be interrupted, v1

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

tl;dr - instead of preventing new calls to autoFocus() while there is an outstanding call, cancel the earlier calls
Attachment #8437067 - Flags: review?(dhylands)
Umm. I see try run failures for this test.

14:02:39     INFO -  702 INFO TEST-UNEXPECTED-FAIL | /tests/dom/camera/test/test_bug1022766.html | uncaught exception - TypeError: Camera.cameraObj is null at http://mochi.test:8888/tests/dom/camera/test/test_bug1022766.html:82
(In reply to Dave Hylands [:dhylands] from comment #4)
>
> Umm. I see try run failures for this test.

Weird, the tests pass fine locally. Thanks for calling that out--I'll take a look.
Status: NEW → ASSIGNED
Comment on attachment 8437067 [details] [diff] [review]
allow autoFocus() calls to be interrupted, v1

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

So I can't give it an r+ with test failures...

::: dom/camera/test/test_bug1022766.html
@@ +59,5 @@
> +  },
> +  failureTwo: function test_failureTwo(error) {
> +    ok(false, "Second call to autoFocus() failed unexpectedly with: " + error);
> +  },
> +  

nit: trailing spaces

@@ +71,5 @@
> +      // this is just testing the sequencing.
> +      camera.autoFocus(Camera.successOne, Camera.failureOne);
> +      camera.autoFocus(Camera.successTwo, Camera.failureTwo);
> +    };
> +    

nit: trailing spaces
Attachment #8437067 - Flags: review?(dhylands) → review-
Created attachment 8438823 [details] [diff] [review]
allow autoFocus() calls to be interrupted, v2

Incorporate review feedback; fix test. The problem with running this test locally is that the 'beforeunload' event never fires, so the null object is never hit.

new try-server push: https://tbpl.mozilla.org/?tree=Try&rev=ecd5f45d2fa3
Attachment #8437067 - Attachment is obsolete: true
Attachment #8437067 - Flags: feedback?(dmarcos)
Attachment #8438823 - Flags: review?(dhylands)
Comment on attachment 8438823 [details] [diff] [review]
allow autoFocus() calls to be interrupted, v2

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

Looks good now :)
Attachment #8438823 - Flags: review?(dhylands) → review+
https://hg.mozilla.org/mozilla-central/rev/583635fb7525
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S4 (20june)
Blocks: 1042072
You need to log in before you can comment on or make changes to this bug.