Avoid creating consecutive whitespace renderers.

third_party/WebKit/Source/core/layout/LayoutTextTest.cpp

Index: third_party/WebKit/Source/core/layout/LayoutTextTest.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutTextTest.cpp b/third_party/WebKit/Source/core/layout/LayoutTextTest.cpp
index 9c551bbcf4df3a69b7405733b165cb34ab898c3b..99f195be7e03c2e96e15148b828bf08d23d52c52 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTextTest.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTextTest.cpp
@@ -85,4 +85,70 @@ TEST_F(LayoutTextTest, WidthLengthBeyondLength)
     ASSERT_LE(width, 20.f);
 }
 
+TEST_F(LayoutTextTest, ConsecutiveWhitespaceRenderers)
+{
+    setBodyInnerHTML("<span></span> <span id='span'></span>");
+
+    Element* span = document().getElementById("span");
+    Node* whitespace1 = span->previousSibling();
+    Node* whitespace2 = Text::create(document(), " ");
+    document().body()->insertBefore(whitespace2, span, ASSERT_NO_EXCEPTION);
+    document().view()->updateAllLifecyclePhases();
+
+    EXPECT_TRUE(whitespace1->layoutObject());
+    EXPECT_FALSE(whitespace2->layoutObject());
+}

[mstensho (USE GERRIT), 2016/09/28 11:19:09]

How about a test similar to this one, except that you insert the new whitespace
node BEFORE the existing whitespace node?

The expectation then would be that the old whitespace node would lose its
LayoutObject and have a LayoutObject created for the new node instead.

But you just explained to me f2f that it won't work like that, and that both
whitespace nodes would end up with a LayoutObject, because
Text::textLayoutObjectIsNeeded() only searches backwards. I don't know if
creating different layout trees with the same DOM, depending on the order things
are inserted in could be a potential problem, but it sounds reasonable to add a
test for it, with an explanation.

We're already inconsistent in textLayoutObjectIsNeeded(), anyway, because we
limit the search to 50 nodes, with a comment saying that it shouldn't be a
problem.

+
+TEST_F(LayoutTextTest, ConsecutiveWhitespaceRenderersDisplayNone)
+{
+    setBodyInnerHTML("<span></span> <span style='display:none'></span> <span></span>");
+
+    // First <span>
+    Node* child = document().body()->firstChild();
+    ASSERT_TRUE(child);
+
+    // First whitespace node
+    child = child->nextSibling();
+    ASSERT_TRUE(child);
+    EXPECT_TRUE(child->isTextNode());
+    EXPECT_TRUE(child->layoutObject());
+
+    // <span display:none>
+    child = child->nextSibling();
+    ASSERT_TRUE(child);
+    EXPECT_FALSE(child->layoutObject());
+
+    // Second whitespace node
+    child = child->nextSibling();
+    ASSERT_TRUE(child);
+    EXPECT_TRUE(child->isTextNode());
+    EXPECT_FALSE(child->layoutObject());
+}
+
+TEST_F(LayoutTextTest, ConsecutiveWhitespaceRenderersComment)
+{
+    setBodyInnerHTML("<span></span> <!-- --> <span></span>");
+
+    // First <span>
+    Node* child = document().body()->firstChild();
+    ASSERT_TRUE(child);
+
+    // First whitespace node
+    child = child->nextSibling();
+    ASSERT_TRUE(child);
+    EXPECT_TRUE(child->isTextNode());
+    EXPECT_TRUE(child->layoutObject());
+
+    // <span display:none>

[rune, 2016/09/27 13:54:14]

This should be the comment <!-- -->

[rune, 2016/09/28 12:13:10]

Fixed.

+    child = child->nextSibling();
+    ASSERT_TRUE(child);
+    EXPECT_FALSE(child->layoutObject());
+
+    // Second whitespace node
+    child = child->nextSibling();
+    ASSERT_TRUE(child);
+    EXPECT_TRUE(child->isTextNode());
+    EXPECT_FALSE(child->layoutObject());
+}
+
 } // namespace blink