Reattach whitespace siblings only when needed

Source/core/dom/Node.cpp

Index: Source/core/dom/Node.cpp
diff --git a/Source/core/dom/Node.cpp b/Source/core/dom/Node.cpp
index 60e09a205ed5da49f28b78bc9f696885e66f7f40..c5cda1929e190464d847b43d4e94ebd1504c2541 100644
--- a/Source/core/dom/Node.cpp
+++ b/Source/core/dom/Node.cpp
@@ -953,17 +953,22 @@ void Node::detach(const AttachContext& context)
 #endif
 }
 
-void Node::reattachWhitespaceSiblings(Text* start)
+void Node::reattachWhitespaceSiblingsIfNeeded(Text* start)
 {
+    if (!start)
+        return;
+
+    RenderObject* parentRenderer = NodeRenderingTraversal::parent(start)->renderer();

[esprehn, 2014/11/04 22:03:55]

This function can probably be called in situations where you're out of
composition, you need to null check the result of parent().

+    ASSERT(parentRenderer && parentRenderer->style());

[esprehn, 2014/11/04 22:03:55]

asserting ->style() on a RenderObject doesn't make sense.

     for (Node* sibling = start; sibling; sibling = sibling->nextSibling()) {
         if (sibling->isTextNode() && toText(sibling)->containsOnlyWhitespace()) {
             bool hadRenderer = !!sibling->renderer();
-            sibling->reattach();
-            // If the reattach didn't toggle the visibility of the whitespace we don't
-            // need to continue reattaching siblings since they won't toggle visibility
-            // either.
-            if (hadRenderer == !!sibling->renderer())
+            bool rendererIsNeeded = toText(sibling)->textRendererIsNeeded(*parentRenderer->style(), *parentRenderer);

[esprehn, 2014/11/04 22:03:55]

This is going to be a perf regression, calling textRendererIsNeeded is slow (and
sometimes n^2), and we're going to call it inside reattach() too.

[Xianzhu, 2014/11/05 01:30:13]

To avoid the twice calling to textRendererIsNeeded, the new patch set is based
on https://codereview.chromium.org/699213003/.

Added Text::reattachIfNeeded().

+            // If rendererIsNeeded didn't change we don't need to continue checking
+            // siblings since their rendererIsNeeded won't change either.
+            if (hadRenderer == rendererIsNeeded)
                 return;
+            sibling->reattach();
         } else if (sibling->renderer()) {
             return;
         }