Fix overflow:overlay scrollbar width for paint.

Fix overflow:overlay scrollbar width for paint.

This CL went too far:

https://codereview.chromium.org/2725763002

Some callers to verticalScrollbarWidth -- in particular, the paint
code -- need the old behavior, where overflow:overlay scrollbars
are subtracted from the overflow clip rect.

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

Review-Url: https://codereview.chromium.org/2730343003
Cr-Commit-Position: refs/heads/master@{#457251}
Committed: https://chromium.googlesource.com/chromium/src/+/7ab46eae8507137607e258543118ef8fb04984de

[szager1, 2017-03-07 03:10:13]

Description was changed from

==========
Fix overflow:overlay scrollbar width for paint.

This CL went too far:

https://codereview.chromium.org/2725763002

Some callers to verticalScrollbarWidth -- in particular, the paint
code -- need the old behavior, where overflow:overlay scrollbars
are subtracted from the overflow clip rect.

BUG=697751
==========

to

==========
Fix overflow:overlay scrollbar width for paint.

This CL went too far:

https://codereview.chromium.org/2725763002

Some callers to verticalScrollbarWidth -- in particular, the paint
code -- need the old behavior, where overflow:overlay scrollbars
are subtracted from the overflow clip rect.

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

[szager1, 2017-03-07 03:10:38]

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

[commit-bot: I haz the power, 2017-03-07 03:11:12]

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-03-07 05:08:13]

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

[commit-bot: I haz the power, 2017-03-07 05:08:15]

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

[szager1, 2017-03-07 16:36:52]

Description was changed from

==========
Fix overflow:overlay scrollbar width for paint.

This CL went too far:

https://codereview.chromium.org/2725763002

Some callers to verticalScrollbarWidth -- in particular, the paint
code -- need the old behavior, where overflow:overlay scrollbars
are subtracted from the overflow clip rect.

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

to

==========
Fix overflow:overlay scrollbar width for paint.

This CL went too far:

https://codereview.chromium.org/2725763002

Some callers to verticalScrollbarWidth -- in particular, the paint
code -- need the old behavior, where overflow:overlay scrollbars
are subtracted from the overflow clip rect.

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

[szager1, 2017-03-07 16:36:52]

szager@chromium.org changed reviewers:
+ skobes@chromium.org

[skobes, 2017-03-07 16:42:47]

https://codereview.chromium.org/2730343003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/2730343003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:507:
pixelSnappedIntRect(box().overflowClipRect(box().location())).size();
Could we add a value to the OverlayScrollbarClipBehavior enum indicating that
both regular-overlay and overflow:overlay scrollbars should be ignored, and pass
it here?

[szager1, 2017-03-07 19:01:31]

https://codereview.chromium.org/2730343003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/2730343003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:507:
pixelSnappedIntRect(box().overflowClipRect(box().location())).size();
On 2017/03/07 16:42:47, skobes wrote:
> Could we add a value to the OverlayScrollbarClipBehavior enum indicating that
> both regular-overlay and overflow:overlay scrollbars should be ignored, and
pass
> it here?

Yeah, that was my first attempt.  It works, but it's a more invasive change.  If
you prefer that, I'll upload that version.

[skobes, 2017-03-07 19:09:12]

https://codereview.chromium.org/2730343003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/2730343003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:507:
pixelSnappedIntRect(box().overflowClipRect(box().location())).size();
On 2017/03/07 19:01:31, szager1 wrote:
> Yeah, that was my first attempt.  It works, but it's a more invasive change. 
If
> you prefer that, I'll upload that version.

I probably do, though I'll have to see the patch. :)

If we have existing plumbing to conditionally subtract scrollbar space, it's
cleaner to refine the conditions than to "undo" the subtraction after the fact.

[szager1, 2017-03-07 19:20:08]

On 2017/03/07 19:09:12, skobes wrote:
>
https://codereview.chromium.org/2730343003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
(right):
> 
>
https://codereview.chromium.org/2730343003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:507:
> pixelSnappedIntRect(box().overflowClipRect(box().location())).size();
> On 2017/03/07 19:01:31, szager1 wrote:
> > Yeah, that was my first attempt.  It works, but it's a more invasive change.

> If
> > you prefer that, I'll upload that version.
> 
> I probably do, though I'll have to see the patch. :)
> 
> If we have existing plumbing to conditionally subtract scrollbar space, it's
> cleaner to refine the conditions than to "undo" the subtraction after the
fact.

I uploaded the other version, PTAL.

[szager1, 2017-03-07 19:20:13]

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

[commit-bot: I haz the power, 2017-03-07 19:21:42]

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

[skobes, 2017-03-07 19:28:07]

https://codereview.chromium.org/2730343003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/2730343003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:1308:
(overlayScrollbarClipBehavior == IgnorePlatformOverlayScrollbarSize ||
Don't you need to check for IgnorePlatformAndCSSOverlayScrollbarSize in this
branch too?

[szager1, 2017-03-07 19:44:38]

https://codereview.chromium.org/2730343003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/2730343003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:1308:
(overlayScrollbarClipBehavior == IgnorePlatformOverlayScrollbarSize ||
On 2017/03/07 19:28:07, skobes wrote:
> Don't you need to check for IgnorePlatformAndCSSOverlayScrollbarSize in this
> branch too?

Whoops!  Fixed.

[skobes, 2017-03-07 19:47:18]

https://codereview.chromium.org/2730343003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/2730343003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:1327:
(overlayScrollbarClipBehavior == IgnorePlatformOverlayScrollbarSize ||
same fix here (|| IgnorePlatformAndCSSOverlayScrollbarSize)

[szager1, 2017-03-07 20:07:50]

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

[szager1, 2017-03-07 20:07:52]

https://codereview.chromium.org/2730343003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/2730343003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:1327:
(overlayScrollbarClipBehavior == IgnorePlatformOverlayScrollbarSize ||
On 2017/03/07 19:47:18, skobes wrote:
> same fix here (|| IgnorePlatformAndCSSOverlayScrollbarSize)

Jeez, picky picky!

Fixed.

[commit-bot: I haz the power, 2017-03-07 20:08:56]

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

[skobes, 2017-03-07 20:13:24]

lgtm :)

[commit-bot: I haz the power, 2017-03-07 22:52:47]

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

[commit-bot: I haz the power, 2017-03-07 22:52:48]

Dry run: This issue passed the CQ dry run.

[szager1, 2017-03-08 01:18:51]

The CQ bit was checked by szager@chromium.org

[commit-bot: I haz the power, 2017-03-08 01:20:02]

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-03-08 01:32:02]

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

[commit-bot: I haz the power, 2017-03-08 01:32:03]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[szager1, 2017-03-08 20:18:51]

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

[szager1, 2017-03-08 20:19:03]

+pdr for platform/OWNERS

[pdr., 2017-03-08 20:33:20]

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

https://codereview.chromium.org/2730343003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:509:
IgnorePlatformAndCSSOverlayScrollbarSize))
Is this the only callsite using IgnorePlatformAndCSSOverlayScrollbarSize? The
difference between IgnorePlatformAndCSSOverlayScrollbarSize and
IgnorePlatformOverlayScrollbarSize is subtle and it's not obvious which should
be used in which callsites. Is it possible to move the CSSOverlay code needed
for paint into a paint-specific function?

[szager1, 2017-03-08 20:43:09]

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

https://codereview.chromium.org/2730343003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:509:
IgnorePlatformAndCSSOverlayScrollbarSize))
On 2017/03/08 20:33:20, pdr. wrote:
> Is this the only callsite using IgnorePlatformAndCSSOverlayScrollbarSize? The
> difference between IgnorePlatformAndCSSOverlayScrollbarSize and
> IgnorePlatformOverlayScrollbarSize is subtle and it's not obvious which should
> be used in which callsites. Is it possible to move the CSSOverlay code needed
> for paint into a paint-specific function?


My initial patch did something like that, but skobes@ expressed a preference for
a new enum value.  So I will let the reviewers duke it out.

[szager1, 2017-03-08 20:44:50]

On 2017/03/08 20:43:09, szager1 wrote:
>
https://codereview.chromium.org/2730343003/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
(right):
> 
>
https://codereview.chromium.org/2730343003/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:509:
> IgnorePlatformAndCSSOverlayScrollbarSize))
> On 2017/03/08 20:33:20, pdr. wrote:
> > Is this the only callsite using IgnorePlatformAndCSSOverlayScrollbarSize?
The
> > difference between IgnorePlatformAndCSSOverlayScrollbarSize and
> > IgnorePlatformOverlayScrollbarSize is subtle and it's not obvious which
should
> > be used in which callsites. Is it possible to move the CSSOverlay code
needed
> > for paint into a paint-specific function?
> 
> 
> My initial patch did something like that, but skobes@ expressed a preference
for
> a new enum value.  So I will let the reviewers duke it out.

I also don't think the difference between platform and css overlay scrollbars is
particularly subtle, but maybe I'm to steeped in scrolling to judge.  I could
add a comment, if you like.

[skobes, 2017-03-08 21:24:11]

On 2017/03/08 20:33:20, pdr. wrote:
> Is this the only callsite using IgnorePlatformAndCSSOverlayScrollbarSize? The
> difference between IgnorePlatformAndCSSOverlayScrollbarSize and
> IgnorePlatformOverlayScrollbarSize is subtle and it's not obvious which should
> be used in which callsites. Is it possible to move the CSSOverlay code needed
> for paint into a paint-specific function?

I objected to a version of the patch in which PLSA called overflowClipRect in a
mode that subtracted scrollbars, and then reversed the subtraction for
overflow:overlay.

It is probably good to force callers to think about the difference and decide
which one they really want.  I'm not convinced that PLSA is the only place that
got this wrong.  Some comments on the enum would help.

Another option would be for overflowClipRect to take a "bool excludeScrollbars",
and provide a helper like SA::hasCSSOverlayScrollbars in addition to
SA::hasOverlayScrollbars.  Maybe that's easier to reason about.

[szager1, 2017-03-08 21:53:21]

On 2017/03/08 21:24:11, skobes wrote:

> Another option would be for overflowClipRect to take a "bool
excludeScrollbars",
> and provide a helper like SA::hasCSSOverlayScrollbars in addition to
> SA::hasOverlayScrollbars.  Maybe that's easier to reason about.

I don't quite follow this suggestion.  How would a bool argument to
overflowClipRect satisfy all callers.

Also, we can't add hasCSSOverlayScrollbars to ScrollableArea, because platform
code can't access style code.  I guess we could make it virtual, but to me that
seems worse than the current patch.

[skobes, 2017-03-08 22:06:39]

On 2017/03/08 21:53:21, szager1 wrote:
> On 2017/03/08 21:24:11, skobes wrote:
> 
> > Another option would be for overflowClipRect to take a "bool
> excludeScrollbars",
> > and provide a helper like SA::hasCSSOverlayScrollbars in addition to
> > SA::hasOverlayScrollbars.  Maybe that's easier to reason about.
> 
> I don't quite follow this suggestion.  How would a bool argument to
> overflowClipRect satisfy all callers.

The caller would own the logic for whether to exclude.

  overflowClipRect(..., hasOverlayScrollbars() || hasCSSOverlayScrollbars())

> Also, we can't add hasCSSOverlayScrollbars to ScrollableArea, because platform
> code can't access style code.  I guess we could make it virtual, but to me
that
> seems worse than the current patch.

Yeah PLSA would have to override.  I think I prefer the current patch as well,
just throwing the idea out there.

[pdr., 2017-03-08 22:08:56]

On 2017/03/08 at 21:53:21, szager wrote:
> On 2017/03/08 21:24:11, skobes wrote:
> 
> > Another option would be for overflowClipRect to take a "bool
excludeScrollbars",
> > and provide a helper like SA::hasCSSOverlayScrollbars in addition to
> > SA::hasOverlayScrollbars.  Maybe that's easier to reason about.
> 
> I don't quite follow this suggestion.  How would a bool argument to
overflowClipRect satisfy all callers.
> 
> Also, we can't add hasCSSOverlayScrollbars to ScrollableArea, because platform
code can't access style code.  I guess we could make it virtual, but to me that
seems worse than the current patch.

Can you help me understand why we don't treat platform overlay and css overlay
scrollbars the same everywhere? This patch is fixing a bug where we need to
ignore css overlay with computing visibleSize which makes sense to me. Why are
the other callsites not doing this too? For example, LayoutBox::clippingRect is
calling overflowClipRect with only the platform overlay excluded but with css
overlay included--why don't we need to exclude both platform and css there? At
least on osx on http://jsfiddle.net/rNxgD, the css overlay scrollbars are
semi-transparent so we shouldn't clip out the content under the scrollbar.

[szager1, 2017-03-08 22:21:56]

On 2017/03/08 22:08:56, pdr. wrote:
> On 2017/03/08 at 21:53:21, szager wrote:
> > On 2017/03/08 21:24:11, skobes wrote:
> > 
> > > Another option would be for overflowClipRect to take a "bool
> excludeScrollbars",
> > > and provide a helper like SA::hasCSSOverlayScrollbars in addition to
> > > SA::hasOverlayScrollbars.  Maybe that's easier to reason about.
> > 
> > I don't quite follow this suggestion.  How would a bool argument to
> overflowClipRect satisfy all callers.
> > 
> > Also, we can't add hasCSSOverlayScrollbars to ScrollableArea, because
platform
> code can't access style code.  I guess we could make it virtual, but to me
that
> seems worse than the current patch.
> 
> Can you help me understand why we don't treat platform overlay and css overlay
> scrollbars the same everywhere? This patch is fixing a bug where we need to
> ignore css overlay with computing visibleSize which makes sense to me. Why are
> the other callsites not doing this too? For example, LayoutBox::clippingRect
is
> calling overflowClipRect with only the platform overlay excluded but with css
> overlay included--why don't we need to exclude both platform and css there? At
> least on osx on http://jsfiddle.net/rNxgD, the css overlay scrollbars are
> semi-transparent so we shouldn't clip out the content under the scrollbar.

CSS overlay scrollbars are always visible; platform overlay scrollbars are not
necessarily.

When computing maximum scroll offset, we need to reduce the visible content rect
by the size of overlay scrollbars (CSS or platform).

When painting, we paint the overflow content underneath platform overlay
scrollbars because it might be visible; but we don't paint the content
underneath CSS overlay scrollbars, for the same reason we don't paint the
content underneath overflow:scroll scrollbars.

[szager1, 2017-03-08 22:35:12]

On 2017/03/08 22:21:56, szager1 wrote:
> On 2017/03/08 22:08:56, pdr. wrote:
> > On 2017/03/08 at 21:53:21, szager wrote:
> > > On 2017/03/08 21:24:11, skobes wrote:
> > > 
> > > > Another option would be for overflowClipRect to take a "bool
> > excludeScrollbars",
> > > > and provide a helper like SA::hasCSSOverlayScrollbars in addition to
> > > > SA::hasOverlayScrollbars.  Maybe that's easier to reason about.
> > > 
> > > I don't quite follow this suggestion.  How would a bool argument to
> > overflowClipRect satisfy all callers.
> > > 
> > > Also, we can't add hasCSSOverlayScrollbars to ScrollableArea, because
> platform
> > code can't access style code.  I guess we could make it virtual, but to me
> that
> > seems worse than the current patch.
> > 
> > Can you help me understand why we don't treat platform overlay and css
overlay
> > scrollbars the same everywhere? This patch is fixing a bug where we need to
> > ignore css overlay with computing visibleSize which makes sense to me. Why
are
> > the other callsites not doing this too? For example, LayoutBox::clippingRect
> is
> > calling overflowClipRect with only the platform overlay excluded but with
css
> > overlay included--why don't we need to exclude both platform and css there?
At
> > least on osx on http://jsfiddle.net/rNxgD, the css overlay scrollbars are
> > semi-transparent so we shouldn't clip out the content under the scrollbar.
> 
> CSS overlay scrollbars are always visible; platform overlay scrollbars are not
> necessarily.
> 
> When computing maximum scroll offset, we need to reduce the visible content
rect
> by the size of overlay scrollbars (CSS or platform).
> 
> When painting, we paint the overflow content underneath platform overlay
> scrollbars because it might be visible; but we don't paint the content
> underneath CSS overlay scrollbars, for the same reason we don't paint the
> content underneath overflow:scroll scrollbars.

To expand on this a bit: with platform overlay scrollbars, the platform is
responsible for painting them, and it always paints them on top of Chrome. 
However -- as evidenced by the bug -- CSS overlay scrollbars are painted by
Chrome, and can be affected by overpainting the visible content rect.

[pdr., 2017-03-08 23:05:49]

On 2017/03/08 at 22:35:12, szager wrote:
> On 2017/03/08 22:21:56, szager1 wrote:
> > On 2017/03/08 22:08:56, pdr. wrote:
> > > On 2017/03/08 at 21:53:21, szager wrote:
> > > > On 2017/03/08 21:24:11, skobes wrote:
> > > > 
> > > > > Another option would be for overflowClipRect to take a "bool
> > > excludeScrollbars",
> > > > > and provide a helper like SA::hasCSSOverlayScrollbars in addition to
> > > > > SA::hasOverlayScrollbars.  Maybe that's easier to reason about.
> > > > 
> > > > I don't quite follow this suggestion.  How would a bool argument to
> > > overflowClipRect satisfy all callers.
> > > > 
> > > > Also, we can't add hasCSSOverlayScrollbars to ScrollableArea, because
> > platform
> > > code can't access style code.  I guess we could make it virtual, but to me
> > that
> > > seems worse than the current patch.
> > > 
> > > Can you help me understand why we don't treat platform overlay and css
overlay
> > > scrollbars the same everywhere? This patch is fixing a bug where we need
to
> > > ignore css overlay with computing visibleSize which makes sense to me. Why
are
> > > the other callsites not doing this too? For example,
LayoutBox::clippingRect
> > is
> > > calling overflowClipRect with only the platform overlay excluded but with
css
> > > overlay included--why don't we need to exclude both platform and css
there? At
> > > least on osx on http://jsfiddle.net/rNxgD, the css overlay scrollbars are
> > > semi-transparent so we shouldn't clip out the content under the scrollbar.
> > 
> > CSS overlay scrollbars are always visible; platform overlay scrollbars are
not
> > necessarily.
> > 
> > When computing maximum scroll offset, we need to reduce the visible content
rect
> > by the size of overlay scrollbars (CSS or platform).
> > 
> > When painting, we paint the overflow content underneath platform overlay
> > scrollbars because it might be visible; but we don't paint the content
> > underneath CSS overlay scrollbars, for the same reason we don't paint the
> > content underneath overflow:scroll scrollbars.
> 
> To expand on this a bit: with platform overlay scrollbars, the platform is
responsible for painting them, and it always paints them on top of Chrome. 
However -- as evidenced by the bug -- CSS overlay scrollbars are painted by
Chrome, and can be affected by overpainting the visible content rect.

Ahh... The scrollbars on Android/OSX are called "overlay" and they are
semi-transparent rectangles so you would need to draw the content under them.
The distinction here is that we're drawing (presumably opaque?) non-overlay
scrollbars as overlay scrollbars, and when you do that you need these special
cases. Does css overlay have no effect on systems with real overlay scrollbars?

This seems real subtle to me, but I don't see an easy way to clean it up. I
agree with Steve that the callers do need to know about this distinction. LGTM
as long as my understanding is right.

[szager1, 2017-03-08 23:11:01]

On 2017/03/08 23:05:49, pdr. wrote:
> 
> Ahh... The scrollbars on Android/OSX are called "overlay" and they are
> semi-transparent rectangles so you would need to draw the content under them.
> The distinction here is that we're drawing (presumably opaque?) non-overlay
> scrollbars as overlay scrollbars, and when you do that you need these special
> cases. Does css overlay have no effect on systems with real overlay
scrollbars?

On such systems, overflow:overlay behaves the same as overflow:auto.

[szager1, 2017-03-08 23:14:37]

The CQ bit was checked by szager@chromium.org

[szager1, 2017-03-08 23:14:38]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/2730343003/#ps80001
(title: "comment")

[commit-bot: I haz the power, 2017-03-08 23:15:14]

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-03-09 00:54:50]

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

[commit-bot: I haz the power, 2017-03-09 00:54:52]

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

[szager1, 2017-03-09 01:02:45]

The CQ bit was checked by szager@chromium.org

[commit-bot: I haz the power, 2017-03-09 01:03: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-03-09 03:07:37]

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

[commit-bot: I haz the power, 2017-03-09 03:07:39]

Failed to apply patch for
third_party/WebKit/Source/platform/scroll/ScrollableArea.h:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/platform/scroll/ScrollableArea.h:331
  error: third_party/WebKit/Source/platform/scroll/ScrollableArea.h: patch does
not apply

Patch:       third_party/WebKit/Source/platform/scroll/ScrollableArea.h
Index: third_party/WebKit/Source/platform/scroll/ScrollableArea.h
diff --git a/third_party/WebKit/Source/platform/scroll/ScrollableArea.h
b/third_party/WebKit/Source/platform/scroll/ScrollableArea.h
index
6c5a92464f643a060839ea2e5bb88cc2682393ef..50f7dbc624969bbf9fdbb206bf7f4f40868d40b6
100644
--- a/third_party/WebKit/Source/platform/scroll/ScrollableArea.h
+++ b/third_party/WebKit/Source/platform/scroll/ScrollableArea.h
@@ -331,9 +331,9 @@ class PLATFORM_EXPORT ScrollableArea : public
GarbageCollectedMixin,
   IntSize excludeScrollbars(const IntSize&) const;
 
   virtual int verticalScrollbarWidth(
-      OverlayScrollbarClipBehavior = IgnoreOverlayScrollbarSize) const;
+      OverlayScrollbarClipBehavior = IgnorePlatformOverlayScrollbarSize) const;
   virtual int horizontalScrollbarHeight(
-      OverlayScrollbarClipBehavior = IgnoreOverlayScrollbarSize) const;
+      OverlayScrollbarClipBehavior = IgnorePlatformOverlayScrollbarSize) const;
 
   // Returns the widget associated with this ScrollableArea.
   virtual FrameViewBase* getWidget() { return nullptr; }

[szager1, 2017-03-15 21:22:13]

The CQ bit was checked by szager@chromium.org

[szager1, 2017-03-15 21:22:14]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/2730343003/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2017-03-15 21:23:15]

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-03-15 23:04:49]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1489612933778470,
"parent_rev": "914e3e2720a22f8b6e0c32ef9e440b76d6ddac3c", "commit_rev":
"7ab46eae8507137607e258543118ef8fb04984de"}

[commit-bot: I haz the power, 2017-03-15 23:05:36]

Description was changed from

==========
Fix overflow:overlay scrollbar width for paint.

This CL went too far:

https://codereview.chromium.org/2725763002

Some callers to verticalScrollbarWidth -- in particular, the paint
code -- need the old behavior, where overflow:overlay scrollbars
are subtracted from the overflow clip rect.

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

to

==========
Fix overflow:overlay scrollbar width for paint.

This CL went too far:

https://codereview.chromium.org/2725763002

Some callers to verticalScrollbarWidth -- in particular, the paint
code -- need the old behavior, where overflow:overlay scrollbars
are subtracted from the overflow clip rect.

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

Review-Url: https://codereview.chromium.org/2730343003
Cr-Commit-Position: refs/heads/master@{#457251}
Committed:
https://chromium.googlesource.com/chromium/src/+/7ab46eae8507137607e258543118...
==========

[commit-bot: I haz the power, 2017-03-15 23:05:37]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/7ab46eae8507137607e258543118...

[tzik, 2017-03-16 02:27:56]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/2749863006/ by tzik@chromium.org.

The reason for reverting is: A layout test gets red after this CL due to the
mismatched test expectation.

The error log is:
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11%20%28...
fast/overflow/scroll-nested-positioned-layer-in-overflow.html
.