Reattach whitespace siblings only when needed

Source/core/dom/Node.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 935 matching lines @@
     if (StyleResolver* resolver = document().styleResolver())
         resolver->ruleFeatureSet().styleInvalidator().clearInvalidation(*this);
     clearChildNeedsStyleInvalidation();
     clearNeedsStyleInvalidation();
 
 #if ENABLE(ASSERT)
     detachingNode = nullptr;
 #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();
+            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();
-            // 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())
-                return;
         } else if (sibling->renderer()) {
             return;
         }
     }
 }
 
 // FIXME: This code is used by editing.  Seems like it could move over there and not pollute Node.
 Node* Node::previousNodeConsideringAtomicNodes() const
 {
     if (previousSibling()) {
@@ skipping 1479 matching lines @@
         node->showTreeForThis();
 }
 
 void showNodePath(const blink::Node* node)
 {
     if (node)
         node->showNodePathForThis();
 }
 
 #endif