Commit 5f5cb637 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[FragmentItem] Fix NGOutlineUtils::ShouldPaintOutline

This patch changes |NGOutlineUtils::ShouldPaintOutline| to
support FragmentItem. This fix is needed to compute ink
overflow (WIP crrev.com/c/1771360).

This patch also discovered a bug in |NGInlineCursor| where
|MoveTo(const LayoutObject&)| does not work if the root is
not given, when FragmentItem is enabled. This patch adds a
test for the case and fix this.

This patch fixes ~260 tests and regresses ~80. The ~80 used
to pass accidentally, revealing these failures is a progress.
I will update FlagExpectations separately to avoid conflict
as the change is large.

Bug: 982194
Change-Id: Ia4cf6b15624c0ef73bb90c76a57ef326d19c5d26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1890374Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711227}
parent 327bf7bb
...@@ -23,8 +23,7 @@ void NGInlineCursor::MoveToItem(const ItemsSpan::iterator& iter) { ...@@ -23,8 +23,7 @@ void NGInlineCursor::MoveToItem(const ItemsSpan::iterator& iter) {
void NGInlineCursor::SetRoot(const NGFragmentItems& fragment_items, void NGInlineCursor::SetRoot(const NGFragmentItems& fragment_items,
ItemsSpan items) { ItemsSpan items) {
DCHECK(items.data() || !items.size()); DCHECK(items.data() || !items.size());
DCHECK_EQ(root_paint_fragment_, nullptr); DCHECK(!HasRoot());
DCHECK_EQ(current_paint_fragment_, nullptr);
fragment_items_ = &fragment_items; fragment_items_ = &fragment_items;
items_ = items; items_ = items;
MoveToItem(items_.begin()); MoveToItem(items_.begin());
...@@ -36,16 +35,17 @@ void NGInlineCursor::SetRoot(const NGFragmentItems& items) { ...@@ -36,16 +35,17 @@ void NGInlineCursor::SetRoot(const NGFragmentItems& items) {
void NGInlineCursor::SetRoot(const NGPaintFragment& root_paint_fragment) { void NGInlineCursor::SetRoot(const NGPaintFragment& root_paint_fragment) {
DCHECK(&root_paint_fragment); DCHECK(&root_paint_fragment);
DCHECK(!HasRoot());
root_paint_fragment_ = &root_paint_fragment; root_paint_fragment_ = &root_paint_fragment;
current_paint_fragment_ = root_paint_fragment.FirstChild(); current_paint_fragment_ = root_paint_fragment.FirstChild();
} }
NGInlineCursor::NGInlineCursor(const LayoutBlockFlow& block_flow) { void NGInlineCursor::SetRoot(const LayoutBlockFlow& block_flow) {
DCHECK(&block_flow); DCHECK(&block_flow);
DCHECK(!HasRoot());
if (const NGPhysicalBoxFragment* fragment = block_flow.CurrentFragment()) { if (const NGPhysicalBoxFragment* fragment = block_flow.CurrentFragment()) {
if (const NGFragmentItems* items = fragment->Items()) { if (const NGFragmentItems* items = fragment->Items()) {
fragment_items_ = items;
SetRoot(*items); SetRoot(*items);
return; return;
} }
...@@ -61,6 +61,10 @@ NGInlineCursor::NGInlineCursor(const LayoutBlockFlow& block_flow) { ...@@ -61,6 +61,10 @@ NGInlineCursor::NGInlineCursor(const LayoutBlockFlow& block_flow) {
// See external/wpt/css/css-scroll-anchoring/wrapped-text.html // See external/wpt/css/css-scroll-anchoring/wrapped-text.html
} }
NGInlineCursor::NGInlineCursor(const LayoutBlockFlow& block_flow) {
SetRoot(block_flow);
}
NGInlineCursor::NGInlineCursor(const NGFragmentItems& fragment_items, NGInlineCursor::NGInlineCursor(const NGFragmentItems& fragment_items,
ItemsSpan items) { ItemsSpan items) {
SetRoot(fragment_items, items); SetRoot(fragment_items, items);
...@@ -526,18 +530,29 @@ void NGInlineCursor::InternalMoveTo(const LayoutObject& layout_object) { ...@@ -526,18 +530,29 @@ void NGInlineCursor::InternalMoveTo(const LayoutObject& layout_object) {
void NGInlineCursor::MoveTo(const LayoutObject& layout_object) { void NGInlineCursor::MoveTo(const LayoutObject& layout_object) {
DCHECK(layout_object.IsInLayoutNGInlineFormattingContext()) << layout_object; DCHECK(layout_object.IsInLayoutNGInlineFormattingContext()) << layout_object;
InternalMoveTo(layout_object); InternalMoveTo(layout_object);
if (IsNotNull() || !layout_object.IsLayoutInline()) { if (*this) {
layout_inline_ = nullptr; layout_inline_ = nullptr;
return; return;
} }
// In case of |layout_object| is cullred inline.
if (!fragment_items_ && !root_paint_fragment_) { // This |layout_object| did not produce any fragments.
root_paint_fragment_ = //
layout_object.RootInlineFormattingContext()->PaintFragment(); // Try to find ancestors if this is a culled inline.
if (!root_paint_fragment_) layout_inline_ = ToLayoutInlineOrNull(&layout_object);
return MakeNull(); if (!layout_inline_)
return;
// If this cursor is rootless, find the root of the inline formatting context.
if (!HasRoot()) {
const LayoutBlockFlow* root = layout_object.RootInlineFormattingContext();
DCHECK(root);
SetRoot(*root);
if (!HasRoot()) {
DCHECK(IsNull());
return;
}
} }
layout_inline_ = ToLayoutInline(&layout_object);
MoveToFirst(); MoveToFirst();
while (IsNotNull() && !IsInclusiveDescendantOf(layout_object)) while (IsNotNull() && !IsInclusiveDescendantOf(layout_object))
MoveToNext(); MoveToNext();
......
...@@ -47,6 +47,10 @@ class CORE_EXPORT NGInlineCursor { ...@@ -47,6 +47,10 @@ class CORE_EXPORT NGInlineCursor {
ItemsSpan items); ItemsSpan items);
explicit NGInlineCursor(const NGPaintFragment& root_paint_fragment); explicit NGInlineCursor(const NGPaintFragment& root_paint_fragment);
NGInlineCursor(const NGInlineCursor& other); NGInlineCursor(const NGInlineCursor& other);
// Creates an |NGInlineCursor| without the root. Even when callers don't know
// the root of the inline formatting context, this cursor can |MoveTo()|
// specific |LayoutObject|.
NGInlineCursor(); NGInlineCursor();
bool operator==(const NGInlineCursor& other) const; bool operator==(const NGInlineCursor& other) const;
...@@ -57,6 +61,10 @@ class CORE_EXPORT NGInlineCursor { ...@@ -57,6 +61,10 @@ class CORE_EXPORT NGInlineCursor {
bool IsItemCursor() const { return fragment_items_; } bool IsItemCursor() const { return fragment_items_; }
bool IsPaintFragmentCursor() const { return root_paint_fragment_; } bool IsPaintFragmentCursor() const { return root_paint_fragment_; }
// True if this cursor has the root to traverse. Only the default constructor
// creates a cursor without the root.
bool HasRoot() const { return IsItemCursor() || IsPaintFragmentCursor(); }
const NGFragmentItems& Items() const { const NGFragmentItems& Items() const {
DCHECK(fragment_items_); DCHECK(fragment_items_);
return *fragment_items_; return *fragment_items_;
...@@ -265,6 +273,7 @@ class CORE_EXPORT NGInlineCursor { ...@@ -265,6 +273,7 @@ class CORE_EXPORT NGInlineCursor {
void SetRoot(const NGFragmentItems& items); void SetRoot(const NGFragmentItems& items);
void SetRoot(const NGFragmentItems& fragment_items, ItemsSpan items); void SetRoot(const NGFragmentItems& fragment_items, ItemsSpan items);
void SetRoot(const NGPaintFragment& root_paint_fragment); void SetRoot(const NGPaintFragment& root_paint_fragment);
void SetRoot(const LayoutBlockFlow& block_flow);
void MoveToItem(const ItemsSpan::iterator& iter); void MoveToItem(const ItemsSpan::iterator& iter);
void MoveToNextItem(); void MoveToNextItem();
......
...@@ -98,7 +98,7 @@ TEST_P(NGInlineCursorTest, ContainingLine) { ...@@ -98,7 +98,7 @@ TEST_P(NGInlineCursorTest, ContainingLine) {
EXPECT_EQ(line2, cursor); EXPECT_EQ(line2, cursor);
} }
TEST_P(NGInlineCursorTest, CulledInline) { TEST_P(NGInlineCursorTest, CulledInlineWithRoot) {
NGInlineCursor cursor = NGInlineCursor cursor =
SetupCursor("<div id=root><a><b>abc</b><br><i>xyz</i></a></div>"); SetupCursor("<div id=root><a><b>abc</b><br><i>xyz</i></a></div>");
const LayoutInline& layout_inline = const LayoutInline& layout_inline =
...@@ -112,6 +112,21 @@ TEST_P(NGInlineCursorTest, CulledInline) { ...@@ -112,6 +112,21 @@ TEST_P(NGInlineCursorTest, CulledInline) {
EXPECT_THAT(list, ElementsAre("abc", "", "xyz")); EXPECT_THAT(list, ElementsAre("abc", "", "xyz"));
} }
TEST_P(NGInlineCursorTest, CulledInlineWithoutRoot) {
SetBodyInnerHTML(R"HTML(
<div id="root"><a id="a"><b>abc</b><br><i>xyz</i></a></div>
)HTML");
const LayoutObject* layout_inline_a = GetLayoutObjectByElementId("a");
NGInlineCursor cursor;
cursor.MoveTo(*layout_inline_a);
Vector<String> list;
while (cursor) {
list.push_back(ToDebugString(cursor));
cursor.MoveToNextForSameLayoutObject();
}
EXPECT_THAT(list, ElementsAre("abc", "", "xyz"));
}
TEST_P(NGInlineCursorTest, FirstChild) { TEST_P(NGInlineCursorTest, FirstChild) {
// TDOO(yosin): Remove <style> once NGFragmentItem don't do culled inline. // TDOO(yosin): Remove <style> once NGFragmentItem don't do culled inline.
InsertStyleElement("a, b { background: gray; }"); InsertStyleElement("a, b { background: gray; }");
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "third_party/blink/renderer/core/layout/ng/ng_outline_utils.h" #include "third_party/blink/renderer/core/layout/ng/ng_outline_utils.h"
#include "third_party/blink/renderer/core/layout/layout_inline.h" #include "third_party/blink/renderer/core/layout/layout_inline.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_cursor.h"
#include "third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h" #include "third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h"
#include "third_party/blink/renderer/core/style/computed_style.h" #include "third_party/blink/renderer/core/style/computed_style.h"
...@@ -32,13 +33,19 @@ bool NGOutlineUtils::ShouldPaintOutline( ...@@ -32,13 +33,19 @@ bool NGOutlineUtils::ShouldPaintOutline(
if (layout_object->IsElementContinuation()) { if (layout_object->IsElementContinuation()) {
// If the |LayoutInline|'s continuation-root generated a fragment, we // If the |LayoutInline|'s continuation-root generated a fragment, we
// shouldn't paint the outline. // shouldn't paint the outline.
if (layout_object->ContinuationRoot()->FirstInlineFragment()) DCHECK(layout_object->ContinuationRoot());
NGInlineCursor cursor;
cursor.MoveTo(*layout_object->ContinuationRoot());
if (cursor)
return false; return false;
} }
DCHECK(layout_object->FirstInlineFragment()); // The first fragment paints all outlines. Check if this is the first fragment
return &layout_object->FirstInlineFragment()->PhysicalFragment() == // for the |layout_object|.
&physical_fragment; NGInlineCursor cursor;
cursor.MoveTo(*layout_object);
DCHECK(cursor);
return cursor.CurrentBoxFragment() == &physical_fragment;
} }
} // namespace blink } // namespace blink
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