Media Controls: take into consideration high margin on some elements.

Media Controls: take into consideration high margin on some elements.

When computing the width required to show the controls, we should
take into consideration elements with a large margin. It is in
general better to overestimate that under estimate to avoid buttons
being cut.

BUG=650670
TEST=Manual testing on https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html

Committed: https://crrev.com/c8b4bc2d7895981dcdc674e69c9691271dc0c483
Cr-Commit-Position: refs/heads/master@{#422246}

[mlamouri (slow - plz ping), 2016-09-27 15:07:06]

mlamouri@chromium.org changed reviewers:
+ liberato@chromium.org

[mlamouri (slow - plz ping), 2016-09-27 15:07:07]

Frank PTAL.

I'm assuming that there is some history around the code here. I want to make
sure I'm not overlooking some details.

[mlamouri (slow - plz ping), 2016-09-27 15:07:11]

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

[commit-bot: I haz the power, 2016-09-27 15:07:25]

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

[mlamouri (slow - plz ping), 2016-09-27 15:07:36]

Description was changed from

==========
Media Controls: fix issues with buttons being cut from the controls.

BUG=650670
==========

to

==========
Media Controls: fix issues with buttons being cut from the controls.

BUG=650670
TEST=Manual testing on
https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html
==========

[commit-bot: I haz the power, 2016-09-27 16:03:04]

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

[commit-bot: I haz the power, 2016-09-27 16:03:05]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[liberato (no reviews please), 2016-09-27 16:33:53]

https://codereview.chromium.org/2374733002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/shadow/MediaControls.cpp (right):

https://codereview.chromium.org/2374733002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/shadow/MediaControls.cpp:751: int
largeMinimumWidth = minimumWidth * 2;
is this enough on desktop?  there is 18px margin on either side.

might want +36 rather than *2 -- not sure that we need 48*2 on android.

https://codereview.chromium.org/2374733002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/shadow/MediaControls.cpp:819: LOG(INFO) <<
"set first element";
:)

[mlamouri (slow - plz ping), 2016-09-29 12:53:00]

Description was changed from

==========
Media Controls: fix issues with buttons being cut from the controls.

BUG=650670
TEST=Manual testing on
https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html
==========

to

==========
Media Controls: take into consideration high margin on some elements.

When computing the width required to show the controls, we should
take into consideration elements with a large margin. It is in
general better to overestimate that under estimate to avoid buttons
being cut.

BUG=650670
TEST=Manual testing on
https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html
==========

[mlamouri (slow - plz ping), 2016-09-29 13:00:39]

Comments applied. PTAL.

https://codereview.chromium.org/2374733002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/shadow/MediaControls.cpp (right):

https://codereview.chromium.org/2374733002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/shadow/MediaControls.cpp:751: int
largeMinimumWidth = minimumWidth * 2;
On 2016/09/27 16:33:53, liberato wrote:
> is this enough on desktop?  there is 18px margin on either side.
> 
> might want +36 rather than *2 -- not sure that we need 48*2 on android.

Done.

https://codereview.chromium.org/2374733002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/shadow/MediaControls.cpp:819: LOG(INFO) <<
"set first element";
On 2016/09/27 16:33:53, liberato wrote:
> :)

Done.

[liberato (no reviews please), 2016-09-29 14:55:06]

lgtm

[mlamouri (slow - plz ping), 2016-09-30 09:38:37]

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

[commit-bot: I haz the power, 2016-09-30 09:38:52]

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, 2016-09-30 10:42:28]

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

[commit-bot: I haz the power, 2016-09-30 10:42:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mlamouri (slow - plz ping), 2016-09-30 13:34:39]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2016-09-30 13:34:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
liberato@chromium.org
Link to the patchset: https://codereview.chromium.org/2374733002/#ps60001
(title: "rebaseline")

[commit-bot: I haz the power, 2016-09-30 13:34:51]

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, 2016-09-30 14:23:31]

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

[commit-bot: I haz the power, 2016-09-30 14:23:32]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[mlamouri (slow - plz ping), 2016-09-30 15:31:03]

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

[commit-bot: I haz the power, 2016-09-30 15:31:20]

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, 2016-09-30 17:15:06]

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

[commit-bot: I haz the power, 2016-09-30 17:15:07]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mlamouri (slow - plz ping), 2016-09-30 20:48:30]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2016-09-30 20:48:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
liberato@chromium.org
Link to the patchset: https://codereview.chromium.org/2374733002/#ps100001
(title: "rebaseline")

[commit-bot: I haz the power, 2016-09-30 20:48:57]

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, 2016-09-30 22:33:39]

Description was changed from

==========
Media Controls: take into consideration high margin on some elements.

When computing the width required to show the controls, we should
take into consideration elements with a large margin. It is in
general better to overestimate that under estimate to avoid buttons
being cut.

BUG=650670
TEST=Manual testing on
https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html
==========

to

==========
Media Controls: take into consideration high margin on some elements.

When computing the width required to show the controls, we should
take into consideration elements with a large margin. It is in
general better to overestimate that under estimate to avoid buttons
being cut.

BUG=650670
TEST=Manual testing on
https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html
==========

[commit-bot: I haz the power, 2016-09-30 22:33:40]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-30 22:36:20]

Description was changed from

==========
Media Controls: take into consideration high margin on some elements.

When computing the width required to show the controls, we should
take into consideration elements with a large margin. It is in
general better to overestimate that under estimate to avoid buttons
being cut.

BUG=650670
TEST=Manual testing on
https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html
==========

to

==========
Media Controls: take into consideration high margin on some elements.

When computing the width required to show the controls, we should
take into consideration elements with a large margin. It is in
general better to overestimate that under estimate to avoid buttons
being cut.

BUG=650670
TEST=Manual testing on
https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html

Committed: https://crrev.com/c8b4bc2d7895981dcdc674e69c9691271dc0c483
Cr-Commit-Position: refs/heads/master@{#422246}
==========

[commit-bot: I haz the power, 2016-09-30 22:36:21]

Patchset 6 (id:??) landed as
https://crrev.com/c8b4bc2d7895981dcdc674e69c9691271dc0c483
Cr-Commit-Position: refs/heads/master@{#422246}