Fix the overflow clip rect rounding for overflow scrolling

Fix the overflow clip rect rounding for overflow scrolling

We were using the enclosingIntRect, but that sets the clip too
large and enables the scrolling contents to paint over the
container's border. Switch to using the pixelSnappedIntRect
to correctly size the clip to the scrolling layer.

Update a test to catch this case in addition to the existing
fractionally sized layer issues.

Fix a potential source of problems in computing scroll offsets
due to different methods for computing contentsSize.

R=flackr@chromium.org
BUG=651305
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[Stephen Chennney, 2016-10-21 18:54:13]

Description was changed from

==========
Fix the overflow clip rect rounding for overflow scrolling

We were using the enclosingIntRect, but that sets the clip too
large and enables the scrolling contents to paint over the
container's border. Switch to using the pixelSnappedIntRect
to correctly size the clip to the scrolling layer.

Update a test to catch this case in addition to the existing
fractionally sized layer issues.

Fix a potential source of problems in computing scroll offsets
due to different methods for computing contentsSize.

R=flackr@chromium.org
BUG=651305
==========

to

==========
Fix the overflow clip rect rounding for overflow scrolling

We were using the enclosingIntRect, but that sets the clip too
large and enables the scrolling contents to paint over the
container's border. Switch to using the pixelSnappedIntRect
to correctly size the clip to the scrolling layer.

Update a test to catch this case in addition to the existing
fractionally sized layer issues.

Fix a potential source of problems in computing scroll offsets
due to different methods for computing contentsSize.

R=flackr@chromium.org
BUG=651305
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Stephen Chennney, 2016-10-21 18:54:49]

Need new baselines. Let me know if you think I should leave the test unchanged
and add another test.

[Stephen Chennney, 2016-10-24 15:20:29]

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

[Stephen Chennney, 2016-10-24 15:20:40]

Ready for review.

[commit-bot: I haz the power, 2016-10-24 15:20:59]

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

[flackr, 2016-10-24 15:32:34]

LGTM

[Stephen Chennney, 2016-10-24 15:41:25]

The CQ bit was unchecked by schenney@chromium.org

[Stephen Chennney, 2016-10-24 15:41:27]

The CQ bit was checked by schenney@chromium.org

[Stephen Chennney, 2016-10-24 15:41:31]

The CQ bit was checked by schenney@chromium.org

[commit-bot: I haz the power, 2016-10-24 15:41:38]

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-24 16:58:42]

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

[commit-bot: I haz the power, 2016-10-24 16:58:43]

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

[Stephen Chennney, 2016-10-24 19:10:35]

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

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

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

[Stephen Chennney, 2016-10-24 19:40:21]

Description was changed from

==========
Fix the overflow clip rect rounding for overflow scrolling

We were using the enclosingIntRect, but that sets the clip too
large and enables the scrolling contents to paint over the
container's border. Switch to using the pixelSnappedIntRect
to correctly size the clip to the scrolling layer.

Update a test to catch this case in addition to the existing
fractionally sized layer issues.

Fix a potential source of problems in computing scroll offsets
due to different methods for computing contentsSize.

R=flackr@chromium.org
BUG=651305
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Fix the overflow clip rect rounding for overflow scrolling

We were using the enclosingIntRect, but that sets the clip too
large and enables the scrolling contents to paint over the
container's border. Switch to using the pixelSnappedIntRect
to correctly size the clip to the scrolling layer.

Update a test to catch this case in addition to the existing
fractionally sized layer issues.

Fix a potential source of problems in computing scroll offsets
due to different methods for computing contentsSize.

R=flackr@chromium.org
BUG=651305
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Stephen Chennney, 2016-10-24 19:40:22]

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

[Stephen Chennney, 2016-10-24 19:40:45]

Adding pdr@ to ok the spv2 fix.

[pdr., 2016-10-24 22:11:08]

On 2016/10/24 at 19:40:45, schenney wrote:
> Adding pdr@ to ok the spv2 fix.

Overall the spv2 change looks fine.

I wonder if we could clean this up though? The paint property tree builder code
uses overflowClipRect for hasOverflowClip/containsPaint and SVGRoot's viewport.
SVGRoot's overflowClipRect already returns snapped rects and this patch relies
on that to avoid spv2 differences.

I don't have a great suggestion for how to clean it up. Many callers to
overflowClipRect do snapping already (e.g., BoxClipper,
PaintLayerScrollableArea::maximumScrollOffsetInt), or have todos that they
should (e.g., BoxPainter::paintFillLayer). Is there an underlying reason why
some callers need snapping and others don't?

[commit-bot: I haz the power, 2016-10-24 22:19:19]

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

[commit-bot: I haz the power, 2016-10-24 22:19:20]

Dry run: This issue passed the CQ dry run.

[Stephen Chennney, 2016-10-25 17:24:12]

On 2016/10/24 22:11:08, pdr. wrote:
> On 2016/10/24 at 19:40:45, schenney wrote:
> > Adding pdr@ to ok the spv2 fix.
> 
> Overall the spv2 change looks fine.
> 
> I wonder if we could clean this up though? The paint property tree builder
code
> uses overflowClipRect for hasOverflowClip/containsPaint and SVGRoot's
viewport.
> SVGRoot's overflowClipRect already returns snapped rects and this patch relies
> on that to avoid spv2 differences.
> 
> I don't have a great suggestion for how to clean it up. Many callers to
> overflowClipRect do snapping already (e.g., BoxClipper,
> PaintLayerScrollableArea::maximumScrollOffsetInt), or have todos that they
> should (e.g., BoxPainter::paintFillLayer). Is there an underlying reason why
> some callers need snapping and others don't?

I propose landing this and then looking at the clean up as a separate issue.
This is a release blocking bug, after all.

[pdr., 2016-10-25 17:46:43]

On 2016/10/25 at 17:24:12, schenney wrote:
> On 2016/10/24 22:11:08, pdr. wrote:
> > On 2016/10/24 at 19:40:45, schenney wrote:
> > > Adding pdr@ to ok the spv2 fix.
> > 
> > Overall the spv2 change looks fine.
> > 
> > I wonder if we could clean this up though? The paint property tree builder
code
> > uses overflowClipRect for hasOverflowClip/containsPaint and SVGRoot's
viewport.
> > SVGRoot's overflowClipRect already returns snapped rects and this patch
relies
> > on that to avoid spv2 differences.
> > 
> > I don't have a great suggestion for how to clean it up. Many callers to
> > overflowClipRect do snapping already (e.g., BoxClipper,
> > PaintLayerScrollableArea::maximumScrollOffsetInt), or have todos that they
> > should (e.g., BoxPainter::paintFillLayer). Is there an underlying reason why
> > some callers need snapping and others don't?
> 
> I propose landing this and then looking at the clean up as a separate issue.
This is a release blocking bug, after all.

SGTM, LGTM

[Stephen Chennney, 2016-10-25 17:59:10]

The CQ bit was checked by schenney@chromium.org

[Stephen Chennney, 2016-10-25 17:59:10]

The patchset sent to the CQ was uploaded after l-g-t-m from flackr@chromium.org
Link to the patchset: https://codereview.chromium.org/2442833002/#ps40001
(title: "Fix Slimming Paint v2")

[commit-bot: I haz the power, 2016-10-25 17:59:57]

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-25 18:05:48]

Description was changed from

==========
Fix the overflow clip rect rounding for overflow scrolling

We were using the enclosingIntRect, but that sets the clip too
large and enables the scrolling contents to paint over the
container's border. Switch to using the pixelSnappedIntRect
to correctly size the clip to the scrolling layer.

Update a test to catch this case in addition to the existing
fractionally sized layer issues.

Fix a potential source of problems in computing scroll offsets
due to different methods for computing contentsSize.

R=flackr@chromium.org
BUG=651305
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Fix the overflow clip rect rounding for overflow scrolling

We were using the enclosingIntRect, but that sets the clip too
large and enables the scrolling contents to paint over the
container's border. Switch to using the pixelSnappedIntRect
to correctly size the clip to the scrolling layer.

Update a test to catch this case in addition to the existing
fractionally sized layer issues.

Fix a potential source of problems in computing scroll offsets
due to different methods for computing contentsSize.

R=flackr@chromium.org
BUG=651305
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[commit-bot: I haz the power, 2016-10-25 18:05:49]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-25 18:16:25]

Description was changed from

==========
Fix the overflow clip rect rounding for overflow scrolling

We were using the enclosingIntRect, but that sets the clip too
large and enables the scrolling contents to paint over the
container's border. Switch to using the pixelSnappedIntRect
to correctly size the clip to the scrolling layer.

Update a test to catch this case in addition to the existing
fractionally sized layer issues.

Fix a potential source of problems in computing scroll offsets
due to different methods for computing contentsSize.

R=flackr@chromium.org
BUG=651305
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Fix the overflow clip rect rounding for overflow scrolling

We were using the enclosingIntRect, but that sets the clip too
large and enables the scrolling contents to paint over the
container's border. Switch to using the pixelSnappedIntRect
to correctly size the clip to the scrolling layer.

Update a test to catch this case in addition to the existing
fractionally sized layer issues.

Fix a potential source of problems in computing scroll offsets
due to different methods for computing contentsSize.

R=flackr@chromium.org
BUG=651305
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[commit-bot: I haz the power, 2016-10-25 18:16:26]

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