Fix treatment of non-autosizing containers.

Source/core/rendering/FastTextAutosizer.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 40 matching lines @@
     , m_renderViewInfoPrepared(false)
 #endif
 {
 }
 
 void FastTextAutosizer::record(RenderBlock* block)
 {
     if (!enabled())
         return;
 
-    if (!shouldBeClusterRoot(block))
+    if (!isFingerprintingCandidate(block))
         return;
 
     AtomicString fingerprint = computeFingerprint(block);
     if (fingerprint.isNull())
         return;
 
     m_fingerprintMapper.add(block, fingerprint);
 }
 
 void FastTextAutosizer::destroy(RenderBlock* block)
 {
     m_fingerprintMapper.remove(block);
 }
 
 void FastTextAutosizer::beginLayout(RenderBlock* block)
 {
     if (!enabled())
         return;
 
     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)
 {
     if (m_clusterStack.isEmpty())
         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.
         }
     }
 }
 
 void FastTextAutosizer::endLayout(RenderBlock* block)
 {
     if (!enabled())
         return;
 
     if (!m_clusterStack.isEmpty() && m_clusterStack.last()->m_root == block)
@@ skipping 25 matching lines @@
     m_baseMultiplier = m_document->settings()->accessibilityFontScaleFactor();
     // If the page has a meta viewport or @viewport, don't apply the device scale adjustment.
     const ViewportDescription& viewportDescription = m_document->page()->mainFrame()->document()->viewportDescription();
     if (!viewportDescription.isSpecifiedByAuthor())
         m_baseMultiplier *= m_document->settings()->deviceScaleAdjustment();
 #ifndef NDEBUG
     m_renderViewInfoPrepared = true;
 #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()
         || (TextAutosizer::isAutosizingContainer(block)
             && TextAutosizer::isIndependentDescendant(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)
 {
     // FIXME(crbug.com/322340): Implement a fingerprinting algorithm.
     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);
     RenderBlock* superRoot = deepestCommonAncestor(roots);
 
     bool shouldAutosize = false;
     for (BlockSet::iterator it = roots.begin(); it != roots.end(); ++it)
         shouldAutosize |= clusterWantsAutosizing(*it);
 
     float multiplier = shouldAutosize ? computeMultiplier(superRoot) : 1.0f;
 
     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)
             result = cluster;
     }
     return result;
 }
 
 RenderBlock* FastTextAutosizer::deepestCommonAncestor(BlockSet& blocks)
 {
@@ skipping 64 matching lines @@
 {
     return m_fingerprints.get(block);
 }
 
 FastTextAutosizer::BlockSet& FastTextAutosizer::FingerprintMapper::getBlocks(AtomicString fingerprint)
 {
     return *m_blocksForFingerprint.get(fingerprint);
 }
 
 } // namespace WebCore