Commit 6e17f939 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

Avoid falling back to legacy if |CanTraversePhysicalFragments|

This is a re-land of r824316 <crrev.com/c/2515429>, which was
reverted for <crbug.com/1146797>, with following changes in
|NGBoxFragmentPainter::HitTestChildBoxFragment|:
* When |!FragmentRequiresLegacyFallback|, check
  |IsPaintedAtomically| before |IsInline|. I'm not sure why we
  do so for legacy, but this was the root cause of the
  regression in <crbug.com/1146797>.
* No code changes when |FragmentRequiresLegacyFallback|.
* Test for crbug.com/1146797 paasses.

Original description follows:

When painting and hit-testing NG fragments,
|NGBoxFragmentPainter| falls back to |LayoutObject| at the BFC
boundary, because we already have a logic in |LayoutObject|
to switch painters for NG and legacy.

This patch changes to do so only when |!CanTraverse|, because
|LayoutObject| cannot handle block fragmented NG objects.

This patch eliminates |CurrentFragment| from
|LayoutNGBlockFlowMixin::Paint| by making sure it is not block
fragmented.

Also changes |HitTestChildBoxFragment| so that it can handle
atomically-painted objects in NG code path. Due to the change
in |FragmentRequiresLegacyFallback|, such objects go to NG
code path without going through |LayoutObject|.

Bug: 1061423, 829028
Change-Id: Ie795d5ab091b60535f4ee83ed548287e158cfaab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2526874Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827170}
parent 835a73ab
...@@ -218,6 +218,14 @@ void LayoutNGBlockFlowMixin<Base>::SetPaintFragment( ...@@ -218,6 +218,14 @@ void LayoutNGBlockFlowMixin<Base>::SetPaintFragment(
template <typename Base> template <typename Base>
void LayoutNGBlockFlowMixin<Base>::Paint(const PaintInfo& paint_info) const { void LayoutNGBlockFlowMixin<Base>::Paint(const PaintInfo& paint_info) const {
// When |this| is NG block fragmented, the painter should traverse fragments
// instead of |LayoutObject|, because this function cannot handle block
// fragmented objects. We can come here only when |this| cannot traverse
// fragments, or the parent is legacy.
DCHECK(!Base::CanTraversePhysicalFragments() ||
!Base::Parent()->CanTraversePhysicalFragments());
DCHECK_LE(Base::PhysicalFragmentCount(), 1u);
// Avoid painting dirty objects because descendants maybe already destroyed. // Avoid painting dirty objects because descendants maybe already destroyed.
if (UNLIKELY(Base::NeedsLayout() && if (UNLIKELY(Base::NeedsLayout() &&
!Base::ChildLayoutBlockedByDisplayLock())) { !Base::ChildLayoutBlockedByDisplayLock())) {
...@@ -226,7 +234,9 @@ void LayoutNGBlockFlowMixin<Base>::Paint(const PaintInfo& paint_info) const { ...@@ -226,7 +234,9 @@ void LayoutNGBlockFlowMixin<Base>::Paint(const PaintInfo& paint_info) const {
} }
if (UNLIKELY(RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled())) { if (UNLIKELY(RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled())) {
if (const NGPhysicalBoxFragment* fragment = CurrentFragment()) { if (Base::PhysicalFragmentCount()) {
const NGPhysicalBoxFragment* fragment = Base::GetPhysicalFragment(0);
DCHECK(fragment);
if (fragment->HasItems()) { if (fragment->HasItems()) {
NGBoxFragmentPainter(*fragment).Paint(paint_info); NGBoxFragmentPainter(*fragment).Paint(paint_info);
return; return;
...@@ -239,7 +249,9 @@ void LayoutNGBlockFlowMixin<Base>::Paint(const PaintInfo& paint_info) const { ...@@ -239,7 +249,9 @@ void LayoutNGBlockFlowMixin<Base>::Paint(const PaintInfo& paint_info) const {
return; return;
} }
if (const NGPhysicalBoxFragment* fragment = CurrentFragment()) { if (Base::PhysicalFragmentCount()) {
const NGPhysicalBoxFragment* fragment = Base::GetPhysicalFragment(0);
DCHECK(fragment);
NGBoxFragmentPainter(*fragment).Paint(paint_info); NGBoxFragmentPainter(*fragment).Paint(paint_info);
return; return;
} }
......
...@@ -231,10 +231,13 @@ bool HitTestCulledInlineAncestors( ...@@ -231,10 +231,13 @@ bool HitTestCulledInlineAncestors(
// |NGBoxFragmentPainter| for all fragments, and we still want this // |NGBoxFragmentPainter| for all fragments, and we still want this
// oprimization. // oprimization.
bool FragmentRequiresLegacyFallback(const NGPhysicalFragment& fragment) { bool FragmentRequiresLegacyFallback(const NGPhysicalFragment& fragment) {
// Fallback to LayoutObject if this is a root of NG block layout. // If |fragment| is |IsFormattingContextRoot|, it may be legacy.
// If this box is for this painter, LayoutNGBlockFlow will call this back. // Avoid falling back to |LayoutObject| if |CanTraverse|, because it
// Otherwise it calls legacy painters. // cannot handle block fragmented objects.
return fragment.IsFormattingContextRoot(); if (!fragment.IsFormattingContextRoot() || fragment.CanTraverse())
return false;
DCHECK(!To<NGPhysicalBoxFragment>(&fragment)->BreakToken());
return true;
} }
// Returns a vector of backplates that surround the paragraphs of text within // Returns a vector of backplates that surround the paragraphs of text within
...@@ -2200,10 +2203,12 @@ bool NGBoxFragmentPainter::HitTestChildBoxFragment( ...@@ -2200,10 +2203,12 @@ bool NGBoxFragmentPainter::HitTestChildBoxFragment(
return false; return false;
if (!FragmentRequiresLegacyFallback(fragment)) { if (!FragmentRequiresLegacyFallback(fragment)) {
DCHECK(!fragment.IsAtomicInline());
DCHECK(!fragment.IsFloating());
if (const NGPaintFragment* paint_fragment = if (const NGPaintFragment* paint_fragment =
backward_cursor.Current().PaintFragment()) { backward_cursor.Current().PaintFragment()) {
if (fragment.IsPaintedAtomically()) {
return HitTestAllPhasesInFragment(fragment, hit_test.location,
physical_offset, hit_test.result);
}
if (fragment.IsInlineBox()) { if (fragment.IsInlineBox()) {
return NGBoxFragmentPainter(*paint_fragment) return NGBoxFragmentPainter(*paint_fragment)
.NodeAtPoint(hit_test, physical_offset); .NodeAtPoint(hit_test, physical_offset);
...@@ -2214,6 +2219,10 @@ bool NGBoxFragmentPainter::HitTestChildBoxFragment( ...@@ -2214,6 +2219,10 @@ bool NGBoxFragmentPainter::HitTestChildBoxFragment(
.NodeAtPoint(*hit_test.result, hit_test.location, physical_offset, .NodeAtPoint(*hit_test.result, hit_test.location, physical_offset,
hit_test.action); hit_test.action);
} }
if (fragment.IsPaintedAtomically()) {
return HitTestAllPhasesInFragment(fragment, hit_test.location,
physical_offset, hit_test.result);
}
NGInlineCursor cursor(backward_cursor); NGInlineCursor cursor(backward_cursor);
const NGFragmentItem* item = cursor.Current().Item(); const NGFragmentItem* item = cursor.Current().Item();
DCHECK(item); DCHECK(item);
......
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