New issue
Advanced search Search tips

Issue 718765 link

Starred by 2 users
Status: Fixed
Owner:
mcasas@chromium.org
Closed: May 2017
Cc:
mcasas@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 718632


Sign in to add a comment

Image Capture: reject settings/options if value is beyond supported and/or can't be applied

Project Member Reported by fbeaufort@chromium.org, May 5 2017 
If the red eye reduction capability says "never", then takePhoto({advanced: [{redEyeReduction : true}]}) should fail. This should apply to all photoSettings options.

Comment 1 by mcasas@chromium.org, May 11 2017

Labels: Pri-2

Comment 2 by mcasas@chromium.org, May 11 2017

Labels: -OS-Chrome OS-All
Owner: mcasas@chromium.org
Status: Assigned (was: Unconfirmed)
Summary: Image Capture: reject settings/options if value is beyond supported and/or can't be applied (was: Image Capture: takePhoto(options) should reject if options can't be applied)
this also applies to e.g. trying to configure a capability
to a not supported range, e.g. if theTrack.getCapabilities()  
says {... brightness : { min: 0, max: 255, step:1}...}, then

theTrack.applyConstraints({advanced: [{ brightness: 345 }] });

should be rejected.
Project Member

Comment 4 by bugdroid1@chromium.org, May 16 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64b5e1fad912daf7c311c7f984db673bcae7713c

commit 64b5e1fad912daf7c311c7f984db673bcae7713c
Author: mcasas <mcasas@chromium.org>
Date: Tue May 16 17:21:12 2017

Image Capture: reject setOptions()/applyConstraints() if any argument is unsupported

This CL adds rejection of applyConstraints()/setOptions() if the control value
is out of ranges or unsupported, e.g. trying to set zoom to 1000 where the
capability range is {1.0 - 10.0}.

LayoutTests for each of those methods are added.

BUG= 718765 

Review-Url: https://codereview.chromium.org/2877273002
Cr-Commit-Position: refs/heads/master@{#472143}

[modify] https://crrev.com/64b5e1fad912daf7c311c7f984db673bcae7713c/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-getSettings.html
[add] https://crrev.com/64b5e1fad912daf7c311c7f984db673bcae7713c/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-reject.html
[modify] https://crrev.com/64b5e1fad912daf7c311c7f984db673bcae7713c/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints.html
[modify] https://crrev.com/64b5e1fad912daf7c311c7f984db673bcae7713c/third_party/WebKit/LayoutTests/imagecapture/resources/mock-imagecapture.js
[add] https://crrev.com/64b5e1fad912daf7c311c7f984db673bcae7713c/third_party/WebKit/LayoutTests/imagecapture/setOptions-reject.html
[modify] https://crrev.com/64b5e1fad912daf7c311c7f984db673bcae7713c/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/64b5e1fad912daf7c311c7f984db673bcae7713c/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/64b5e1fad912daf7c311c7f984db673bcae7713c/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.cpp
[modify] https://crrev.com/64b5e1fad912daf7c311c7f984db673bcae7713c/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.h
Project Member

Comment 5 by bugdroid1@chromium.org, May 16 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28c903b63eeb69ca5a3d954ce9549c59d9c10bef

commit 28c903b63eeb69ca5a3d954ce9549c59d9c10bef
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Tue May 16 18:25:39 2017

Image Capture: correct LayoutTests name case

This CL is a follow up to https://crrev.com/2877273002, where tests
could not be renamed due to some platforms' file systems being case
insensitive.

Here we Rename a few tests:

- getphotocapabilities.html => getPhotoCapabilities.html
- setoptions.html => setOptions.html
- takephoto-with-photosettings.html => takePhoto-with-PhotoSettings.html
- takephoto.html => takePhoto.html

**No new code.**

TBR=emircan@chromium.org

Bug:  718765 
Change-Id: I4dc03a95a86a43a4422f3d2311ed163322d656ba
Reviewed-on: https://chromium-review.googlesource.com/506506
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#472167}
[rename] https://crrev.com/28c903b63eeb69ca5a3d954ce9549c59d9c10bef/third_party/WebKit/LayoutTests/imagecapture/getPhotoCapabilities.html
[rename] https://crrev.com/28c903b63eeb69ca5a3d954ce9549c59d9c10bef/third_party/WebKit/LayoutTests/imagecapture/setOptions.html
[rename] https://crrev.com/28c903b63eeb69ca5a3d954ce9549c59d9c10bef/third_party/WebKit/LayoutTests/imagecapture/takePhoto-with-PhotoSettings.html
[rename] https://crrev.com/28c903b63eeb69ca5a3d954ce9549c59d9c10bef/third_party/WebKit/LayoutTests/imagecapture/takePhoto.html

Comment 6 by mcasas@chromium.org, May 16 2017

Labels: Merge-Request-59
Status: Fixed (was: Assigned)
Project Member

Comment 7 by sheriffbot@chromium.org, May 17 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
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
Project Member

Comment 8 by bugdroid1@chromium.org, May 17 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb3563bb431e6c26dace10a4b5325d4d4b812da5

commit eb3563bb431e6c26dace10a4b5325d4d4b812da5
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Wed May 17 20:02:29 2017

Image Capture: reject setOptions()/applyConstraints() if any argument is unsupported

This CL adds rejection of applyConstraints()/setOptions() if the control value
is out of ranges or unsupported, e.g. trying to set zoom to 1000 where the
capability range is {1.0 - 10.0}.

LayoutTests for each of those methods are added.

BUG= 718765 

Review-Url: https://codereview.chromium.org/2877273002
Cr-Original-Commit-Position: refs/heads/master@{#472143}
Review-Url: https://codereview.chromium.org/2887043003 .
Cr-Commit-Position: refs/branch-heads/3071@{#605}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/eb3563bb431e6c26dace10a4b5325d4d4b812da5/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-getSettings.html
[add] https://crrev.com/eb3563bb431e6c26dace10a4b5325d4d4b812da5/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-reject.html
[modify] https://crrev.com/eb3563bb431e6c26dace10a4b5325d4d4b812da5/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints.html
[modify] https://crrev.com/eb3563bb431e6c26dace10a4b5325d4d4b812da5/third_party/WebKit/LayoutTests/imagecapture/resources/mock-imagecapture.js
[add] https://crrev.com/eb3563bb431e6c26dace10a4b5325d4d4b812da5/third_party/WebKit/LayoutTests/imagecapture/setOptions-reject.html
[modify] https://crrev.com/eb3563bb431e6c26dace10a4b5325d4d4b812da5/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
[modify] https://crrev.com/eb3563bb431e6c26dace10a4b5325d4d4b812da5/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
[modify] https://crrev.com/eb3563bb431e6c26dace10a4b5325d4d4b812da5/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.cpp
[modify] https://crrev.com/eb3563bb431e6c26dace10a4b5325d4d4b812da5/third_party/WebKit/Source/modules/imagecapture/PhotoCapabilities.h
Project Member

Comment 9 by bugdroid1@chromium.org, May 17 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb02d95f0e169c0271a81baa94d144bb19b1d1c5

commit eb02d95f0e169c0271a81baa94d144bb19b1d1c5
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Wed May 17 20:05:23 2017

Image Capture: correct LayoutTests name case

This CL is a follow up to https://crrev.com/2877273002, where tests
could not be renamed due to some platforms' file systems being case
insensitive.

Here we Rename a few tests:

- getphotocapabilities.html => getPhotoCapabilities.html
- setoptions.html => setOptions.html
- takephoto-with-photosettings.html => takePhoto-with-PhotoSettings.html
- takephoto.html => takePhoto.html

**No new code.**

TBR=emircan@chromium.org

Bug:  718765 
Change-Id: I4dc03a95a86a43a4422f3d2311ed163322d656ba
Reviewed-on: https://chromium-review.googlesource.com/506506
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#472167}
Review-Url: https://codereview.chromium.org/2883283005 .
Cr-Commit-Position: refs/branch-heads/3071@{#606}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[rename] https://crrev.com/eb02d95f0e169c0271a81baa94d144bb19b1d1c5/third_party/WebKit/LayoutTests/imagecapture/getPhotoCapabilities.html
[rename] https://crrev.com/eb02d95f0e169c0271a81baa94d144bb19b1d1c5/third_party/WebKit/LayoutTests/imagecapture/setOptions.html
[rename] https://crrev.com/eb02d95f0e169c0271a81baa94d144bb19b1d1c5/third_party/WebKit/LayoutTests/imagecapture/takePhoto-with-PhotoSettings.html
[rename] https://crrev.com/eb02d95f0e169c0271a81baa94d144bb19b1d1c5/third_party/WebKit/LayoutTests/imagecapture/takePhoto.html

Comment 10 by fbeaufort@chromium.org, May 31 2017 

Verified in Chromium 61.0.3117.0
►

Sign in to add a comment