SkPDF: SkPDFFont class changes

SkPDFFont:
  * inline some one-line methdods.
      - SkPDFFont::typeface()
      - SkPDFFont::fontInfo()
      - SkPDFFont::firstGlyphID()
      - SkPDFFont::lastGlyphID()
      - SkPDFFont::getFontDescriptor()
  * de-virtualize some methods:
      - SkPDFFont::getType()
      - SkPDFFont::multiByteGlyphs()
  * Constructor takes more arguments:
    fontType, multiByteGlyphs
  * re-order fields (pointers before shorts)
  * use sk_sp<T> more, T* less

SkAdvancedTypefaceMetrics:
  * SkAdvancedTypefaceMetrics::fFont now a uint8_t
  * other enumes are sized.
  * SkAdvancedTypefaceMetrics::fStyle now big enough.
  * remove use of SkTBitOr, replaced with fancy templates

No public API changes.
TBR=reed@google.com

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2246903002

Review-Url: https://codereview.chromium.org/2246903002

8 files changed, 220 insertions, 205 deletions

diff --git a/include/core/SkTypeface.h b/include/core/SkTypeface.h
index 23ca15e1f9..b2c288ebcd 100644
--- a/include/core/SkTypeface.h
+++ b/include/core/SkTypeface.h
@@ -8,6 +8,7 @@
 #ifndef SkTypeface_DEFINED
 #define SkTypeface_DEFINED
 
+#include "../private/SkBitmaskEnum.h"
 #include "../private/SkOnce.h"
 #include "../private/SkWeakRefCnt.h"
 #include "SkFontStyle.h"
@@ -421,4 +422,8 @@ private:
     typedef SkWeakRefCnt INHERITED;
 };
 
+namespace skstd {
+template <> struct is_bitmask_enum<SkTypeface::PerGlyphInfo> : std::true_type {};
+}
+
 #endif
diff --git a/include/private/SkBitmaskEnum.h b/include/private/SkBitmaskEnum.h
new file mode 100644
index 0000000000..f787d3b04d
--- /dev/null
+++ b/include/private/SkBitmaskEnum.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+#ifndef SkEnumOperators_DEFINED
+#define SkEnumOperators_DEFINED
+
+#include "SkTLogic.h"
+
+namespace skstd {
+template <typename T> struct is_bitmask_enum : std::false_type {};
+}
+
+template <typename E> SK_WHEN(skstd::is_bitmask_enum<E>::value, E) operator|(E l, E r) {
+    using U = skstd::underlying_type_t<E>;
+    return static_cast<E>(static_cast<U>(l) | static_cast<U>(r));
+}
+
+template <typename E> SK_WHEN(skstd::is_bitmask_enum<E>::value, E&) operator|=(E& l, E r) {
+    return l = l | r;
+}
+
+template <typename E> SK_WHEN(skstd::is_bitmask_enum<E>::value, E) operator&(E l, E r) {
+    using U = skstd::underlying_type_t<E>;
+    return static_cast<E>(static_cast<U>(l) & static_cast<U>(r));
+}
+
+template <typename E> SK_WHEN(skstd::is_bitmask_enum<E>::value, E&) operator&=(E& l, E r) {
+    return l = l & r;
+}
+
+#endif  // SkEnumOperators_DEFINED
diff --git a/src/core/SkAdvancedTypefaceMetrics.h b/src/core/SkAdvancedTypefaceMetrics.h
index 1b490e0199..17255ab217 100644
--- a/src/core/SkAdvancedTypefaceMetrics.h
+++ b/src/core/SkAdvancedTypefaceMetrics.h
@@ -5,16 +5,14 @@
  * found in the LICENSE file.
  */
 
-
 #ifndef SkAdvancedTypefaceMetrics_DEFINED
 #define SkAdvancedTypefaceMetrics_DEFINED
 
+#include "SkBitmaskEnum.h"
 #include "SkRect.h"
 #include "SkRefCnt.h"
 #include "SkString.h"
 #include "SkTDArray.h"
-#include "SkTemplates.h"
-#include "SkSinglyLinkedList.h"
 
 /** \class SkAdvancedTypefaceMetrics
 
@@ -22,16 +20,15 @@
     embed typefaces. This class is created and filled in with information by
     SkTypeface::getAdvancedTypefaceMetrics.
 */
-
 class SkAdvancedTypefaceMetrics : public SkRefCnt {
 public:
 
     SkAdvancedTypefaceMetrics()
         : fType(SkAdvancedTypefaceMetrics::kOther_Font)
-        , fFlags(SkAdvancedTypefaceMetrics::kEmpty_FontFlag)
+        , fFlags((FontFlags)0)
         , fLastGlyphID(0)
         , fEmSize(0)
-        , fStyle(0)
+        , fStyle((StyleFlags)0)
         , fItalicAngle(0)
         , fAscent(0)
         , fDescent(0)
@@ -43,7 +40,7 @@ public:
 
     SkString fFontName;
 
-    enum FontType {
+    enum FontType : uint8_t {
         kType1_Font,
         kType1CID_Font,
         kCFF_Font,
@@ -55,29 +52,28 @@ public:
     // information will never be populated.
     FontType fType;
 
-    enum FontFlags {
-        kEmpty_FontFlag          = 0x0,  //!<No flags set
-        kMultiMaster_FontFlag    = 0x1,  //!<May be true for Type1, CFF, or TrueType fonts.
-        kNotEmbeddable_FontFlag  = 0x2,  //!<May not be embedded.
-        kNotSubsettable_FontFlag = 0x4,  //!<May not be subset.
+    enum FontFlags : uint8_t {
+        kMultiMaster_FontFlag    = 0x01,  //!<May be true for Type1, CFF, or TrueType fonts.
+        kNotEmbeddable_FontFlag  = 0x02,  //!<May not be embedded.
+        kNotSubsettable_FontFlag = 0x04,  //!<May not be subset.
     };
-    // Global font flags.
-    FontFlags fFlags;
+    FontFlags fFlags;  // Global font flags.
 
     uint16_t fLastGlyphID; // The last valid glyph ID in the font.
     uint16_t fEmSize;  // The size of the em box (defines font units).
 
     // These enum values match the values used in the PDF file format.
-    enum StyleFlags {
-        kFixedPitch_Style  = 0x00001,
-        kSerif_Style       = 0x00002,
-        kScript_Style      = 0x00008,
-        kItalic_Style      = 0x00040,
-        kAllCaps_Style     = 0x10000,
-        kSmallCaps_Style   = 0x20000,
-        kForceBold_Style   = 0x40000
+    enum StyleFlags : uint32_t {
+        kFixedPitch_Style  = 0x00000001,
+        kSerif_Style       = 0x00000002,
+        kScript_Style      = 0x00000008,
+        kItalic_Style      = 0x00000040,
+        kAllCaps_Style     = 0x00010000,
+        kSmallCaps_Style   = 0x00020000,
+        kForceBold_Style   = 0x00040000
     };
-    uint16_t fStyle;        // Font style characteristics.
+    StyleFlags fStyle;        // Font style characteristics.
+
     int16_t fItalicAngle;   // Counterclockwise degrees from vertical of the
                             // dominant vertical stroke for an Italic face.
     // The following fields are all in font units.
@@ -99,5 +95,9 @@ private:
     typedef SkRefCnt INHERITED;
 };
 
+namespace skstd {
+template <> struct is_bitmask_enum<SkAdvancedTypefaceMetrics::FontFlags> : std::true_type {};
+template <> struct is_bitmask_enum<SkAdvancedTypefaceMetrics::StyleFlags> : std::true_type {};
+}
 
 #endif
diff --git a/src/core/SkTypeface.cpp b/src/core/SkTypeface.cpp
index 24e1a67864..cda9b5ce5e 100644
--- a/src/core/SkTypeface.cpp
+++ b/src/core/SkTypeface.cpp
@@ -298,14 +298,10 @@ SkAdvancedTypefaceMetrics* SkTypeface::getAdvancedTypefaceMetrics(
                 (os2table.version.v2.fsType.field.Restricted &&
                  !(os2table.version.v2.fsType.field.PreviewPrint ||
                    os2table.version.v2.fsType.field.Editable))) {
-                result->fFlags = SkTBitOr<SkAdvancedTypefaceMetrics::FontFlags>(
-                        result->fFlags,
-                        SkAdvancedTypefaceMetrics::kNotEmbeddable_FontFlag);
+                result->fFlags |= SkAdvancedTypefaceMetrics::kNotEmbeddable_FontFlag;
             }
             if (os2table.version.v2.fsType.field.NoSubsetting) {
-                result->fFlags = SkTBitOr<SkAdvancedTypefaceMetrics::FontFlags>(
-                        result->fFlags,
-                        SkAdvancedTypefaceMetrics::kNotSubsettable_FontFlag);
+                result->fFlags |= SkAdvancedTypefaceMetrics::kNotSubsettable_FontFlag;
             }
         }
     }
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index c24737ffad..937360578c 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -56,10 +56,10 @@ struct AdvanceMetric {
 
 class SkPDFType0Font final : public SkPDFFont {
 public:
-    SkPDFType0Font(const SkAdvancedTypefaceMetrics* info,
-                   SkTypeface* typeface);
+    SkPDFType0Font(sk_sp<const SkAdvancedTypefaceMetrics> info,
+                   sk_sp<SkTypeface> typeface,
+                   SkAdvancedTypefaceMetrics::FontType type);
     virtual ~SkPDFType0Font();
-    bool multiByteGlyphs() const override { return true; }
     sk_sp<SkPDFObject> getFontSubset(const SkPDFGlyphSet* usage) override;
 #ifdef SK_DEBUG
     void emitObject(SkWStream*,
@@ -77,11 +77,11 @@ private:
 
 class SkPDFCIDFont final : public SkPDFFont {
 public:
-    SkPDFCIDFont(const SkAdvancedTypefaceMetrics* info,
-                 SkTypeface* typeface,
+    SkPDFCIDFont(sk_sp<const SkAdvancedTypefaceMetrics> info,
+                 sk_sp<SkTypeface> typeface,
+                 SkAdvancedTypefaceMetrics::FontType fontType,
                  const SkPDFGlyphSet* subset);
     virtual ~SkPDFCIDFont();
-    bool multiByteGlyphs() const override { return true; }
 
 private:
     bool populate(const SkPDFGlyphSet* subset);
@@ -91,12 +91,11 @@ private:
 
 class SkPDFType1Font final : public SkPDFFont {
 public:
-    SkPDFType1Font(const SkAdvancedTypefaceMetrics* info,
-                   SkTypeface* typeface,
+    SkPDFType1Font(sk_sp<const SkAdvancedTypefaceMetrics> info,
+                   sk_sp<SkTypeface> typeface,
                    uint16_t glyphID,
-                   SkPDFDict* relatedFontDescriptor);
+                   sk_sp<SkPDFDict> relatedFontDescriptor);
     virtual ~SkPDFType1Font();
-    bool multiByteGlyphs() const override { return false; }
 
 private:
     bool populate(int16_t glyphID);
@@ -105,8 +104,9 @@ private:
 
 class SkPDFType3Font final : public SkPDFFont {
 public:
-    SkPDFType3Font(const SkAdvancedTypefaceMetrics* info,
-                   SkTypeface* typeface,
+    SkPDFType3Font(sk_sp<const SkAdvancedTypefaceMetrics> info,
+                   sk_sp<SkTypeface> typeface,
+                   SkAdvancedTypefaceMetrics::FontType fontType,
                    uint16_t glyphID);
     virtual ~SkPDFType3Font() {}
     void emitObject(SkWStream*,
@@ -115,7 +115,6 @@ public:
         SkDEBUGFAIL("should call getFontSubset!");
     }
     sk_sp<SkPDFObject> getFontSubset(const SkPDFGlyphSet* usage) override;
-    bool multiByteGlyphs() const override { return false; }
 };
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -468,14 +467,6 @@ SkPDFGlyphSet* SkPDFGlyphSetMap::getGlyphSetForFont(SkPDFFont* font) {
 
 SkPDFFont::~SkPDFFont() {}
 
-SkTypeface* SkPDFFont::typeface() {
-    return fTypeface.get();
-}
-
-SkAdvancedTypefaceMetrics::FontType SkPDFFont::getType() {
-    return fFontType;
-}
-
 bool SkPDFFont::canEmbed() const {
     if (!fFontInfo.get()) {
         SkASSERT(fFontType == SkAdvancedTypefaceMetrics::kOther_Font);
@@ -533,16 +524,15 @@ int SkPDFFont::glyphsToPDFFontEncodingCount(const SkGlyphID* glyphIDs,
 
 // static
 SkPDFFont* SkPDFFont::GetFontResource(SkPDFCanon* canon,
-                                      SkTypeface* typeface,
+                                      SkTypeface* face,
                                       uint16_t glyphID) {
     SkASSERT(canon);
-    const uint32_t fontID = SkTypeface::UniqueID(typeface);
+    const uint32_t fontID = SkTypeface::UniqueID(face);
     SkPDFFont* relatedFont;
     if (SkPDFFont* pdfFont = canon->findFont(fontID, glyphID, &relatedFont)) {
         return SkRef(pdfFont);
     }
-    SkAutoResolveDefaultTypeface autoResolve(typeface);
-    typeface = autoResolve.get();
+    sk_sp<SkTypeface> typeface(face ? sk_ref_sp(face) : SkTypeface::MakeDefault());
     SkASSERT(typeface);
     int glyphCount = typeface->countGlyphs();
     if (glyphCount < 1 ||  // typeface lacks even a NOTDEF glyph.
@@ -551,122 +541,106 @@ SkPDFFont* SkPDFFont::GetFontResource(SkPDFCanon* canon,
         return nullptr;
     }
     sk_sp<const SkAdvancedTypefaceMetrics> fontMetrics;
-    SkPDFDict* relatedFontDescriptor = nullptr;
+    sk_sp<SkPDFDict> relatedFontDescriptor;
     if (relatedFont) {
-        fontMetrics.reset(SkSafeRef(relatedFont->fontInfo()));
-        relatedFontDescriptor = relatedFont->getFontDescriptor();
+        fontMetrics = relatedFont->refFontInfo();
+        relatedFontDescriptor = relatedFont->refFontDescriptor();
 
         // This only is to catch callers who pass invalid glyph ids.
         // If glyph id is invalid, then we will create duplicate entries
         // for TrueType fonts.
-        SkAdvancedTypefaceMetrics::FontType fontType =
-            fontMetrics.get() ? fontMetrics.get()->fType :
-                                SkAdvancedTypefaceMetrics::kOther_Font;
-
-        if (fontType == SkAdvancedTypefaceMetrics::kType1CID_Font ||
-            fontType == SkAdvancedTypefaceMetrics::kTrueType_Font) {
-            return SkRef(relatedFont);
-        }
+        SkDEBUGCODE(SkAdvancedTypefaceMetrics::FontType fontType = relatedFont->getType());
+        SkASSERT(fontType != SkAdvancedTypefaceMetrics::kType1CID_Font);
+        SkASSERT(fontType != SkAdvancedTypefaceMetrics::kTrueType_Font);
     } else {
-        SkTypeface::PerGlyphInfo info =
-            SkTBitOr(SkTypeface::kGlyphNames_PerGlyphInfo,
-                     SkTypeface::kToUnicode_PerGlyphInfo);
+        SkTypeface::PerGlyphInfo info = SkTypeface::kGlyphNames_PerGlyphInfo |
+                                        SkTypeface::kToUnicode_PerGlyphInfo;
         fontMetrics.reset(
             typeface->getAdvancedTypefaceMetrics(info, nullptr, 0));
     }
 
-    SkPDFFont* font = SkPDFFont::Create(canon, fontMetrics.get(), typeface,
-                                        glyphID, relatedFontDescriptor);
-    canon->addFont(font, fontID, font->fFirstGlyphID);
-    return font;
-}
-
-sk_sp<SkPDFObject> SkPDFFont::getFontSubset(const SkPDFGlyphSet*) {
-    return nullptr;  // Default: no support.
-}
-
-// TODO: take a sk_sp<SkAdvancedTypefaceMetrics> and sk_sp<SkTypeface>
-SkPDFFont::SkPDFFont(const SkAdvancedTypefaceMetrics* info,
-                     SkTypeface* typeface,
-                     SkPDFDict* relatedFontDescriptor)
-    : SkPDFDict("Font")
-    , fTypeface(ref_or_default(typeface))
-    , fFirstGlyphID(1)
-    , fLastGlyphID(info ? info->fLastGlyphID : 0)
-    , fFontInfo(SkSafeRef(info))
-    , fDescriptor(SkSafeRef(relatedFontDescriptor))
-    , fFontType((!info || info->fFlags & SkAdvancedTypefaceMetrics::kMultiMaster_FontFlag)
-                ? SkAdvancedTypefaceMetrics::kOther_Font
-                : info->fType) {
-    SkASSERT(fTypeface);
-    if (0 == fLastGlyphID) {
-        fLastGlyphID = SkToU16(fTypeface->countGlyphs() - 1);
-    }
-}
-
-// static
-SkPDFFont* SkPDFFont::Create(SkPDFCanon* canon,
-                             const SkAdvancedTypefaceMetrics* info,
-                             SkTypeface* typeface,
-                             uint16_t glyphID,
-                             SkPDFDict* relatedFontDescriptor) {
     SkAdvancedTypefaceMetrics::FontType type =
-        info ? info->fType : SkAdvancedTypefaceMetrics::kOther_Font;
-    SkAdvancedTypefaceMetrics::FontFlags flags =
-        info ? info->fFlags : SkAdvancedTypefaceMetrics::kEmpty_FontFlag;
-    if (SkToBool(flags & SkAdvancedTypefaceMetrics::kMultiMaster_FontFlag)) {
-        return new SkPDFType3Font(info, typeface, glyphID);
+        fontMetrics ? fontMetrics->fType : SkAdvancedTypefaceMetrics::kOther_Font;
+    if (fontMetrics &&
+        SkToBool(fontMetrics->fFlags &
+                 SkAdvancedTypefaceMetrics::kMultiMaster_FontFlag)) {
+        // force Type3 fallback.
+        type = SkAdvancedTypefaceMetrics::kOther_Font;
     }
+
+    sk_sp<SkPDFFont> font;
     switch (type) {
         case SkAdvancedTypefaceMetrics::kType1CID_Font:
         case SkAdvancedTypefaceMetrics::kTrueType_Font:
             SkASSERT(relatedFontDescriptor == nullptr);
-            SkASSERT(info != nullptr);
-            return new SkPDFType0Font(info, typeface);
+            SkASSERT(fontMetrics != nullptr);
+            font = sk_make_sp<SkPDFType0Font>(std::move(fontMetrics),
+                                              std::move(typeface),
+                                              type);
+            break;
         case SkAdvancedTypefaceMetrics::kType1_Font:
-            SkASSERT(info != nullptr);
-            return new SkPDFType1Font(info, typeface, glyphID, relatedFontDescriptor);
+            SkASSERT(fontMetrics != nullptr);
+            font = sk_make_sp<SkPDFType1Font>(std::move(fontMetrics),
+                                              std::move(typeface),
+                                              glyphID,
+                                              std::move(relatedFontDescriptor));
+            break;
         case SkAdvancedTypefaceMetrics::kCFF_Font:
-            SkASSERT(info != nullptr);
+            SkASSERT(fontMetrics != nullptr);
             // fallthrough
         case SkAdvancedTypefaceMetrics::kOther_Font:
-            return new SkPDFType3Font(info, typeface, glyphID);
+            font = sk_make_sp<SkPDFType3Font>(std::move(fontMetrics),
+                                              std::move(typeface),
+                                              type,
+                                              glyphID);
+            break;
+        default:
+            SkDEBUGFAIL("invalid SkAdvancedTypefaceMetrics::FontType");
+            return nullptr;
     }
-    SkDEBUGFAIL("invalid SkAdvancedTypefaceMetrics::FontType");
-    return nullptr;
+    // When firstGlyphID==0, SkFont::IsMatch() matches all glyphs in font.
+    SkGlyphID firstGlyphID = font->multiByteGlyphs() ? 0 : font->fFirstGlyphID;
+    // TODO(halcanary) Make SkCanon::addFont take sk_sp<SkPDFFont>.
+    canon->addFont(font.get(), fontID, firstGlyphID);
+    return font.release();  // TODO(halcanary) return sk_sp<SkPDFFont>.
 }
 
-const SkAdvancedTypefaceMetrics* SkPDFFont::fontInfo() {
-    return fFontInfo.get();
+sk_sp<SkPDFObject> SkPDFFont::getFontSubset(const SkPDFGlyphSet*) {
+    return nullptr;  // Default: no support.
 }
 
-void SkPDFFont::setFontInfo(const SkAdvancedTypefaceMetrics* info) {
-    if (info == nullptr || info == fFontInfo.get()) {
-        return;
+// TODO: take a sk_sp<SkAdvancedTypefaceMetrics> and sk_sp<SkTypeface>
+SkPDFFont::SkPDFFont(sk_sp<const SkAdvancedTypefaceMetrics> info,
+                     sk_sp<SkTypeface> typeface,
+                     sk_sp<SkPDFDict> relatedFontDescriptor,
+                     SkAdvancedTypefaceMetrics::FontType fontType,
+                     bool multiByteGlyphs)
+    : SkPDFDict("Font")
+    , fTypeface(std::move(typeface))
+    , fFontInfo(std::move(info))
+    , fDescriptor(std::move(relatedFontDescriptor))
+    , fFirstGlyphID(1)
+    , fFontType(fontType)
+    , fMultiByteGlyphs(multiByteGlyphs) {
+    SkASSERT(fTypeface);
+    fLastGlyphID = fFontInfo ? fFontInfo->fLastGlyphID : 0;
+    if (0 == fLastGlyphID) {
+        fLastGlyphID = SkToU16(fTypeface->countGlyphs() - 1);
     }
-    fFontInfo.reset(info);
-    SkSafeRef(info);
-}
-
-uint16_t SkPDFFont::firstGlyphID() const {
-    return fFirstGlyphID;
 }
 
-uint16_t SkPDFFont::lastGlyphID() const {
-    return fLastGlyphID;
+void SkPDFFont::setFontInfo(sk_sp<const SkAdvancedTypefaceMetrics> info) {
+    if (info) {
+        fFontInfo = std::move(info);
+    }
 }
 
 void SkPDFFont::setLastGlyphID(uint16_t glyphID) {
     fLastGlyphID = glyphID;
 }
 
-SkPDFDict* SkPDFFont::getFontDescriptor() {
-    return fDescriptor.get();
-}
-
-void SkPDFFont::setFontDescriptor(SkPDFDict* descriptor) {
-    fDescriptor.reset(descriptor);
-    SkSafeRef(descriptor);
+void SkPDFFont::setFontDescriptor(sk_sp<SkPDFDict> descriptor) {
+    fDescriptor = std::move(descriptor);
 }
 
 bool SkPDFFont::addCommonFontDescriptorEntries(int16_t defaultWidth) {
@@ -677,7 +651,7 @@ bool SkPDFFont::addCommonFontDescriptorEntries(int16_t defaultWidth) {
     const uint16_t emSize = fFontInfo->fEmSize;
 
     fDescriptor->insertName("FontName", fFontInfo->fFontName);
-    fDescriptor->insertInt("Flags", fFontInfo->fStyle | kPdfSymbolic);
+    fDescriptor->insertInt("Flags", (size_t)(fFontInfo->fStyle | kPdfSymbolic));
     fDescriptor->insertScalar("Ascent",
             scaleFromFontUnits(fFontInfo->fAscent, emSize));
     fDescriptor->insertScalar("Descent",
@@ -722,8 +696,10 @@ void SkPDFFont::populateToUnicodeTable(const SkPDFGlyphSet* subset) {
 // class SkPDFType0Font
 ///////////////////////////////////////////////////////////////////////////////
 
-SkPDFType0Font::SkPDFType0Font(const SkAdvancedTypefaceMetrics* info, SkTypeface* typeface)
-    : SkPDFFont(info, typeface, nullptr) {
+SkPDFType0Font::SkPDFType0Font(sk_sp<const SkAdvancedTypefaceMetrics> info,
+                               sk_sp<SkTypeface> typeface,
+                               SkAdvancedTypefaceMetrics::FontType fontType)
+    : SkPDFFont(std::move(info), std::move(typeface), nullptr, fontType, true) {
     SkDEBUGCODE(fPopulated = false);
     if (!canSubset()) {
         this->populate(nullptr);
@@ -736,7 +712,7 @@ sk_sp<SkPDFObject>  SkPDFType0Font::getFontSubset(const SkPDFGlyphSet* subset) {
     if (!canSubset()) {
         return nullptr;
     }
-    auto newSubset = sk_make_sp<SkPDFType0Font>(fontInfo(), typeface());
+    auto newSubset = sk_make_sp<SkPDFType0Font>(refFontInfo(), refTypeface(), getType());
     newSubset->populate(subset);
     return newSubset;
 }
@@ -752,11 +728,14 @@ void SkPDFType0Font::emitObject(SkWStream* stream,
 
 bool SkPDFType0Font::populate(const SkPDFGlyphSet* subset) {
     insertName("Subtype", "Type0");
-    insertName("BaseFont", fontInfo()->fFontName);
+    insertName("BaseFont", this->getFontInfo()->fFontName);
     insertName("Encoding", "Identity-H");
 
     sk_sp<SkPDFCIDFont> newCIDFont(
-            new SkPDFCIDFont(fontInfo(), typeface(), subset));
+            sk_make_sp<SkPDFCIDFont>(this->refFontInfo(),
+                                     this->refTypeface(),
+                                     this->getType(),
+                                     subset));
     auto descendantFonts = sk_make_sp<SkPDFArray>();
     descendantFonts->appendObjRef(std::move(newCIDFont));
     this->insertObject("DescendantFonts", std::move(descendantFonts));
@@ -771,10 +750,12 @@ bool SkPDFType0Font::populate(const SkPDFGlyphSet* subset) {
 // class SkPDFCIDFont
 ///////////////////////////////////////////////////////////////////////////////
 
-SkPDFCIDFont::SkPDFCIDFont(const SkAdvancedTypefaceMetrics* info,
-                           SkTypeface* typeface,
+SkPDFCIDFont::SkPDFCIDFont(sk_sp<const SkAdvancedTypefaceMetrics> info,
+                           sk_sp<SkTypeface> typeface,
+                           SkAdvancedTypefaceMetrics::FontType fontType,
                            const SkPDFGlyphSet* subset)
-    : SkPDFFont(info, typeface, nullptr) {
+    : SkPDFFont(std::move(info), std::move(typeface), nullptr,
+                fontType , /* multiByteGlyphs = */ true) {
     this->populate(subset);
 }
 
@@ -836,7 +817,7 @@ static sk_sp<SkPDFObject> get_subset_font_stream(
 bool SkPDFCIDFont::addFontDescriptor(int16_t defaultWidth,
                                      const SkTDArray<uint32_t>* subset) {
     auto descriptor = sk_make_sp<SkPDFDict>("FontDescriptor");
-    setFontDescriptor(descriptor.get());
+    setFontDescriptor(descriptor);
     if (!addCommonFontDescriptorEntries(defaultWidth)) {
         this->insertObjRef("FontDescriptor", std::move(descriptor));
         return false;
@@ -860,8 +841,9 @@ bool SkPDFCIDFont::addFontDescriptor(int16_t defaultWidth,
 
             #ifdef SK_SFNTLY_SUBSETTER
             if (this->canSubset() && subset) {
+                const char* name = this->getFontInfo()->fFontName.c_str();
                 sk_sp<SkPDFObject> subsetStream = get_subset_font_stream(
-                        std::move(fontAsset), *subset, fontInfo()->fFontName.c_str());
+                        std::move(fontAsset), *subset, name);
                 if (subsetStream) {
                     descriptor->insertObjRef("FontFile2", std::move(subsetStream));
                     break;
@@ -941,19 +923,19 @@ bool SkPDFCIDFont::populate(const SkPDFGlyphSet* subset) {
         }
         subset->exportTo(&glyphIDs);
     }
-    if (fontInfo()->fType == SkAdvancedTypefaceMetrics::kTrueType_Font) {
+    if (this->getType() == SkAdvancedTypefaceMetrics::kTrueType_Font) {
         uint32_t* glyphs = (glyphIDs.count() == 0) ? nullptr : glyphIDs.begin();
         uint32_t glyphsCount = glyphs ? glyphIDs.count() : 0;
         sk_sp<const SkAdvancedTypefaceMetrics> fontMetrics =
             SkPDFFont::GetFontMetricsWithGlyphNames(this->typeface(), glyphs, glyphsCount);
-        this->setFontInfo(fontMetrics.get());
+        this->setFontInfo(std::move(fontMetrics));
         this->addFontDescriptor(0, &glyphIDs);
     } else {
         // Other CID fonts
         addFontDescriptor(0, nullptr);
     }
 
-    insertName("BaseFont", fontInfo()->fFontName);
+    insertName("BaseFont", this->getFontInfo()->fFontName);
 
     if (getType() == SkAdvancedTypefaceMetrics::kType1CID_Font) {
         insertName("Subtype", "CIDFontType0");
@@ -973,7 +955,7 @@ bool SkPDFCIDFont::populate(const SkPDFGlyphSet* subset) {
     SkSinglyLinkedList<AdvanceMetric> tmpMetrics;
     set_glyph_widths(this->typeface(), &glyphIDs, &tmpMetrics);
     int16_t defaultWidth = 0;
-    uint16_t emSize = (uint16_t)this->fontInfo()->fEmSize;
+    uint16_t emSize = (uint16_t)this->getFontInfo()->fEmSize;
     sk_sp<SkPDFArray> widths = composeAdvanceData(tmpMetrics, emSize, &defaultWidth);
     if (widths->size()) {
         this->insertObject("W", std::move(widths));
@@ -988,25 +970,28 @@ bool SkPDFCIDFont::populate(const SkPDFGlyphSet* subset) {
 // class SkPDFType1Font
 ///////////////////////////////////////////////////////////////////////////////
 
-SkPDFType1Font::SkPDFType1Font(const SkAdvancedTypefaceMetrics* info,
-                               SkTypeface* typeface,
+SkPDFType1Font::SkPDFType1Font(sk_sp<const SkAdvancedTypefaceMetrics> info,
+                               sk_sp<SkTypeface> typeface,
                                uint16_t glyphID,
-                               SkPDFDict* relatedFontDescriptor)
-    : SkPDFFont(info, typeface, relatedFontDescriptor) {
-    this->populate(glyphID);
+                               sk_sp<SkPDFDict> relatedFontDescriptor)
+    : SkPDFFont(std::move(info),
+                std::move(typeface),
+                std::move(relatedFontDescriptor),
+                SkAdvancedTypefaceMetrics::kType1_Font,
+                /* multiByteGlyphs = */ false) {
+    this->populate(glyphID);  // TODO(halcanary): subset this.
 }
 
 SkPDFType1Font::~SkPDFType1Font() {}
 
 bool SkPDFType1Font::addFontDescriptor(int16_t defaultWidth) {
-    if (SkPDFDict* descriptor = getFontDescriptor()) {
-        this->insertObjRef("FontDescriptor",
-                           sk_ref_sp(descriptor));
+    if (sk_sp<SkPDFDict> descriptor = this->refFontDescriptor()) {
+        this->insertObjRef("FontDescriptor", std::move(descriptor));
         return true;
     }
 
     auto descriptor = sk_make_sp<SkPDFDict>("FontDescriptor");
-    setFontDescriptor(descriptor.get());
+    setFontDescriptor(descriptor);
 
     int ttcIndex;
     size_t header SK_INIT_TO_AVOID_WARNING;
@@ -1032,7 +1017,7 @@ bool SkPDFType1Font::addFontDescriptor(int16_t defaultWidth) {
 
 bool SkPDFType1Font::populate(int16_t glyphID) {
     this->insertName("Subtype", "Type1");
-    this->insertName("BaseFont", fontInfo()->fFontName);
+    this->insertName("BaseFont", this->getFontInfo()->fFontName);
     adjustGlyphRangeForSingleByteEncoding(glyphID);
     SkGlyphID firstGlyphID = this->firstGlyphID();
     SkGlyphID lastGlyphID = this->lastGlyphID();
@@ -1051,7 +1036,7 @@ bool SkPDFType1Font::populate(int16_t glyphID) {
         SkAutoGlyphCache glyphCache(tmpPaint, &props, nullptr);
         auto widths = sk_make_sp<SkPDFArray>();
         SkScalar advance = glyphCache->getGlyphIDAdvance(0).fAdvanceX;
-        const uint16_t emSize = this->fontInfo()->fEmSize;
+        const uint16_t emSize = this->getFontInfo()->fEmSize;
         widths->appendScalar(from_font_units(advance, emSize));
         for (unsigned gID = firstGlyphID; gID <= lastGlyphID; gID++) {
             advance = glyphCache->getGlyphIDAdvance(gID).fAdvanceX;
@@ -1065,7 +1050,7 @@ bool SkPDFType1Font::populate(int16_t glyphID) {
     auto encDiffs = sk_make_sp<SkPDFArray>();
     encDiffs->reserve(lastGlyphID - firstGlyphID + 3);
     encDiffs->appendInt(0);
-    const SkTArray<SkString>& glyphNames = this->fontInfo()->fGlyphNames;
+    const SkTArray<SkString>& glyphNames = this->getFontInfo()->fGlyphNames;
     SkASSERT(glyphNames.count() > lastGlyphID);
     encDiffs->appendName(glyphNames[0].c_str());
     const SkString unknown("UNKNOWN");
@@ -1235,12 +1220,14 @@ static void add_type3_font_info(SkPDFDict* font,
     font->insertObject("CharProcs", std::move(charProcs));
 }
 
-SkPDFType3Font::SkPDFType3Font(const SkAdvancedTypefaceMetrics* info,
-                               SkTypeface* typeface,
+SkPDFType3Font::SkPDFType3Font(sk_sp<const SkAdvancedTypefaceMetrics> info,
+                               sk_sp<SkTypeface> typeface,
+                               SkAdvancedTypefaceMetrics::FontType fontType,
                                uint16_t glyphID)
-    : SkPDFFont(info, typeface, nullptr) {
+    : SkPDFFont(std::move(info), std::move(typeface), nullptr,
+                fontType, /* multiByteGlyphs = */ false) {
     // If fLastGlyphID isn't set (because there is not fFontInfo), look it up.
-    this->setLastGlyphID(SkToU16(typeface->countGlyphs() - 1));
+    this->setLastGlyphID(SkToU16(this->typeface()->countGlyphs() - 1));
     this->adjustGlyphRangeForSingleByteEncoding(glyphID);
 }
 
@@ -1248,7 +1235,7 @@ sk_sp<SkPDFObject> SkPDFType3Font::getFontSubset(const SkPDFGlyphSet* usage) {
     // All fonts are subset before serialization.
     // TODO(halcanary): all fonts should follow this pattern.
     auto font = sk_make_sp<SkPDFDict>("Font");
-    const SkAdvancedTypefaceMetrics* info = this->fontInfo();
+    const SkAdvancedTypefaceMetrics* info = this->getFontInfo();
     uint16_t emSize = info && info->fEmSize > 0 ? info->fEmSize : 1000;
     add_type3_font_info(font.get(), this->typeface(), (SkScalar)emSize, usage,
                         this->firstGlyphID(), this->lastGlyphID());
diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h
index c2e34e44e4..9e36b53d16 100644
--- a/src/pdf/SkPDFFont.h
+++ b/src/pdf/SkPDFFont.h
@@ -75,16 +75,16 @@ public:
     /** Returns the typeface represented by this class. Returns nullptr for the
      *  default typeface.
      */
-    SkTypeface* typeface();
+    SkTypeface* typeface() { return fTypeface.get(); }
 
     /** Returns the font type represented in this font.  For Type0 fonts,
      *  returns the type of the decendant font.
      */
-    virtual SkAdvancedTypefaceMetrics::FontType getType();
+    SkAdvancedTypefaceMetrics::FontType getType() { return fFontType; }
 
     /** Returns true if this font encoding supports glyph IDs above 255.
      */
-    virtual bool multiByteGlyphs() const = 0;
+    bool multiByteGlyphs() const { return fMultiByteGlyphs; }
 
     /** Returns true if the machine readable licensing bits allow embedding.
      */
@@ -158,20 +158,27 @@ public:
 
 protected:
     // Common constructor to handle common members.
-    SkPDFFont(const SkAdvancedTypefaceMetrics* fontInfo,
-              SkTypeface* typeface,
-              SkPDFDict* relatedFontDescriptor);
+    SkPDFFont(sk_sp<const SkAdvancedTypefaceMetrics> fontInfo,
+              sk_sp<SkTypeface> typeface,
+              sk_sp<SkPDFDict> relatedFontDescriptor,
+              SkAdvancedTypefaceMetrics::FontType fontType,
+              bool multiByteGlyphs);
 
     // Accessors for subclass.
-    const SkAdvancedTypefaceMetrics* fontInfo();
-    void setFontInfo(const SkAdvancedTypefaceMetrics* info);
-    uint16_t firstGlyphID() const;
-    uint16_t lastGlyphID() const;
+    const SkAdvancedTypefaceMetrics* getFontInfo() const { return fFontInfo.get(); }
+    sk_sp<const SkAdvancedTypefaceMetrics> refFontInfo() const { return fFontInfo; }
+
+    void setFontInfo(sk_sp<const SkAdvancedTypefaceMetrics> info);
+    SkGlyphID firstGlyphID() const { return fFirstGlyphID; }
+    SkGlyphID lastGlyphID() const { return fLastGlyphID; }
     void setLastGlyphID(uint16_t glyphID);
 
     // Accessors for FontDescriptor associated with this object.
-    SkPDFDict* getFontDescriptor();
-    void setFontDescriptor(SkPDFDict* descriptor);
+    SkPDFDict* getFontDescriptor() const { return fDescriptor.get(); }
+    sk_sp<SkPDFDict> refFontDescriptor() const { return fDescriptor; }
+    void setFontDescriptor(sk_sp<SkPDFDict> descriptor);
+
+    sk_sp<SkTypeface> refTypeface() const { return fTypeface; }
 
     // Add common entries to FontDescriptor.
     bool addCommonFontDescriptorEntries(int16_t defaultWidth);
@@ -185,28 +192,21 @@ protected:
     // If subset is nullptr, all available glyph ids will be used.
     void populateToUnicodeTable(const SkPDFGlyphSet* subset);
 
-    // Create instances of derived types based on fontInfo.
-    static SkPDFFont* Create(SkPDFCanon* canon,
-                             const SkAdvancedTypefaceMetrics* fontInfo,
-                             SkTypeface* typeface,
-                             uint16_t glyphID,
-                             SkPDFDict* relatedFontDescriptor);
-
     static bool Find(uint32_t fontID, uint16_t glyphID, int* index);
 
     void drop() override;
 
 private:
     sk_sp<SkTypeface> fTypeface;
-
-    // The glyph IDs accessible with this font.  For Type1 (non CID) fonts,
-    // this will be a subset if the font has more than 255 glyphs.
-    uint16_t fFirstGlyphID;
-    uint16_t fLastGlyphID;
     sk_sp<const SkAdvancedTypefaceMetrics> fFontInfo;
     sk_sp<SkPDFDict> fDescriptor;
 
+    // The glyph IDs accessible with this font.  For Type1 (non CID) fonts,
+    // this will be a subset if the font has more than 255 glyphs.
+    SkGlyphID fFirstGlyphID;
+    SkGlyphID fLastGlyphID;
     SkAdvancedTypefaceMetrics::FontType fFontType;
+    bool fMultiByteGlyphs;
 
     typedef SkPDFDict INHERITED;
 };
diff --git a/src/ports/SkFontHost_FreeType.cpp b/src/ports/SkFontHost_FreeType.cpp
index 5c3034162d..6681c9c1ef 100644
--- a/src/ports/SkFontHost_FreeType.cpp
+++ b/src/ports/SkFontHost_FreeType.cpp
@@ -464,20 +464,15 @@ SkAdvancedTypefaceMetrics* SkTypeface_FreeType::onGetAdvancedTypefaceMetrics(
 
     SkAdvancedTypefaceMetrics* info = new SkAdvancedTypefaceMetrics;
     info->fFontName.set(FT_Get_Postscript_Name(face));
-    info->fFlags = SkAdvancedTypefaceMetrics::kEmpty_FontFlag;
+
     if (FT_HAS_MULTIPLE_MASTERS(face)) {
-        info->fFlags = SkTBitOr<SkAdvancedTypefaceMetrics::FontFlags>(
-                info->fFlags, SkAdvancedTypefaceMetrics::kMultiMaster_FontFlag);
+        info->fFlags |= SkAdvancedTypefaceMetrics::kMultiMaster_FontFlag;
     }
     if (!canEmbed(face)) {
-        info->fFlags = SkTBitOr<SkAdvancedTypefaceMetrics::FontFlags>(
-                info->fFlags,
-                SkAdvancedTypefaceMetrics::kNotEmbeddable_FontFlag);
+        info->fFlags |= SkAdvancedTypefaceMetrics::kNotEmbeddable_FontFlag;
     }
     if (!canSubset(face)) {
-        info->fFlags = SkTBitOr<SkAdvancedTypefaceMetrics::FontFlags>(
-                info->fFlags,
-                SkAdvancedTypefaceMetrics::kNotSubsettable_FontFlag);
+        info->fFlags |= SkAdvancedTypefaceMetrics::kNotSubsettable_FontFlag;
     }
     info->fLastGlyphID = face->num_glyphs - 1;
     info->fEmSize = 1000;
@@ -499,7 +494,7 @@ SkAdvancedTypefaceMetrics* SkTypeface_FreeType::onGetAdvancedTypefaceMetrics(
         info->fType = SkAdvancedTypefaceMetrics::kOther_Font;
     }
 
-    info->fStyle = 0;
+    info->fStyle = (SkAdvancedTypefaceMetrics::StyleFlags)0;
     if (FT_IS_FIXED_WIDTH(face)) {
         info->fStyle |= SkAdvancedTypefaceMetrics::kFixedPitch_Style;
     }
diff --git a/src/ports/SkFontHost_win.cpp b/src/ports/SkFontHost_win.cpp
index a2d9da7a39..45e76c797d 100644
--- a/src/ports/SkFontHost_win.cpp
+++ b/src/ports/SkFontHost_win.cpp
@@ -1761,9 +1761,7 @@ SkAdvancedTypefaceMetrics* LogFontTypeface::onGetAdvancedTypefaceMetrics(
     // If bit 1 is clear, the font can be embedded.
     // If bit 2 is set, the embedding is read-only.
     if (otm.otmfsType & 0x1) {
-        info->fFlags = SkTBitOr<SkAdvancedTypefaceMetrics::FontFlags>(
-                info->fFlags,
-                SkAdvancedTypefaceMetrics::kNotEmbeddable_FontFlag);
+        info->fFlags |= SkAdvancedTypefaceMetrics::kNotEmbeddable_FontFlag;
     }
 
     if (perGlyphInfo & kToUnicode_PerGlyphInfo) {