Commit ba8c7800 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[LayoutNG] Check consistency between NGPhysicalFragment and LayoutObject

This patch adds DCHECKs to check consistency between
NGPhysicalFragment and LayoutObject in construction, when
read from cache, and when NGPaintFragment is constructed.

These DCHECKs should catch when we have old fragments after
LayoutObject was changed, such as r671460
(crrev.com/c/1670146).

Change-Id: I65cb1d9080ca79d6ce904fd544500ab6d5909207
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1669916Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671937}
parent cbca4e1e
...@@ -2448,6 +2448,8 @@ scoped_refptr<const NGLayoutResult> LayoutBox::CachedLayoutResult( ...@@ -2448,6 +2448,8 @@ scoped_refptr<const NGLayoutResult> LayoutBox::CachedLayoutResult(
if (*out_cache_status == NGLayoutCacheStatus::kNeedsSimplifiedLayout) if (*out_cache_status == NGLayoutCacheStatus::kNeedsSimplifiedLayout)
return nullptr; return nullptr;
physical_fragment.CheckType();
DCHECK_EQ(*out_cache_status, NGLayoutCacheStatus::kHit); DCHECK_EQ(*out_cache_status, NGLayoutCacheStatus::kHit);
// We can safely re-use this fragment if we are positioned, and only our // We can safely re-use this fragment if we are positioned, and only our
......
...@@ -261,6 +261,7 @@ scoped_refptr<const NGLayoutResult> NGBoxFragmentBuilder::ToBoxFragment( ...@@ -261,6 +261,7 @@ scoped_refptr<const NGLayoutResult> NGBoxFragmentBuilder::ToBoxFragment(
scoped_refptr<const NGPhysicalBoxFragment> fragment = scoped_refptr<const NGPhysicalBoxFragment> fragment =
NGPhysicalBoxFragment::Create(this, block_or_line_writing_mode); NGPhysicalBoxFragment::Create(this, block_or_line_writing_mode);
fragment->CheckType();
return base::AdoptRef(new NGLayoutResult(std::move(fragment), this)); return base::AdoptRef(new NGLayoutResult(std::move(fragment), this));
} }
......
...@@ -357,6 +357,58 @@ const NGPhysicalFragment* NGPhysicalFragment::PostLayout() const { ...@@ -357,6 +357,58 @@ const NGPhysicalFragment* NGPhysicalFragment::PostLayout() const {
} }
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
void NGPhysicalFragment::CheckType() const {
switch (Type()) {
case kFragmentBox:
case kFragmentRenderedLegend:
if (IsInlineBox()) {
DCHECK(layout_object_.IsLayoutInline());
} else {
DCHECK(layout_object_.IsBox());
}
if (IsColumnBox()) {
// TODO(kojii): Column box can fail following checks, needs review.
break;
}
if (layout_object_.IsLayoutNGListMarker()) {
// List marker is an atomic inline if it appears in a line box, or a
// block box.
DCHECK(!IsFloating());
DCHECK(!IsOutOfFlowPositioned());
DCHECK(IsAtomicInline() || (IsBox() && BoxType() == kBlockFlowRoot));
break;
}
DCHECK_EQ(IsFloating(), layout_object_.IsFloating());
DCHECK_EQ(IsOutOfFlowPositioned(),
layout_object_.IsOutOfFlowPositioned());
DCHECK_EQ(IsAtomicInline(), layout_object_.IsInline() &&
layout_object_.IsAtomicInlineLevel());
break;
case kFragmentText:
if (To<NGPhysicalTextFragment>(this)->IsGeneratedText()) {
// Ellipsis has the truncated in-flow LayoutObject.
DCHECK(layout_object_.IsText() ||
(layout_object_.IsInline() &&
layout_object_.IsAtomicInlineLevel()) ||
layout_object_.IsLayoutInline());
} else {
DCHECK(layout_object_.IsText());
}
DCHECK(!IsFloating());
DCHECK(!IsOutOfFlowPositioned());
DCHECK(!IsInlineBox());
DCHECK(!IsAtomicInline());
break;
case kFragmentLineBox:
DCHECK(layout_object_.IsLayoutBlockFlow());
DCHECK(!IsFloating());
DCHECK(!IsOutOfFlowPositioned());
DCHECK(!IsInlineBox());
DCHECK(!IsAtomicInline());
break;
}
}
void NGPhysicalFragment::CheckCanUpdateInkOverflow() const { void NGPhysicalFragment::CheckCanUpdateInkOverflow() const {
if (!GetLayoutObject()) if (!GetLayoutObject())
return; return;
......
...@@ -242,6 +242,7 @@ class CORE_EXPORT NGPhysicalFragment ...@@ -242,6 +242,7 @@ class CORE_EXPORT NGPhysicalFragment
String ToString() const; String ToString() const;
void CheckType() const;
void CheckCanUpdateInkOverflow() const; void CheckCanUpdateInkOverflow() const;
enum DumpFlag { enum DumpFlag {
...@@ -335,6 +336,7 @@ struct CORE_EXPORT NGPhysicalFragmentWithOffset { ...@@ -335,6 +336,7 @@ struct CORE_EXPORT NGPhysicalFragmentWithOffset {
}; };
#if !DCHECK_IS_ON() #if !DCHECK_IS_ON()
inline void NGPhysicalFragment::CheckType() const {}
inline void NGPhysicalFragment::CheckCanUpdateInkOverflow() const {} inline void NGPhysicalFragment::CheckCanUpdateInkOverflow() const {}
#endif #endif
......
...@@ -415,6 +415,8 @@ void NGPaintFragment::PopulateDescendants(CreateContext* parent_context) { ...@@ -415,6 +415,8 @@ void NGPaintFragment::PopulateDescendants(CreateContext* parent_context) {
!box_physical_fragment || box_physical_fragment->ChildrenInline(); !box_physical_fragment || box_physical_fragment->ChildrenInline();
for (const NGLink& child_fragment : container.Children()) { for (const NGLink& child_fragment : container.Children()) {
child_fragment->CheckType();
// OOF objects are not needed because they always have self painting layer. // OOF objects are not needed because they always have self painting layer.
if (UNLIKELY(child_fragment->IsOutOfFlowPositioned())) if (UNLIKELY(child_fragment->IsOutOfFlowPositioned()))
continue; continue;
......
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