Let getSettings() return the constrained track resolution, not source.

Let getSettings() return the constrained track resolution, not source.

This lets MediaStreamTrack.getSettings() on a track that has been
limited in size due to constraints return the constrained size rather
than the original size.

Test note: Due to limits of the mock devices, tests don't work with
content_shell. They have been verified to work in a browser, and
upstreamed to web-platform-tests.
https://github.com/w3c/web-platform-tests/pull/5114

BUG=626595
Review-Url: https://codereview.chromium.org/2742893003
Cr-Commit-Position: refs/heads/master@{#457064}
Committed: https://chromium.googlesource.com/chromium/src/+/977056bc7c6be8e98eb98f9702b3f4ed3eb992dd

[hta - Chromium, 2017-03-13 09:48:08]

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

[commit-bot: I haz the power, 2017-03-13 09:48:14]

Dry run: 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-13 11:11:39]

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

[commit-bot: I haz the power, 2017-03-13 11:11:39]

Dry run: This issue passed the CQ dry run.

[hta - Chromium, 2017-03-13 11:43:10]

Description was changed from

==========
Let getSettings() return the constrained track resolution, not source.

This lets MediaStreamTrack.getSettings() on a track that has been
limited in size due to constraints return the constrained size rather
than the original size.

Test note: Due to limits of the mock devices, tests don't work with
content_shell. They have been verified to work in a browser, and
upstreamed to web-platform-tests.
https://github.com/w3c/web-platform-tests/pull/5114

BUG=626595
==========

to

==========
Let getSettings() return the constrained track resolution, not source.

This lets MediaStreamTrack.getSettings() on a track that has been
limited in size due to constraints return the constrained size rather
than the original size.

Test note: Due to limits of the mock devices, tests don't work with
content_shell. They have been verified to work in a browser, and
upstreamed to web-platform-tests.
https://github.com/w3c/web-platform-tests/pull/5114

BUG=626595
==========

[hta - Chromium, 2017-03-13 11:43:10]

hta@chromium.org changed reviewers:
+ guidou@chromium.org

[hta - Chromium, 2017-03-13 11:43:55]

I think this is ready for review.

[Guido Urdaneta, 2017-03-13 14:12:37]

https://codereview.chromium.org/2742893003/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_video_track.h (right):

https://codereview.chromium.org/2742893003/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_video_track.h:66: void SetTargetSize(int
width, int height) {
Is there a reason why the track cannot set these values itself during
initialization?

https://codereview.chromium.org/2742893003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-getSettings.html
(right):

https://codereview.chromium.org/2742893003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-getSettings.html:73:
// assert_equals(settings.width, settings1.width / 2,
would it make sense to make a content_browsertest?

[hta - Chromium, 2017-03-13 15:04:35]

Dialogue...

https://codereview.chromium.org/2742893003/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_video_track.h (right):

https://codereview.chromium.org/2742893003/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_video_track.h:66: void SetTargetSize(int
width, int height) {
On 2017/03/13 14:12:37, Guido Urdaneta wrote:
> Is there a reason why the track cannot set these values itself during
> initialization?

Yes - when it is created, I don't think the resolution is decided yet, and after
creation, it can't easily navigate the hierarchy of objects involved.

It is better for the track to be told by upstream what the resolution is than to
have the track ask the upstream about it.

The way it is set, these variables are empty if not set.
I also wanted to have the opportunty to change them if something changes
upstream (such as calling applyConstraints) - which requires a function anyway.

https://codereview.chromium.org/2742893003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-getSettings.html
(right):

https://codereview.chromium.org/2742893003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-getSettings.html:73:
// assert_equals(settings.width, settings1.width / 2,
On 2017/03/13 14:12:37, Guido Urdaneta wrote:
> would it make sense to make a content_browsertest?

Probably. Does content_browsertest fully support constraints?

[Guido Urdaneta, 2017-03-13 15:08:24]

lgtm

https://codereview.chromium.org/2742893003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-getSettings.html
(right):

https://codereview.chromium.org/2742893003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-getSettings.html:73:
// assert_equals(settings.width, settings1.width / 2,
On 2017/03/13 15:04:35, hta - Chromium wrote:
> On 2017/03/13 14:12:37, Guido Urdaneta wrote:
> > would it make sense to make a content_browsertest?
> 
> Probably. Does content_browsertest fully support constraints?
> 

Yes, it does.

[hta - Chromium, 2017-03-14 08:59:27]

Due to timing issues (away from the computer for a few days), I'll commit this
now, and leave off writing a content_browsertest. The WPT test passes.

Added a reference to the bug about blink_tests and constraints.

[hta - Chromium, 2017-03-14 08:59:41]

The CQ bit was checked by hta@chromium.org

[hta - Chromium, 2017-03-14 08:59:42]

The patchset sent to the CQ was uploaded after l-g-t-m from guidou@chromium.org
Link to the patchset: https://codereview.chromium.org/2742893003/#ps40001
(title: "Added bug # for non-working test")

[commit-bot: I haz the power, 2017-03-14 08:59:53]

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-14 09:05:54]

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

[commit-bot: I haz the power, 2017-03-14 09:05:55]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hta - Chromium, 2017-03-14 09:13:01]

hta@chromium.org changed reviewers:
+ tommi@chromium.org

[hta - Chromium, 2017-03-14 09:13:18]

Adding tommi for owner approval

[hta - Chromium, 2017-03-15 11:43:40]

Submitting since Guido is now in owners.

[hta - Chromium, 2017-03-15 11:43:49]

The CQ bit was checked by hta@chromium.org

[commit-bot: I haz the power, 2017-03-15 11:44:16]

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-15 13:05:41]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489578229665940,
"parent_rev": "b74fc1f0f68a2924581dda2e148ad1f1a0c5b587", "commit_rev":
"977056bc7c6be8e98eb98f9702b3f4ed3eb992dd"}

[commit-bot: I haz the power, 2017-03-15 13:06:22]

Description was changed from

==========
Let getSettings() return the constrained track resolution, not source.

This lets MediaStreamTrack.getSettings() on a track that has been
limited in size due to constraints return the constrained size rather
than the original size.

Test note: Due to limits of the mock devices, tests don't work with
content_shell. They have been verified to work in a browser, and
upstreamed to web-platform-tests.
https://github.com/w3c/web-platform-tests/pull/5114

BUG=626595
==========

to

==========
Let getSettings() return the constrained track resolution, not source.

This lets MediaStreamTrack.getSettings() on a track that has been
limited in size due to constraints return the constrained size rather
than the original size.

Test note: Due to limits of the mock devices, tests don't work with
content_shell. They have been verified to work in a browser, and
upstreamed to web-platform-tests.
https://github.com/w3c/web-platform-tests/pull/5114

BUG=626595

Review-Url: https://codereview.chromium.org/2742893003
Cr-Commit-Position: refs/heads/master@{#457064}
Committed:
https://chromium.googlesource.com/chromium/src/+/977056bc7c6be8e98eb98f9702b3...
==========

[commit-bot: I haz the power, 2017-03-15 13:06:24]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/977056bc7c6be8e98eb98f9702b3...