Reattach whitespace siblings only when needed

Source/core/dom/Text.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 256 matching lines @@
             return false;
     } else {
         if (parent.isRenderBlock() && !parent.childrenInline() && (!prev || !prev->isInline()))
             return false;
 
         // Avoiding creation of a Renderer for the text node is a non-essential memory optimization.
         // So to avoid blowing up on very wide DOMs, we limit the number of siblings to visit.
         unsigned maxSiblingsToVisit = 50;
 
         RenderObject* first = parent.slowFirstChild();
-        while (first && first->isFloatingOrOutOfFlowPositioned() && maxSiblingsToVisit--)
+        while (first && (first == renderer() || first->isFloatingOrOutOfFlowPositioned()) && maxSiblingsToVisit--)

[leviw_travelin_and_unemployed, 2014/11/05 19:23:15]

You still want to store renderer() in a local variable instead of constantly
calling it in this loop. Otherwise I think this looks good.

[Xianzhu, 2014/11/05 20:15:33]

Moved first==renderer() out of the loop into the following 'if'. If the text has
already renderer and the first non-floating-non-out-of-flow renderer is itself,
return false so that we can detach it.

             first = first->nextSibling();
         if (!first || NodeRenderingTraversal::nextSiblingRenderer(this) == first)
             // Whitespace at the start of a block just goes away.  Don't even
             // make a render object for this text.
             return false;
     }
     return true;
 }
 
 static bool isSVGText(Text* text)
@@ skipping 13 matching lines @@
 
     return new RenderText(this, dataImpl());
 }
 
 void Text::attach(const AttachContext& context)
 {
     RenderTreeBuilderForText(this).createRendererIfNeeded();
     CharacterData::attach(context);
 }
 
+void Text::reattachIfNeeded(const AttachContext& context)
+{
+    RenderTreeBuilderForText treeBuilder(this);
+    bool hadRenderer = !!renderer();

[leviw_travelin_and_unemployed, 2014/11/05 19:23:15]

You shouldn't need the !! when assigning into a bool.

[Xianzhu, 2014/11/05 20:15:33]

Done.

+    bool rendererIsNeeded = treeBuilder.rendererIsNeeded();
+    if (rendererIsNeeded == hadRenderer)
+        return;
+
+    // The following is almost the same as Node::attach() except that we create renderer only if needed.
+    // Not calling reattach() to avoid repeated calls to Text::textRendererIsNeeded().
+    AttachContext reattachContext(context);
+    reattachContext.performingReattach = true;
+
+    if (styleChangeType() < NeedsReattachStyleChange)
+        detach(reattachContext);
+    if (rendererIsNeeded)
+        treeBuilder.createRenderer();
+    CharacterData::attach(reattachContext);
+}
+
 void Text::recalcTextStyle(StyleRecalcChange change, Text* nextTextSibling)
 {
     if (RenderText* renderer = this->renderer()) {
         if (change != NoChange || needsStyleRecalc())
             renderer->setStyle(document().ensureStyleResolver().styleForText(this));
         if (needsStyleRecalc())
             renderer->setText(dataImpl());
         clearNeedsStyleRecalc();
     } else if (needsStyleRecalc() || needsWhitespaceRenderer()) {
         reattach();
         if (this->renderer())
-            reattachWhitespaceSiblings(nextTextSibling);
+            reattachWhitespaceSiblingsIfNeeded(nextTextSibling);
     }
 }
 
 // If a whitespace node had no renderer and goes through a recalcStyle it may
 // need to create one if the parent style now has white-space: pre.
 bool Text::needsWhitespaceRenderer()
 {
     ASSERT(!renderer());
     if (RenderStyle* style = parentRenderStyle())
         return style->preserveNewline();
@@ skipping 34 matching lines @@
             result.appendLiteral("; ");
         result.appendLiteral("value=");
         result.append(s);
     }
 
     strncpy(buffer, result.toString().utf8().data(), length - 1);
 }
 #endif
 
 } // namespace blink