Adjust overlay scrollbar appearance.

Adjust overlay scrollbar appearance.

- share more constants between Blink and native (Views)
- make Views paint a 1px stroke instead of a 1dip stroke, and update
  constants as per latest spec.
- fix a bug with Blink overlay scrollbars where stroke was painted on
  all sides instead of just three sides.

BUG=710550
Review-Url: https://codereview.chromium.org/2818033002
Cr-Commit-Position: refs/heads/master@{#465319}
Committed: https://chromium.googlesource.com/chromium/src/+/41aea3751213e055f31f13d449bf21518432c6cf

[Evan Stade, 2017-04-13 23:25:47]

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

[commit-bot: I haz the power, 2017-04-13 23:26:30]

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-04-14 00:34:43]

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

[commit-bot: I haz the power, 2017-04-14 00:34:43]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2017-04-14 16:48:42]

estade@chromium.org changed reviewers:
+ bokan@chromium.org, sadrul@chromium.org

[Evan Stade, 2017-04-14 16:48:43]

+bokan ptal at NativeThemeAura changes
+sadrul please review

[Evan Stade, 2017-04-17 19:48:51]

On 2017/04/14 16:48:43, Evan Stade wrote:
> +bokan ptal at NativeThemeAura changes
> +sadrul please review

ping

[bokan, 2017-04-18 13:17:56]

Sorry for the delay, Canada's been off for Easter a few days.

https://codereview.chromium.org/2818033002/diff/1/ui/native_theme/native_them...
File ui/native_theme/native_theme_aura.cc (right):

https://codereview.chromium.org/2818033002/diff/1/ui/native_theme/native_them...
ui/native_theme/native_theme_aura.cc:248: stroke_rect.Inset(stroke_insets +
edge_adjust_insets);
My understanding of Skia isn't so good, why didn't the old code work? (I can see
it didn't, but I don't understand why)

Does this new code mean that we're drawing the stroke for the omitted edge
outside the thumb rect, since the stroke_rect edge runs through the center of
the pixels just outside the rect? Do we have to do that? Shouldn't the fill
below paint over the right-most column (in the case of a vertical scrollbar) of
pixels in the canvas? Or is there some kind of blending going on?

[Evan Stade, 2017-04-18 14:13:31]

https://codereview.chromium.org/2818033002/diff/1/ui/native_theme/native_them...
File ui/native_theme/native_theme_aura.cc (right):

https://codereview.chromium.org/2818033002/diff/1/ui/native_theme/native_them...
ui/native_theme/native_theme_aura.cc:248: stroke_rect.Inset(stroke_insets +
edge_adjust_insets);
On 2017/04/18 13:17:56, bokan wrote:
> My understanding of Skia isn't so good, why didn't the old code work? (I can
see
> it didn't, but I don't understand why)

It worked fine for the fill portion, this is just applying the same for the
stroke.

> 
> Does this new code mean that we're drawing the stroke for the omitted edge
> outside the thumb rect, since the stroke_rect edge runs through the center of
> the pixels just outside the rect?

Yes

> Do we have to do that? Shouldn't the fill
> below paint over the right-most column (in the case of a vertical scrollbar)
of
> pixels in the canvas? Or is there some kind of blending going on?

Correct, the fill is not fully opaque.

[bokan, 2017-04-18 14:28:40]

On 2017/04/18 14:13:31, Evan Stade wrote:
>
https://codereview.chromium.org/2818033002/diff/1/ui/native_theme/native_them...
> File ui/native_theme/native_theme_aura.cc (right):
> 
>
https://codereview.chromium.org/2818033002/diff/1/ui/native_theme/native_them...
> ui/native_theme/native_theme_aura.cc:248: stroke_rect.Inset(stroke_insets +
> edge_adjust_insets);
> On 2017/04/18 13:17:56, bokan wrote:
> > My understanding of Skia isn't so good, why didn't the old code work? (I can
> see
> > it didn't, but I don't understand why)
> 
> It worked fine for the fill portion, this is just applying the same for the
> stroke.
> 
> > 
> > Does this new code mean that we're drawing the stroke for the omitted edge
> > outside the thumb rect, since the stroke_rect edge runs through the center
of
> > the pixels just outside the rect?
> 
> Yes
> 
> > Do we have to do that? Shouldn't the fill
> > below paint over the right-most column (in the case of a vertical scrollbar)
> of
> > pixels in the canvas? Or is there some kind of blending going on?
> 
> Correct, the fill is not fully opaque.

Bah, of course - our paint has alpha. Ok thanks for the explanations.
native_theme bits lgtm

[sadrul, 2017-04-18 17:32:31]

lgtm

[Evan Stade, 2017-04-18 18:21:04]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-04-18 18:21: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-04-18 19:17:13]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1492539664972540, "parent_rev":
"4f6af158631b459dee6bba39d3802aae8205f9d0", "commit_rev":
"41aea3751213e055f31f13d449bf21518432c6cf"}

[commit-bot: I haz the power, 2017-04-18 19:17:23]

Description was changed from

==========
Adjust overlay scrollbar appearance.

- share more constants between Blink and native (Views)
- make Views paint a 1px stroke instead of a 1dip stroke, and update
  constants as per latest spec.
- fix a bug with Blink overlay scrollbars where stroke was painted on
  all sides instead of just three sides.

BUG=710550
==========

to

==========
Adjust overlay scrollbar appearance.

- share more constants between Blink and native (Views)
- make Views paint a 1px stroke instead of a 1dip stroke, and update
  constants as per latest spec.
- fix a bug with Blink overlay scrollbars where stroke was painted on
  all sides instead of just three sides.

BUG=710550

Review-Url: https://codereview.chromium.org/2818033002
Cr-Commit-Position: refs/heads/master@{#465319}
Committed:
https://chromium.googlesource.com/chromium/src/+/41aea3751213e055f31f13d449bf...
==========

[commit-bot: I haz the power, 2017-04-18 19:17:24]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/41aea3751213e055f31f13d449bf...