Resizes corner rect to match device scale factor

Resizes corner rect to match device scale factor

Corner rect resizer for scrollable area now considers device scale
factor when computing its bounds.

BUG=692238
COMPONENT=ScrollableAreaPainter, Corner Resizer
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2698543002
Cr-Commit-Position: refs/heads/master@{#451844}
Committed: https://chromium.googlesource.com/chromium/src/+/c7657d65b7ca9a5623883676a8b641cf509847d9

[malaykeshav, 2017-02-14 22:10:36]

Description was changed from

==========
Resizes corner rect to match device scale factor

Crner rect resizer for scrollable area now considers device scale
factor when computing its bounds.

BUG=692238
COMPONENT=ScrollableAreaPainter, Corner Resizer
==========

to

==========
Resizes corner rect to match device scale factor

Crner rect resizer for scrollable area now considers device scale
factor when computing its bounds.

BUG=692238
COMPONENT=ScrollableAreaPainter, Corner Resizer
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[malaykeshav, 2017-02-14 22:12:26]

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

[commit-bot: I haz the power, 2017-02-14 22:13:17]

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

[malaykeshav, 2017-02-14 23:43:03]

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

[malaykeshav, 2017-02-14 23:43:04]

PTAL

[malaykeshav, 2017-02-14 23:43:21]

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

[commit-bot: I haz the power, 2017-02-14 23:43:59]

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

[malaykeshav, 2017-02-15 00:12:29]

Patchset #2 (id:20001) has been deleted

[malaykeshav, 2017-02-15 00:12:35]

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

[commit-bot: I haz the power, 2017-02-15 00:13:21]

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

[malaykeshav, 2017-02-15 00:19:45]

Patchset #2 (id:40001) has been deleted

[malaykeshav, 2017-02-15 00:19:49]

Patchset #2 (id:60001) has been deleted

[malaykeshav, 2017-02-15 00:19:51]

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

[commit-bot: I haz the power, 2017-02-15 00:20:59]

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-02-15 00:47:37]

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

[commit-bot: I haz the power, 2017-02-15 00:47:38]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[malaykeshav, 2017-02-15 00:56:11]

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

[commit-bot: I haz the power, 2017-02-15 00:57:53]

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

[malaykeshav, 2017-02-15 02:09:32]

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

[commit-bot: I haz the power, 2017-02-15 02:10:59]

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-02-15 05:15:58]

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

[commit-bot: I haz the power, 2017-02-15 05:15:59]

Dry run: This issue passed the CQ dry run.

[malaykeshav, 2017-02-15 09:05:58]

All test cases pass.
PTAL

[oshima, 2017-02-15 22:22:16]

https://codereview.chromium.org/2698543002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/ScrollableAreaPainter.cpp (right):

https://codereview.chromium.org/2698543002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/ScrollableAreaPainter.cpp:88: 1.f /
blink::deviceScaleFactor(getScrollableArea().box().frame()));
blink::deviceScaleFactor may return 2> on Android. I'm not so sure if this is
used on Android, but you probably want to do the same as before (just check if
it's 2> then scale 0.5f.

[malaykeshav, 2017-02-15 22:25:46]

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

[commit-bot: I haz the power, 2017-02-15 22:26:34]

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-02-16 00:30:50]

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

[commit-bot: I haz the power, 2017-02-16 00:30:52]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[malaykeshav, 2017-02-16 22:09:13]

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

[commit-bot: I haz the power, 2017-02-16 22:10:58]

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

[malaykeshav, 2017-02-17 00:35:10]

PTAL

[malaykeshav, 2017-02-17 00:35:17]

On 2017/02/15 at 22:22:16, oshima wrote:
>
https://codereview.chromium.org/2698543002/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/paint/ScrollableAreaPainter.cpp (right):
> 
>
https://codereview.chromium.org/2698543002/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/paint/ScrollableAreaPainter.cpp:88: 1.f /
blink::deviceScaleFactor(getScrollableArea().box().frame()));
> blink::deviceScaleFactor may return 2> on Android. I'm not so sure if this is
used on Android, but you probably want to do the same as before (just check if
it's 2> then scale 0.5f.

Done

[commit-bot: I haz the power, 2017-02-17 03:40:04]

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

[commit-bot: I haz the power, 2017-02-17 03:40:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[oshima, 2017-02-17 16:59:59]

lgtm with a nit

https://codereview.chromium.org/2698543002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/ScrollableAreaPainter.cpp (right):

https://codereview.chromium.org/2698543002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/ScrollableAreaPainter.cpp:87: if
(blink::deviceScaleFactor(getScrollableArea().box().frame()) >= 2)
nit: cache this value first, like

float oldBlinkDeviceScaleFactor = .

then use it both above and here.

[malaykeshav, 2017-02-17 18:26:05]

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

[commit-bot: I haz the power, 2017-02-17 18:26:37]

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

[malaykeshav, 2017-02-17 18:39:09]

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

[malaykeshav, 2017-02-17 18:39:10]

+wangxianzhu@
PTAL

[Xianzhu, 2017-02-17 18:42:17]

On 2017/02/17 18:39:10, malaykeshav wrote:
> +wangxianzhu@
> PTAL

rs lgtm.

[Xianzhu, 2017-02-17 18:42:33]

Description was changed from

==========
Resizes corner rect to match device scale factor

Crner rect resizer for scrollable area now considers device scale
factor when computing its bounds.

BUG=692238
COMPONENT=ScrollableAreaPainter, Corner Resizer
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Resizes corner rect to match device scale factor

Corner rect resizer for scrollable area now considers device scale
factor when computing its bounds.

BUG=692238
COMPONENT=ScrollableAreaPainter, Corner Resizer
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-02-17 18:45:22]

I think I need to learn basics about device scale factor handling in blink. Can
you give me some links?

(Based on my old experience on Android, device scale factors were all handled
outside of blink and it worked well.)

[malaykeshav, 2017-02-17 19:11:09]

On 2017/02/17 at 18:45:22, wangxianzhu wrote:
> I think I need to learn basics about device scale factor handling in blink.
Can you give me some links?
> 
> (Based on my old experience on Android, device scale factors were all handled
outside of blink and it worked well.)

Yes, but I think it changed when we started using zoom for DSF. (Still not
enabled in Android as of now)

This document has a basic overview:
https://docs.google.com/document/d/1CZSCPzOYujdUMyChocwzOBPKxYAoTsEoezMye30Hdcs

[commit-bot: I haz the power, 2017-02-17 20:17:41]

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

[commit-bot: I haz the power, 2017-02-17 20:17:42]

Dry run: This issue passed the CQ dry run.

[malaykeshav, 2017-02-17 20:49:07]

The CQ bit was checked by malaykeshav@chromium.org

[malaykeshav, 2017-02-17 20:49:08]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2698543002/#ps180001
(title: "Resizes corner rect to match device scale factor")

[commit-bot: I haz the power, 2017-02-17 20:49: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, 2017-02-17 22:52:07]

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

[commit-bot: I haz the power, 2017-02-17 22:52:09]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[Xianzhu, 2017-02-21 06:54:36]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2017-02-21 06:54:55]

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-02-21 08:29:27]

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

[commit-bot: I haz the power, 2017-02-21 08:29:28]

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, 2017-02-21 19:47:16]

The CQ bit was checked by malaykeshav@chromium.org

[commit-bot: I haz the power, 2017-02-21 19:48:30]

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-02-21 21:53:06]

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

[commit-bot: I haz the power, 2017-02-21 21:53:07]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[malaykeshav, 2017-02-21 21:55:25]

The CQ bit was checked by malaykeshav@chromium.org

[commit-bot: I haz the power, 2017-02-21 21:56:49]

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-02-21 22:06:12]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1487714125446350,
"parent_rev": "e2221203908694ce026c037e9d084485a1f92c81", "commit_rev":
"c7657d65b7ca9a5623883676a8b641cf509847d9"}

[commit-bot: I haz the power, 2017-02-21 22:06:50]

Description was changed from

==========
Resizes corner rect to match device scale factor

Corner rect resizer for scrollable area now considers device scale
factor when computing its bounds.

BUG=692238
COMPONENT=ScrollableAreaPainter, Corner Resizer
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Resizes corner rect to match device scale factor

Corner rect resizer for scrollable area now considers device scale
factor when computing its bounds.

BUG=692238
COMPONENT=ScrollableAreaPainter, Corner Resizer
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2698543002
Cr-Commit-Position: refs/heads/master@{#451844}
Committed:
https://chromium.googlesource.com/chromium/src/+/c7657d65b7ca9a5623883676a8b6...
==========

[commit-bot: I haz the power, 2017-02-21 22:06:51]

Committed patchset #7 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/c7657d65b7ca9a5623883676a8b6...