Image Capture: MediaStreamTrack::applyConstraints()

Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not (TBD later).

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.

applyConstraints() when called without the optional
argument |constraints|, and is supposed to do... nothing? 
Filed Spec bug:
https://github.com/w3c/mediacapture-main/issues/438

BUG=700607
Review-Url: https://codereview.chromium.org/2757673005
Cr-Commit-Position: refs/heads/master@{#457771}
Committed: https://chromium.googlesource.com/chromium/src/+/ef23b14712be35baab5b30b0176808911f9bb638

[mcasas, 2017-03-16 22:39:03]

Patchset #1 (id:1) has been deleted

[mcasas, 2017-03-16 22:43:22]

Description was changed from

==========
Image Capture: MediaStreamTrack::applyConstraints

MediaStreamTrack.applyConstraints()


BUG=700607
==========

to

==========
Image Capture: MediaStreamTrack::applyConstraints

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not.

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.


BUG=700607
==========

[mcasas, 2017-03-16 22:43:34]

Description was changed from

==========
Image Capture: MediaStreamTrack::applyConstraints

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not.

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.


BUG=700607
==========

to

==========
Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not.

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.


BUG=700607
==========

[mcasas, 2017-03-16 22:44:41]

Description was changed from

==========
Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not.

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.


BUG=700607
==========

to

==========
Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not.

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.

applyConstraints() when called without the optional
argument |constraints|, is supposed to do... nothing? 
Filed Spec bug:
https://github.com/w3c/mediacapture-main/issues/438

BUG=700607
==========

[mcasas, 2017-03-16 22:44:52]

Patchset #1 (id:20001) has been deleted

[mcasas, 2017-03-16 22:45:14]

Description was changed from

==========
Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not.

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.

applyConstraints() when called without the optional
argument |constraints|, is supposed to do... nothing? 
Filed Spec bug:
https://github.com/w3c/mediacapture-main/issues/438

BUG=700607
==========

to

==========
Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not (TBD later).

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.

applyConstraints() when called without the optional
argument |constraints|, is supposed to do... nothing? 
Filed Spec bug:
https://github.com/w3c/mediacapture-main/issues/438

BUG=700607
==========

[mcasas, 2017-03-16 22:56:21]

Description was changed from

==========
Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not (TBD later).

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.

applyConstraints() when called without the optional
argument |constraints|, is supposed to do... nothing? 
Filed Spec bug:
https://github.com/w3c/mediacapture-main/issues/438

BUG=700607
==========

to

==========
Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not (TBD later).

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.

applyConstraints() when called without the optional
argument |constraints|, and is supposed to do... nothing? 
Filed Spec bug:
https://github.com/w3c/mediacapture-main/issues/438

BUG=700607
==========

[mcasas, 2017-03-16 22:58:36]

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

[mcasas, 2017-03-16 22:58:37]

reillyg@ PTAL at *imagecapture*

guidou@ PTAL/RS mediastream parts.

[Reilly Grant (use Gerrit), 2017-03-16 23:43:25]

lgtm with nit

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-applyConstraints.html
(right):

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-applyConstraints.html:10:
// when called with no parameters, returns a Promise() that is resolved. This
s/()//

[Guido Urdaneta, 2017-03-17 00:30:28]

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp (right):

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp:286:
ScriptPromiseResolver* resolver,
Maybe add a TODO indicating that the constraints algorithm is not spec
compliant.

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp (right):

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp:295:
DOMException::create(NotFoundError, "Image Capture not found"));
Change the error message to something like "Track type not currently supported"
or something like that. This method will eventually support other track types.

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5 (right):

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5:576: name:
"MediaTrackConstraints",
Since this is only for applyConstraints, call this flag
MediaTrackApplyConstraints or similar.

[mcasas, 2017-03-17 00:49:21]

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

[mcasas, 2017-03-17 00:49:22]

guidou@ PTAL

tkent@ RS plz the extra runtime flag.

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-applyConstraints.html
(right):

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-applyConstraints.html:10:
// when called with no parameters, returns a Promise() that is resolved. This
On 2017/03/16 23:43:25, Reilly Grant wrote:
> s/()//

Done.

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp (right):

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp:286:
ScriptPromiseResolver* resolver,
On 2017/03/17 00:30:27, Guido Urdaneta wrote:
> Maybe add a TODO indicating that the constraints algorithm is not spec
> compliant.

Done.

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp (right):

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp:295:
DOMException::create(NotFoundError, "Image Capture not found"));
On 2017/03/17 00:30:27, Guido Urdaneta wrote:
> Change the error message to something like "Track type not currently
supported"
> or something like that. This method will eventually support other track types.

Done.

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5 (right):

https://codereview.chromium.org/2757673005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5:576: name:
"MediaTrackConstraints",
On 2017/03/17 00:30:27, Guido Urdaneta wrote:
> Since this is only for applyConstraints, call this flag
> MediaTrackApplyConstraints or similar.

Done.

[mcasas, 2017-03-17 00:51:05]

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

[commit-bot: I haz the power, 2017-03-17 00:51:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tkent, 2017-03-17 01:00:13]

lgtm

[commit-bot: I haz the power, 2017-03-17 02:28:45]

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

[commit-bot: I haz the power, 2017-03-17 02:28:46]

Dry run: This issue passed the CQ dry run.

[Guido Urdaneta, 2017-03-17 09:40:03]

lgtm

[haraken, 2017-03-17 09:43:51]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2017-03-17 09:43:52]

Implementation-wise LGTM

https://codereview.chromium.org/2757673005/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp (right):

https://codereview.chromium.org/2757673005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp:289:
resolver->resolve();

Would it be better to reject with NotSupportedError?

[mcasas, 2017-03-17 15:12:48]

https://codereview.chromium.org/2757673005/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp (right):

https://codereview.chromium.org/2757673005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp:289:
resolver->resolve();
On 2017/03/17 09:43:51, haraken wrote:
> 
> Would it be better to reject with NotSupportedError?

|constraints| is a dictionary having just one member,
|advanced| (strange, right?). The spec says that the 
|constraints| parameter is optional and doesn't specify 
what to do if it's not there. This doesn't make any sense, 
so I filed the Spec-bug in l.287, but I guessed not 
having an optional parameter is ok-ish and not a reason 
to reject().

[mcasas, 2017-03-17 15:13:56]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2017-03-17 15:13:56]

The patchset sent to the CQ was uploaded after l-g-t-m from reillyg@chromium.org
Link to the patchset: https://codereview.chromium.org/2757673005/#ps60001
(title: "guidou@ and reillyg@ comments")

[commit-bot: I haz the power, 2017-03-17 15:14:14]

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-03-17 15:19:49]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1489763636422670,
"parent_rev": "a8b24819979eaf3727465ecdbf89e462cf98f817", "commit_rev":
"ef23b14712be35baab5b30b0176808911f9bb638"}

[commit-bot: I haz the power, 2017-03-17 15:20:36]

Description was changed from

==========
Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not (TBD later).

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.

applyConstraints() when called without the optional
argument |constraints|, and is supposed to do... nothing? 
Filed Spec bug:
https://github.com/w3c/mediacapture-main/issues/438

BUG=700607
==========

to

==========
Image Capture: MediaStreamTrack::applyConstraints()

This CL adds MediaStreamTrack.applyConstraints()
method behind a flag (that is implied if ImageCapture 
is on).  The method essentially derives to
ImageCapture::setMediaTrackConstraints(); this one 
prepares a mojo::PhotoSettings and applies it via
mojo interface.

Only simple Constrains are supported now, i.e., 
{zoom : 3.14} is supported, but {zoom : { ideal : 100}}
or {zoom : { exact : 27}} are not (TBD later).

LayoutTests are added where appropriate.

Also a cleanup: methods that return Promises are not
supposed to throw exceptions; so I removed an unused
ExceptionState from some ImageCapture methods.

applyConstraints() when called without the optional
argument |constraints|, and is supposed to do... nothing? 
Filed Spec bug:
https://github.com/w3c/mediacapture-main/issues/438

BUG=700607

Review-Url: https://codereview.chromium.org/2757673005
Cr-Commit-Position: refs/heads/master@{#457771}
Committed:
https://chromium.googlesource.com/chromium/src/+/ef23b14712be35baab5b30b01768...
==========

[commit-bot: I haz the power, 2017-03-17 15:20:38]

Committed patchset #2 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/ef23b14712be35baab5b30b01768...