986024 - [Camera][Gecko] Expose cancelAutoFocus()

Status: RESOLVED FIXED

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

Description

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

Expose cancelAutoFocus() so that the Camera app can resume continuous auto-focus, per http://developer.android.com/reference/android/hardware/Camera.Parameters.html#FOCUS_MODE_CONTINUOUS_PICTURE

Comment 1

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

Attachment: Expose resumeContinuousFocus() method to JS/DOM, v1

David, if you're comfortable building your own Gecko, this patch should apply on top of a recent b2g-inbound (and likely m-c) tree.

Comment 2

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

Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

Dave, would you mind reviewing the non-DOM changes?
Johnny, do you have a minute to look over CameraControl.webidl and DOMCameraControl.* ?

Comment 3

[Dave Hylands [:dhylands]]

Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

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

Just one minor comment...

::: dom/camera/CameraControlListener.h
@@ +94,5 @@
>      kInStopRecording,
>      kInSetConfiguration,
>      kInStartPreview,
>      kInStopPreview,
> +    kInResumeContinuousFocus,

Shouldn't there also be a corresponding string added to OnError/OnUserError strings?

Comment 4

[David Flanagan [:djf]]

Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

Works for me. I would love, love, love to get this in 1.4. We've got a deadline today to land stuff before QC starts testing the 1.4 camera.

Comment 5

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

Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

bz, sorry to ask so last-minute, but is there any chance you could last-minute review this one today? It's a pretty formulaic change that just exposes a new camera method to the DOM. No arguments, no callbacks.

Comment 6

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

(In reply to Dave Hylands [:dhylands] (away - back Tue April 15) from comment #3)
>
> Shouldn't there also be a corresponding string added to OnError/OnUserError
> strings?

Yes, thanks! I'll fix that up before landing.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

Hmm.  The AutoFocus API with this change is a bit bizarre, in that it can call the error callback before the call returns of an existing autofocus is in progress.  Is that behavior really as-designed?  That's definitely not the sort of behavior people would expect out of a callback in general...

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

r=me if we call the error callback async.

Comment 9

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

Attachment: Expose resumeContinuousFocus() method to JS/DOM, v2

Incorporate review feedback: fix error strings, make "AlreadyInProgress" callbacks async. Carrying r+en forward.

try-server push: https://tbpl.mozilla.org/?tree=Try&rev=67dd594012d2&showall=1

Comment 10

[David Flanagan [:djf]]

Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

This is a low-risk patch that we'd like to get into v1.4

Comment 11

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

Attachment: Expose resumeContinuousFocus() method to JS/DOM, v2.1

Fix local and try bustages.

try-server (re)push: https://tbpl.mozilla.org/?tree=Try&rev=7f49dbb1dd9f&showall=1

Comment 12

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

The linux opt build is fine, but the debug build is failing for a reason I don't understand--looks like a MOZ_CRASH during the -build- stage.

Here's a non-Linux try-server push:
https://tbpl.mozilla.org/?tree=Try&rev=a75408c4f90d&showall=1

Comment 13

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

Both the linux (comment 11) and linux64 (comment 12) DEBUG builds are failing on this error:

Executing /builds/slave/try-l64-d-00000000000000000000/build/obj-firefox/dist/bin/xpcshell -g /builds/slave/try-l64-d-00000000000000000000/build/obj-firefox/dist/bin/ -a /builds/slave/try-l64-d-00000000000000000000/build/obj-firefox/dist/bin/ -f /builds/slave/try-l64-d-00000000000000000000/build/toolkit/mozapps/installer/precompile_cache.js -e precompile_startupcache("resource://gre/");
  ...
[4896] ###!!! ABORT: file /builds/slave/try-l64-d-00000000000000000000/build/ipc/chromium/src/base/message_loop.h, line 517
Hit MOZ_CRASH() at /builds/slave/try-l64-d-00000000000000000000/build/memory/mozalloc/mozalloc_abort.cpp:30
  ...
mozpack.errors.ErrorMessage: Error: Error while running startup cache precompilation

The Win32 DEBUG build worked just fine, as did the MacOSX variants.

Comment 14

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

I see "Error while running startup cache precompilation" mentioned in bug 872439 and bug 902221. Not sure if/how they're related to this issue.

The Windows 7 and 8 variants are also building fine.

I've kicked off the linux builds one more time, but it's well past my bedtime now, and my time to try to fix this. Will look again in the morning.

Comment 15

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

With only some extra logging statements added, this builds:
https://tbpl.mozilla.org/?tree=Try&rev=0823b01425db

Trying again, in case the try server has decided to be sane again:
https://tbpl.mozilla.org/?tree=Try&rev=d404d74a3cc4

Comment 16

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

Finally, a sane try result.

BAM!
https://hg.mozilla.org/integration/b2g-inbound/rev/7151a7e50317

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7151a7e50317

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Doesn't apply to Aurora. Also not sure if non-blockers are being taken for v1.4 anymore, so you may want to request blocking status on this bug if you want it uplifted.

Comment 19

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

Attachment: Expose resumeContinuousFocus() method to JS/DOM, v2.1 [aurora]

Version of the patch rebased for aurora, pending blocking-1.4+.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/df67d32e6282