Commit 09ee8b8e authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

Revert "Avoid falling back to legacy if |CanTraversePhysicalFragments|"

This reverts commit 2493e000.

Reason for revert: Hit test regression at crbug.com/1146797

Original change's description:
> Avoid falling back to legacy if |CanTraversePhysicalFragments|
>
> 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: Ife31d6dcd209668a489243b1158112dac44fc273
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515429
> Commit-Queue: Koji Ishii <kojii@chromium.org>
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#824316}

TBR=yosin@chromium.org,tkent@chromium.org,kojii@chromium.org,mstensho@chromium.org

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

Bug: 1146797, 1146935, 829028, 1061423
Change-Id: Id228899a56bbcdd118a860e8e73ecb18a9b0d96b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2525882
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825301}
parent 2f25a632
...@@ -215,14 +215,6 @@ void LayoutNGBlockFlowMixin<Base>::SetPaintFragment( ...@@ -215,14 +215,6 @@ 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())) {
...@@ -231,9 +223,7 @@ void LayoutNGBlockFlowMixin<Base>::Paint(const PaintInfo& paint_info) const { ...@@ -231,9 +223,7 @@ void LayoutNGBlockFlowMixin<Base>::Paint(const PaintInfo& paint_info) const {
} }
if (UNLIKELY(RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled())) { if (UNLIKELY(RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled())) {
if (Base::PhysicalFragmentCount()) { if (const NGPhysicalBoxFragment* fragment = CurrentFragment()) {
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;
...@@ -246,9 +236,7 @@ void LayoutNGBlockFlowMixin<Base>::Paint(const PaintInfo& paint_info) const { ...@@ -246,9 +236,7 @@ void LayoutNGBlockFlowMixin<Base>::Paint(const PaintInfo& paint_info) const {
return; return;
} }
if (Base::PhysicalFragmentCount()) { if (const NGPhysicalBoxFragment* fragment = CurrentFragment()) {
const NGPhysicalBoxFragment* fragment = Base::GetPhysicalFragment(0);
DCHECK(fragment);
NGBoxFragmentPainter(*fragment).Paint(paint_info); NGBoxFragmentPainter(*fragment).Paint(paint_info);
return; return;
} }
......
...@@ -231,13 +231,10 @@ bool HitTestCulledInlineAncestors( ...@@ -231,13 +231,10 @@ 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) {
// If |fragment| is |IsFormattingContextRoot|, it may be legacy. // Fallback to LayoutObject if this is a root of NG block layout.
// Avoid falling back to |LayoutObject| if |CanTraverse|, because it // If this box is for this painter, LayoutNGBlockFlow will call this back.
// cannot handle block fragmented objects. // Otherwise it calls legacy painters.
if (!fragment.IsFormattingContextRoot() || fragment.CanTraverse()) return fragment.IsFormattingContextRoot();
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
...@@ -2201,15 +2198,9 @@ bool NGBoxFragmentPainter::HitTestChildBoxFragment( ...@@ -2201,15 +2198,9 @@ bool NGBoxFragmentPainter::HitTestChildBoxFragment(
if (fragment.IsFloating() && hit_test.action != kHitTestFloat) if (fragment.IsFloating() && hit_test.action != kHitTestFloat)
return false; return false;
if (fragment.IsInline() && hit_test.action != kHitTestForeground)
return false;
if (fragment.IsPaintedAtomically()) {
return HitTestAllPhasesInFragment(fragment, hit_test.location,
physical_offset, hit_test.result);
}
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.IsInlineBox()) { if (fragment.IsInlineBox()) {
...@@ -2237,6 +2228,14 @@ bool NGBoxFragmentPainter::HitTestChildBoxFragment( ...@@ -2237,6 +2228,14 @@ bool NGBoxFragmentPainter::HitTestChildBoxFragment(
hit_test.action); hit_test.action);
} }
if (fragment.IsInline() && hit_test.action != kHitTestForeground)
return false;
if (fragment.IsPaintedAtomically()) {
return HitTestAllPhasesInFragment(fragment, hit_test.location,
physical_offset, hit_test.result);
}
return fragment.GetMutableLayoutObject()->NodeAtPoint( return fragment.GetMutableLayoutObject()->NodeAtPoint(
*hit_test.result, hit_test.location, physical_offset, hit_test.action); *hit_test.result, hit_test.location, physical_offset, hit_test.action);
} }
......
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