Scale focus outline thickness with zoom level.

Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from vanishing when zoomed out, I floored its thickness at 1px.

Focus rings draw the same at 1x device scale. A subtle effect of this
patch is that elements will have their focus rings scaled by the css
zoom property.

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

Committed: https://crrev.com/87ad798087aebf914351caab62a8cf072fe9d388
Cr-Commit-Position: refs/heads/master@{#424596}

[Bret, 2016-10-01 23:22:34]

Description was changed from

==========
Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from being teeny when zooming out, I floored its thickness at 1px.

BUG=651510
==========

to

==========
Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from being teeny when zooming out, I floored its thickness at 1px.

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

[Bret, 2016-10-01 23:25:16]

Description was changed from

==========
Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from being teeny when zooming out, I floored its thickness at 1px.

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

to

==========
Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from vanishing when zoomed out, I floored its thickness at 1px.

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

[Bret, 2016-10-01 23:25:16]

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

[Bret, 2016-10-01 23:26:13]

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

[Bret, 2016-10-01 23:30:46]

bsep@chromium.org changed reviewers:
+ chrishtr@chromium.org
- junov@chromium.org, wangxianzhu@chromium.org

[Bret, 2016-10-02 01:30:15]

Seems like this breaks something about focusing on imagemaps. Hold off on
reviewing this for now while I investigate.

[Bret, 2016-10-05 20:12:02]

Description was changed from

==========
Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from vanishing when zoomed out, I floored its thickness at 1px.

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

to

==========
Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from vanishing when zoomed out, I floored its thickness at 1px.

Focus rings draw the same at 1x device scale. A subtle effect of this
patch is that elements will have their focus rings scaled by the css
zoom property.

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

[Bret, 2016-10-05 23:00:49]

On 2016/10/02 01:30:15, Bret Sepulveda wrote:
> Seems like this breaks something about focusing on imagemaps. Hold off on
> reviewing this for now while I investigate.

This is ready for review. I couldn't find any information on how focus rings are
"supposed" to behave, but this seems like reasonable behavior to me. If we want
to strictly restore the old functionality instead it's not hard to change this
to scale only via device scale (instead of total zoom level).

[chrishtr, 2016-10-07 21:14:21]

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/ObjectPainter.cpp (right):

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/ObjectPainter.cpp:222: return
style.outlineWidth();
Why this special case?

Also, it apears there is the opposite heuristic in GraphicsContext.h?

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graph...

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/GraphicsContext.h (right):

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/GraphicsContext.h:361: return
focusRingOffset(offset) + (focusRingWidth(width) + 1) / 2;
Change this call site to focusRingWidth also?

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/GraphicsContext.h:365: static int
focusRingWidth(int width) { return width; }
change to float?

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/GraphicsContext.h:367: static int
focusRingWidth(int width) { return 1; }
Doesn't this defeat the purpose of this feature?

[Bret, 2016-10-08 01:32:41]

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/ObjectPainter.cpp (right):

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/ObjectPainter.cpp:222: return
style.outlineWidth();
On 2016/10/07 21:14:21, chrishtr wrote:
> Why this special case?
> 
> Also, it apears there is the opposite heuristic in GraphicsContext.h?
> 
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graph...

This is the same case, just moved around. See my other comment.

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/GraphicsContext.h (right):

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/GraphicsContext.h:361: return
focusRingOffset(offset) + (focusRingWidth(width) + 1) / 2;
On 2016/10/07 21:14:21, chrishtr wrote:
> Change this call site to focusRingWidth also?

Done. Not sure what this callsite is for, something about layout, but now it
follows the same logic as the rest of the change.

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/GraphicsContext.h:365: static int
focusRingWidth(int width) { return width; }
On 2016/10/07 21:14:21, chrishtr wrote:
> change to float?

This is deleted.

https://codereview.chromium.org/2386033002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/GraphicsContext.h:367: static int
focusRingWidth(int width) { return 1; }
On 2016/10/07 21:14:21, chrishtr wrote:
> Doesn't this defeat the purpose of this feature?

I just forgot to delete this block. I moved this into the
ObjectPainter/ComputedStyle layer and GraphicsContext just uses the width it's
given. The "return 1" is now "return effectiveZoom()".

[chrishtr, 2016-10-10 21:16:13]

lgtm

[Bret, 2016-10-11 19:25:14]

The CQ bit was checked by bsep@chromium.org

[commit-bot: I haz the power, 2016-10-11 19:25:45]

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-10-11 19:28:04]

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

[commit-bot: I haz the power, 2016-10-11 19:28:05]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Bret, 2016-10-11 21:58:15]

The CQ bit was checked by bsep@chromium.org

[Bret, 2016-10-11 21:58:16]

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

[commit-bot: I haz the power, 2016-10-11 21:58:37]

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-10-11 23:36:13]

Description was changed from

==========
Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from vanishing when zoomed out, I floored its thickness at 1px.

Focus rings draw the same at 1x device scale. A subtle effect of this
patch is that elements will have their focus rings scaled by the css
zoom property.

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

to

==========
Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from vanishing when zoomed out, I floored its thickness at 1px.

Focus rings draw the same at 1x device scale. A subtle effect of this
patch is that elements will have their focus rings scaled by the css
zoom property.

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

[commit-bot: I haz the power, 2016-10-11 23:36:14]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-11 23:42:44]

Description was changed from

==========
Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from vanishing when zoomed out, I floored its thickness at 1px.

Focus rings draw the same at 1x device scale. A subtle effect of this
patch is that elements will have their focus rings scaled by the css
zoom property.

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

to

==========
Scale focus outline thickness with zoom level.

Focus rings were only being scaled by the dsf, which after the launch of
use-zoom-for-dsf meant that they weren't being scaled at all. It seems
like we should keep the ring in proportion with the rest of the page, so
I changed it to scale with the total zoom level. In order to keep the
ring from vanishing when zoomed out, I floored its thickness at 1px.

Focus rings draw the same at 1x device scale. A subtle effect of this
patch is that elements will have their focus rings scaled by the css
zoom property.

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

Committed: https://crrev.com/87ad798087aebf914351caab62a8cf072fe9d388
Cr-Commit-Position: refs/heads/master@{#424596}
==========

[commit-bot: I haz the power, 2016-10-11 23:42:45]

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

[loyso (OOO), 2016-10-12 05:46:00]

It has produced bad tests expectations here:
https://codereview.chromium.org/2405343003
for color-suggestion-picker-appearance-zoom200 test :(
I'll disable the test.

[loyso (OOO), 2016-10-12 06:02:12]

https://bugs.chromium.org/p/chromium/issues/detail?id=654999