Mac: adjust sizes of progress bars to be equal to painted size.

Mac: adjust sizes of progress bars to be equal to painted size.

Moves code to compute the size into the style adjustment phase, and simplifies
it according to the actual margins specified in the style configs.

BUG=632319
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[chrishtr, 2016-09-02 22:59:35]

Description was changed from

==========
none


none

BUG=
==========

to

==========
none


none

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2016-09-02 23:01:09]

Description was changed from

==========
none


none

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Mac: adjust sizes of progress bars to be equal to painted size.

BUG=632319
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2016-09-02 23:01:51]

Description was changed from

==========
Mac: adjust sizes of progress bars to be equal to painted size.

BUG=632319
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Mac: adjust sizes of progress bars to be equal to painted size.

Moves code to compute the size into the style adjustment phase, and simplifies
it according to the actual margins specified in the style configs.

BUG=632319
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2016-09-02 23:28:37]

chrishtr@chromium.org changed reviewers:
+ pdr@chromium.org

[chrishtr, 2016-09-02 23:28:38]

https://codereview.chromium.org/2305133002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutThemeMac.mm (left):

https://codereview.chromium.org/2305133002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutThemeMac.mm:697: { 0, 0, 1, 0 },
Note for reviewer: this struct says that the margin-bottom is expanded by 1 px
regardless of control
size, and no other margins are expanded.

[chrishtr, 2016-09-02 23:28:47]

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

[commit-bot: I haz the power, 2016-09-02 23:29:45]

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-03 00:55:29]

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

[commit-bot: I haz the power, 2016-09-03 00:55:31]

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

[chrishtr, 2016-09-03 01:21:10]

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

[commit-bot: I haz the power, 2016-09-03 01:21:37]

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-03 01:24:24]

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

[commit-bot: I haz the power, 2016-09-03 01:24:25]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[chrishtr, 2016-09-03 01:35:37]

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

[commit-bot: I haz the power, 2016-09-03 01:35:51]

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-03 04:03:36]

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

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

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_...)

[chrishtr, 2016-09-03 19:42:41]

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

[commit-bot: I haz the power, 2016-09-03 19:42:43]

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-03 20:51:33]

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

[commit-bot: I haz the power, 2016-09-03 20:51:34]

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_...)

[pdr., 2016-09-06 17:25:00]

Looks good overall. Does the inspector still show the wrong rectangle when
hovering the progress bar with this patch though?

https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutThemeMac.mm (right):

https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutThemeMac.mm:921: IntSize size =
progressBarSizes()[controlSize];
Refactor progressBarSizes to just return a height instead of a size?

[pdr., 2016-09-06 17:26:18]

https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutThemeMac.mm (right):

https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutThemeMac.mm:921: IntSize size =
progressBarSizes()[controlSize];
On 2016/09/06 at 17:25:00, pdr. wrote:
> Refactor progressBarSizes to just return a height instead of a size?

Nvm, this may be too messy. Can you just assert that the width is zero?

[chrishtr, 2016-09-06 17:55:15]

On 2016/09/06 at 17:25:00, pdr wrote:
> Looks good overall. Does the inspector still show the wrong rectangle when
hovering the progress bar with this patch though?

It shows the correct rectangle.

> 
>
https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/LayoutThemeMac.mm (right):
> 
>
https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/LayoutThemeMac.mm:921: IntSize size =
progressBarSizes()[controlSize];
> Refactor progressBarSizes to just return a height instead of a size?

[pdr., 2016-09-06 17:56:24]

On 2016/09/06 at 17:55:15, chrishtr wrote:
> On 2016/09/06 at 17:25:00, pdr wrote:
> > Looks good overall. Does the inspector still show the wrong rectangle when
hovering the progress bar with this patch though?
> 
> It shows the correct rectangle.
> 
> > 
> >
https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/layout/LayoutThemeMac.mm (right):
> > 
> >
https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/layout/LayoutThemeMac.mm:921: IntSize size =
progressBarSizes()[controlSize];
> > Refactor progressBarSizes to just return a height instead of a size?

LGTM with a lil' dcheck

[pdr., 2016-09-06 17:56:56]

On 2016/09/06 at 17:56:24, pdr. wrote:
> On 2016/09/06 at 17:55:15, chrishtr wrote:
> > On 2016/09/06 at 17:25:00, pdr wrote:
> > > Looks good overall. Does the inspector still show the wrong rectangle when
hovering the progress bar with this patch though?
> > 
> > It shows the correct rectangle.
> > 
> > > 
> > >
https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/core/layout/LayoutThemeMac.mm (right):
> > > 
> > >
https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/core/layout/LayoutThemeMac.mm:921: IntSize size
= progressBarSizes()[controlSize];
> > > Refactor progressBarSizes to just return a height instead of a size?
> 
> LGTM with a lil' dcheck

oh I see it's in the latest patch. LGTM

[chrishtr, 2016-09-06 17:57:05]

https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutThemeMac.mm (right):

https://codereview.chromium.org/2305133002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutThemeMac.mm:921: IntSize size =
progressBarSizes()[controlSize];
On 2016/09/06 at 17:26:18, pdr. wrote:
> On 2016/09/06 at 17:25:00, pdr. wrote:
> > Refactor progressBarSizes to just return a height instead of a size?
> 
> Nvm, this may be too messy. Can you just assert that the width is zero?

I was actually able to do it, when I made another change to setMinHeight rather
than comparing height
with the minimum. This matches other approaches in this file, and avoids me
having to convert from
percentage/other heights to pixels.

[chrishtr, 2016-09-06 17:57:30]

The CQ bit was checked by chrishtr@chromium.org

[pdr., 2016-09-06 17:57:46]

On 2016/09/06 at 17:56:56, pdr. wrote:
> 
> oh I see it's in the latest patch. LGTM

(where it == progressBarHeights refactor)

[commit-bot: I haz the power, 2016-09-06 17:58:05]

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-06 20:11:26]

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

[commit-bot: I haz the power, 2016-09-06 20:11:28]

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_...)

[chrishtr, 2016-09-06 22:28:21]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-09-06 22:28:21]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2305133002/#ps120001
(title: "none")

[commit-bot: I haz the power, 2016-09-06 22:28:56]

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-07 00:25:40]

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

[commit-bot: I haz the power, 2016-09-07 00:25:43]

Exceeded global retry quota

[chrishtr, 2016-09-07 14:51:18]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-09-07 14:51:18]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2305133002/#ps140001
(title: "none")

[commit-bot: I haz the power, 2016-09-07 14:51:35]

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-07 16:24:07]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-09-07 16:26:11]

Description was changed from

==========
Mac: adjust sizes of progress bars to be equal to painted size.

Moves code to compute the size into the style adjustment phase, and simplifies
it according to the actual margins specified in the style configs.

BUG=632319
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Mac: adjust sizes of progress bars to be equal to painted size.

Moves code to compute the size into the style adjustment phase, and simplifies
it according to the actual margins specified in the style configs.

BUG=632319
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[commit-bot: I haz the power, 2016-09-07 16:26:12]

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