Set multiplier to 1 on blocks with no direct text children.

Source/core/rendering/FastTextAutosizer.cpp

Index: Source/core/rendering/FastTextAutosizer.cpp
diff --git a/Source/core/rendering/FastTextAutosizer.cpp b/Source/core/rendering/FastTextAutosizer.cpp
index 97039c667df51feb4e0390daae391468451d2115..7b932ae4528a7329b629849925c459bcfa8e747a 100644
--- a/Source/core/rendering/FastTextAutosizer.cpp
+++ b/Source/core/rendering/FastTextAutosizer.cpp
@@ -372,7 +372,7 @@ void FastTextAutosizer::beginLayout(RenderBlock* block)
 
     // Cells in auto-layout tables are handled separately by inflateAutoTable.
     bool isAutoTableCell = block->isTableCell() && !toRenderTableCell(block)->table()->style()->isFixedTableLayout();
-    if (block->childrenInline() && block->firstChild() && !isAutoTableCell)
+    if (!isAutoTableCell && !m_clusterStack.isEmpty())
         inflate(block);
 }
 
@@ -443,27 +443,44 @@ void FastTextAutosizer::endLayout(RenderBlock* block)
 
 void FastTextAutosizer::inflate(RenderBlock* block)

[pdr., 2014/06/05 20:13:49]

I think this would be a little cleaner if this returned the multiplier and took
a float multiplier as an argument that has a default value of 0. This would
remove the need for a second function and the by-ref float.

[skobes, 2014/06/05 21:19:19]

Done.

 {
-    Cluster* cluster = currentCluster();
     float multiplier = 0;
-    RenderObject* descendant = block->firstChild();
-    while (descendant) {
-        // Skip block descendants because they will be inflate()'d on their own.
-        if (descendant->isRenderBlock()) {
-            descendant = descendant->nextInPreOrderAfterChildren(block);
-            continue;
-        }
-        if (descendant->isText()) {
+    inflateRecursive(block, multiplier);
+}
+
+void FastTextAutosizer::inflateRecursive(RenderObject* parent, float& multiplier)
+{
+    Cluster* cluster = currentCluster();
+    bool hasTextChild = false;
+
+    RenderObject* child = 0;
+    if (parent->isRenderBlock() && parent->childrenInline())

[pdr., 2014/06/05 20:13:49]

The fast firstChild checks only remove a single virtual call so these checks
will actually end up being more expensive. I think we can refactor this as:
if (parent->isRenderBlock() && parent->childrenInline() ||
parent->isRenderInline())
    child = parent->firstChild();

[skobes, 2014/06/05 21:19:20]

We can't directly call parent->firstChild() because firstChild does not exist on
RenderObject.  Or do you mean we should call slowFirstChild()?

+        child = toRenderBlock(parent)->firstChild();
+    else if (parent->isRenderInline())
+        child = toRenderInline(parent)->firstChild();
+
+    while (child) {
+        if (child->isText()) {
+            hasTextChild = true;
             // We only calculate this multiplier on-demand to ensure the parent block of this text
             // has entered layout.
             if (!multiplier)
                 multiplier = cluster->m_flags & SUPPRESSING ? 1.0f : clusterMultiplier(cluster);
-            applyMultiplier(descendant, multiplier);
-            applyMultiplier(descendant->parent(), multiplier); // Parent handles line spacing.
+            applyMultiplier(child, multiplier);
             // FIXME: Investigate why MarkOnlyThis is sufficient.
-            if (descendant->parent()->isRenderInline())
-                descendant->setPreferredLogicalWidthsDirty(MarkOnlyThis);
+            if (parent->isRenderInline())
+                child->setPreferredLogicalWidthsDirty(MarkOnlyThis);
+        } else if (child->isRenderInline()) {
+            inflateRecursive(child, multiplier);
         }
-        descendant = descendant->nextInPreOrder(block);
+        child = child->nextSibling();
+    }
+
+    if (hasTextChild) {
+        applyMultiplier(parent, multiplier); // Parent handles line spacing.
+    } else if (!parent->isListItem()) {
+        // For consistency, a block with no immediate text child should always have a
+        // multiplier of 1 (except for list items which are handled in inflateListItem).
+        applyMultiplier(parent, 1);

[pdr., 2014/06/05 20:13:49]

Should we only do this when textAutosizingMultiplier != 1? Otherwise we will
create many more unique styles which may be unnecessary.

[skobes, 2014/06/05 21:19:19]

applyMultiplier does this check.

     }
 }