Image Capture: teach takePhoto() to accept an optional PhotoSettings dictionary

Image Capture: teach takePhoto() to accept an optional PhotoSettings dictionary

This CL:
- Adds an optional PhotoSettings arg to takePhoto(),

- cleanup: the 3 private methods used to receive mojo callbacks 
 (OnPhotoCapabilities, OnSetOptions and OnTakePhoto) get a Mojo 
prefix, for clarity.

- adds a |trigger_take_photo| argument to setOptions() and a few
others that pass it along; takePhoto() uses it to setOptions() when
appropriate.  The sequence of method calls in ImageCapture.cpp then 
would be:
 - without options
   takePhoto() -> OnMojoTakePhoto()
 - with options:
   takePhoto(options) -> setOptions(trigger_take_photo == true)  --> OnMojoSetOptions(true) --> OnMojoPhotoCapabilities(true) --> OnMojoTakePhoto()


- methods OnCapabilitiesUpdate() and OnCapabilitiesUpdateInternal() 
(which are essentially the same thing) are merged and the resulting
method renamed to UpdateMediaTrackCapabilities() to make more 
evident what it does.

BUG=718632
Review-Url: https://codereview.chromium.org/2865563002
Cr-Commit-Position: refs/heads/master@{#469766}
Committed: https://chromium.googlesource.com/chromium/src/+/3fd00a51e22a0ffa55d0ab14725f9f300b2fc56b

[mcasas, 2017-05-04 23:03:01]

Description was changed from

==========
Image Capture: teach takePhoto() to accept an optional PhotoSettings dictionary

Added optional PhotoSettings arg to takePhoto() and cleaned up the impl and
naming

BUG=718632
==========

to

==========
Image Capture: teach takePhoto() to accept an optional PhotoSettings dictionary

This CL:
- Adds an optional PhotoSettings arg to takePhoto(),

- the 3 private methods used to receive mojo callbacks (OnPhotoCapabilities,
 OnSetOptions and OnTakePhoto) get a Mojo prefix, for clarity.

- adds a |trigger_take_photo| argument to setOptions() and a few
others that pass it along; takePhoto() uses it to setOptions() when
appropriate.  The sequence of method calls in ImageCapture.cpp then 
would be:
 - without options
   takePhoto() -> OnMojoTakePhoto()
 - with options:
   takePhoto(options) -> setOptions(trigger_take_photo == true)  -->
OnMojoSetOptions(true) --> OnMojoPhotoCapabilities(true) --> OnMojoTakePhoto()


- methods OnCapabilitiesUpdate() and OnCapabilitiesUpdateInternal() 
(which are essentially the same thing) are merged and the resulting
method renamed to UpdateMediaTrackCapabilities() to make more 
evident what it does.

BUG=718632
==========

[mcasas, 2017-05-04 23:05:06]

Description was changed from

==========
Image Capture: teach takePhoto() to accept an optional PhotoSettings dictionary

This CL:
- Adds an optional PhotoSettings arg to takePhoto(),

- the 3 private methods used to receive mojo callbacks (OnPhotoCapabilities,
 OnSetOptions and OnTakePhoto) get a Mojo prefix, for clarity.

- adds a |trigger_take_photo| argument to setOptions() and a few
others that pass it along; takePhoto() uses it to setOptions() when
appropriate.  The sequence of method calls in ImageCapture.cpp then 
would be:
 - without options
   takePhoto() -> OnMojoTakePhoto()
 - with options:
   takePhoto(options) -> setOptions(trigger_take_photo == true)  -->
OnMojoSetOptions(true) --> OnMojoPhotoCapabilities(true) --> OnMojoTakePhoto()


- methods OnCapabilitiesUpdate() and OnCapabilitiesUpdateInternal() 
(which are essentially the same thing) are merged and the resulting
method renamed to UpdateMediaTrackCapabilities() to make more 
evident what it does.

BUG=718632
==========

to

==========
Image Capture: teach takePhoto() to accept an optional PhotoSettings dictionary

This CL:
- Adds an optional PhotoSettings arg to takePhoto(),

- cleanup: the 3 private methods used to receive mojo callbacks 
 (OnPhotoCapabilities, OnSetOptions and OnTakePhoto) get a Mojo 
prefix, for clarity.

- adds a |trigger_take_photo| argument to setOptions() and a few
others that pass it along; takePhoto() uses it to setOptions() when
appropriate.  The sequence of method calls in ImageCapture.cpp then 
would be:
 - without options
   takePhoto() -> OnMojoTakePhoto()
 - with options:
   takePhoto(options) -> setOptions(trigger_take_photo == true)  -->
OnMojoSetOptions(true) --> OnMojoPhotoCapabilities(true) --> OnMojoTakePhoto()


- methods OnCapabilitiesUpdate() and OnCapabilitiesUpdateInternal() 
(which are essentially the same thing) are merged and the resulting
method renamed to UpdateMediaTrackCapabilities() to make more 
evident what it does.

BUG=718632
==========

[mcasas, 2017-05-04 23:05:17]

Patchset #1 (id:1) has been deleted

[mcasas, 2017-05-05 00:09:28]

mcasas@chromium.org changed reviewers:
+ reillyg@chromium.org

[mcasas, 2017-05-05 00:09:29]

reillyg@ PTAL

[Reilly Grant (use Gerrit), 2017-05-05 16:00:28]

https://codereview.chromium.org/2865563002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/imagecapture/ImageCapture.h (right):

https://codereview.chromium.org/2865563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagecapture/ImageCapture.h:59: ScriptPromise
setOptions(ScriptState*, const PhotoSettings&, bool = false);
Since the type name "bool" does not describe the argument please add the
variable name here.

https://codereview.chromium.org/2865563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagecapture/ImageCapture.h:84: void
OnMojoSetOptions(ScriptPromiseResolver*, bool, bool);
Please add variable names for these bools as well.

[mcasas, 2017-05-05 16:38:07]

Ptal

https://codereview.chromium.org/2865563002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/imagecapture/ImageCapture.h (right):

https://codereview.chromium.org/2865563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagecapture/ImageCapture.h:59: ScriptPromise
setOptions(ScriptState*, const PhotoSettings&, bool = false);
On 2017/05/05 16:00:28, Reilly Grant wrote:
> Since the type name "bool" does not describe the argument please add the
> variable name here.

Done.

https://codereview.chromium.org/2865563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagecapture/ImageCapture.h:84: void
OnMojoSetOptions(ScriptPromiseResolver*, bool, bool);
On 2017/05/05 16:00:28, Reilly Grant wrote:
> Please add variable names for these bools as well.

Done.

[Reilly Grant (use Gerrit), 2017-05-05 17:18:00]

lgtm

https://codereview.chromium.org/2865563002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/imagecapture/takephoto-with-photosettings.html
(right):

https://codereview.chromium.org/2865563002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/imagecapture/takephoto-with-photosettings.html:16:
async_test(function(t) {
nit: why not a promise_test?

[mcasas, 2017-05-05 17:48:37]

Patchset #3 (id:60001) has been deleted

[mcasas, 2017-05-05 18:43:00]

The CQ bit was checked by mcasas@chromium.org to run a CQ dry run

[mcasas, 2017-05-05 18:43:08]

https://codereview.chromium.org/2865563002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/imagecapture/takephoto-with-photosettings.html
(right):

https://codereview.chromium.org/2865563002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/imagecapture/takephoto-with-photosettings.html:16:
async_test(function(t) {
On 2017/05/05 17:18:00, Reilly Grant wrote:
> nit: why not a promise_test?

It could perfectly be one, I think I wanted to catch the error 
in l.66 explicitly and print out the error message, whereas
the promise_test [1] IIRC dumps just [Object: object] or a
similar thing.


[1]
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/resources...

[commit-bot: I haz the power, 2017-05-05 18:43:50]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 20:29:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 20:29:29]

Dry run: This issue passed the CQ dry run.

[mcasas, 2017-05-05 20:29:53]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2017-05-05 20:30:52]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 20:37:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494016193983760,
"parent_rev": "48e975e533eaf4b951551df1ea01e90ab5930fd8", "commit_rev":
"3fd00a51e22a0ffa55d0ab14725f9f300b2fc56b"}

[commit-bot: I haz the power, 2017-05-05 20:37:29]

Description was changed from

==========
Image Capture: teach takePhoto() to accept an optional PhotoSettings dictionary

This CL:
- Adds an optional PhotoSettings arg to takePhoto(),

- cleanup: the 3 private methods used to receive mojo callbacks 
 (OnPhotoCapabilities, OnSetOptions and OnTakePhoto) get a Mojo 
prefix, for clarity.

- adds a |trigger_take_photo| argument to setOptions() and a few
others that pass it along; takePhoto() uses it to setOptions() when
appropriate.  The sequence of method calls in ImageCapture.cpp then 
would be:
 - without options
   takePhoto() -> OnMojoTakePhoto()
 - with options:
   takePhoto(options) -> setOptions(trigger_take_photo == true)  -->
OnMojoSetOptions(true) --> OnMojoPhotoCapabilities(true) --> OnMojoTakePhoto()


- methods OnCapabilitiesUpdate() and OnCapabilitiesUpdateInternal() 
(which are essentially the same thing) are merged and the resulting
method renamed to UpdateMediaTrackCapabilities() to make more 
evident what it does.

BUG=718632
==========

to

==========
Image Capture: teach takePhoto() to accept an optional PhotoSettings dictionary

This CL:
- Adds an optional PhotoSettings arg to takePhoto(),

- cleanup: the 3 private methods used to receive mojo callbacks 
 (OnPhotoCapabilities, OnSetOptions and OnTakePhoto) get a Mojo 
prefix, for clarity.

- adds a |trigger_take_photo| argument to setOptions() and a few
others that pass it along; takePhoto() uses it to setOptions() when
appropriate.  The sequence of method calls in ImageCapture.cpp then 
would be:
 - without options
   takePhoto() -> OnMojoTakePhoto()
 - with options:
   takePhoto(options) -> setOptions(trigger_take_photo == true)  -->
OnMojoSetOptions(true) --> OnMojoPhotoCapabilities(true) --> OnMojoTakePhoto()


- methods OnCapabilitiesUpdate() and OnCapabilitiesUpdateInternal() 
(which are essentially the same thing) are merged and the resulting
method renamed to UpdateMediaTrackCapabilities() to make more 
evident what it does.

BUG=718632

Review-Url: https://codereview.chromium.org/2865563002
Cr-Commit-Position: refs/heads/master@{#469766}
Committed:
https://chromium.googlesource.com/chromium/src/+/3fd00a51e22a0ffa55d0ab14725f...
==========

[commit-bot: I haz the power, 2017-05-05 20:37:30]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/3fd00a51e22a0ffa55d0ab14725f...