New issue
Advanced search Search tips

Issue 819372 link

Starred by 2 users

Issue metadata

Status: Closed
Owner:
Closed: Dec 3
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 812981



Sign in to add a comment

[LayoutNG] Implement invalidation task

Project Member Reported by atotic@chromium.org, Mar 6 2018

Issue description

Tracking bug for NG invalidation task. Invalidaton problems are:

* Paint invalidation not invalidating DisplayItem
* Stale style cached in style object
* Cacheing causing crashes

 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 8 2018

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

commit e2502ab0e1d1af2a4f70b485f5088b03efdfe748
Author: Aleks Totic <atotic@chromium.org>
Date: Thu Mar 08 23:26:03 2018

[LayoutNG] fix NGLayoutResult clone null bug

This happens when ng cacheing is enabled.

Following tests crash with Cache flag without the fix:
compositing/geometry/composited-in-columns.html
external/wpt/css/css-multicol/multicol-break-000.xht
external/wpt/css/css-multicol/multicol-break-001.xht
fast/block/float/floats-do-not-overhang-from-block-formatting-context.html
fast/block/positioning/offsetLeft-offsetTop-multicolumn.html
fast/css/getComputedStyle/getComputedStyle-zoom-and-background-size.html
fast/css-intrinsic-dimensions/multicol.html
fast/forms/number/number-spinbutton-in-multi-column.html
fast/forms/select/listbox-in-multi-column.html
fast/multicol/auto-height-forced-break-complex-margin-collapsing.html
fast/multicol/balance-break-inside-avoid.html
fast/multicol/balance-floats.html
fast/multicol/balance-short-trailing-empty-block.html
fast/multicol/balance-trailing-border-after-break.html
fast/multicol/balance-trailing-border.html
fast/multicol/balance-unbreakable.html
fast/multicol/break-after-always-bottom-margin.html
fast/multicol/break-before-first-line-in-first-child.html
fast/multicol/break-properties.html
fast/multicol/client-rects-rtl.html
fast/multicol/client-rects.html
fast/multicol/composited-layer.html
fast/multicol/cssom-view.html
fast/multicol/first-line-in-block-below-next-column-top.html
fast/multicol/first-line-in-block-with-padding-exact-fit.html
fast/multicol/first-line-in-block-with-padding.html
fast/multicol/first-line-in-float-below-next-column-top.html
fast/multicol/first-line-in-float-with-margin.html
fast/multicol/fixed-column-percent-logical-height-orthogonal-writing-mode.html
fast/multicol/fixedpos-in-transform-at-column-boundary.html
fast/multicol/flipped-blocks-hit-test.html
fast/multicol/float-beside-bfc.html
fast/multicol/float-moved-by-child-line-and-unbreakable.html
fast/multicol/float-with-margin-moved-by-child-block-and-unbreakable.html
fast/multicol/float-with-margin-moved-by-child-block.html
fast/multicol/float-with-margin-moved-by-child-line-and-unbreakable.html
fast/multicol/float-with-margin-moved-by-child-line.html
fast/multicol/float-with-margin-moved-unbreakable.html
fast/multicol/forced-break-before-complex-margin-collapsing.html
fast/multicol/forced-break-in-nested-columns.html
fast/multicol/hit-test-above-or-below.html
fast/multicol/hit-test-end-of-column-with-line-height.html
fast/multicol/hit-test-end-of-column.html
fast/multicol/hit-test-float.html
fast/multicol/hit-test-translate-z.html
fast/multicol/image-inside-nested-blocks-with-border.html
fast/multicol/inline-getclientrects.html
fast/multicol/inner-multicol-moved-into-continuation.html
fast/multicol/insane-column-count-and-padding-nested-crash.html
fast/multicol/margin-bottom-and-break-after.html
fast/multicol/min-height-greater-than-content.html
fast/multicol/min-height-greater-than-height.html
fast/multicol/min-height-less-than-content.html
fast/multicol/min-height-less-than-height.html
fast/multicol/min-height-much-greater-than-content.html
fast/multicol/nested-balancing-with-line-at-exact-top.html
fast/multicol/nested-balancing-with-lines-and-space-left-in-previous-row.html
fast/multicol/nested-inner-auto-height-outer-extra-space.html
fast/multicol/nested-one-line-in-inner.html
fast/multicol/nested-short-first-row-extra-tall-line.html
fast/multicol/nested-short-first-row-unsplittable-block.html
fast/multicol/nested-uneven-inner-column-height.html
fast/multicol/nested-with-forced-breaks-in-eariler-rows.html
fast/multicol/orphaned-line-at-exact-top-of-column.html
fast/multicol/orphans-relayout.html
fast/multicol/relayout-and-push-float.html
fast/multicol/shadow-breaking.html
fast/multicol/three-inner-rows.html
fast/multicol/unforced-break-after-complex-margin-collapsing.html
fast/multicol/widows-and-orphans.html
fast/multicol/widows.html
fast/multicol/widows2.html
fast/multicol/dynamic/bottom-aligned-abspos-change-column-height.html
fast/multicol/newmulticol/balance-images.html
fast/multicol/newmulticol/balance-maxheight1.html
fast/multicol/newmulticol/balance-maxheight2.html
fast/multicol/newmulticol/balance1.html
fast/multicol/newmulticol/balance10.html
fast/multicol/newmulticol/balance2.html
fast/multicol/newmulticol/balance3.html
fast/multicol/newmulticol/balance4.html
fast/multicol/newmulticol/balance5.html
fast/multicol/newmulticol/balance6.html
fast/multicol/newmulticol/balance7.html
fast/multicol/newmulticol/balance8.html
fast/multicol/newmulticol/balance9.html
fast/multicol/newmulticol/orphans-and-widows-balance.html
fast/multicol/span/autofill-after-spanner.html
fast/multicol/vertical-lr/break-properties.html
fast/multicol/vertical-lr/balancing/balance-short-trailing-empty-block.html
fast/multicol/vertical-lr/balancing/balance-trailing-border-after-break.html
fast/multicol/vertical-lr/balancing/balance-trailing-border.html
fast/multicol/vertical-lr/balancing/balance-unbreakable.html
fast/multicol/vertical-rl/break-properties.html
fast/multicol/vertical-rl/balancing/balance-short-trailing-empty-block.html
fast/multicol/vertical-rl/balancing/balance-trailing-border-after-break.html
fast/multicol/vertical-rl/balancing/balance-trailing-border.html
fast/multicol/vertical-rl/balancing/balance-unbreakable.html
fast/pagination/multicol.html
fragmentation/abspos-after-forced-break.html
fragmentation/avoid-break-inside-first-child-nested.html
fragmentation/avoid-break-inside-first-child.html
fragmentation/block-with-float-and-1-orphaned-line.html
fragmentation/break-after-last-child.html
fragmentation/break-before-empty-child-block.html
fragmentation/break-before-first-child.html
fragmentation/break-in-caption-before-tbody.html
fragmentation/break-in-first-table-row-only.html
fragmentation/break-in-first-table-section.html
fragmentation/break-in-second-table-section.html
fragmentation/break-in-tbody-after-caption.html
fragmentation/break-inside-avoid-with-forced-break.html
fragmentation/cell-taller-than-col-straddles-columns.html
fragmentation/class-c-break-after-clearance.html
fragmentation/collapsing-class-a-breakpoints.html
fragmentation/float-after-forced-break.html
fragmentation/image-block-as-first-child.html
fragmentation/multi-line-cells.html
fragmentation/repeating-thead-exceeds-page-size.html
fragmentation/repeating-thead-no-room-for-content-row-on-first-page.html
fragmentation/single-cell-too-large-for-page.html
fragmentation/single-cells-multiple-tables-no-repeating-thead.html
fragmentation/single-line-cells-in-multiple-table-sections.html
fragmentation/single-line-cells.html
fragmentation/table-in-subpixel-fragmentainer.html
fragmentation/table-overlapping-rowspan.html
fragmentation/table-row-dimensions-break-freely.html
fragmentation/table-row-dimensions-with-thead.html
fragmentation/table-row-dimensions.html
fragmentation/tbody-before-thead.html
paint/invalidation/overflow/paged-with-overflowing-block-rl.html
paint/pagination/pagination-change-clip-crash.html
printing/multicol.html
printing/pageProperty-with-multicol.html

Bug:  819372 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I5df07ebf451527a640d7a5f4125d10a3e7441bc8
Reviewed-on: https://chromium-review.googlesource.com/952262
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541931}
[modify] https://crrev.com/e2502ab0e1d1af2a4f70b485f5088b03efdfe748/third_party/WebKit/Source/core/layout/ng/ng_layout_result.cc

Blocking: 812981
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 13 2018

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

commit fe3d571e7612763137a9e0d36c23fe921426af4d
Author: Aleks Totic <atotic@chromium.org>
Date: Tue Mar 13 00:22:57 2018

[LayoutNG] Fix stale style on a fragment after paint invalidation

This makes ~50 additional tests to pass, and 3 to fail.

Paint invalidation can cause styles on a NGPhysicalFragment to
get out of sync with LayoutObject styles.

There were two ways to fix this, I found a) to be cleaner for now.

a) Make PhysicalFragments fetch style from LayoutObject
Tricky part here is that :first-line and overflow:ellipsis get
special styles.

b) Rebuild PhysicalFragment tree with a shallow clonewithstyle
I was unable to find a clean way to trigger tree rebuild here.
Sample experiment to convert paint invalidations into
layout invalidations triggered 100s DCHECK(!NeedsLayout())
Possible fix would be to trap all places that call NeedsLayout()
with intent to relayout, and replace them with NeedsLayout() ||
NeedsNGLayout()

This does not fix all invalidation bugs. DisplayItems outside of
LayoutObject tree do not get invalidated correctly.

fast/css/first-letter-hover.html
fast/history/visited-link-hover-emphasis-color.html

Bug:  819372 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ia11f8288b8f45423762c870a43615707d6a0cc93
Reviewed-on: https://chromium-review.googlesource.com/956552
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542664}
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_item.cc
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_item.h
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/inline/ng_physical_text_fragment.cc
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/inline/ng_physical_text_fragment.h
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/inline/ng_text_fragment_builder.cc
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/inline/ng_text_fragment_builder.h
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/ng_base_fragment_builder.cc
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/ng_base_fragment_builder.h
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/ng_physical_container_fragment.cc
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/ng_physical_fragment.cc
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/ng_physical_fragment.h
[add] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/layout/ng/ng_style_variant.h
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/paint/ng/ng_box_fragment_painter.cc
[modify] https://crrev.com/fe3d571e7612763137a9e0d36c23fe921426af4d/third_party/WebKit/Source/core/paint/ng/ng_fragment_painter.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 17 2018

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

commit b18ebe265c2d059934346aaaea283d9655b555b1
Author: Aleks Totic <atotic@chromium.org>
Date: Sat Mar 17 00:39:49 2018

[LayoutNG] Fix paint invalidation of LayoutText

Root cause of missing invalidations:
LayoutObject::AdjustStyleDifference was not marking LayoutText with NG rendering
as NeedPaintInvalidation. This caused invalidation not to propagate to document.
It was not marked because
LayoutObject:1789 ToLayoutText(this)->HasTextBoxes() always returned false for NG.

The fix was to make HasTextBoxes() NG-aware. This caused 2 callers
of HasTextBoxes() to behave incorrectly. Created HasLegacyTextBoxes(),
and added a TODO to remove them.

Bug:  819372 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: If2c4ddc0c9c707d11bb9115ef26ac578387c8359
Reviewed-on: https://chromium-review.googlesource.com/961526
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543885}
[modify] https://crrev.com/b18ebe265c2d059934346aaaea283d9655b555b1/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/b18ebe265c2d059934346aaaea283d9655b555b1/third_party/WebKit/Source/core/layout/LayoutText.cpp
[modify] https://crrev.com/b18ebe265c2d059934346aaaea283d9655b555b1/third_party/WebKit/Source/core/layout/LayoutText.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 2

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

commit 1a0145ed069fd05bae17caf84a7cacc32127af59
Author: Aleks Totic <atotic@chromium.org>
Date: Mon Jul 02 20:50:21 2018

[LayoutNG] Rebaseline test that has gotten better

Bug:  819372 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I8e1dfc647673327587fbc5505488a68ebc62e139
Reviewed-on: https://chromium-review.googlesource.com/1123170
Commit-Queue: Aleks Totic <atotic@chromium.org>
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571990}
[modify] https://crrev.com/1a0145ed069fd05bae17caf84a7cacc32127af59/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/external/wpt/html/dom/elements/the-innertext-idl-attribute/getter-expected.txt

Status: Closed (was: Assigned)
There is no outstanding work being done for this tracking bug.

Sign in to add a comment