Commit 0ef4a449 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[LayoutNG] Mark previous line dirty when LayoutText was removed

This patch changes LayoutText to use |DirtyLinesFromChangedChild()|
when it is removed. This change makes LayoutText behavior the same
as LayoutInline in this regard.

Before the change, LayoutText marked only the line it belongs to.
When it is removed, however, it should mark the line its previous
sibling belongs to. E.g.,
  line 1<br>line 2
and removing "line 2" should invalidate the 1st line because its
break token needs to be updated. Another example:
  <span>foo part</span>OfLongWord
and removing "OfLongWord" may make "part" to fit to the previous
line.

There are tests that remove <span>s, but there were no tests that
remove text nodes unfortunately. Given it turned out that we have
separate code for each type on removal, this patch adds some
tests, but will add more in following patches.

Bug: 918812
Change-Id: I6ada5c9d9dbec51d03ed3f1fc9db8d70c031bdd8
Reviewed-on: https://chromium-review.googlesource.com/c/1405127Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622072}
parent 5798283a
...@@ -218,10 +218,7 @@ void LayoutText::RemoveAndDestroyTextBoxes() { ...@@ -218,10 +218,7 @@ void LayoutText::RemoveAndDestroyTextBoxes() {
for (InlineTextBox* box : TextBoxes()) for (InlineTextBox* box : TextBoxes())
box->Remove(); box->Remove();
} else if (Parent()) { } else if (Parent()) {
if (NGPaintFragment* first_fragment = FirstInlineFragment()) Parent()->DirtyLinesFromChangedChild(this);
first_fragment->MarkContainingLineBoxDirty();
else
Parent()->DirtyLinesFromChangedChild(this);
} }
} }
DeleteTextBoxes(); DeleteTextBoxes();
......
...@@ -180,6 +180,18 @@ class NGInlineNodeTest : public NGLayoutTest { ...@@ -180,6 +180,18 @@ class NGInlineNodeTest : public NGLayoutTest {
FontCachePurgePreventer purge_preventer_; FontCachePurgePreventer purge_preventer_;
}; };
class NodeParameterTest : public NGInlineNodeTest,
public testing::WithParamInterface<const char*> {};
INSTANTIATE_TEST_CASE_P(
NGInlineNodeTest,
NodeParameterTest,
testing::Values("text",
"<span>span</span>",
"<span>1234 12345678</span>",
"<span style='display: inline-block'>box</span>",
"<img>"));
#define TEST_ITEM_TYPE_OFFSET(item, type, start, end) \ #define TEST_ITEM_TYPE_OFFSET(item, type, start, end) \
EXPECT_EQ(NGInlineItem::type, item.Type()); \ EXPECT_EQ(NGInlineItem::type, item.Type()); \
EXPECT_EQ(start, item.StartOffset()); \ EXPECT_EQ(start, item.StartOffset()); \
...@@ -759,15 +771,17 @@ TEST_F(NGInlineNodeTest, MarkLineBoxesDirtyOnRemove) { ...@@ -759,15 +771,17 @@ TEST_F(NGInlineNodeTest, MarkLineBoxesDirtyOnRemove) {
} }
// Test marking line boxes when removing a span. // Test marking line boxes when removing a span.
TEST_F(NGInlineNodeTest, MarkLineBoxesDirtyOnRemoveFirst) { TEST_P(NodeParameterTest, MarkLineBoxesDirtyOnRemoveFirst) {
SetupHtml("container", R"HTML( SetupHtml("container", String(R"HTML(
<div id=container style="font-size: 10px; width: 10ch"> <div id=container style="font-size: 10px; width: 10ch">)HTML") +
<span id=t>1234</span>5678 GetParam() + R"HTML(<span>after</span>
</div> </div>
)HTML"); )HTML");
Element* span = GetElementById("t"); Element* container = GetElementById("container");
span->remove(); Node* node = container->firstChild();
ASSERT_TRUE(node);
node->remove();
auto lines = MarkLineBoxesDirty(); auto lines = MarkLineBoxesDirty();
EXPECT_TRUE(lines[0]->IsDirty()); EXPECT_TRUE(lines[0]->IsDirty());
...@@ -790,6 +804,27 @@ TEST_F(NGInlineNodeTest, MarkLineBoxesDirtyOnRemove2) { ...@@ -790,6 +804,27 @@ TEST_F(NGInlineNodeTest, MarkLineBoxesDirtyOnRemove2) {
EXPECT_TRUE(lines[1]->IsDirty()); EXPECT_TRUE(lines[1]->IsDirty());
} }
// Test marking line boxes when removing a text node on 2nd line.
TEST_P(NodeParameterTest, MarkLineBoxesDirtyOnRemoveAfterBR) {
SetupHtml("container", String(R"HTML(
<div id=container style="font-size: 10px; width: 10ch">
line 1
<br>)HTML") + GetParam() +
"</div>");
Element* container = GetElementById("container");
Node* node = container->lastChild();
ASSERT_TRUE(node);
node->remove();
auto lines = MarkLineBoxesDirty();
EXPECT_TRUE(lines[0]->IsDirty());
// Currently, only the first dirty line is marked.
EXPECT_FALSE(lines[1]->IsDirty());
ForceLayout(); // Ensure running layout does not crash.
}
// Test marking line boxes when the first span has NeedsLayout. The span is // Test marking line boxes when the first span has NeedsLayout. The span is
// culled. // culled.
TEST_F(NGInlineNodeTest, MarkLineBoxesDirtyOnNeedsLayoutFirst) { TEST_F(NGInlineNodeTest, MarkLineBoxesDirtyOnNeedsLayoutFirst) {
......
...@@ -482,9 +482,10 @@ TEST_F(NGPaintFragmentTest, MarkLineBoxesDirtyByRemoveChild) { ...@@ -482,9 +482,10 @@ TEST_F(NGPaintFragmentTest, MarkLineBoxesDirtyByRemoveChild) {
Element& target = *GetDocument().getElementById("target"); Element& target = *GetDocument().getElementById("target");
target.remove(); target.remove();
const NGPaintFragment& container = *GetPaintFragmentByElementId("container"); const NGPaintFragment& container = *GetPaintFragmentByElementId("container");
EXPECT_TRUE(container.FirstChild()->IsDirty()); auto lines = ToList(container.Children());
EXPECT_TRUE(ToList(container.Children())[1]->IsDirty()); EXPECT_TRUE(lines[0]->IsDirty());
EXPECT_FALSE(ToList(container.Children())[2]->IsDirty()); EXPECT_FALSE(lines[1]->IsDirty());
EXPECT_FALSE(lines[2]->IsDirty());
} }
TEST_F(NGPaintFragmentTest, MarkLineBoxesDirtyByRemoveSpanWithBr) { TEST_F(NGPaintFragmentTest, MarkLineBoxesDirtyByRemoveSpanWithBr) {
......
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