[FastTextAutosizer] Remove printing checks from enabled()

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 302 matching lines @@
 #ifndef NDEBUG
         m_blocksThatHaveBegunLayout.add(block);
 #endif
         if (Cluster* cluster = maybeCreateCluster(block))
             m_clusterStack.append(adoptPtr(cluster));
     }
 }
 
 void FastTextAutosizer::beginLayout(RenderBlock* block)
 {
-    ASSERT(enabled() && shouldHandleLayout());
+    ASSERT(shouldHandleLayout());
 
     if (prepareForLayout(block) == StopLayout)
         return;
 
     if (Cluster* cluster = maybeCreateCluster(block)) {
         m_clusterStack.append(adoptPtr(cluster));
         if (block->isTable())
             inflateTable(toRenderTable(block));
     }
 
     if (block->childrenInline() && block->firstChild())
         inflate(block);
 }
 
 void FastTextAutosizer::inflateListItem(RenderListItem* listItem, RenderListMarker* listItemMarker)
 {
-    if (!enabled() || !shouldHandleLayout())
+    if (!shouldHandleLayout())
         return;
     ASSERT(listItem && listItemMarker);
 
     if (prepareForLayout(listItem) == StopLayout)
         return;
 
     // Force the LI to be inside the DBCAT when computing the multiplier.
     // This guarantees that the DBCAT has entered layout, so we can ask for its width.
     // It also makes sense because the list marker is autosized like a text node.
     float multiplier = clusterMultiplier(currentCluster());
@@ skipping 61 matching lines @@
                         }
                     }
                 }
             }
         }
     }
 }
 
 void FastTextAutosizer::endLayout(RenderBlock* block)
 {
-    ASSERT(enabled() && shouldHandleLayout());
+    ASSERT(shouldHandleLayout());
 
     if (block == m_firstBlockToBeginLayout) {
         m_firstBlockToBeginLayout = 0;
         m_clusterStack.clear();
         m_superclusters.clear();
         m_stylesRetainedDuringLayout.clear();
 #ifndef NDEBUG
         m_blocksThatHaveBegunLayout.clear();
 #endif
     } else if (currentCluster()->m_root == block) {
@@ skipping 20 matching lines @@
             applyMultiplier(descendant, multiplier);
             applyMultiplier(descendant->parent(), multiplier); // Parent handles line spacing.
             // FIXME: Investigate why MarkOnlyThis is sufficient.
             if (descendant->parent()->isRenderInline())
                 descendant->setPreferredLogicalWidthsDirty(MarkOnlyThis);
         }
         descendant = descendant->nextInPreOrder(block);
     }
 }
 
 // FIXME(crbug.com/367864): Unify this with m_pageInfo.m_settingEnabled.

[skobes, 2014/04/30 18:28:08]

Remove this? Or is there more to address?

[pdr., 2014/04/30 18:49:01]

There's a little more to address: I want to further refactor enabled() (possibly
remove it) in a followup. This patch sets everything up so that patch will be
easy.

 bool FastTextAutosizer::enabled() const
 {
-    if (!m_document->settings() || !m_document->page() || m_document->printing())
-        return false;
-
-    return m_document->settings()->textAutosizingEnabled();
+    return m_document->settings() && m_document->settings()->textAutosizingEnabled();
 }
 
 bool FastTextAutosizer::shouldHandleLayout() const
 {
-    return m_pageInfo.m_pageNeedsAutosizing && !m_updatePageInfoDeferred;
+    return enabled() && m_pageInfo.m_pageNeedsAutosizing && !m_updatePageInfoDeferred;
 }
 
 void FastTextAutosizer::updatePageInfoInAllFrames()
 {
     ASSERT(!enabled() || m_document->frame()->isMainFrame());
 
     for (LocalFrame* frame = m_document->frame(); frame; frame = frame->tree().traverseNext()) {
         if (FastTextAutosizer* textAutosizer = frame->document()->fastTextAutosizer())
             textAutosizer->updatePageInfo();
     }
 }
 
 void FastTextAutosizer::updatePageInfo()
 {
+    if (m_updatePageInfoDeferred || !m_document->page())
+        return;
+
     PageInfo previousPageInfo(m_pageInfo);
+    m_pageInfo.m_settingEnabled = m_document->settings() && m_document->settings()->textAutosizingEnabled();

[skobes, 2014/04/30 18:28:08]

This could just be

m_pageInfo.m_settingEnabled = enabled();

[pdr., 2014/04/30 18:49:01]

Done.

 
-    m_pageInfo.m_settingEnabled = m_document->settings() && m_document->settings()->textAutosizingEnabled();
-    if (!enabled()) {
-        if (previousPageInfo.m_hasAutosized)
-            resetMultipliers();
-        return;
+    if (!enabled() || m_document->printing()) {
+        m_pageInfo.m_pageNeedsAutosizing = false;
+    } else {
+        RenderView* renderView = toRenderView(m_document->renderer());
+        bool horizontalWritingMode = isHorizontalWritingMode(renderView->style()->writingMode());
+
+        LocalFrame* mainFrame = m_document->page()->mainFrame();
+        IntSize frameSize = m_document->settings()->textAutosizingWindowSizeOverride();
+        if (frameSize.isEmpty())
+            frameSize = mainFrame->view()->unscaledVisibleContentSize(IncludeScrollbars);
+        m_pageInfo.m_frameWidth = horizontalWritingMode ? frameSize.width() : frameSize.height();
+
+        IntSize layoutSize = mainFrame->view()->layoutSize();
+        m_pageInfo.m_layoutWidth = horizontalWritingMode ? layoutSize.width() : layoutSize.height();
+
+        // Compute the base font scale multiplier based on device and accessibility settings.
+        m_pageInfo.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 = mainFrame->document()->viewportDescription();
+        if (!viewportDescription.isSpecifiedByAuthor()) {
+            float deviceScaleAdjustment = m_document->settings()->deviceScaleAdjustment();
+            m_pageInfo.m_baseMultiplier *= deviceScaleAdjustment;
+        }
+
+        m_pageInfo.m_pageNeedsAutosizing = !!m_pageInfo.m_frameWidth
+            && (m_pageInfo.m_baseMultiplier * (static_cast<float>(m_pageInfo.m_layoutWidth) / m_pageInfo.m_frameWidth) > 1.0f);
     }
 
-    if (m_updatePageInfoDeferred)
-        return;
-
-    RenderView* renderView = toRenderView(m_document->renderer());
-    bool horizontalWritingMode = isHorizontalWritingMode(renderView->style()->writingMode());
-
-    LocalFrame* mainFrame = m_document->page()->mainFrame();
-    IntSize frameSize = m_document->settings()->textAutosizingWindowSizeOverride();
-    if (frameSize.isEmpty())
-        frameSize = mainFrame->view()->unscaledVisibleContentSize(IncludeScrollbars);
-    m_pageInfo.m_frameWidth = horizontalWritingMode ? frameSize.width() : frameSize.height();
-
-    IntSize layoutSize = m_document->page()->mainFrame()->view()->layoutSize();
-    m_pageInfo.m_layoutWidth = horizontalWritingMode ? layoutSize.width() : layoutSize.height();
-
-    // Compute the base font scale multiplier based on device and accessibility settings.
-    m_pageInfo.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()) {
-        float deviceScaleAdjustment = m_document->settings()->deviceScaleAdjustment();
-        m_pageInfo.m_baseMultiplier *= deviceScaleAdjustment;
-    }
-
-    m_pageInfo.m_pageNeedsAutosizing = !!m_pageInfo.m_frameWidth
-        && (m_pageInfo.m_baseMultiplier * (static_cast<float>(m_pageInfo.m_layoutWidth) / m_pageInfo.m_frameWidth) > 1.0f);
-
-    // If we are no longer autosizing the page, we won't do anything during the next layout.
-    // Set all the multipliers back to 1 now.
-    if (!m_pageInfo.m_pageNeedsAutosizing && previousPageInfo.m_hasAutosized)
-        resetMultipliers();
-
-    // If page info has changed, multipliers may have changed. Force a layout to recompute them.
-    if (m_pageInfo.m_pageNeedsAutosizing
-        && (m_pageInfo.m_frameWidth != previousPageInfo.m_frameWidth
+    if (m_pageInfo.m_pageNeedsAutosizing) {
+        // If page info has changed, multipliers may have changed. Force a layout to recompute them.
+        if (m_pageInfo.m_frameWidth != previousPageInfo.m_frameWidth
             || m_pageInfo.m_layoutWidth != previousPageInfo.m_layoutWidth
             || m_pageInfo.m_baseMultiplier != previousPageInfo.m_baseMultiplier
-            || m_pageInfo.m_settingEnabled != previousPageInfo.m_settingEnabled))
-        setAllTextNeedsLayout();
+            || m_pageInfo.m_settingEnabled != previousPageInfo.m_settingEnabled)
+            setAllTextNeedsLayout();
+    } else if (previousPageInfo.m_hasAutosized) {
+        // If we are no longer autosizing the page, we won't do anything during the next layout.
+        // Set all the multipliers back to 1 now.
+        resetMultipliers();
+        m_pageInfo.m_hasAutosized = false;
+    }
 }
 
 void FastTextAutosizer::resetMultipliers()
 {
     RenderObject* renderer = m_document->renderer();
     while (renderer) {
         if (RenderStyle* style = renderer->style()) {
             if (style->textAutosizingMultiplier() != 1)
                 applyMultiplier(renderer, 1, LayoutNeeded);
         }
         renderer = renderer->nextInPreOrder();
     }
-    m_pageInfo.m_hasAutosized = false;
 }
 
 void FastTextAutosizer::setAllTextNeedsLayout()
 {
     RenderObject* renderer = m_document->renderer();
     while (renderer) {
         if (renderer->isText())
             renderer->setNeedsLayout();
         renderer = renderer->nextInPreOrder();
     }
@@ skipping 504 matching lines @@
     return *m_blocksForFingerprint.get(fingerprint);
 }
 
 FastTextAutosizer::LayoutScope::LayoutScope(RenderBlock* block)
     : m_textAutosizer(block->document().fastTextAutosizer())
     , m_block(block)
 {
     if (!m_textAutosizer)
         return;
 
-    if (m_textAutosizer->enabled() && m_textAutosizer->shouldHandleLayout())
+    if (m_textAutosizer->shouldHandleLayout())
         m_textAutosizer->beginLayout(m_block);
     else
         m_textAutosizer = 0;
 }
 
 FastTextAutosizer::LayoutScope::~LayoutScope()
 {
     if (m_textAutosizer)
         m_textAutosizer->endLayout(m_block);
 }
@@ skipping 10 matching lines @@
 FastTextAutosizer::DeferUpdatePageInfo::~DeferUpdatePageInfo()
 {
     if (FastTextAutosizer* textAutosizer = m_mainFrame->document()->fastTextAutosizer()) {
         ASSERT(textAutosizer->m_updatePageInfoDeferred);
         textAutosizer->m_updatePageInfoDeferred = false;
         textAutosizer->updatePageInfoInAllFrames();
     }
 }
 
 } // namespace WebCore