Image Capture takePhoto() should accept an optional dictionary of PhotoSettings |
||||
Issue description
Catch-up with the spec, takePhoto() should be [1]:
Promise<Blob> takePhoto(optional PhotoSettings photoSettings);
and it is now [2]
[CallWith=ScriptState] Promise<void> setOptions(PhotoSettings photoSettings);
[CallWith=ScriptState] Promise<Blob> takePhoto();
[1] https://w3c.github.io/mediacapture-image/#imagecaptureapi
[2] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl?q=imagecapture.idl&sq=package:chromium&dr&l=20
,
May 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3fd00a51e22a0ffa55d0ab14725f9f300b2fc56b commit 3fd00a51e22a0ffa55d0ab14725f9f300b2fc56b Author: mcasas <mcasas@chromium.org> Date: Fri May 05 20:37:06 2017 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} [add] https://crrev.com/3fd00a51e22a0ffa55d0ab14725f9f300b2fc56b/third_party/WebKit/LayoutTests/imagecapture/takephoto-with-photosettings.html [modify] https://crrev.com/3fd00a51e22a0ffa55d0ab14725f9f300b2fc56b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp [modify] https://crrev.com/3fd00a51e22a0ffa55d0ab14725f9f300b2fc56b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h [modify] https://crrev.com/3fd00a51e22a0ffa55d0ab14725f9f300b2fc56b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl
,
May 5 2017
,
May 6 2017
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dcc52283947a13edbd756eb739ee55a372fa5483 commit dcc52283947a13edbd756eb739ee55a372fa5483 Author: Miguel Casas-Sanchez <mcasas@chromium.org> Date: Mon May 08 17:34:12 2017 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} (cherry picked from commit 3fd00a51e22a0ffa55d0ab14725f9f300b2fc56b) Review-Url: https://codereview.chromium.org/2866053002 . Cr-Commit-Position: refs/branch-heads/3071@{#451} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [add] https://crrev.com/dcc52283947a13edbd756eb739ee55a372fa5483/third_party/WebKit/LayoutTests/imagecapture/takephoto-with-photosettings.html [modify] https://crrev.com/dcc52283947a13edbd756eb739ee55a372fa5483/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp [modify] https://crrev.com/dcc52283947a13edbd756eb739ee55a372fa5483/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h [modify] https://crrev.com/dcc52283947a13edbd756eb739ee55a372fa5483/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl
,
May 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d0f1c8d5aa6c645af631bd1772b5aa2afda2cae7 commit d0f1c8d5aa6c645af631bd1772b5aa2afda2cae7 Author: khmel <khmel@chromium.org> Date: Mon May 08 19:24:12 2017 Revert of Image Capture: teach takePhoto() to accept an optional PhotoSettings dictionary (patchset #1 id:1 of https://codereview.chromium.org/2866053002/ ) Reason for revert: This breaks build ../../third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp:570:61: error: function definition is not allowed here media::mojom::blink::PhotoCapabilitiesPtr capabilities) { Sorry, Miguel Original issue's description: > 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} > (cherry picked from commit 3fd00a51e22a0ffa55d0ab14725f9f300b2fc56b) > > Review-Url: https://codereview.chromium.org/2866053002 . > Cr-Commit-Position: refs/branch-heads/3071@{#451} > Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} > Committed: https://chromium.googlesource.com/chromium/src/+/dcc52283947a13edbd756eb739ee55a372fa5483 TBR=mcasas@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 718632 Review-Url: https://codereview.chromium.org/2872713002 Cr-Commit-Position: refs/branch-heads/3071@{#457} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [delete] https://crrev.com/0d67f4a02abc54313bbb9fef0fb4acdfd6eac7d4/third_party/WebKit/LayoutTests/imagecapture/takephoto-with-photosettings.html [modify] https://crrev.com/d0f1c8d5aa6c645af631bd1772b5aa2afda2cae7/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp [modify] https://crrev.com/d0f1c8d5aa6c645af631bd1772b5aa2afda2cae7/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h [modify] https://crrev.com/d0f1c8d5aa6c645af631bd1772b5aa2afda2cae7/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl
,
May 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b4d3ac8136a52dbdcd521ac47fefb55bda7dbe88 commit b4d3ac8136a52dbdcd521ac47fefb55bda7dbe88 Author: Miguel Casas-Sanchez <mcasas@chromium.org> Date: Mon May 08 21:26:10 2017 Image Capture: teach takePhoto() to accept an optional PhotoSettings dictionary (2nd landing) [Note] The first landing was reverted due to a wrong rebase on my side: forgot to close a { :-P 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} NOPRESUBMIT=true NOTRY=true TBR=reillyg@chromium.org (reviewer of the original CL) Review-Url: https://codereview.chromium.org/2871653003 . Cr-Commit-Position: refs/branch-heads/3071@{#463} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [add] https://crrev.com/b4d3ac8136a52dbdcd521ac47fefb55bda7dbe88/third_party/WebKit/LayoutTests/imagecapture/takephoto-with-photosettings.html [modify] https://crrev.com/b4d3ac8136a52dbdcd521ac47fefb55bda7dbe88/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp [modify] https://crrev.com/b4d3ac8136a52dbdcd521ac47fefb55bda7dbe88/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h [modify] https://crrev.com/b4d3ac8136a52dbdcd521ac47fefb55bda7dbe88/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl |
||||
►
Sign in to add a comment |
||||
Comment 1 by fbeaufort@chromium.org
, May 5 2017