Skip to content

Commit

Permalink
Image Capture: Reject applyConstraints() if passed an unsupported con…
Browse files Browse the repository at this point in the history
…straint

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 635372d)

NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2819233002
Cr-Commit-Position: refs/branch-heads/3071@{#13}
Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
  • Loading branch information
yellowdoge authored and Commit bot committed Apr 17, 2017
1 parent db95542 commit dfb6460
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,26 @@
return videoTrack.applyConstraints();
}, 'MediaStreamTrack.applyConstraints()');

// This test verifies that applyConstraints() rejects the returned Promise if
// passed a non-supported image-capture constraint (https://crbug.com/711694).
promise_test(function(t) {
var canvas = document.getElementById('canvas');
var context = canvas.getContext("2d");
context.fillStyle = "red";
context.fillRect(0, 0, 10, 10);

var stream = canvas.captureStream();
var videoTrack = stream.getVideoTracks()[0];

var expectedException =
new DOMException('Unsupported constraint(s)', 'NotSupportedError');

// Use e.g. |torch| as an example of unsupported constraint.
assert_false("torch" in videoTrack.getCapabilities());
return promise_rejects(
t, expectedException,
videoTrack.applyConstraints({advanced : [ {torch : true} ]}));
}, 'MediaStreamTrack.applyConstraints() with unsupported constraint');

// This test verifies that applyConstraints() rejects the returned Promise if
// passed a non-supported constraint.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@
var stream = canvas.captureStream();
videoTrack = stream.getVideoTracks()[0];

// |videoTrack|'s capabilities gathering, just like the actual capture, is
// a process kicked off right after creation, we introduce a small delay
// to allow for those to be collected, since they are needed to understand
// which constraints are supported in applyConstraints().
// TODO(mcasas): this shouldn't be needed, https://crbug.com/711524.
return new Promise(resolve => setTimeout(resolve, 100));
})
.then(function() {
return videoTrack.applyConstraints(constraints);
})
.then(function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,26 @@
}]};

var theMock = null;
var videoTrack = null;
mockImageCaptureReady
.then(mock => {
theMock = mock;
var stream = canvas.captureStream();
var videoTrack = stream.getVideoTracks()[0];

return videoTrack.applyConstraints(constraints);
videoTrack = stream.getVideoTracks()[0];

// |videoTrack|'s capabilities gathering, just like the actual capture, is
// a process kicked off right after creation, we introduce a small delay
// to allow for those to be collected, since they are needed to understand
// which constraints are supported in applyConstraints().
// TODO(mcasas): this shouldn't be needed, https://crbug.com/711524.
return new Promise(resolve => setTimeout(resolve, 100));
},
error => {
assert_unreached("Error creating MockImageCapture: " + error);
})
.then(function() {
return videoTrack.applyConstraints(constraints);
})
.then(function() {
assert_equals(constraints.advanced[0].whiteBalanceMode,
meteringModeNames[theMock.options().white_balance_mode],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@
var videoTrack = stream.getVideoTracks()[0];
assert_equals(typeof videoTrack.getCapabilities, 'function');

// |videoTrack|s capabilities, just like the actual capture, is a process
// kicked right after creation, we introduce a small delay to allow for
// those to be collected.
// |videoTrack|'s capabilities gathering, just like the actual capture, is
// a process kicked off right after creation, we introduce a small delay
// to allow for those to be collected.
// TODO(mcasas): this shouldn't be needed, https://crbug.com/711524.
setTimeout(() => {
capabilities = videoTrack.getCapabilities();
assert_equals(typeof capabilities, 'object');
Expand Down
20 changes: 20 additions & 0 deletions third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,26 @@ void ImageCapture::SetMediaTrackConstraints(
// TODO(mcasas): add support more than one single advanced constraint.
const auto constraints = constraints_vector[0];

if ((constraints.hasWhiteBalanceMode() &&
!capabilities_.hasWhiteBalanceMode()) ||
(constraints.hasExposureMode() && !capabilities_.hasExposureMode()) ||
(constraints.hasFocusMode() && !capabilities_.hasFocusMode()) ||
(constraints.hasExposureCompensation() &&
!capabilities_.hasExposureCompensation()) ||
(constraints.hasColorTemperature() &&
!capabilities_.hasColorTemperature()) ||
(constraints.hasIso() && !capabilities_.hasIso()) ||
(constraints.hasBrightness() && !capabilities_.hasBrightness()) ||
(constraints.hasContrast() && !capabilities_.hasContrast()) ||
(constraints.hasSaturation() && !capabilities_.hasSaturation()) ||
(constraints.hasSharpness() && !capabilities_.hasSharpness()) ||
(constraints.hasZoom() && !capabilities_.hasZoom()) ||
(constraints.hasTorch() && !capabilities_.hasTorch())) {
resolver->Reject(
DOMException::Create(kNotSupportedError, "Unsupported constraint(s)"));
return;
}

auto settings = media::mojom::blink::PhotoSettings::New();

// TODO(mcasas): support other Mode types beyond simple string i.e. the
Expand Down

0 comments on commit dfb6460

Please sign in to comment.