Revert "SkAdvancedTypefaceMetrics: factor out GlyphToUnicode"

This reverts commit 1c2bcd8b14e029a70e88b1e81acd29553cab0d1c.

Reason for revert: breaking chrome roll

Original change's description:
> SkAdvancedTypefaceMetrics: factor out GlyphToUnicode
> 
> Change-Id: Iedce8c1ea2c405d5ab64ccac353970d5cd2b9d63
> Reviewed-on: https://skia-review.googlesource.com/126507
> Commit-Queue: Hal Canary <halcanary@google.com>
> Reviewed-by: Ben Wagner <bungeman@google.com>

TBR=halcanary@google.com,bungeman@google.com,reed@google.com

Change-Id: Ib1ff8484ffd09cdb88d461ac00745aa32c191124
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/127000
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
diff --git a/include/core/SkTypeface.h b/include/core/SkTypeface.h
index 6a59b26..37878cc 100644
--- a/include/core/SkTypeface.h
+++ b/include/core/SkTypeface.h
@@ -318,11 +318,6 @@
     // Backends that do not suport type1 fonts should not override.
     virtual void getPostScriptGlyphNames(SkString*) const {}
 
-    // The mapping from glyph to Unicode; array indices are glyph ids.
-    // For each glyph, give the default Unicode value, if it exists.
-    // dstArray is non-null, and points to an array of size this->countGlyphs().
-    virtual void getGlyphToUnicodeMap(SkUnichar* dstArray) const = 0;
-
     virtual SkStreamAsset* onOpenStream(int* ttcIndex) const = 0;
     // TODO: make pure virtual.
     virtual std::unique_ptr<SkFontData> onMakeFontData() const;
diff --git a/src/core/SkAdvancedTypefaceMetrics.h b/src/core/SkAdvancedTypefaceMetrics.h
index e9af831..6be3d44 100644
--- a/src/core/SkAdvancedTypefaceMetrics.h
+++ b/src/core/SkAdvancedTypefaceMetrics.h
@@ -10,7 +10,9 @@
 
 #include "SkBitmaskEnum.h"
 #include "SkRect.h"
+#include "SkRefCnt.h"
 #include "SkString.h"
+#include "SkTDArray.h"
 
 /** \class SkAdvancedTypefaceMetrics
 
@@ -19,6 +21,11 @@
     SkTypeface::getAdvancedMetrics.
 */
 struct SkAdvancedTypefaceMetrics {
+    SkAdvancedTypefaceMetrics() {}
+    SkAdvancedTypefaceMetrics(const SkAdvancedTypefaceMetrics&) = delete;
+    SkAdvancedTypefaceMetrics& operator=(const SkAdvancedTypefaceMetrics&) = delete;
+    ~SkAdvancedTypefaceMetrics() {}
+
     // The PostScript name of the font. See `FontName` and `BaseFont` in PDF standard.
     SkString fPostScriptName;
     SkString fFontName;
@@ -63,6 +70,9 @@
     int16_t fCapHeight = 0;    // Height (from baseline) of top of flat capitals.
 
     SkIRect fBBox = {0, 0, 0, 0};  // The bounding box of all glyphs (in font units).
+
+    // The mapping from glyph to Unicode; array indices are glyph ids.
+    SkTDArray<SkUnichar> fGlyphToUnicode;
 };
 
 namespace skstd {
diff --git a/src/core/SkTypeface.cpp b/src/core/SkTypeface.cpp
index af3e810..c6337d7 100644
--- a/src/core/SkTypeface.cpp
+++ b/src/core/SkTypeface.cpp
@@ -50,7 +50,6 @@
         return nullptr;
     }
     void onFilterRec(SkScalerContextRec*) const override { }
-    void getGlyphToUnicodeMap(SkUnichar*) const override { }
     std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override {
         return nullptr;
     }
diff --git a/src/core/SkTypeface_remote.h b/src/core/SkTypeface_remote.h
index ad729ad..691b124 100644
--- a/src/core/SkTypeface_remote.h
+++ b/src/core/SkTypeface_remote.h
@@ -111,10 +111,6 @@
     void onGetFontDescriptor(SkFontDescriptor*, bool*) const override {
         SK_ABORT("Should never be called.");
     }
-    void getGlyphToUnicodeMap(SkUnichar*) const override {
-        SK_ABORT("Should never be called.");
-    }
-
     std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override {
         SK_ABORT("Should never be called.");
         return nullptr;
diff --git a/src/pdf/SkPDFCanon.h b/src/pdf/SkPDFCanon.h
index 8e97db0..8d0e8f8 100644
--- a/src/pdf/SkPDFCanon.h
+++ b/src/pdf/SkPDFCanon.h
@@ -42,7 +42,6 @@
 
     SkTHashMap<uint32_t, std::unique_ptr<SkAdvancedTypefaceMetrics>> fTypefaceMetrics;
     SkTHashMap<uint32_t, std::vector<SkString>> fType1GlyphNames;
-    SkTHashMap<uint32_t, std::vector<SkUnichar>> fToUnicodeMap;
     SkTHashMap<uint32_t, sk_sp<SkPDFDict>> fFontDescriptors;
     SkTHashMap<uint64_t, sk_sp<SkPDFFont>> fFontMap;
 
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index c18219a..6e36fb3 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -1052,8 +1052,8 @@
 };
 }  // namespace
 
-static SkUnichar map_glyph(const std::vector<SkUnichar>& glyphToUnicode, SkGlyphID glyph) {
-    return glyph < glyphToUnicode.size() ? glyphToUnicode[SkToInt(glyph)] : -1;
+static SkUnichar map_glyph(const SkTDArray<SkUnichar>& glyphToUnicode, SkGlyphID glyph) {
+    return SkToInt(glyph) < glyphToUnicode.count() ? glyphToUnicode[SkToInt(glyph)] : -1;
 }
 
 static void update_font(SkWStream* wStream, int fontIndex, SkScalar textSize) {
@@ -1200,9 +1200,6 @@
     if (!metrics) {
         return;
     }
-    const std::vector<SkUnichar>& glyphToUnicode = SkPDFFont::GetUnicodeMap(
-        typeface, fDocument->canon());
-
     SkClusterator clusterator(sourceText, sourceByteCount, paint,
                               clusters, textByteLength, utf8Text);
     const SkGlyphID* glyphs = clusterator.glyphs();
@@ -1247,6 +1244,7 @@
             return;
         }
         SkDynamicMemoryWStream* out = content.stream();
+        const SkTDArray<SkUnichar>& glyphToUnicode = metrics->fGlyphToUnicode;
 
         out->writeText("BT\n");
         SK_AT_SCOPE_EXIT(out->writeText("ET\n"));
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index 3b89f62..9ba4114 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -186,19 +186,6 @@
     return canon->fTypefaceMetrics.set(id, std::move(metrics))->get();
 }
 
-const std::vector<SkUnichar>& SkPDFFont::GetUnicodeMap(const SkTypeface* typeface,
-                                                       SkPDFCanon* canon) {
-    SkASSERT(typeface);
-    SkASSERT(canon);
-    SkFontID id = typeface->uniqueID();
-    if (std::vector<SkUnichar>* ptr = canon->fToUnicodeMap.find(id)) {
-        return *ptr;
-    }
-    std::vector<SkUnichar> buffer(typeface->countGlyphs());
-    typeface->getGlyphToUnicodeMap(buffer.data());
-    return *canon->fToUnicodeMap.set(id, std::move(buffer));
-}
-
 SkAdvancedTypefaceMetrics::FontType SkPDFFont::FontType(const SkAdvancedTypefaceMetrics& metrics) {
     if (SkToBool(metrics.fFlags & SkAdvancedTypefaceMetrics::kMultiMaster_FontFlag) ||
         SkToBool(metrics.fFlags & SkAdvancedTypefaceMetrics::kNotEmbeddable_FontFlag)) {
@@ -490,15 +477,14 @@
     descendantFonts->appendObjRef(std::move(newCIDFont));
     this->insertObject("DescendantFonts", std::move(descendantFonts));
 
-    const std::vector<SkUnichar>& glyphToUnicode =
-        SkPDFFont::GetUnicodeMap(this->typeface(), canon);
-    SkASSERT(SkToSizeT(this->typeface()->countGlyphs()) == glyphToUnicode.size());
-    this->insertObjRef("ToUnicode",
-                       SkPDFMakeToUnicodeCmap(glyphToUnicode.data(),
-                                              &this->glyphUsage(),
-                                              this->multiByteGlyphs(),
-                                              this->firstGlyphID(),
-                                              this->lastGlyphID()));
+    if (metrics.fGlyphToUnicode.count() > 0) {
+        this->insertObjRef("ToUnicode",
+                           SkPDFMakeToUnicodeCmap(metrics.fGlyphToUnicode,
+                                                  &this->glyphUsage(),
+                                                  multiByteGlyphs(),
+                                                  firstGlyphID(),
+                                                  lastGlyphID()));
+    }
     SkDEBUGCODE(fPopulated = true);
     return;
 }
@@ -736,15 +722,14 @@
     fontBBox->appendInt(bbox.top());
     font->insertObject("FontBBox", std::move(fontBBox));
     font->insertName("CIDToGIDMap", "Identity");
-
-    const std::vector<SkUnichar>& glyphToUnicode = SkPDFFont::GetUnicodeMap(typeface, canon);
-    SkASSERT(glyphToUnicode.size() == SkToSizeT(typeface->countGlyphs()));
-    font->insertObjRef("ToUnicode",
-                       SkPDFMakeToUnicodeCmap(glyphToUnicode.data(),
-                                              &subset,
-                                              false,
-                                              firstGlyphID,
-                                              lastGlyphID));
+    if (metrics && metrics->fGlyphToUnicode.count() > 0) {
+        font->insertObjRef("ToUnicode",
+                           SkPDFMakeToUnicodeCmap(metrics->fGlyphToUnicode,
+                                                  &subset,
+                                                  false,
+                                                  firstGlyphID,
+                                                  lastGlyphID));
+    }
     auto descriptor = sk_make_sp<SkPDFDict>("FontDescriptor");
     int32_t fontDescriptorFlags = kPdfSymbolic;
     if (metrics) {
diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h
index 1441eed..9dc4655 100644
--- a/src/pdf/SkPDFFont.h
+++ b/src/pdf/SkPDFFont.h
@@ -95,9 +95,6 @@
     static const SkAdvancedTypefaceMetrics* GetMetrics(SkTypeface* typeface,
                                                        SkPDFCanon* canon);
 
-    static const std::vector<SkUnichar>& GetUnicodeMap(const SkTypeface* typeface,
-                                                       SkPDFCanon* canon);
-
     /** Subset the font based on current usage.
      *  Must be called before emitObject().
      */
diff --git a/src/pdf/SkPDFMakeToUnicodeCmap.cpp b/src/pdf/SkPDFMakeToUnicodeCmap.cpp
index c93aa6f..afe7732 100644
--- a/src/pdf/SkPDFMakeToUnicodeCmap.cpp
+++ b/src/pdf/SkPDFMakeToUnicodeCmap.cpp
@@ -147,12 +147,15 @@
 // For the worst case (having 65536 continuous unicode and we use every other
 // one of them), the possible savings by aggressive optimization is 416KB
 // pre-compressed and does not provide enough motivation for implementation.
-void SkPDFAppendCmapSections(const SkUnichar* glyphToUnicode,
+void SkPDFAppendCmapSections(const SkTDArray<SkUnichar>& glyphToUnicode,
                              const SkBitSet* subset,
                              SkDynamicMemoryWStream* cmap,
                              bool multiByteGlyphs,
                              SkGlyphID firstGlyphID,
                              SkGlyphID lastGlyphID) {
+    if (glyphToUnicode.isEmpty()) {
+        return;
+    }
     int glyphOffset = 0;
     if (!multiByteGlyphs) {
         glyphOffset = firstGlyphID - 1;
@@ -163,7 +166,8 @@
 
     BFRange currentRangeEntry = {0, 0, 0};
     bool rangeEmpty = true;
-    const int limit = (int)lastGlyphID + 1 - glyphOffset;
+    const int limit =
+            SkMin32(lastGlyphID + 1, glyphToUnicode.count()) - glyphOffset;
 
     for (int i = firstGlyphID - glyphOffset; i < limit + 1; ++i) {
         bool inSubset = i < limit &&
@@ -206,7 +210,7 @@
 }
 
 sk_sp<SkPDFStream> SkPDFMakeToUnicodeCmap(
-        const SkUnichar* glyphToUnicode,
+        const SkTDArray<SkUnichar>& glyphToUnicode,
         const SkBitSet* subset,
         bool multiByteGlyphs,
         SkGlyphID firstGlyphID,
diff --git a/src/pdf/SkPDFMakeToUnicodeCmap.h b/src/pdf/SkPDFMakeToUnicodeCmap.h
index 656af91..0c4d1c3 100644
--- a/src/pdf/SkPDFMakeToUnicodeCmap.h
+++ b/src/pdf/SkPDFMakeToUnicodeCmap.h
@@ -12,14 +12,14 @@
 #include "SkStream.h"
 
 sk_sp<SkPDFStream> SkPDFMakeToUnicodeCmap(
-        const SkUnichar* glyphToUnicode,
+        const SkTDArray<SkUnichar>& glyphToUnicode,
         const SkBitSet* subset,
         bool multiByteGlyphs,
         SkGlyphID firstGlyphID,
         SkGlyphID lastGlyphID);
 
 // Exposed for unit testing.
-void SkPDFAppendCmapSections(const SkUnichar* glyphToUnicode,
+void SkPDFAppendCmapSections(const SkTDArray<SkUnichar>& glyphToUnicode,
                              const SkBitSet* subset,
                              SkDynamicMemoryWStream* cmap,
                              bool multiByteGlyphs,
diff --git a/src/ports/SkFontHost_FreeType.cpp b/src/ports/SkFontHost_FreeType.cpp
index 3e8e957..f1c1b55 100644
--- a/src/ports/SkFontHost_FreeType.cpp
+++ b/src/ports/SkFontHost_FreeType.cpp
@@ -511,6 +511,23 @@
     return (fsType & FT_FSTYPE_NO_SUBSETTING) == 0;
 }
 
+static void populate_glyph_to_unicode(FT_Face& face, SkTDArray<SkUnichar>* glyphToUnicode) {
+    FT_Long numGlyphs = face->num_glyphs;
+    glyphToUnicode->setCount(SkToInt(numGlyphs));
+    sk_bzero(glyphToUnicode->begin(), sizeof((*glyphToUnicode)[0]) * numGlyphs);
+
+    FT_UInt glyphIndex;
+    SkUnichar charCode = FT_Get_First_Char(face, &glyphIndex);
+    while (glyphIndex) {
+        SkASSERT(glyphIndex < SkToUInt(numGlyphs));
+        // Use the first character that maps to this glyphID. https://crbug.com/359065
+        if (0 == (*glyphToUnicode)[glyphIndex]) {
+            (*glyphToUnicode)[glyphIndex] = charCode;
+        }
+        charCode = FT_Get_Next_Char(face, charCode, &glyphIndex);
+    }
+}
+
 static SkAdvancedTypefaceMetrics::FontType get_font_type(FT_Face face) {
     const char* fontType = FT_Get_X11_Font_Format(face);
     static struct { const char* s; SkAdvancedTypefaceMetrics::FontType t; } values[] = {
@@ -585,26 +602,17 @@
     }
     info->fBBox = SkIRect::MakeLTRB(face->bbox.xMin, face->bbox.yMax,
                                     face->bbox.xMax, face->bbox.yMin);
-    return info;
-}
 
-void SkTypeface_FreeType::getGlyphToUnicodeMap(SkUnichar* dstArray) const {
-    SkASSERT(dstArray);
-    AutoFTAccess fta(this);
-    FT_Face face = fta.face();
-    FT_Long numGlyphs = face->num_glyphs;
-    sk_bzero(dstArray, sizeof(SkUnichar) * numGlyphs);
+    bool perGlyphInfo = FT_IS_SCALABLE(face);
 
-    FT_UInt glyphIndex;
-    SkUnichar charCode = FT_Get_First_Char(face, &glyphIndex);
-    while (glyphIndex) {
-        SkASSERT(glyphIndex < SkToUInt(numGlyphs));
-        // Use the first character that maps to this glyphID. https://crbug.com/359065
-        if (0 == dstArray[glyphIndex]) {
-            dstArray[glyphIndex] = charCode;
-        }
-        charCode = FT_Get_Next_Char(face, charCode, &glyphIndex);
+    if (perGlyphInfo &&
+        info->fType != SkAdvancedTypefaceMetrics::kType1_Font &&
+        face->num_charmaps)
+    {
+        populate_glyph_to_unicode(face, &(info->fGlyphToUnicode));
     }
+
+    return info;
 }
 
 void SkTypeface_FreeType::getPostScriptGlyphNames(SkString* dstArray) const {
diff --git a/src/ports/SkFontHost_FreeType_common.h b/src/ports/SkFontHost_FreeType_common.h
index eacce04..3aabdde 100644
--- a/src/ports/SkFontHost_FreeType_common.h
+++ b/src/ports/SkFontHost_FreeType_common.h
@@ -91,7 +91,6 @@
     virtual SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
                                                    const SkDescriptor*) const override;
     void onFilterRec(SkScalerContextRec*) const override;
-    void getGlyphToUnicodeMap(SkUnichar*) const override;
     std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override;
     void getPostScriptGlyphNames(SkString* dstArray) const override;
     int onGetUPEM() const override;
diff --git a/src/ports/SkFontHost_mac.cpp b/src/ports/SkFontHost_mac.cpp
index 3cc51d4..8047c43 100644
--- a/src/ports/SkFontHost_mac.cpp
+++ b/src/ports/SkFontHost_mac.cpp
@@ -719,7 +719,6 @@
                                            const SkDescriptor*) const override;
     void onFilterRec(SkScalerContextRec*) const override;
     void onGetFontDescriptor(SkFontDescriptor*, bool*) const override;
-    void getGlyphToUnicodeMap(SkUnichar*) const override;
     std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override;
     int onCharsToGlyphs(const void* chars, Encoding,
                         uint16_t glyphs[], int glyphCount) const override;
@@ -1606,7 +1605,9 @@
 // Iterate through the font in this case. The existing caller caches the result,
 // so the performance impact isn't too bad.
 static void populate_glyph_to_unicode_slow(CTFontRef ctFont, CFIndex glyphCount,
-                                           SkUnichar* out) {
+                                           SkTDArray<SkUnichar>* glyphToUnicode) {
+    glyphToUnicode->setCount(SkToInt(glyphCount));
+    SkUnichar* out = glyphToUnicode->begin();
     sk_bzero(out, glyphCount * sizeof(SkUnichar));
     UniChar unichar = 0;
     while (glyphCount > 0) {
@@ -1627,8 +1628,7 @@
 // Unicode code points that require conjugate pairs in utf16 are not
 // supported.
 static void populate_glyph_to_unicode(CTFontRef ctFont, CFIndex glyphCount,
-                                      SkUnichar* glyphToUnicode) {
-    sk_bzero(glyphToUnicode, sizeof(SkUnichar) * glyphCount);
+                                      SkTDArray<SkUnichar>* glyphToUnicode) {
     UniqueCFRef<CFCharacterSetRef> charSet(CTFontCopyCharacterSet(ctFont));
     if (!charSet) {
         populate_glyph_to_unicode_slow(ctFont, glyphCount, glyphToUnicode);
@@ -1651,7 +1651,9 @@
         length = 8192;
     }
     const UInt8* bits = CFDataGetBytePtr(bitmap.get());
-    sk_bzero(glyphToUnicode, glyphCount * sizeof(SkUnichar));
+    glyphToUnicode->setCount(SkToInt(glyphCount));
+    SkUnichar* out = glyphToUnicode->begin();
+    sk_bzero(out, glyphCount * sizeof(SkUnichar));
     for (int i = 0; i < length; i++) {
         int mask = bits[i];
         if (!mask) {
@@ -1661,7 +1663,7 @@
             CGGlyph glyph;
             UniChar unichar = static_cast<UniChar>((i << 3) + j);
             if (mask & (1 << j) && CTFontGetGlyphsForCharacters(ctFont, &unichar, &glyph, 1)) {
-                glyphToUnicode[glyph] = unichar;
+                out[glyph] = unichar;
             }
         }
     }
@@ -1679,14 +1681,6 @@
     dst->resize(strlen(dst->c_str()));
 }
 
-void SkTypeface_Mac::getGlyphToUnicodeMap(SkUnichar* dstArray) const {
-    AUTO_CG_LOCK();
-    UniqueCFRef<CTFontRef> ctFont =
-            ctfont_create_exact_copy(fFontRef.get(), CTFontGetUnitsPerEm(fFontRef.get()), nullptr);
-    CFIndex glyphCount = CTFontGetGlyphCount(ctFont.get());
-    populate_glyph_to_unicode(ctFont.get(), glyphCount, dstArray);
-}
-
 std::unique_ptr<SkAdvancedTypefaceMetrics> SkTypeface_Mac::onGetAdvancedMetrics() const {
 
     AUTO_CG_LOCK();
@@ -1716,6 +1710,10 @@
         }
     }
 
+    CFIndex glyphCount = CTFontGetGlyphCount(ctFont.get());
+
+    populate_glyph_to_unicode(ctFont.get(), glyphCount, &info->fGlyphToUnicode);
+
     SkOTTableOS2_V4::Type fsType;
     if (sizeof(fsType) == this->getTableData(SkTEndian_SwapBE32(SkOTTableOS2::TAG),
                                              offsetof(SkOTTableOS2_V4, fsType),
diff --git a/src/ports/SkFontHost_win.cpp b/src/ports/SkFontHost_win.cpp
index f1da805..35a0a74 100644
--- a/src/ports/SkFontHost_win.cpp
+++ b/src/ports/SkFontHost_win.cpp
@@ -261,7 +261,6 @@
     SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
                                            const SkDescriptor*) const override;
     void onFilterRec(SkScalerContextRec*) const override;
-    void getGlyphToUnicodeMap(SkUnichar*) const override;
     std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override;
     void onGetFontDescriptor(SkFontDescriptor*, bool*) const override;
     int onCharsToGlyphs(const void* chars, Encoding encoding,
@@ -363,8 +362,7 @@
 // require parsing the TTF cmap table (platform 4, encoding 12) directly instead
 // of calling GetFontUnicodeRange().
 static void populate_glyph_to_unicode(HDC fontHdc, const unsigned glyphCount,
-                                      SkUnichar* glyphToUnicode) {
-    sk_bzero(glyphToUnicode, sizeof(SkUnichar) * glyphCount);
+                                      SkTDArray<SkUnichar>* glyphToUnicode) {
     DWORD glyphSetBufferSize = GetFontUnicodeRanges(fontHdc, nullptr);
     if (!glyphSetBufferSize) {
         return;
@@ -377,6 +375,8 @@
         return;
     }
 
+    glyphToUnicode->setCount(glyphCount);
+    memset(glyphToUnicode->begin(), 0, glyphCount * sizeof(SkUnichar));
     for (DWORD i = 0; i < glyphSet->cRanges; ++i) {
         // There is no guarantee that within a Unicode range, the corresponding
         // glyph id in a font file are continuous. So, even if we have ranges,
@@ -399,8 +399,9 @@
         // unlikely to have collisions since glyph reuse happens mostly for
         // different Unicode pages.
         for (USHORT j = 0; j < count; ++j) {
-            if (glyph[j] != 0xFFFF && glyph[j] < glyphCount && glyphToUnicode[glyph[j]] == 0) {
-                glyphToUnicode[glyph[j]] = chars[j];
+            if (glyph[j] != 0xffff && glyph[j] < glyphCount &&
+                (*glyphToUnicode)[glyph[j]] == 0) {
+                (*glyphToUnicode)[glyph[j]] = chars[j];
             }
         }
     }
@@ -1706,23 +1707,6 @@
     *isLocalStream = this->fSerializeAsStream;
 }
 
-void LogFontTypeface::getGlyphToUnicodeMap(SkUnichar* dstArray) const {
-    HDC hdc = ::CreateCompatibleDC(nullptr);
-    HFONT font = CreateFontIndirect(&fLogFont);
-    HFONT savefont = (HFONT)SelectObject(hdc, font);
-    LOGFONT lf = fLogFont;
-    HFONT designFont = CreateFontIndirect(&lf);
-    SelectObject(hdc, designFont);
-
-    unsigned int glyphCount = calculateGlyphCount(hdc, fLogFont);
-    populate_glyph_to_unicode(hdc, glyphCount, dstArray);
-
-    SelectObject(hdc, savefont);
-    DeleteObject(designFont);
-    DeleteObject(font);
-    DeleteDC(hdc);
-}
-
 std::unique_ptr<SkAdvancedTypefaceMetrics> LogFontTypeface::onGetAdvancedMetrics() const {
     LOGFONT lf = fLogFont;
     std::unique_ptr<SkAdvancedTypefaceMetrics> info(nullptr);
@@ -1773,6 +1757,8 @@
         }
     }
 
+    populate_glyph_to_unicode(hdc, glyphCount, &(info->fGlyphToUnicode));
+
     if (glyphCount > 0 &&
         (otm.otmTextMetrics.tmPitchAndFamily & TMPF_TRUETYPE)) {
         info->fType = SkAdvancedTypefaceMetrics::kTrueType_Font;
diff --git a/src/ports/SkTypeface_win_dw.cpp b/src/ports/SkTypeface_win_dw.cpp
index 38e68dc..69bf7ff 100644
--- a/src/ports/SkTypeface_win_dw.cpp
+++ b/src/ports/SkTypeface_win_dw.cpp
@@ -288,31 +288,42 @@
 ///////////////////////////////////////////////////////////////////////////////
 //PDF Support
 
-void DWriteFontTypeface::getGlyphToUnicodeMap(SkUnichar* glyphToUnicode) const {
-    unsigned glyphCount = fDWriteFontFace->GetGlyphCount();
-    sk_bzero(glyphToUnicode, sizeof(SkUnichar) * glyphCount);
-    IDWriteFontFace* fontFace = fDWriteFontFace.get();
+// Construct Glyph to Unicode table.
+// Unicode code points that require conjugate pairs in utf16 are not
+// supported.
+// TODO(bungeman): This never does what anyone wants.
+// What is really wanted is the text to glyphs mapping
+static void populate_glyph_to_unicode(IDWriteFontFace* fontFace,
+                                      const unsigned glyphCount,
+                                      SkTDArray<SkUnichar>* glyphToUnicode) {
+    //Do this like free type instead
+    SkAutoTMalloc<SkUnichar> glyphToUni(
+            (SkUnichar*)sk_calloc_throw(sizeof(SkUnichar) * glyphCount));
     int maxGlyph = -1;
     unsigned remainingGlyphCount = glyphCount;
     for (UINT32 c = 0; c < 0x10FFFF && remainingGlyphCount != 0; ++c) {
         UINT16 glyph = 0;
-        HRVM(fontFace->GetGlyphIndices(&c, 1, &glyph), "Failed to get glyph index.");
+        HRVM(fontFace->GetGlyphIndices(&c, 1, &glyph),
+             "Failed to get glyph index.");
         // Intermittent DW bug on Windows 10. See crbug.com/470146.
         if (glyph >= glyphCount) {
-            return;
+          return;
         }
-        if (0 < glyph && glyphToUnicode[glyph] == 0) {
+        if (0 < glyph && glyphToUni[glyph] == 0) {
             maxGlyph = SkTMax(static_cast<int>(glyph), maxGlyph);
-            glyphToUnicode[glyph] = c;  // Always use lowest-index unichar.
+            glyphToUni[glyph] = c;  // Always use lowest-index unichar.
             --remainingGlyphCount;
         }
     }
+    SkTDArray<SkUnichar>(glyphToUni, maxGlyph + 1).swap(*glyphToUnicode);
 }
 
 std::unique_ptr<SkAdvancedTypefaceMetrics> DWriteFontTypeface::onGetAdvancedMetrics() const {
 
     std::unique_ptr<SkAdvancedTypefaceMetrics> info(nullptr);
 
+    const unsigned glyphCount = fDWriteFontFace->GetGlyphCount();
+
     DWRITE_FONT_METRICS dwfm;
     fDWriteFontFace->GetMetrics(&dwfm);
 
@@ -347,6 +358,9 @@
         info->fPostScriptName = info->fFontName;
     }
 
+
+    populate_glyph_to_unicode(fDWriteFontFace.get(), glyphCount, &(info->fGlyphToUnicode));
+
     DWRITE_FONT_FACE_TYPE fontType = fDWriteFontFace->GetType();
     if (fontType != DWRITE_FONT_FACE_TYPE_TRUETYPE &&
         fontType != DWRITE_FONT_FACE_TYPE_TRUETYPE_COLLECTION)
diff --git a/src/ports/SkTypeface_win_dw.h b/src/ports/SkTypeface_win_dw.h
index 75be040..7abbde5 100644
--- a/src/ports/SkTypeface_win_dw.h
+++ b/src/ports/SkTypeface_win_dw.h
@@ -104,7 +104,6 @@
     SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
                                            const SkDescriptor*) const override;
     void onFilterRec(SkScalerContextRec*) const override;
-    void getGlyphToUnicodeMap(SkUnichar* glyphToUnicode) const override;
     std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override;
     void onGetFontDescriptor(SkFontDescriptor*, bool*) const override;
     int onCharsToGlyphs(const void* chars, Encoding encoding,
diff --git a/tests/FontMgrTest.cpp b/tests/FontMgrTest.cpp
index 0f035a2..17a60bb 100644
--- a/tests/FontMgrTest.cpp
+++ b/tests/FontMgrTest.cpp
@@ -133,7 +133,6 @@
             return nullptr;
         }
         void onFilterRec(SkScalerContextRec*) const override { }
-        void getGlyphToUnicodeMap(SkUnichar*) const override { }
         std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override {
             return nullptr;
         }
diff --git a/tests/PDFGlyphsToUnicodeTest.cpp b/tests/PDFGlyphsToUnicodeTest.cpp
index 2aeedf0..332520b 100644
--- a/tests/PDFGlyphsToUnicodeTest.cpp
+++ b/tests/PDFGlyphsToUnicodeTest.cpp
@@ -74,12 +74,9 @@
     glyphsInSubset.push(0x101);
     glyphToUnicode.push(0x1013);
 
-    SkGlyphID lastGlyphID = SkToU16(glyphToUnicode.count() - 1);
-
     SkDynamicMemoryWStream buffer;
     subset.setAll(glyphsInSubset.begin(), glyphsInSubset.count());
-    SkPDFAppendCmapSections(&glyphToUnicode[0], &subset, &buffer, true, 0,
-                            SkTMin<SkGlyphID>(0xFFFF,  lastGlyphID));
+    SkPDFAppendCmapSections(glyphToUnicode, &subset, &buffer, true, 0, 0xFFFF);
 
     char expectedResult[] =
 "4 beginbfchar\n\
@@ -101,8 +98,7 @@
     // Remove characters and ranges.
     buffer.reset();
 
-    SkPDFAppendCmapSections(&glyphToUnicode[0], &subset, &buffer, true, 8,
-                            SkTMin<SkGlyphID>(0x00FF, lastGlyphID));
+    SkPDFAppendCmapSections(glyphToUnicode, &subset, &buffer, true, 8, 0x00FF);
 
     char expectedResultChop1[] =
 "2 beginbfchar\n\
@@ -120,8 +116,7 @@
     // Remove characters from range to downdrade it to one char.
     buffer.reset();
 
-    SkPDFAppendCmapSections(&glyphToUnicode[0], &subset, &buffer, true, 0x00D,
-                            SkTMin<SkGlyphID>(0x00FE, lastGlyphID));
+    SkPDFAppendCmapSections(glyphToUnicode, &subset, &buffer, true, 0x00D, 0x00FE);
 
     char expectedResultChop2[] =
 "2 beginbfchar\n\
@@ -134,8 +129,7 @@
 
     buffer.reset();
 
-    SkPDFAppendCmapSections(&glyphToUnicode[0], nullptr, &buffer, false, 0xFC,
-                            SkTMin<SkGlyphID>(0x110, lastGlyphID));
+    SkPDFAppendCmapSections(glyphToUnicode, nullptr, &buffer, false, 0xFC, 0x110);
 
     char expectedResultSingleBytes[] =
 "2 beginbfchar\n\
@@ -161,7 +155,6 @@
     for (SkUnichar i = 0; i < 100; ++i) {
       glyphToUnicode.push(i + 29);
     }
-    lastGlyphID = SkToU16(glyphToUnicode.count() - 1);
 
     glyphsInSubset.push(0x2C);
     glyphsInSubset.push(0x44);
@@ -172,8 +165,7 @@
 
     SkDynamicMemoryWStream buffer2;
     subset2.setAll(glyphsInSubset.begin(), glyphsInSubset.count());
-    SkPDFAppendCmapSections(&glyphToUnicode[0], &subset2, &buffer2, true, 0,
-                            SkTMin<SkGlyphID>(0xFFFF, lastGlyphID));
+    SkPDFAppendCmapSections(glyphToUnicode, &subset2, &buffer2, true, 0, 0xffff);
 
     char expectedResult2[] =
 "4 beginbfchar\n\
diff --git a/tests/TypefaceTest.cpp b/tests/TypefaceTest.cpp
index d791f75..b6b71ae 100644
--- a/tests/TypefaceTest.cpp
+++ b/tests/TypefaceTest.cpp
@@ -203,11 +203,11 @@
 
 namespace {
 
-class EmptyTypeface : public SkTypeface {
+class SkEmptyTypeface : public SkTypeface {
 public:
-    static sk_sp<SkTypeface> Create() { return sk_sp<SkTypeface>(new EmptyTypeface()); }
+    static sk_sp<SkTypeface> Create() { return sk_sp<SkTypeface>(new SkEmptyTypeface()); }
 protected:
-    EmptyTypeface() : SkTypeface(SkFontStyle(), true) { }
+    SkEmptyTypeface() : SkTypeface(SkFontStyle(), true) { }
 
     SkStreamAsset* onOpenStream(int* ttcIndex) const override { return nullptr; }
     SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
@@ -215,7 +215,6 @@
         return nullptr;
     }
     void onFilterRec(SkScalerContextRec*) const override { }
-    void getGlyphToUnicodeMap(SkUnichar*) const override { }
     std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override {
         return nullptr;
     }
@@ -256,12 +255,12 @@
 }
 
 DEF_TEST(TypefaceCache, reporter) {
-    sk_sp<SkTypeface> t1(EmptyTypeface::Create());
+    sk_sp<SkTypeface> t1(SkEmptyTypeface::Create());
     {
         SkTypefaceCache cache;
         REPORTER_ASSERT(reporter, count(reporter, cache) == 0);
         {
-            sk_sp<SkTypeface> t0(EmptyTypeface::Create());
+            sk_sp<SkTypeface> t0(SkEmptyTypeface::Create());
             cache.add(t0.get());
             REPORTER_ASSERT(reporter, count(reporter, cache) == 1);
             cache.add(t1.get());
diff --git a/tools/fonts/SkRandomScalerContext.cpp b/tools/fonts/SkRandomScalerContext.cpp
index 9379759..2d47212 100644
--- a/tools/fonts/SkRandomScalerContext.cpp
+++ b/tools/fonts/SkRandomScalerContext.cpp
@@ -164,10 +164,6 @@
     rec->fMaskFormat = SkMask::kARGB32_Format;
 }
 
-void SkRandomTypeface::getGlyphToUnicodeMap(SkUnichar* glyphToUnicode) const {
-    fProxy->getGlyphToUnicodeMap(glyphToUnicode);
-}
-
 std::unique_ptr<SkAdvancedTypefaceMetrics> SkRandomTypeface::onGetAdvancedMetrics() const {
     return fProxy->getAdvancedMetrics();
 }
diff --git a/tools/fonts/SkRandomScalerContext.h b/tools/fonts/SkRandomScalerContext.h
index 5dbdac6..b71689d 100644
--- a/tools/fonts/SkRandomScalerContext.h
+++ b/tools/fonts/SkRandomScalerContext.h
@@ -27,7 +27,6 @@
     SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
                                            const SkDescriptor*) const override;
     void onFilterRec(SkScalerContextRec*) const override;
-    void getGlyphToUnicodeMap(SkUnichar*) const override;
     std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override;
     SkStreamAsset* onOpenStream(int* ttcIndex) const override;
     void onGetFontDescriptor(SkFontDescriptor*, bool* isLocal) const override;
diff --git a/tools/fonts/SkTestSVGTypeface.cpp b/tools/fonts/SkTestSVGTypeface.cpp
index 199c3b5..ceb523a 100644
--- a/tools/fonts/SkTestSVGTypeface.cpp
+++ b/tools/fonts/SkTestSVGTypeface.cpp
@@ -100,17 +100,15 @@
     rec->setHinting(SkPaint::kNo_Hinting);
 }
 
-void SkTestSVGTypeface::getGlyphToUnicodeMap(SkUnichar* glyphToUnicode) const {
-    SkDEBUGCODE(unsigned glyphCount = this->countGlyphs());
-    fCMap.foreach([=](const SkUnichar& c, const SkGlyphID& g) {
-        SkASSERT(g < glyphCount);
-        glyphToUnicode[g] = c;
-    });
-}
-
 std::unique_ptr<SkAdvancedTypefaceMetrics> SkTestSVGTypeface::onGetAdvancedMetrics() const {
     std::unique_ptr<SkAdvancedTypefaceMetrics> info(new SkAdvancedTypefaceMetrics);
-    info->fFontName = fName;
+    info->fFontName.set(fName);
+
+    SkTDArray<SkUnichar>& toUnicode = info->fGlyphToUnicode;
+    toUnicode.setCount(fGlyphCount);
+    fCMap.foreach([&toUnicode](const SkUnichar& c, const SkGlyphID& g) {
+        toUnicode[g] = c;
+    });
     return info;
 }
 
diff --git a/tools/fonts/SkTestSVGTypeface.h b/tools/fonts/SkTestSVGTypeface.h
index 249cd8b..0b53245 100644
--- a/tools/fonts/SkTestSVGTypeface.h
+++ b/tools/fonts/SkTestSVGTypeface.h
@@ -77,7 +77,6 @@
     SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
                                            const SkDescriptor* desc) const override;
     void onFilterRec(SkScalerContextRec* rec) const override;
-    void getGlyphToUnicodeMap(SkUnichar*) const override;
     std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override;
 
     SkStreamAsset* onOpenStream(int* ttcIndex) const override {
diff --git a/tools/fonts/SkTestTypeface.cpp b/tools/fonts/SkTestTypeface.cpp
index bf3a1a3..d7caa5a 100644
--- a/tools/fonts/SkTestTypeface.cpp
+++ b/tools/fonts/SkTestTypeface.cpp
@@ -119,16 +119,17 @@
     rec->setHinting(SkPaint::kNo_Hinting);
 }
 
-void SkTestTypeface::getGlyphToUnicodeMap(SkUnichar* glyphToUnicode) const {
-    unsigned glyphCount = fTestFont->fCharCodesCount;
-    for (unsigned gid = 0; gid < glyphCount; ++gid) {
-        glyphToUnicode[gid] = SkTo<SkUnichar>(fTestFont->fCharCodes[gid]);
-    }
-}
-
 std::unique_ptr<SkAdvancedTypefaceMetrics> SkTestTypeface::onGetAdvancedMetrics() const { // pdf only
     std::unique_ptr<SkAdvancedTypefaceMetrics> info(new SkAdvancedTypefaceMetrics);
     info->fFontName.set(fTestFont->fName);
+    int glyphCount = this->onCountGlyphs();
+
+    SkTDArray<SkUnichar>& toUnicode = info->fGlyphToUnicode;
+    toUnicode.setCount(glyphCount);
+    SkASSERT(glyphCount == SkToInt(fTestFont->fCharCodesCount));
+    for (int gid = 0; gid < glyphCount; ++gid) {
+        toUnicode[gid] = SkToS32(fTestFont->fCharCodes[gid]);
+    }
     return info;
 }
 
diff --git a/tools/fonts/SkTestTypeface.h b/tools/fonts/SkTestTypeface.h
index 205d82c..d3365f9 100644
--- a/tools/fonts/SkTestTypeface.h
+++ b/tools/fonts/SkTestTypeface.h
@@ -71,7 +71,6 @@
     SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
                                            const SkDescriptor* desc) const override;
     void onFilterRec(SkScalerContextRec* rec) const override;
-    void getGlyphToUnicodeMap(SkUnichar* glyphToUnicode) const override;
     std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override;
 
     SkStreamAsset* onOpenStream(int* ttcIndex) const override {