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

Status: RESOLVED FIXED

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

Comment 1

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

Attachment: allow autoFocus() calls to be interrupted, v1

try-server push: https://tbpl.mozilla.org/?tree=Try&rev=74e8b52f063b

Comment 2

[Justin D'Arcangelo [:justindarc]]

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).

Comment 3

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

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

Comment 4

[Dave Hylands [: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

Comment 5

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

(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.

Comment 6

[Dave Hylands [:dhylands]]

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

Comment 7

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

Attachment: 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

Comment 8

[Dave Hylands [: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 :)

Comment 9

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

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

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/583635fb7525