Use the pixelSnappedScrollWidth/Height for contentsSize

Use the pixelSnappedScrollWidth/Height for contentsSize

PaintLayerScrollableArea::contentsSize was using the LayoutUnit::toInt
method to convert to int for the size. However, when sizing a Graphicslayer
from this value, it results in layers that are too narrow in some cases,
giving single pixel gaps between the scrolling content and the scrollbars.

The correct value is the pixelSnappedScrollWidth, that accounts for
sub-pixel position of surrounding elements.

Adds a test that fails in m55 and passes with the change, derived from the
site that the bug was reported for.

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

Committed: https://crrev.com/6c715c7a413e006594b26a2b59783d2d9d01f311
Cr-Commit-Position: refs/heads/master@{#425727}

[Stephen Chennney, 2016-10-13 20:37:05]

Description was changed from

==========
Use the pixelSnappedScrollWidth/Height for contentsSize

PaintLayerScrollableArea::contentsSize was using the LayoutUnit::toInt
method to convert to int for the size. However, when sizing a Graphicslayer
from this value, it results in layers that are too narrow in some cases,
giving single pixel gaps between the scrolling content and the scrollbars.

The correct value is the pixelSnappedScrollWidth, that accounts for
sub-pixel position of surrounding elements.

Adds a test that fails in m55 and passes with the change, derived from the
site that the bug was reported for.

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

to

==========
Use the pixelSnappedScrollWidth/Height for contentsSize

PaintLayerScrollableArea::contentsSize was using the LayoutUnit::toInt
method to convert to int for the size. However, when sizing a Graphicslayer
from this value, it results in layers that are too narrow in some cases,
giving single pixel gaps between the scrolling content and the scrollbars.

The correct value is the pixelSnappedScrollWidth, that accounts for
sub-pixel position of surrounding elements.

Adds a test that fails in m55 and passes with the change, derived from the
site that the bug was reported for.

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

[Stephen Chennney, 2016-10-13 20:39:48]

This fix was the least invasive I could find. I considered making the
contentSize a LayoutUnit everywhere, but it's used for FrameViews and other
top-level objects, and it seems like a really bad idea to make those
LayoutUnits.

Interestingly, no test that we have changes for this patch.

Tests are expected to fail for the newly added test. A follow on patch will add
the baselines.

[flackr, 2016-10-13 21:07:43]

This looks good. I assume this works correctly for elements in a scaled parent
as well?
i.e. http://jsbin.com/seqayud/edit?html,css,output

https://codereview.chromium.org/2416453006/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/compositing/overflow/fractional-sized-scrolling-layer.html
(right):

https://codereview.chromium.org/2416453006/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/compositing/overflow/fractional-sized-scrolling-layer.html:16:
width: 575.06px;
If we can set fractional widths, then is the outer container necessary to get
#container to be a fractional width?

[Stephen Chennney, 2016-10-13 21:27:41]

I'll check the scaled thing tomorrow when I can. I have a mega interview slate
tomorrow so may not get to it.

https://codereview.chromium.org/2416453006/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/compositing/overflow/fractional-sized-scrolling-layer.html
(right):

https://codereview.chromium.org/2416453006/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/compositing/overflow/fractional-sized-scrolling-layer.html:16:
width: 575.06px;
On 2016/10/13 21:07:43, flackr wrote:
> If we can set fractional widths, then is the outer container necessary to get
> #container to be a fractional width?

Turns out it is necessary. There is clearly some different snapping behaviour
when working from percentage widths.

[Stephen Chennney, 2016-10-14 13:42:43]

Turns out this fixes an existing test result on Windows. Yippee.

The results on the scaled-body jsbin file you provided are better than m54,
though there are still some grey lines at the bottom of the scrolling content
that shouldn't be there. What exactly were you expecting the result to be on
that test case? Regardless, I'll add it as a test so we track the behavior.

[Stephen Chennney, 2016-10-14 14:42:03]

Still need one more set of baselines. Otherwise, what do you think?

[flackr, 2016-10-14 14:45:59]

On 2016/10/14 at 13:42:43, schenney wrote:
> Turns out this fixes an existing test result on Windows. Yippee.
> 
> The results on the scaled-body jsbin file you provided are better than m54,
though there are still some grey lines at the bottom of the scrolling content
that shouldn't be there. What exactly were you expecting the result to be on
that test case? Regardless, I'll add it as a test so we track the behavior.

There should be no gaps in the test case, I believe there are none if the
scrollers aren't promoted. Anyways it may be a separate bug.

[flackr, 2016-10-14 14:48:10]

On 2016/10/14 at 14:42:03, schenney wrote:
> Still need one more set of baselines. Otherwise, what do you think?

LGTM

[Stephen Chennney, 2016-10-14 17:54:48]

The CQ bit was checked by schenney@chromium.org

[Stephen Chennney, 2016-10-14 17:54:49]

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/2416453006/#ps40001
(title: "More baselines")

[commit-bot: I haz the power, 2016-10-14 17:55:33]

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-14 18:33:17]

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

[commit-bot: I haz the power, 2016-10-14 18:33:18]

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-17 14:51:06]

Patchset #4 (id:60001) has been deleted

[Stephen Chennney, 2016-10-17 15:40:39]

The CQ bit was checked by schenney@chromium.org

[Stephen Chennney, 2016-10-17 15:40:39]

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/2416453006/#ps80001
(title: "Spv2 expectations")

[commit-bot: I haz the power, 2016-10-17 15:40: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-17 16:25:17]

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

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

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Stephen Chennney, 2016-10-17 17:36:27]

The CQ bit was checked by schenney@chromium.org

[commit-bot: I haz the power, 2016-10-17 17:37:01]

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-17 18:34:06]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-10-17 18:36:29]

Description was changed from

==========
Use the pixelSnappedScrollWidth/Height for contentsSize

PaintLayerScrollableArea::contentsSize was using the LayoutUnit::toInt
method to convert to int for the size. However, when sizing a Graphicslayer
from this value, it results in layers that are too narrow in some cases,
giving single pixel gaps between the scrolling content and the scrollbars.

The correct value is the pixelSnappedScrollWidth, that accounts for
sub-pixel position of surrounding elements.

Adds a test that fails in m55 and passes with the change, derived from the
site that the bug was reported for.

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

to

==========
Use the pixelSnappedScrollWidth/Height for contentsSize

PaintLayerScrollableArea::contentsSize was using the LayoutUnit::toInt
method to convert to int for the size. However, when sizing a Graphicslayer
from this value, it results in layers that are too narrow in some cases,
giving single pixel gaps between the scrolling content and the scrollbars.

The correct value is the pixelSnappedScrollWidth, that accounts for
sub-pixel position of surrounding elements.

Adds a test that fails in m55 and passes with the change, derived from the
site that the bug was reported for.

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

Committed: https://crrev.com/6c715c7a413e006594b26a2b59783d2d9d01f311
Cr-Commit-Position: refs/heads/master@{#425727}
==========

[commit-bot: I haz the power, 2016-10-17 18:36:31]

Patchset 4 (id:??) landed as
https://crrev.com/6c715c7a413e006594b26a2b59783d2d9d01f311
Cr-Commit-Position: refs/heads/master@{#425727}