New issue
Advanced search Search tips

Issue 910300 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Unpredictable truncation of margins in bottom-aligned abspos in multicol / printing

Project Member Reported by mstensho@chromium.org, Nov 29

Issue description

It looks like we think that we get a fragmentainer break inside a bottom-aligned abspos, and therefore truncate the margin. Probably because we initially position it at the bottom of the fragmentainer and let the content flow into the next fragmentainer, before we adjust the position after having calculated the height.

This is the reason why NG "fails" printing/absolute-position-headers-and-footers.html . The number of re-layouts affects the final result. We re-lay out more in NG, because printing is done with legacy, so nothing can be preserved when switching from screen to print. I wonder how flaky this test is across systems and across phases of the moon.

Attaching a simple multicol test that demonstrates the issue. Try resizing the window for an extra amusing effect. :)

To be clear: this is a pure legacy layout bug (NG always switches to legacy for block fragmentation)
 
tc.html
475 bytes View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d5096cb8a5d544acb51f8b4df908775b6584778f

commit d5096cb8a5d544acb51f8b4df908775b6584778f
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Wed Dec 05 10:37:15 2018

Don't estimate the top of bottom-aligned OOFs as bottom.

We'll incorrectly think that we get fragmented in that case, which will
lead to incorrect height, which will lead to incorrect top offset,
which will lead to incorrect fragmentation.

This has probably been the cause for a flaky web test, that behaved
even worse in LayoutNG, because in NG we force legacy layout as soon as
fragmentation is involved (so when entering printing, we typically need
to rebuild the entire layout object tree, with no former height to base
the bottom estimate on (which is what saved us when NG is disabled -
ehm, at least most of the time)).

Bug:  910300 
Change-Id: I8e152f77e9a94bb05e51f02a24be6ef5fe03c6f9
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/c/1355920
Reviewed-by: Aleks Totic <atotic@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613930}
[modify] https://crrev.com/d5096cb8a5d544acb51f8b4df908775b6584778f/third_party/blink/renderer/core/layout/layout_block.cc
[modify] https://crrev.com/d5096cb8a5d544acb51f8b4df908775b6584778f/third_party/blink/web_tests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/d5096cb8a5d544acb51f8b4df908775b6584778f/third_party/blink/web_tests/TestExpectations
[add] https://crrev.com/d5096cb8a5d544acb51f8b4df908775b6584778f/third_party/blink/web_tests/external/wpt/css/css-break/block-end-aligned-abspos-with-overflow-ref.html
[add] https://crrev.com/d5096cb8a5d544acb51f8b4df908775b6584778f/third_party/blink/web_tests/external/wpt/css/css-break/block-end-aligned-abspos-with-overflow.html
[add] https://crrev.com/d5096cb8a5d544acb51f8b4df908775b6584778f/third_party/blink/web_tests/external/wpt/css/css-break/block-end-aligned-abspos.html

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2eb894c22bfe57d2145e5a7fed388dff7ddf39fd

commit 2eb894c22bfe57d2145e5a7fed388dff7ddf39fd
Author: Roger McFarlane <rogerm@chromium.org>
Date: Wed Dec 05 18:03:46 2018

[Sheriff] Revert "Don't estimate the top of bottom-aligned OOFs as bottom."

This reverts commit d5096cb8a5d544acb51f8b4df908775b6584778f.

Reason for revert:

This looks like a likely culprit breaking WebKit Win10

  printing/absolute-position-headers-and-footers.html 
  virtual/threaded/printing/absolute-position-headers-and-footers.html

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/WebKit%20Win10

Original change's description:
> Don't estimate the top of bottom-aligned OOFs as bottom.
> 
> We'll incorrectly think that we get fragmented in that case, which will
> lead to incorrect height, which will lead to incorrect top offset,
> which will lead to incorrect fragmentation.
> 
> This has probably been the cause for a flaky web test, that behaved
> even worse in LayoutNG, because in NG we force legacy layout as soon as
> fragmentation is involved (so when entering printing, we typically need
> to rebuild the entire layout object tree, with no former height to base
> the bottom estimate on (which is what saved us when NG is disabled -
> ehm, at least most of the time)).
> 
> Bug:  910300 
> Change-Id: I8e152f77e9a94bb05e51f02a24be6ef5fe03c6f9
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
> Reviewed-on: https://chromium-review.googlesource.com/c/1355920
> Reviewed-by: Aleks Totic <atotic@chromium.org>
> Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#613930}

TBR=eae@chromium.org,atotic@chromium.org,mstensho@chromium.org

Change-Id: I5949a6a6e0b9e5cfa281b7abcd266ad63f7af85f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  910300 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/c/1363646
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Commit-Queue: Roger McFarlane <rogerm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614025}
[modify] https://crrev.com/2eb894c22bfe57d2145e5a7fed388dff7ddf39fd/third_party/blink/renderer/core/layout/layout_block.cc
[modify] https://crrev.com/2eb894c22bfe57d2145e5a7fed388dff7ddf39fd/third_party/blink/web_tests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/2eb894c22bfe57d2145e5a7fed388dff7ddf39fd/third_party/blink/web_tests/TestExpectations
[delete] https://crrev.com/65d4114d417ff6e5b4b917bd835c4e567d3c7d49/third_party/blink/web_tests/external/wpt/css/css-break/block-end-aligned-abspos-with-overflow-ref.html
[delete] https://crrev.com/65d4114d417ff6e5b4b917bd835c4e567d3c7d49/third_party/blink/web_tests/external/wpt/css/css-break/block-end-aligned-abspos-with-overflow.html
[delete] https://crrev.com/65d4114d417ff6e5b4b917bd835c4e567d3c7d49/third_party/blink/web_tests/external/wpt/css/css-break/block-end-aligned-abspos.html

Status: Assigned (was: Fixed)
Cc: jyasskin@google.com mstensho@chromium.org rogerm@chromium.org
 Issue 912314  has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18bc33e913c9e619b21daf050e6505df2a676cf1

commit 18bc33e913c9e619b21daf050e6505df2a676cf1
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Mon Dec 10 11:33:24 2018

Don't estimate the top of bottom-aligned OOFs as bottom.

(This is a reland of
https://chromium-review.googlesource.com/c/chromium/src/+/1355920 with
proper rebaselining)

We'll incorrectly think that we get fragmented in that case, which will
lead to incorrect height, which will lead to incorrect top offset,
which will lead to incorrect fragmentation.

This has probably been the cause for a flaky web test, that behaved
even worse in LayoutNG, because in NG we force legacy layout as soon as
fragmentation is involved (so when entering printing, we typically need
to rebuild the entire layout object tree, with no former height to base
the bottom estimate on (which is what saved us when NG is disabled -
ehm, at least most of the time)).

TBR=atotic@chromium.org,eae@chromium.org

Bug:  910300 
Change-Id: I00bce89900c50d6db9b39a4b72dbd6ab653184a7
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/c/1365436
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615089}
[modify] https://crrev.com/18bc33e913c9e619b21daf050e6505df2a676cf1/third_party/blink/renderer/core/layout/layout_block.cc
[modify] https://crrev.com/18bc33e913c9e619b21daf050e6505df2a676cf1/third_party/blink/web_tests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/18bc33e913c9e619b21daf050e6505df2a676cf1/third_party/blink/web_tests/TestExpectations
[add] https://crrev.com/18bc33e913c9e619b21daf050e6505df2a676cf1/third_party/blink/web_tests/external/wpt/css/css-break/block-end-aligned-abspos-with-overflow-ref.html
[add] https://crrev.com/18bc33e913c9e619b21daf050e6505df2a676cf1/third_party/blink/web_tests/external/wpt/css/css-break/block-end-aligned-abspos-with-overflow.html
[add] https://crrev.com/18bc33e913c9e619b21daf050e6505df2a676cf1/third_party/blink/web_tests/external/wpt/css/css-break/block-end-aligned-abspos.html
[delete] https://crrev.com/c74a57b141eb944e5911fbb0e16bbbf6ab6eeea9/third_party/blink/web_tests/platform/mac/virtual/threaded/printing/absolute-position-headers-and-footers-expected.png
[modify] https://crrev.com/18bc33e913c9e619b21daf050e6505df2a676cf1/third_party/blink/web_tests/platform/win/printing/absolute-position-headers-and-footers-expected.png
[delete] https://crrev.com/c74a57b141eb944e5911fbb0e16bbbf6ab6eeea9/third_party/blink/web_tests/platform/win7/printing/absolute-position-headers-and-footers-expected.png
[delete] https://crrev.com/c74a57b141eb944e5911fbb0e16bbbf6ab6eeea9/third_party/blink/web_tests/platform/win7/virtual/threaded/printing/absolute-position-headers-and-footers-expected.png
[delete] https://crrev.com/c74a57b141eb944e5911fbb0e16bbbf6ab6eeea9/third_party/blink/web_tests/virtual/threaded/printing/absolute-position-headers-and-footers-expected.png

Status: Fixed (was: Assigned)

Sign in to add a comment