New issue
Advanced search Search tips

Issue 711694 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Image Capture: applyConstraints() should reject unsupported constraints

Project Member Reported by mcasas@chromium.org, Apr 14 2017

Issue description

If e.g. |torch| is not supported (getCapabilities() says torch= false or doesn't list it), then applyConstraints({advanced: [ torch: true ]}) should reject. It doesn’t.

This is because support for Constraint X is not checked in 
SetMediaTrackConstraints() [1] before applying it.

mediacapture-streams [2] says that 
> It is possible that constraints can be applied to a track which a source is unable to satisfy, either because the source itself cannot satisfy the constraint or because the source is already satisfying a conflicting constraint. When this happens, the promise returned from applyConstraints() will be rejected, without applying any of the new constraints. 

albeit in a non-normative section; however it says elsewhere [3] that
> and if it is not possible to satisfy simultaneously all of those individual constraints for the indicated property names, the User Agent must reject the returned promise.

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp?type=cs&q=imagecapture.cpp+supports_torch&l=252
[2] https://www.w3.org/TR/mediacapture-streams/#the-model-sources-sinks-constraints-and-settings
[3] https://www.w3.org/TR/mediacapture-streams/#constrainable-interface
 

Comment 2 by mcasas@chromium.org, Apr 14 2017

Status: Fixed (was: Assigned)
fbeaufort@ #1 should fix this issue.

Comment 3 by mcasas@chromium.org, Apr 15 2017

Labels: Merge-Request-59
M59 branched as 464641, so we missed it by a jiffie.
Project Member

Comment 4 by sheriffbot@chromium.org, Apr 15 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 5 by bugdroid1@chromium.org, Apr 17 2017

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

commit dfb6460559869ea71010b5c5df456e63858d401b
Author: mcasas <mcasas@chromium.org>
Date: Mon Apr 17 15:27:06 2017

Image Capture: Reject applyConstraints() if passed an unsupported constraint

This CL adds a check to reject applyConstraints() if it is passed an
unsupported constraint. LayoutTests gets an updated, albeit a bit a
cumbersome one because they hit an unrelated bug (https://crbug.com/711524).

BUG= 711694 

Review-Url: https://codereview.chromium.org/2819653003
Cr-Commit-Position: refs/heads/master@{#464820}
(cherry picked from commit 635372d1d6a0318a44dbc1be736d8060a126d68a)

NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2819233002
Cr-Commit-Position: refs/branch-heads/3071@{#13}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/dfb6460559869ea71010b5c5df456e63858d401b/third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-applyConstraints.html
[modify] https://crrev.com/dfb6460559869ea71010b5c5df456e63858d401b/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints-getSettings.html
[modify] https://crrev.com/dfb6460559869ea71010b5c5df456e63858d401b/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-applyConstraints.html
[modify] https://crrev.com/dfb6460559869ea71010b5c5df456e63858d401b/third_party/WebKit/LayoutTests/imagecapture/MediaStreamTrack-getCapabilities.html
[modify] https://crrev.com/dfb6460559869ea71010b5c5df456e63858d401b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp

Sign in to add a comment