Applies zoom factor to the spinner size

Applies zoom factor to the spinner size

The spinner element was not being scaled on browser zoom before this
modification. This also led to the same spinner element being laid out
differently when displayed on HiDPI displays with zoom-for-dsf enabled.

BUG=640256
COMPONENT=Layout Theme, Zoom DSF

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

[malaykeshav, 2016-09-20 21:57:59]

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

[commit-bot: I haz the power, 2016-09-20 21:58:37]

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

[malaykeshav, 2016-09-20 22:05:01]

malaykeshav@chromium.org changed reviewers:
+ oshima@chromium.org

[commit-bot: I haz the power, 2016-09-21 01:39:53]

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

[commit-bot: I haz the power, 2016-09-21 01:39:54]

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

[malaykeshav, 2016-09-21 16:53:57]

Description was changed from

==========
Applies zoom factor to the spinner size

The spinner element was not being scaled on browser zoom before this
modification.

BUG=640256
COMPONENT=Layout Theme, Zoom DSF
==========

to

==========
Applies zoom factor to the spinner size

The spinner element was not being scaled on browser zoom before this
modification. This also led to the same spinner element being laid out 
differently when displayed on HiDPI displays with zoom-for-dsf enabled.

BUG=640256
COMPONENT=Layout Theme, Zoom DSF
==========

[malaykeshav, 2016-09-21 16:54:04]

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

[commit-bot: I haz the power, 2016-09-21 16:54:33]

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-21 18:49:23]

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

[commit-bot: I haz the power, 2016-09-21 18:49:24]

Dry run: This issue passed the CQ dry run.

[malaykeshav, 2016-09-21 18:51:24]

malaykeshav@chromium.org changed reviewers:
+ wangxianzhu@chromium.org

[malaykeshav, 2016-09-21 18:51:24]

PTAL.

[Xianzhu, 2016-09-21 19:04:23]

Is this the standard way to apply zoom factor? How do we apply zoom for a width
like '1.5em'?

[malaykeshav, 2016-09-21 19:49:03]

On 2016/09/21 at 19:04:23, wangxianzhu wrote:
> Is this the standard way to apply zoom factor? How do we apply zoom for a
width like '1.5em'?

Effectively, when we view an element on a 2x (or 1.5x or 1.25x) display, the
element should cover as many logical pixels on the screen as the element would
cover on a 1x display. A browser zoom should not change the relative positioning
or layout of the elements when used as a mode of scaling for different DPI
screens. In the current bug, all the elements of the date picker were being
scaled except for the spinner element which led to it being displayed
differently based on the level of browser zoom. Multiplying its dimensions by
effective zoom, scales it to match the rest of its already scaled siblings and
parent.
I am not sure how the use of 1.5em as size would have an effect on this. Waiting
for @oshima to pitch in.

[malaykeshav, 2016-09-21 19:51:49]

malaykeshav@chromium.org changed reviewers:
+ bsep@chromium.org

[malaykeshav, 2016-09-21 19:51:49]

@oshima is OOO this week. Adding @bsep

[Xianzhu, 2016-09-21 19:57:34]

I think we should not apply zoom in this way. We should let the theme engine
return sizes in zoom-neutral units instead of the fixed unit.

[Bret, 2016-09-21 20:05:35]

On 2016/09/21 19:57:34, Xianzhu wrote:
> I think we should not apply zoom in this way. We should let the theme engine
> return sizes in zoom-neutral units instead of the fixed unit.

lgtm

I'm not very familiar with this code, but the zoom level is used the same way
elsewhere in this file. This is reasonable to me as a targeted fix.

[Xianzhu, 2016-09-21 22:34:50]

OK. lgtm.

However, I feel we should optimize how we apply zoom.

[malaykeshav, 2016-09-21 22:35:04]

On 2016/09/21 at 19:57:34, wangxianzhu wrote:
> I think we should not apply zoom in this way. We should let the theme engine
return sizes in zoom-neutral units instead of the fixed unit.

The theme engine returns the sizes in CSS pixels to which the browser zoom is
applied and used for layout.

[malaykeshav, 2016-09-21 22:35:45]

The CQ bit was checked by malaykeshav@chromium.org

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

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-21 22:42:19]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-21 22:44:57]

Description was changed from

==========
Applies zoom factor to the spinner size

The spinner element was not being scaled on browser zoom before this
modification. This also led to the same spinner element being laid out 
differently when displayed on HiDPI displays with zoom-for-dsf enabled.

BUG=640256
COMPONENT=Layout Theme, Zoom DSF
==========

to

==========
Applies zoom factor to the spinner size

The spinner element was not being scaled on browser zoom before this
modification. This also led to the same spinner element being laid out
differently when displayed on HiDPI displays with zoom-for-dsf enabled.

BUG=640256
COMPONENT=Layout Theme, Zoom DSF

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

[commit-bot: I haz the power, 2016-09-21 22:44:59]

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

[Xianzhu, 2016-09-21 23:26:23]

Sorry I'm not familiar with the area. A dumb question: for a css 'width: 15px',
and zoom==2, does the zoom double the size of CSS pixels (so computed width is
still 15px) or double the computed width to 30px?

[malaykeshav, 2016-09-22 01:25:30]

On 2016/09/21 at 23:26:23, wangxianzhu wrote:
> Sorry I'm not familiar with the area. A dumb question: for a css 'width:
15px', and zoom==2, does the zoom double the size of CSS pixels (so computed
width is still 15px) or double the computed width to 30px?

Logical width would still be 15px. But if it too x physical pixels to draw the
line of width 15px on 100% browser zoom, then it would take 2 times the number
of physical pixels (2 * x pixels) to draw the same line on 200% browser zoom.

[malaykeshav, 2016-09-22 01:25:41]

On 2016/09/22 at 01:25:30, malaykeshav wrote:
> On 2016/09/21 at 23:26:23, wangxianzhu wrote:
> > Sorry I'm not familiar with the area. A dumb question: for a css 'width:
15px', and zoom==2, does the zoom double the size of CSS pixels (so computed
width is still 15px) or double the computed width to 30px?
> 
> Logical width would still be 15px. But if it too x physical pixels to draw the
line of width 15px on 100% browser zoom, then it would take 2 times the number
of physical pixels (2 * x pixels) to draw the same line on 200% browser zoom.

took*