Add ASSERT childNeedsDistributionRecalc in ComposedTreeTraversal

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 259 matching lines @@
 
     if (!containsOnlyWhitespace())
         return true;
 
     if (!canHaveWhitespaceChildren(parent))
         return false;
 
     if (style.preserveNewline()) // pre/pre-wrap/pre-line always make renderers.
         return true;
 
+    // childNeedsDistributionRecalc() here is rare, only happens JS calling surroundContents() etc. from DOMNodeInsertedIntoDocument etc.
+    if (document().childNeedsDistributionRecalc())

[hayato, 2015/01/23 04:06:30]

Could you help me to understand why it is okay to return false  if
document().childNeedsDistributionRecalc() is true here?

It's unclear to me.

[kojii, 2015/01/23 04:23:13]

In short, this fix disables a non-essential memory optimization in a rare
condition.

Comments in line 296 says this function is to avoid creation of a Renderer for a
non-essential memory optimization, and and if it takes too long to figure that
out precisely, this function returns true. It does so by limiting the number of
siblings to traverse up to 50.

So, returning true here should be no worse than creating a possibly unnecessary
Renderer. WebKit does not have this optimization, so when this hits, we're as
not-optimized as WebKit.

The only case I found to reach here is when a JavaScript handler for
DOMNodeInsertedIntoDocument calls Range::surroundContents() and when it tries to
delete some text, it calls Text::updateTextRenderer(). No other tests hit this
condition, so I consider this is rare enough.

+        return true;
+
     const RenderObject* prev = NodeRenderingTraversal::previousSiblingRenderer(*this);
     if (prev && prev->isBR()) // <span><br/> <br/></span>
         return false;
 
     if (parent.isRenderInline()) {
         // <span><div/> <div/></span>
         if (prev && !prev->isInline())
             return false;
     } else {
         if (parent.isRenderBlock() && !parent.childrenInline() && (!prev || !prev->isInline()))
@@ skipping 128 matching lines @@
             result.appendLiteral("; ");
         result.appendLiteral("value=");
         result.append(s);
     }
 
     strncpy(buffer, result.toString().utf8().data(), length - 1);
 }
 #endif
 
 } // namespace blink