Commit 696fd3af authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

[LayoutNG] Turn AssertLaidOut into a CHECK for LayoutNG round 4

This reverts commit 0af2a61e.

Reason for revert: Need another round of crash capturing.

Original change's description:
> Revert "Reland "[LayoutNG] Turn AssertLaidOut into a CHECK for LayoutNG" round 3"
> 
> This reverts commit cbdaf211.
> 
> Reason for revert: Crash data already collected in 3763 and 3764
> 
> Original change's description:
> > Reland "[LayoutNG] Turn AssertLaidOut into a CHECK for LayoutNG" round 3
> > 
> > This reverts commit 33b5c673.
> > 
> > Reason for revert: Adding production CHECKs to collect LayoutNG failures
> > for the 3rd time.
> > 
> > Original change's description:
> > > Revert "Reland "[LayoutNG] Turn AssertLaidOut into a CHECK for LayoutNG""
> > > 
> > > This reverts commit 26ca019a.
> > > 
> > > Reason for revert: Crash data already collected
> > > 
> > > Original change's description:
> > > > Reland "[LayoutNG] Turn AssertLaidOut into a CHECK for LayoutNG"
> > > > 
> > > > This reverts commit ec11fe81.
> > > > 
> > > > Reason for revert: Try to catch more crashes in the wild after
> > > > a fix (crrev.com/c/1546713) has been landed.
> > > > 
> > > > Original change's description:
> > > > > Revert "[LayoutNG] Turn AssertLaidOut into a CHECK for LayoutNG"
> > > > > 
> > > > > This reverts commit bade42c5.
> > > > > 
> > > > > Reason for revert: New crashes already caught. Reverting CHECKs
> > > > > back into DCHECKs to reduce noises.
> > > > > 
> > > > > Original change's description:
> > > > > > [LayoutNG] Turn AssertLaidOut into a CHECK for LayoutNG
> > > > > > 
> > > > > > Bug: 946004
> > > > > > Change-Id: I9e2aa69a32f7a0373196d57e629256dbc7d78d2a
> > > > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1540305
> > > > > > Reviewed-by: Koji Ishii <kojii@chromium.org>
> > > > > > Reviewed-by: Emil A Eklund <eae@chromium.org>
> > > > > > Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> > > > > > Cr-Commit-Position: refs/heads/master@{#644907}
> > > > > 
> > > > > TBR=eae@chromium.org,kojii@chromium.org,xiaochengh@chromium.org
> > > > > 
> > > > > Change-Id: I15fb43370f58757e09e7b7abc7c40d4af4a6101a
> > > > > No-Presubmit: true
> > > > > No-Tree-Checks: true
> > > > > No-Try: true
> > > > > Bug: 946004
> > > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1544156
> > > > > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> > > > > Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> > > > > Cr-Commit-Position: refs/heads/master@{#645407}
> > > > 
> > > > TBR=eae@chromium.org,kojii@chromium.org,xiaochengh@chromium.org
> > > > 
> > > > # Not skipping CQ checks because original CL landed > 1 day ago.
> > > > 
> > > > Bug: 946004
> > > > Change-Id: Iccb8c2b998d28f659381c3f89a1686614600860c
> > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1548248
> > > > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> > > > Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> > > > Cr-Commit-Position: refs/heads/master@{#646516}
> > > 
> > > TBR=eae@chromium.org,kojii@chromium.org,xiaochengh@chromium.org
> > > 
> > > # Not skipping CQ checks because original CL landed > 1 day ago.
> > > 
> > > Bug: 946004
> > > Change-Id: I9794539b39bc6a0053a7229b03bce7db402ecb5b
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1551440
> > > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> > > Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#647418}
> > 
> > TBR=eae@chromium.org,kojii@chromium.org,xiaochengh@chromium.org
> > 
> > # Not skipping CQ checks because original CL landed > 1 day ago.
> > 
> > Bug: 946004
> > Change-Id: I973ccf65be78d4632f67b945614f1a4ee091134d
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1562500
> > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> > Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#649773}
> 
> TBR=eae@chromium.org,kojii@chromium.org,xiaochengh@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 946004
> Change-Id: If61d1cbfaeb6a445b8ceee193dba198978d2719c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1567130
> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#650624}

TBR=eae@chromium.org,kojii@chromium.org,xiaochengh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 946004
Change-Id: I43e61cc397ebbb44fb15f9a5bc0448077c281f60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1600400Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657538}
parent 4d645147
...@@ -941,11 +941,11 @@ void LocalFrameView::UpdateLayout() { ...@@ -941,11 +941,11 @@ void LocalFrameView::UpdateLayout() {
if (nested_layout_count_) if (nested_layout_count_)
return; return;
#if DCHECK_IS_ON() // TODO(crbug.com/946004): Move the following line into a DCHECK_IS_ON() block
// when we no longer see missing layout reports in LayoutNG.
// Post-layout assert that nobody was re-marked as needing layout during // Post-layout assert that nobody was re-marked as needing layout during
// layout. // layout.
GetLayoutView()->AssertSubtreeIsLaidOut(); GetLayoutView()->AssertSubtreeIsLaidOut();
#endif
if (frame_->IsMainFrame()) { if (frame_->IsMainFrame()) {
// Scrollbars changing state can cause a visual viewport size change. // Scrollbars changing state can cause a visual viewport size change.
...@@ -2887,9 +2887,9 @@ void LocalFrameView::UpdateStyleAndLayoutIfNeededRecursive() { ...@@ -2887,9 +2887,9 @@ void LocalFrameView::UpdateStyleAndLayoutIfNeededRecursive() {
// These asserts ensure that parent frames are clean, when child frames // These asserts ensure that parent frames are clean, when child frames
// finished updating layout and style. // finished updating layout and style.
CheckDoesNotNeedLayout(); CheckDoesNotNeedLayout();
#if DCHECK_IS_ON() // TODO(crbug.com/946004): Move the following line into a DCHECK_IS_ON() block
// when we no longer see missing layout reports in LayoutNG.
frame_->GetDocument()->GetLayoutView()->AssertLaidOut(); frame_->GetDocument()->GetLayoutView()->AssertLaidOut();
#endif
UpdateGeometriesIfNeeded(); UpdateGeometriesIfNeeded();
......
...@@ -392,6 +392,11 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver, ...@@ -392,6 +392,11 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
LayoutObject& layout_object_; LayoutObject& layout_object_;
bool preexisting_forbidden_; bool preexisting_forbidden_;
}; };
#endif // DCHECK_IS_ON()
// TODO(crbug.com/946004): When we no longer see missing layouts in LayoutNG,
// move the following two functions back to the DCHECK_IS_ON() block and
// remove relevant branches and CHECK.
void AssertLaidOut() const { void AssertLaidOut() const {
#ifndef NDEBUG #ifndef NDEBUG
...@@ -401,9 +406,18 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver, ...@@ -401,9 +406,18 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
#endif #endif
SECURITY_DCHECK(!NeedsLayout() || SECURITY_DCHECK(!NeedsLayout() ||
LayoutBlockedByDisplayLock(DisplayLockContext::kChildren)); LayoutBlockedByDisplayLock(DisplayLockContext::kChildren));
if (!RuntimeEnabledFeatures::LayoutNGEnabled())
return;
CHECK(!NeedsLayout() ||
LayoutBlockedByDisplayLock(DisplayLockContext::kChildren))
<< this;
} }
void AssertSubtreeIsLaidOut() const { void AssertSubtreeIsLaidOut() const {
#if !DCHECK_IS_ON()
if (!RuntimeEnabledFeatures::LayoutNGEnabled())
return;
#endif
for (const LayoutObject* layout_object = this; layout_object; for (const LayoutObject* layout_object = this; layout_object;
layout_object = layout_object->LayoutBlockedByDisplayLock( layout_object = layout_object->LayoutBlockedByDisplayLock(
DisplayLockContext::kChildren) DisplayLockContext::kChildren)
...@@ -413,6 +427,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver, ...@@ -413,6 +427,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
} }
} }
#if DCHECK_IS_ON()
void AssertClearedPaintInvalidationFlags() const { void AssertClearedPaintInvalidationFlags() const {
#ifndef NDEBUG #ifndef NDEBUG
if (PaintInvalidationStateIsDirty() && !PrePaintBlockedByDisplayLock()) { if (PaintInvalidationStateIsDirty() && !PrePaintBlockedByDisplayLock()) {
...@@ -431,7 +446,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver, ...@@ -431,7 +446,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
} }
} }
#endif #endif // DCHECK_IS_ON()
// LayoutObject tree manipulation // LayoutObject tree manipulation
////////////////////////////////////////// //////////////////////////////////////////
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment