Fix treatment of non-autosizing containers.

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 94afe2061edc83d08c33ac53df3c42fda5761c6a..439e99cbd7c4e0ab5ae0e9e5dd0418952e11055a 100644
--- a/Source/core/rendering/FastTextAutosizer.cpp
+++ b/Source/core/rendering/FastTextAutosizer.cpp
@@ -58,7 +58,7 @@ void FastTextAutosizer::record(RenderBlock* block)
     if (!enabled())
         return;
 
-    if (!shouldBeClusterRoot(block))
+    if (!isFingerprintingCandidate(block))
         return;
 
     AtomicString fingerprint = computeFingerprint(block);
@@ -81,8 +81,8 @@ void FastTextAutosizer::beginLayout(RenderBlock* block)
     if (block->isRenderView())
         prepareRenderViewInfo(toRenderView(block));
 
-    if (shouldBeClusterRoot(block))
-        m_clusterStack.append(getOrCreateCluster(block));
+    if (Cluster* cluster = maybeGetOrCreateCluster(block))
+        m_clusterStack.append(cluster);
 }
 
 void FastTextAutosizer::inflate(RenderBlock* block)
@@ -91,15 +91,14 @@ void FastTextAutosizer::inflate(RenderBlock* block)
         return;
     Cluster* cluster = m_clusterStack.last();
 
-    // localMultiplier is used to prevent inflation of some containers such as a row of links.
-    float localMultiplier = TextAutosizer::containerShouldBeAutosized(block) ? cluster->m_multiplier : 1;
+    float multiplier = cluster->m_autosize ? cluster->m_multiplier : 1.0f;

[pdr., 2014/01/14 02:50:03]

Double-checking my own understanding:
We no longer call containerShouldBeAutosized here because non-inflating
containers (such as a row of links) will now be a cluster already, and therefore
we can rely on the m_autosize bool?

[skobes, 2014/01/14 20:46:30]

Correct.  It was always broken to call containerShouldBeAutosized here, since
the block passed to inflate may not be a container.

 
     // FIXME: Add an optimization to not do this walk if it's not needed.

[pdr., 2014/01/14 02:50:03]

(not for this patch)
I originally added this FIXME for something to do with window sizes (back when
we were stateful), but I've kept it around because it may be the case that we
can skip this if the multiplier is 1 or 0. Is that actually true, or do we need
to "reset" the multipliers from previous layouts?

[skobes, 2014/01/14 20:46:30]

I think we have to reset them.  It is still kind of stateful since the
RenderStyle holds the multiplier.

Should I remove the FIXME?

     for (InlineWalker walker(block); !walker.atEnd(); walker.advance()) {
         RenderObject* inlineObj = walker.current();
         if (inlineObj->isText()) {
-            applyMultiplier(inlineObj, localMultiplier);
-            applyMultiplier(inlineObj->parent(), localMultiplier); // Parent handles line spacing.
+            applyMultiplier(inlineObj, multiplier);
+            applyMultiplier(inlineObj->parent(), multiplier); // Parent handles line spacing.
         }
     }
 }
@@ -145,7 +144,7 @@ void FastTextAutosizer::prepareRenderViewInfo(RenderView* renderView)
 #endif
 }
 
-bool FastTextAutosizer::shouldBeClusterRoot(RenderBlock* block)
+bool FastTextAutosizer::isFingerprintingCandidate(RenderBlock* block)
 {
     // FIXME: move the logic out of TextAutosizer.cpp into this class.
     return block->isRenderView()
@@ -155,8 +154,8 @@ bool FastTextAutosizer::shouldBeClusterRoot(RenderBlock* block)
 
 bool FastTextAutosizer::clusterWantsAutosizing(RenderBlock* root)
 {
-    // FIXME: this should be slightly different.
-    return TextAutosizer::containerShouldBeAutosized(root);
+    // FIXME: this should call (or re-implement) TextAutosizer::clusterShouldBeAutosized.
+    return true;
 }
 
 AtomicString FastTextAutosizer::computeFingerprint(RenderBlock* block)
@@ -165,26 +164,37 @@ AtomicString FastTextAutosizer::computeFingerprint(RenderBlock* block)
     return nullAtom;
 }
 
-FastTextAutosizer::Cluster* FastTextAutosizer::getOrCreateCluster(RenderBlock* block)
+FastTextAutosizer::Cluster* FastTextAutosizer::maybeGetOrCreateCluster(RenderBlock* block)
 {
+    if (!TextAutosizer::isAutosizingContainer(block))
+        return 0;
+
+    bool isIndependentDescendant = TextAutosizer::isIndependentDescendant(block);
+
+    // Create clusters to suppress / unsuppress autosizing based on containerShouldBeAutosized.
+    bool containerCanAutosize = TextAutosizer::containerShouldBeAutosized(block);
+    bool parentClusterCanAutosize = !m_clusterStack.isEmpty() && m_clusterStack.last()->m_autosize;
+
+    if (!isIndependentDescendant && containerCanAutosize == parentClusterCanAutosize)

[pdr., 2014/01/14 02:50:03]

Double-checking my own understanding:
Is this an optimization for when creating a cluster would have no effect? If so,
could you add that as a comment?

[skobes, 2014/01/14 20:46:30]

Correct.  We don't need a cluster for EVERY container, only containers that flip
the autosizing status.

+        return 0;
+
     ClusterMap::AddResult addResult = m_clusters.add(block, PassOwnPtr<Cluster>());
     if (!addResult.isNewEntry)
         return addResult.iterator->value.get();
 
     AtomicString fingerprint = m_fingerprintMapper.get(block);
     if (fingerprint.isNull()) {
-        addResult.iterator->value = adoptPtr(createCluster(block));
+        float multiplier;
+        if (isIndependentDescendant)
+            multiplier = clusterWantsAutosizing(block) ? computeMultiplier(block) : 1.0f;
+        else
+            multiplier = m_clusterStack.last()->m_multiplier;
+        addResult.iterator->value = adoptPtr(new Cluster(block, containerCanAutosize, multiplier));
         return addResult.iterator->value.get();
     }
     return addSupercluster(fingerprint, block);
 }
 
-FastTextAutosizer::Cluster* FastTextAutosizer::createCluster(RenderBlock* block)
-{
-    float multiplier = clusterWantsAutosizing(block) ? computeMultiplier(block) : 1.0f;
-    return new Cluster(block, multiplier);
-}
-
 FastTextAutosizer::Cluster* FastTextAutosizer::addSupercluster(AtomicString fingerprint, RenderBlock* returnFor)
 {
     BlockSet& roots = m_fingerprintMapper.getBlocks(fingerprint);
@@ -198,7 +208,7 @@ FastTextAutosizer::Cluster* FastTextAutosizer::addSupercluster(AtomicString fing
 
     Cluster* result = 0;
     for (BlockSet::iterator it = roots.begin(); it != roots.end(); ++it) {
-        Cluster* cluster = new Cluster(*it, multiplier);
+        Cluster* cluster = new Cluster(*it, TextAutosizer::containerShouldBeAutosized(*it), multiplier);
         m_clusters.set(*it, adoptPtr(cluster));
 
         if (*it == returnFor)