Only adjust composited pagination position to account for lines offset of inlines.

Only adjust composited pagination position to account for lines offset of inlines.

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

Review-Url: https://codereview.chromium.org/2883083004
Cr-Commit-Position: refs/heads/master@{#473092}
Committed: https://chromium.googlesource.com/chromium/src/+/f4166640f3a4119cdb4b33e2f16ebdd28ef6ff7f

[chrishtr, 2017-05-15 23:52:32]

Description was changed from

==========
none


none

BUG=
==========

to

==========
none


none

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

[chrishtr, 2017-05-15 23:53:38]

Description was changed from

==========
none


none

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

to

==========
Only adjust composited pagination position to account for lines offset of
inlines.

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

[chrishtr, 2017-05-16 00:00:28]

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

[chrishtr, 2017-05-16 00:00:29]

https://codereview.chromium.org/2883083004/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-inline.html
(right):

https://codereview.chromium.org/2883083004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-inline.html:2:
<script src="../../resources/testharness.js"></script>
This test was previously broken, because the non-composited -expected.html file
duplicated the
regular one. I had to make it a pixel test to avoid subpixel rendering issues.

[chrishtr, 2017-05-16 00:00:32]

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

[commit-bot: I haz the power, 2017-05-16 00:01:43]

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-05-16 01:26:19]

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

[commit-bot: I haz the power, 2017-05-16 01:26:20]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chrishtr, 2017-05-16 16:34:27]

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

[commit-bot: I haz the power, 2017-05-16 16:35:33]

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-05-16 19:09:42]

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

[commit-bot: I haz the power, 2017-05-16 19:09:43]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[trchen, 2017-05-16 23:26:48]

https://codereview.chromium.org/2883083004/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-inline.html
(right):

https://codereview.chromium.org/2883083004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-inline.html:2:
<script src="../../resources/testharness.js"></script>
On 2017/05/16 00:00:29, chrishtr wrote:
> This test was previously broken, because the non-composited -expected.html
file
> duplicated the
> regular one. I had to make it a pixel test to avoid subpixel rendering issues.

lol. I think it is possible to keep it a ref-test by adding a solid background
color. Or an alternative is to keep will-change:transform on the expectation,
but use absolute position (instead of columns) to layout the test text.

https://codereview.chromium.org/2883083004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-outlined-box.html
(right):

https://codereview.chromium.org/2883083004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-outlined-box.html:1:
<!DOCTYPE html>
Is it possible to reduce this further? (And make it a ref test.)

https://codereview.chromium.org/2883083004/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/2883083004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:893:
.LinesVisualOverflowBoundingBox()
LinesVisualOverflowBoundingBox can be expensive (have to collect all descendant
line boxes.). And I'm not sure we actually want to include visual overflow, as
say, wouldn't top-left box shadow cause the same issue?

How about .FirstLineBoxTopLeft() ? Feels like exactly what we want.

[chrishtr, 2017-05-17 02:00:14]

https://codereview.chromium.org/2883083004/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-inline.html
(right):

https://codereview.chromium.org/2883083004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-inline.html:2:
<script src="../../resources/testharness.js"></script>
On 2017/05/16 at 23:26:47, trchen wrote:
> On 2017/05/16 00:00:29, chrishtr wrote:
> > This test was previously broken, because the non-composited -expected.html
file
> > duplicated the
> > regular one. I had to make it a pixel test to avoid subpixel rendering
issues.
> 
> lol. I think it is possible to keep it a ref-test by adding a solid background
color. Or an alternative is to keep will-change:transform on the expectation,
but use absolute position (instead of columns) to layout the test text.

I tried to make it a ref test, but failed (tried both of your suggestions).
Ok to commit as is?

https://codereview.chromium.org/2883083004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-outlined-box.html
(right):

https://codereview.chromium.org/2883083004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-outlined-box.html:1:
<!DOCTYPE html>
On 2017/05/16 at 23:26:47, trchen wrote:
> Is it possible to reduce this further? (And make it a ref test.)

Done.

https://codereview.chromium.org/2883083004/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/2883083004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:893:
.LinesVisualOverflowBoundingBox()
On 2017/05/16 at 23:26:47, trchen wrote:
> LinesVisualOverflowBoundingBox can be expensive (have to collect all
descendant line boxes.). And I'm not sure we actually want to include visual
overflow, as say, wouldn't top-left box shadow cause the same issue?
> 
> How about .FirstLineBoxTopLeft() ? Feels like exactly what we want.

Done, good idea.

[chrishtr, 2017-05-17 03:02:27]

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

[commit-bot: I haz the power, 2017-05-17 03:04:47]

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-05-17 04:15:02]

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

[commit-bot: I haz the power, 2017-05-17 04:15:03]

Dry run: 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...)

[chrishtr, 2017-05-17 16:52:29]

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

[commit-bot: I haz the power, 2017-05-17 16:53:39]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chrishtr, 2017-05-17 16:55:00]

The CQ bit was unchecked by chrishtr@chromium.org

[chrishtr, 2017-05-17 16:57:22]

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

[commit-bot: I haz the power, 2017-05-17 16:58:55]

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-05-17 18:07:30]

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

[commit-bot: I haz the power, 2017-05-17 18:07:31]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[trchen, 2017-05-17 20:48:50]

https://codereview.chromium.org/2883083004/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-inline.html
(right):

https://codereview.chromium.org/2883083004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/pagination/composited-paginated-inline.html:2:
<script src="../../resources/testharness.js"></script>
On 2017/05/17 02:00:14, chrishtr wrote:
> On 2017/05/16 at 23:26:47, trchen wrote:
> > On 2017/05/16 00:00:29, chrishtr wrote:
> > > This test was previously broken, because the non-composited -expected.html
> file
> > > duplicated the
> > > regular one. I had to make it a pixel test to avoid subpixel rendering
> issues.
> > 
> > lol. I think it is possible to keep it a ref-test by adding a solid
background
> color. Or an alternative is to keep will-change:transform on the expectation,
> but use absolute position (instead of columns) to layout the test text.
> 
> I tried to make it a ref test, but failed (tried both of your suggestions).
> Ok to commit as is?

Hmm weird. Adding solid background worked for me, but I got 1 pixel difference
with will-change:transform;position:absolute too.
I uploaded my working ref test here for you to cherrypick:
https://codereview.chromium.org/2892593004
lgtm either way. You make the call. :)

[chrishtr, 2017-05-17 21:18:33]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-05-17 21:18:34]

The patchset sent to the CQ was uploaded after l-g-t-m from trchen@chromium.org
Link to the patchset: https://codereview.chromium.org/2883083004/#ps140001
(title: "none")

[commit-bot: I haz the power, 2017-05-17 21:19:46]

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-05-17 22:13:40]

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

[commit-bot: I haz the power, 2017-05-17 22:13:41]

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

[chrishtr, 2017-05-18 01:19:51]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-05-18 01:19:51]

The patchset sent to the CQ was uploaded after l-g-t-m from trchen@chromium.org
Link to the patchset: https://codereview.chromium.org/2883083004/#ps160001
(title: "Merge branch 'master' into paginatino")

[commit-bot: I haz the power, 2017-05-18 01:21:19]

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-05-18 02:36:08]

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

[commit-bot: I haz the power, 2017-05-18 02:36:09]

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

[chrishtr, 2017-05-18 03:25:40]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2017-05-18 03:26:48]

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-05-18 04:38:30]

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

[commit-bot: I haz the power, 2017-05-18 04:38:31]

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

[chrishtr, 2017-05-18 15:53:17]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2017-05-18 15:54:25]

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-05-18 17:08:10]

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

[commit-bot: I haz the power, 2017-05-18 17:08:11]

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

[chrishtr, 2017-05-18 20:41:43]

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

[commit-bot: I haz the power, 2017-05-18 20:42:43]

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-05-18 21:58:04]

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

[commit-bot: I haz the power, 2017-05-18 21:58:05]

Dry run: 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...)

[chrishtr, 2017-05-18 22:10:58]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-05-18 22:11:01]

The patchset sent to the CQ was uploaded after l-g-t-m from trchen@chromium.org
Link to the patchset: https://codereview.chromium.org/2883083004/#ps170001
(title: "none")

[commit-bot: I haz the power, 2017-05-18 22:12: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-05-19 01:35:27]

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

[commit-bot: I haz the power, 2017-05-19 01:35:30]

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

[chrishtr, 2017-05-19 02:42:29]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2017-05-19 02:43:11]

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-05-19 05:03:18]

CQ is committing da patch.

Bot data: {"patchset_id": 170001, "attempt_start_ts": 1495161749466500,
"parent_rev": "ec66f57f3315293efbc78564b83895db814086f9", "commit_rev":
"f4166640f3a4119cdb4b33e2f16ebdd28ef6ff7f"}

[commit-bot: I haz the power, 2017-05-19 05:03:31]

Description was changed from

==========
Only adjust composited pagination position to account for lines offset of
inlines.

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

to

==========
Only adjust composited pagination position to account for lines offset of
inlines.

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

Review-Url: https://codereview.chromium.org/2883083004
Cr-Commit-Position: refs/heads/master@{#473092}
Committed:
https://chromium.googlesource.com/chromium/src/+/f4166640f3a4119cdb4b33e2f16e...
==========

[commit-bot: I haz the power, 2017-05-19 05:03:33]

Committed patchset #10 (id:170001) as
https://chromium.googlesource.com/chromium/src/+/f4166640f3a4119cdb4b33e2f16e...