Eliminate FontBuilder::initForStyleResolve.

It's not necessary to do initialization of FontBuilder separately from
StyleResolverState::setStyle. It just adds noise.

Also, use a reference for Document in FontBuilder, since its existence can
be guaranteed in all cases.

R=dglazkov@chromium.org

Review URL: https://codereview.chromium.org/671613003

git-svn-id: svn://svn.chromium.org/blink/trunk@184342 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/css/resolver/FontBuilder.cpp

@@ -60,25 +60,18 @@ private:
     FontDescription m_fontDescription;
 };
 
-FontBuilder::FontBuilder()
-    : m_document(nullptr)
-    , m_style(0)
+FontBuilder::FontBuilder(const Document& document)
+    : m_document(document)
+    , m_style(nullptr)
     , m_fontDirty(false)
-{
-}
-
-void FontBuilder::initForStyleResolve(const Document& document, RenderStyle* style)
 {
     ASSERT(document.frame());
-    m_document = &document;
-    m_style = style;
-    m_fontDirty = false;
 }
 
 void FontBuilder::setInitial(float effectiveZoom)
 {
-    ASSERT(m_document && m_document->settings());
-    if (!m_document || !m_document->settings())
+    ASSERT(m_document.settings());
+    if (!m_document.settings())
         return;
 
     FontDescriptionChangeScope scope(this);
@@ -112,7 +105,7 @@ void FontBuilder::fromSystemFont(CSSValueID valueId, float effectiveZoom)
         return;
 
     // Make sure the rendering mode and printer font settings are updated.
-    const Settings* settings = m_document->settings();
+    const Settings* settings = m_document.settings();
     ASSERT(settings); // If we're doing style resolution, this document should always be in a frame and thus have settings
     if (!settings)
         return;
@@ -131,8 +124,7 @@ FontFamily FontBuilder::standardFontFamily() const
 
 AtomicString FontBuilder::standardFontFamilyName() const
 {
-    ASSERT(m_document);
-    Settings* settings = m_document->settings();
+    Settings* settings = m_document.settings();
     if (settings)
         return settings->genericFontFamilySettings().standard();
     return AtomicString();
@@ -265,7 +257,7 @@ void FontBuilder::setSize(FontDescription& fontDescription, const FontDescriptio
     float specifiedSize = size.value;
 
     if (!specifiedSize && size.keyword)
-        specifiedSize = FontSize::fontSizeForKeyword(m_document, size.keyword, fontDescription.fixedPitchFontType());
+        specifiedSize = FontSize::fontSizeForKeyword(&m_document, size.keyword, fontDescription.fixedPitchFontType());
 
     if (specifiedSize < 0)
         return;
@@ -283,10 +275,10 @@ float FontBuilder::getComputedSizeFromSpecifiedSize(FontDescription& fontDescrip
 {
     float zoomFactor = effectiveZoom;
     // FIXME: Why is this here!!!!?!
-    if (LocalFrame* frame = m_document->frame())
+    if (LocalFrame* frame = m_document.frame())
         zoomFactor *= frame->textZoomFactor();
 
-    return FontSize::getComputedSizeFromSpecifiedSize(m_document, zoomFactor, fontDescription.isAbsoluteSize(), specifiedSize);
+    return FontSize::getComputedSizeFromSpecifiedSize(&m_document, zoomFactor, fontDescription.isAbsoluteSize(), specifiedSize);
 }
 
 static void getFontAndGlyphOrientation(const RenderStyle* style, FontOrientation& fontOrientation, NonCJKGlyphOrientation& glyphOrientation)
@@ -365,9 +357,9 @@ void FontBuilder::checkForGenericFamilyChange(RenderStyle* style, const RenderSt
     // multiplying by our scale factor.
     float size;
     if (scope.fontDescription().keywordSize()) {
-        size = FontSize::fontSizeForKeyword(m_document, scope.fontDescription().keywordSize(), scope.fontDescription().fixedPitchFontType());
+        size = FontSize::fontSizeForKeyword(&m_document, scope.fontDescription().keywordSize(), scope.fontDescription().fixedPitchFontType());
     } else {
-        Settings* settings = m_document->settings();
+        Settings* settings = m_document.settings();
         float fixedScaleFactor = (settings && settings->defaultFixedFontSize() && settings->defaultFontSize())
             ? static_cast<float>(settings->defaultFixedFontSize()) / settings->defaultFontSize()
             : 1;

third_party/WebKit/Source/core/css/resolver/FontBuilder.h

@@ -41,11 +41,9 @@ class FontBuilder {
     STACK_ALLOCATED();
     WTF_MAKE_NONCOPYABLE(FontBuilder);
 public:
-    FontBuilder();
-
-    // FIXME: The name is probably wrong, but matches StyleResolverState callsite for consistency.
-    void initForStyleResolve(const Document&, RenderStyle*);
+    FontBuilder(const Document&);
 
+    void setStyle(RenderStyle* style) { m_style = style; }
     void setInitial(float effectiveZoom);
 
     void didChangeFontParameters(bool);
@@ -107,7 +105,7 @@ private:
 
     float getComputedSizeFromSpecifiedSize(FontDescription&, float effectiveZoom, float specifiedSize);
 
-    RawPtrWillBeMember<const Document> m_document;
+    const Document& m_document;
     // FIXME: This member is here on a short-term lease. The plan is to remove
     // any notion of RenderStyle from here, allowing FontBuilder to build Font objects
     // directly, rather than as a byproduct of calling RenderStyle::setFontDescription.

third_party/WebKit/Source/core/css/resolver/FontBuilderTest.cpp

@@ -4,6 +4,7 @@
 
 #include "config.h"
 #include "core/css/resolver/FontBuilder.h"
+#include "core/testing/DummyPageHolder.h"
 
 #include <gtest/gtest.h>
 
@@ -19,7 +20,8 @@ protected:
 
 TEST_F(FontBuilderTest, StylePointerInitialisation)
 {
-    FontBuilder builder;
+    OwnPtr<DummyPageHolder> dummy = DummyPageHolder::create(IntSize(800, 600));
+    FontBuilder builder(dummy->document());
     EXPECT_EQ(0, getStyle(builder));
 }
 

third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp

@@ -601,8 +601,6 @@ PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderS
         }
     }
 
-    state.fontBuilder().initForStyleResolve(state.document(), state.style());
-
     if (element->isLink()) {
         state.style()->setIsLink(true);
         EInsideLink linkState = state.elementLinkState();
@@ -676,8 +674,6 @@ PassRefPtr<RenderStyle> StyleResolver::styleForKeyframe(Element& element, const
     state.setStyle(RenderStyle::clone(&elementStyle));
     state.setLineHeightValue(0);
 
-    state.fontBuilder().initForStyleResolve(state.document(), state.style());
-
     // We don't need to bother with !important. Since there is only ever one
     // decl, there's nothing to override. So just add the first properties.
     // We also don't need to bother with animation properties since the only
@@ -714,7 +710,6 @@ PassRefPtrWillBeRawPtr<AnimatableValue> StyleResolver::createAnimatableValueSnap
         style = RenderStyle::create();
     StyleResolverState state(element.document(), &element);
     state.setStyle(style);
-    state.fontBuilder().initForStyleResolve(state.document(), state.style());
     return createAnimatableValueSnapshot(state, property, value);
 }
 
@@ -788,7 +783,6 @@ bool StyleResolver::pseudoStyleForElementInternal(Element& element, const Pseudo
     }
 
     state.style()->setStyleType(pseudoStyleRequest.pseudoId);
-    state.fontBuilder().initForStyleResolve(state.document(), state.style());
 
     // Since we don't use pseudo-elements in any of our quirk/print
     // user agent rules, don't waste time walking those rules.
@@ -865,8 +859,6 @@ PassRefPtr<RenderStyle> StyleResolver::styleForPage(int pageIndex)
     ASSERT(rootElementStyle);
     state.style()->inheritFrom(rootElementStyle);
 
-    state.fontBuilder().initForStyleResolve(state.document(), state.style());
-
     PageRuleCollector collector(rootElementStyle, pageIndex);
 
     collector.matchPageRules(CSSDefaultStyleSheets::instance().defaultPrintStyle());
@@ -920,7 +912,6 @@ PassRefPtr<RenderStyle> StyleResolver::defaultStyleForElement()
 {
     StyleResolverState state(document(), 0);
     state.setStyle(RenderStyle::create());
-    state.fontBuilder().initForStyleResolve(document(), state.style());
     state.style()->setLineHeight(RenderStyle::initialLineHeight());
     state.setLineHeightValue(0);
     state.fontBuilder().setInitial(state.style()->effectiveZoom());
@@ -1555,8 +1546,6 @@ void StyleResolver::applyPropertiesToStyle(const CSSPropertyValue* properties, s
     StyleResolverState state(document(), document().documentElement(), style);
     state.setStyle(style);
 
-    state.fontBuilder().initForStyleResolve(document(), style);
-
     for (size_t i = 0; i < count; ++i) {
         if (properties[i].value) {
             // As described in BUG66291, setting font-size and line-height on a font may entail a CSSPrimitiveValue::computeLengthDouble call,

third_party/WebKit/Source/core/css/resolver/StyleResolverState.cpp

@@ -38,6 +38,7 @@ StyleResolverState::StyleResolverState(Document& document, Element* element, Ren
     , m_applyPropertyToRegularStyle(true)
     , m_applyPropertyToVisitedLinkStyle(false)
     , m_lineHeightValue(nullptr)
+    , m_fontBuilder(document)
     , m_styleMap(*this, m_elementStyleResources)
 {
     if (!parentStyle && m_elementContext.parentNode())

third_party/WebKit/Source/core/css/resolver/StyleResolverState.h

@@ -60,7 +60,12 @@ public:
 
     const ElementResolveContext& elementContext() const { return m_elementContext; }
 
-    void setStyle(PassRefPtr<RenderStyle> style) { m_style = style; m_cssToLengthConversionData.setStyle(m_style.get()); }
+    void setStyle(PassRefPtr<RenderStyle> style)
+    {
+        m_style = style;
+        m_cssToLengthConversionData.setStyle(m_style.get());
+        m_fontBuilder.setStyle(m_style.get());
+    }
     const RenderStyle* style() const { return m_style.get(); }
     RenderStyle* style() { return m_style.get(); }
     PassRefPtr<RenderStyle> takeStyle() { return m_style.release(); }

third_party/WebKit/Source/core/dom/Document.cpp

@@ -1661,8 +1661,8 @@ void Document::updateDistributionForNodeIfNeeded(Node* node)
 
 void Document::setupFontBuilder(RenderStyle* documentStyle)
 {
-    FontBuilder fontBuilder;
-    fontBuilder.initForStyleResolve(*this, documentStyle);
+    FontBuilder fontBuilder(*this);
+    fontBuilder.setStyle(documentStyle);
     RefPtrWillBeRawPtr<CSSFontSelector> selector = m_styleEngine->fontSelector();
     fontBuilder.createFontForDocument(selector, documentStyle);
 }