Don't automatically paint clipped backgrounds into scrolling contents layer.

Don't automatically paint clipped backgrounds into scrolling contents layer.

A composited scrolling contents layer currently does not correctly handle css
clip so we only paint into the layer if you explicitly request it with local
attachment.

BUG=645957
TEST=PaintLayerScrollableAreaTest.ShouldPaintBackgroundOntoScrollingContentsLayer
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[flackr, 2016-09-12 14:40:03]

Description was changed from

==========
Don't automatically paint clipped backgrounds into scrolling contents layer.

A composited scrolling contents layer currently does not correctly handle css
clip so we only paint into the layer if you explicitly request it with local
attachment.

BUG=645957
TEST=PaintLayerScrollableAreaTest.ShouldPaintBackgroundOntoScrollingContentsLayer
==========

to

==========
Don't automatically paint clipped backgrounds into scrolling contents layer.

A composited scrolling contents layer currently does not correctly handle css
clip so we only paint into the layer if you explicitly request it with local
attachment.

BUG=645957
TEST=PaintLayerScrollableAreaTest.ShouldPaintBackgroundOntoScrollingContentsLayer
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[flackr, 2016-09-12 14:42:04]

flackr@chromium.org changed reviewers:
+ chrishtr@chromium.org

[flackr, 2016-09-12 14:42:05]

PTAL, this avoids automatically painting backgrounds into a composited scrolling
contents layer when we have a clip defined as composited scrollers currently
don't handle the clip correctly.

[chrishtr, 2016-09-12 18:15:04]

It's unclear to me: what does explicitly requesting local attachment have to do
with CSS clip?

[flackr, 2016-09-12 19:05:38]

On 2016/09/12 at 18:15:04, chrishtr wrote:
> It's unclear to me: what does explicitly requesting local attachment have to
do with CSS clip?

It doesn't have anything to do with it directly, but if you have made a
background local and have a CSS clip we will be wrong either way we work around
the current bugs. Either we ignore the local attachment and the background
doesn't move with scrolling content or we expose the issues with correctly
clipping the scrolling contents layer (which are already exposed on the content
anyways).

[chrishtr, 2016-09-12 19:43:29]

On 2016/09/12 at 19:05:38, flackr wrote:
> On 2016/09/12 at 18:15:04, chrishtr wrote:
> > It's unclear to me: what does explicitly requesting local attachment have to
do with CSS clip?
> 
> It doesn't have anything to do with it directly, but if you have made a
background local and have a CSS clip we will be wrong either way we work around
the current bugs. Either we ignore the local attachment and the background
doesn't move with scrolling content or we expose the issues with correctly
clipping the scrolling contents layer (which are already exposed on the content
anyways).

Ok, but is there actually a new bug you have introduced via your recent work, or
is the set of CSS clip bugs the same as before? If the latter, why the need to
work around?

[flackr, 2016-09-12 20:33:49]

On 2016/09/12 at 19:43:29, chrishtr wrote:
> On 2016/09/12 at 19:05:38, flackr wrote:
> > On 2016/09/12 at 18:15:04, chrishtr wrote:
> > > It's unclear to me: what does explicitly requesting local attachment have
to do with CSS clip?
> > 
> > It doesn't have anything to do with it directly, but if you have made a
background local and have a CSS clip we will be wrong either way we work around
the current bugs. Either we ignore the local attachment and the background
doesn't move with scrolling content or we expose the issues with correctly
clipping the scrolling contents layer (which are already exposed on the content
anyways).
> 
> Ok, but is there actually a new bug you have introduced via your recent work,
or is the set of CSS clip bugs the same as before? If the latter, why the need
to work around?

I have exposed the existing bug in more common cases with my local-equivalent
background checks. If you have a background color and clip on a composited
scroller, previously we would have correctly painted the background color into
the unclipped region and it wouldn't move when scrolled. Now, we will paint it
into the unclipped region and then slide it off with the scrolling contents.

[chrishtr, 2016-09-12 20:44:22]

On 2016/09/12 at 20:33:49, flackr wrote:
> On 2016/09/12 at 19:43:29, chrishtr wrote:
> > On 2016/09/12 at 19:05:38, flackr wrote:
> > > On 2016/09/12 at 18:15:04, chrishtr wrote:
> > > > It's unclear to me: what does explicitly requesting local attachment
have to do with CSS clip?
> > > 
> > > It doesn't have anything to do with it directly, but if you have made a
background local and have a CSS clip we will be wrong either way we work around
the current bugs. Either we ignore the local attachment and the background
doesn't move with scrolling content or we expose the issues with correctly
clipping the scrolling contents layer (which are already exposed on the content
anyways).
> > 
> > Ok, but is there actually a new bug you have introduced via your recent
work, or is the set of CSS clip bugs the same as before? If the latter, why the
need to work around?
> 
> I have exposed the existing bug in more common cases with my local-equivalent
background checks. If you have a background color and clip on a composited
scroller, previously we would have correctly painted the background color into
the unclipped region and it wouldn't move when scrolled. Now, we will paint it
into the unclipped region and then slide it off with the scrolling contents.

Ah. In that case let's just turn off the whole
paint-background-into-scrolling-layer thing when CSS clip is present on the
scroller.

[flackr, 2016-09-13 19:14:23]

On 2016/09/12 at 20:44:22, chrishtr wrote:
> On 2016/09/12 at 20:33:49, flackr wrote:
> > On 2016/09/12 at 19:43:29, chrishtr wrote:
> > > On 2016/09/12 at 19:05:38, flackr wrote:
> > > > On 2016/09/12 at 18:15:04, chrishtr wrote:
> > > > > It's unclear to me: what does explicitly requesting local attachment
have to do with CSS clip?
> > > > 
> > > > It doesn't have anything to do with it directly, but if you have made a
background local and have a CSS clip we will be wrong either way we work around
the current bugs. Either we ignore the local attachment and the background
doesn't move with scrolling content or we expose the issues with correctly
clipping the scrolling contents layer (which are already exposed on the content
anyways).
> > > 
> > > Ok, but is there actually a new bug you have introduced via your recent
work, or is the set of CSS clip bugs the same as before? If the latter, why the
need to work around?
> > 
> > I have exposed the existing bug in more common cases with my
local-equivalent background checks. If you have a background color and clip on a
composited scroller, previously we would have correctly painted the background
color into the unclipped region and it wouldn't move when scrolled. Now, we will
paint it into the unclipped region and then slide it off with the scrolling
contents.
> 
> Ah. In that case let's just turn off the whole
paint-background-into-scrolling-layer thing when CSS clip is present on the
scroller.

Done.

[chrishtr, 2016-09-13 20:16:31]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-09-13 20:16:32]

lgtm

[commit-bot: I haz the power, 2016-09-13 20:17:12]

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-13 21:46:03]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-13 21:48:10]

Description was changed from

==========
Don't automatically paint clipped backgrounds into scrolling contents layer.

A composited scrolling contents layer currently does not correctly handle css
clip so we only paint into the layer if you explicitly request it with local
attachment.

BUG=645957
TEST=PaintLayerScrollableAreaTest.ShouldPaintBackgroundOntoScrollingContentsLayer
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Don't automatically paint clipped backgrounds into scrolling contents layer.

A composited scrolling contents layer currently does not correctly handle css
clip so we only paint into the layer if you explicitly request it with local
attachment.

BUG=645957
TEST=PaintLayerScrollableAreaTest.ShouldPaintBackgroundOntoScrollingContentsLayer
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[commit-bot: I haz the power, 2016-09-13 21:48:11]

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