SkPDF: Glyph validation change

Instead of mapping invaid glyphIDs to zero or maxGlyphID,
don't draw them at all.

Validate glyphs when glyph is written, not ahead of time.

Don't allocate array to copy user-provided glyphs.

Easy early exit from SkPDFDevice::internalDrawText()
    GlyphPositioner::flush() called ~GlyphPositioner()
    SkScopeExit class now exists.

Assume SkTypeface* pointers are now never null in more
places.

precalculate alignmentFactor to clean up code.

SkPDFDevice::updateFont must be called with validated
glyphID.  Skip bad glyphs to make this true.

SkPDFDevice::updateFont always succeeds.

SkPDFFont::GetFontResource always succeeds (preconditions are
asserted).  If GetMetrics fails, don't call GetFontResource.

SkPDFFont::glyphsToPDFFontEncodingCount() becomes
SkPDFFont::countStretch() and is inlined.

SkPDFFont::glyphsToPDFFontEncoding now works one Glyph at a
time and is inlined.

SkPDFFont::noteGlyphUsage() operates one glyph at a time.

Add SkScopeExit.h; also a unit test for it.

SkPostConfig: Fix SK_UNUSED for Win32.

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

BUG=625995

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

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

5 files changed, 228 insertions, 199 deletions

diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index a36125f380..24df879a9d 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -28,11 +28,11 @@
 #include "SkPDFUtils.h"
 #include "SkRasterClip.h"
 #include "SkRRect.h"
+#include "SkScopeExit.h"
 #include "SkString.h"
 #include "SkSurface.h"
 #include "SkTextFormatParams.h"
 #include "SkTemplates.h"
-#include "SkTypefacePriv.h"
 #include "SkXfermodeInterpretation.h"
 
 #define DPI_FOR_RASTER_SCALE_ONE 72
@@ -76,19 +76,6 @@ static SkPaint calculate_text_paint(const SkPaint& paint) {
     return result;
 }
 
-static void set_text_transform(SkScalar x, SkScalar y, SkScalar textSkewX,
-                               SkWStream* content) {
-    // Flip the text about the x-axis to account for origin swap and include
-    // the passed parameters.
-    content->writeText("1 0 ");
-    SkPDFUtils::AppendScalar(0 - textSkewX, content);
-    content->writeText(" -1 ");
-    SkPDFUtils::AppendScalar(x, content);
-    content->writeText(" ");
-    SkPDFUtils::AppendScalar(y, content);
-    content->writeText(" Tm\n");
-}
-
 SkPDFDevice::GraphicStateEntry::GraphicStateEntry()
     : fColor(SK_ColorBLACK)
     , fTextScaleX(SK_Scalar1)
@@ -965,15 +952,22 @@ public:
                     bool defaultPositioning,
                     SkPoint origin)
         : fContent(content)
-        , fCurrentMatrixX(0.0f)
-        , fCurrentMatrixY(0.0f)
+        , fCurrentMatrixOrigin{0.0f, 0.0f}
         , fXAdvance(0.0f)
         , fWideChars(wideChars)
         , fInText(false)
         , fDefaultPositioning(defaultPositioning) {
-        set_text_transform(origin.x(), origin.y(), textSkewX, fContent);
-    }
-    ~GlyphPositioner() { SkASSERT(!fInText);  /* flush first */ }
+        // Flip the text about the x-axis to account for origin swap and include
+        // the passed parameters.
+        fContent->writeText("1 0 ");
+        SkPDFUtils::AppendScalar(0 - textSkewX, fContent);
+        fContent->writeText(" -1 ");
+        SkPDFUtils::AppendScalar(origin.x(), fContent);
+        fContent->writeText(" ");
+        SkPDFUtils::AppendScalar(origin.y(), fContent);
+        fContent->writeText(" Tm\n");
+    }
+    ~GlyphPositioner() { this->flush(); }
     void flush() {
         if (fInText) {
             fContent->writeText("> Tj\n");
@@ -987,21 +981,18 @@ public:
             fWideChars = wideChars;
         }
     }
-    void writeGlyph(SkScalar x,
-                    SkScalar y,
+    void writeGlyph(SkPoint xy,
                     SkScalar advanceWidth,
                     uint16_t glyph) {
         if (!fDefaultPositioning) {
-            SkScalar xPosition = x - fCurrentMatrixX;
-            SkScalar yPosition = y - fCurrentMatrixY;
-            if (xPosition != fXAdvance || yPosition != 0) {
+            SkPoint position = xy - fCurrentMatrixOrigin;
+            if (position != SkPoint{fXAdvance, 0}) {
                 this->flush();
-                SkPDFUtils::AppendScalar(xPosition, fContent);
+                SkPDFUtils::AppendScalar(position.x(), fContent);
                 fContent->writeText(" ");
-                SkPDFUtils::AppendScalar(-yPosition, fContent);
+                SkPDFUtils::AppendScalar(-position.y(), fContent);
                 fContent->writeText(" Td ");
-                fCurrentMatrixX = x;
-                fCurrentMatrixY = y;
+                fCurrentMatrixOrigin = xy;
                 fXAdvance = 0;
             }
             fXAdvance += advanceWidth;
@@ -1020,8 +1011,7 @@ public:
 
 private:
     SkDynamicMemoryWStream* fContent;
-    SkScalar fCurrentMatrixX;
-    SkScalar fCurrentMatrixY;
+    SkPoint fCurrentMatrixOrigin;
     SkScalar fXAdvance;
     bool fWideChars;
     bool fInText;
@@ -1034,12 +1024,13 @@ static void draw_transparent_text(SkPDFDevice* device,
                                   const void* text, size_t len,
                                   SkScalar x, SkScalar y,
                                   const SkPaint& srcPaint) {
-    SkPaint transparent;
-    if (!SkPDFFont::CanEmbedTypeface(transparent.getTypeface(),
-                                     device->getCanon())) {
+    sk_sp<SkTypeface> defaultFace = SkTypeface::MakeDefault();
+    if (!SkPDFFont::CanEmbedTypeface(defaultFace.get(), device->getCanon())) {
         SkDebugf("SkPDF: default typeface should be embeddable");
         return;  // Avoid infinite loop in release.
     }
+    SkPaint transparent;
+    transparent.setTypeface(std::move(defaultFace));
     transparent.setTextSize(srcPaint.getTextSize());
     transparent.setColor(SK_ColorTRANSPARENT);
     switch (srcPaint.getTextEncoding()) {
@@ -1097,11 +1088,14 @@ void SkPDFDevice::internalDrawText(
         SkDebugf("SkPDF: SkTypeface::MakeDefault() returned nullptr.\n");
         return;
     }
-    int typefaceGlyphCount = typeface->countGlyphs();
-    if (typefaceGlyphCount < 1) {
-        SkDebugf("SkPDF: SkTypeface has no glyphs.\n");
+
+    const SkAdvancedTypefaceMetrics* metrics =
+        SkPDFFont::GetMetrics(typeface, fDocument->canon());
+    if (!metrics) {
         return;
     }
+    // TODO(halcanary): use metrics->fGlyphToUnicode to check Unicode mapping.
+    const SkGlyphID maxGlyphID = metrics->fLastGlyphID;
     if (!SkPDFFont::CanEmbedTypeface(typeface, fDocument->canon())) {
         SkPath path; // https://bug.skia.org/3866
         paint.getTextPath(sourceText, sourceByteCount,
@@ -1112,21 +1106,19 @@ void SkPDFDevice::internalDrawText(
                               offset.x(), offset.y(), paint);
         return;
     }
-    // Always make a copy (1) to validate user-input glyphs and
-    // (2) because we may modify the glyphs in place (for
-    // single-byte-glyph-id PDF fonts).
     int glyphCount = paint.textToGlyphs(sourceText, sourceByteCount, nullptr);
-    if (glyphCount <= 0) { return; }
-    SkAutoSTMalloc<128, SkGlyphID> glyphStorage(SkToSizeT(glyphCount));
-    SkGlyphID* glyphs = glyphStorage.get();
-    (void)paint.textToGlyphs(sourceText, sourceByteCount, glyphs);
+    if (glyphCount <= 0) {
+        return;
+    }
+    SkAutoSTMalloc<128, SkGlyphID> glyphStorage;
+    const SkGlyphID* glyphs = nullptr;
     if (paint.getTextEncoding() == SkPaint::kGlyphID_TextEncoding) {
-        // Validate user-input glyphs.
-        SkGlyphID maxGlyphID = SkToU16(typefaceGlyphCount - 1);
-        for (int i = 0; i < glyphCount; ++i) {
-            glyphs[i] = SkTMin(maxGlyphID, glyphs[i]);
-        }
+        glyphs = (const SkGlyphID*)sourceText;
+        // validate input later.
     } else {
+        glyphStorage.reset(SkToSizeT(glyphCount));
+        (void)paint.textToGlyphs(sourceText, sourceByteCount, glyphStorage.get());
+        glyphs = glyphStorage.get();
         paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
     }
 
@@ -1135,89 +1127,92 @@ void SkPDFDevice::internalDrawText(
     SkAutoGlyphCache glyphCache(paint, nullptr, nullptr);
 
     SkPaint::Align alignment = paint.getTextAlign();
+    float alignmentFactor = SkPaint::kLeft_Align   == alignment ?  0.0f :
+                            SkPaint::kCenter_Align == alignment ? -0.5f :
+                            /* SkPaint::kRight_Align */           -1.0f;
     if (defaultPositioning && alignment != SkPaint::kLeft_Align) {
-        SkScalar advance{0};
+        SkScalar advance = 0;
         for (int i = 0; i < glyphCount; ++i) {
             advance += glyphCache->getGlyphIDAdvance(glyphs[i]).fAdvanceX;
         }
-        SkScalar m = alignment == SkPaint::kCenter_Align
-            ? 0.5f * advance : advance;
-        offset -= SkPoint{m, 0};
+        offset.offset(alignmentFactor * advance, 0);
     }
     ScopedContentEntry content(this, d, paint, true);
     if (!content.entry()) {
         return;
     }
     SkDynamicMemoryWStream* out = &content.entry()->fContent;
-    out->writeText("BT\n");
-    if (!this->updateFont(paint, glyphs[0], content.entry())) {
-        SkDebugf("SkPDF: Font error.");
-        out->writeText("ET\n%SkPDF: Font error.\n");
-        return;
+    SkScalar textSize = paint.getTextSize();
+
+    int index = 0;
+    while (glyphs[index] > maxGlyphID) {  // Invalid glyphID for this font.
+        ++index;  // Skip this glyphID
+        if (index == glyphCount) {
+            return;  // all glyphIDs were bad.
+        }
     }
-    SkPDFFont* font = content.entry()->fState.fFont;
+
+    out->writeText("BT\n");
+    SK_AT_SCOPE_EXIT(out->writeText("ET\n"));
+
+    SkPDFFont* font = this->updateFont(
+            typeface, textSize, glyphs[index], content.entry());
+    SkASSERT(font);  // All preconditions for SkPDFFont::GetFontResource are met.
+    if (!font) { return; }
+
     GlyphPositioner glyphPositioner(out,
                                     paint.getTextSkewX(),
                                     font->multiByteGlyphs(),
                                     defaultPositioning,
                                     offset);
-    const SkGlyphID* const glyphsEnd = glyphs + glyphCount;
-
-    while (glyphs < glyphsEnd) {
-        font = content.entry()->fState.fFont;
-        int stretch = font->multiByteGlyphs()
-            ? SkToInt(glyphsEnd - glyphs)
-            : font->glyphsToPDFFontEncodingCount(glyphs, SkToInt(glyphsEnd - glyphs));
-        SkASSERT(glyphs + stretch <= glyphsEnd);
+
+    while (index < glyphCount) {
+        int stretch = font->countStretch(&glyphs[index], glyphCount - index, maxGlyphID);
+        SkASSERT(index + stretch <= glyphCount);
         if (stretch < 1) {
-            SkASSERT(!font->multiByteGlyphs());
             // The current pdf font cannot encode the next glyph.
             // Try to get a pdf font which can encode the next glyph.
             glyphPositioner.flush();
-            if (!this->updateFont(paint, *glyphs, content.entry())) {
-                SkDebugf("SkPDF: Font error.");
-                out->writeText("ET\n%SkPDF: Font error.\n");
-                return;
+            // first, validate the next glyph
+            while (glyphs[index] > maxGlyphID) {
+                ++index;  // Skip this glyphID
+                if (index == glyphCount) {
+                    return;  // all remainng glyphIDs were bad.
+                }
             }
-            font = content.entry()->fState.fFont;
+            SkASSERT(index < glyphCount);
+            font = this->updateFont(typeface, textSize, glyphs[index], content.entry());
+            SkASSERT(font);  // preconditions for SkPDFFont::GetFontResource met.
+            if (!font) { return; }
             glyphPositioner.setWideChars(font->multiByteGlyphs());
-            // try again
-            stretch = font->glyphsToPDFFontEncodingCount(glyphs,
-                                                         SkToInt(glyphsEnd - glyphs));
+            // Get stretch for this new font.
+            stretch = font->countStretch(&glyphs[index], glyphCount - index, maxGlyphID);
             if (stretch < 1) {
                 SkDEBUGFAIL("PDF could not encode glyph.");
-                glyphPositioner.flush();
-                out->writeText("ET\n%SkPDF: Font encoding error.\n");
                 return;
             }
         }
-        font->noteGlyphUsage(glyphs, stretch);
-        if (defaultPositioning) {
-            (void)font->glyphsToPDFFontEncoding(glyphs, SkToInt(glyphsEnd - glyphs));
-            while (stretch-- > 0) {
-                glyphPositioner.writeGlyph(0, 0, 0, *glyphs);
-                ++glyphs;
-            }
-        } else {
-            while (stretch-- > 0) {
-                SkScalar advance = glyphCache->getGlyphIDAdvance(*glyphs).fAdvanceX;
-                SkScalar x = *pos++;
-                // evaluate x and y in order!
-                SkScalar y = SkTextBlob::kFull_Positioning == positioning ? *pos++ : 0;
-                SkPoint xy{x, y};
-                if (alignment != SkPaint::kLeft_Align) {
-                    SkScalar m = alignment == SkPaint::kCenter_Align
-                        ? 0.5f * advance : advance;
-                    xy -= SkPoint{m, 0};
+        while (stretch-- > 0) {
+            SkGlyphID gid = glyphs[index];
+            if (gid <= maxGlyphID) {
+                font->noteGlyphUsage(gid);
+                SkGlyphID encodedGlyph = font->glyphToPDFFontEncoding(gid);
+                if (defaultPositioning) {
+                    glyphPositioner.writeGlyph(SkPoint{0, 0}, 0, encodedGlyph);
+                } else {
+                    SkScalar advance = glyphCache->getGlyphIDAdvance(gid).fAdvanceX;
+                    SkPoint xy = SkTextBlob::kFull_Positioning == positioning
+                               ? SkPoint{pos[2 * index], pos[2 * index + 1]}
+                               : SkPoint{pos[index], 0};
+                    if (alignment != SkPaint::kLeft_Align) {
+                        xy.offset(alignmentFactor * advance, 0);
+                    }
+                    glyphPositioner.writeGlyph(xy, advance, encodedGlyph);
                 }
-                (void)font->glyphsToPDFFontEncoding(glyphs, 1);
-                glyphPositioner.writeGlyph(xy.x(), xy.y(), advance, *glyphs);
-                ++glyphs;
             }
+            ++index;
         }
     }
-    glyphPositioner.flush();
-    out->writeText("ET\n");
 }
 
 void SkPDFDevice::drawText(const SkDraw& d, const void* text, size_t len,
@@ -1903,26 +1898,28 @@ int SkPDFDevice::addXObjectResource(SkPDFObject* xObject) {
     return result;
 }
 
-bool SkPDFDevice::updateFont(const SkPaint& paint, uint16_t glyphID,
-                             SkPDFDevice::ContentEntry* contentEntry) {
-    SkTypeface* typeface = paint.getTypeface();
+SkPDFFont* SkPDFDevice::updateFont(SkTypeface* typeface,
+                                   SkScalar textSize,
+                                   uint16_t glyphID,
+                                   SkPDFDevice::ContentEntry* contentEntry) {
     if (contentEntry->fState.fFont == nullptr ||
-            contentEntry->fState.fTextSize != paint.getTextSize() ||
+            contentEntry->fState.fTextSize != textSize ||
             !contentEntry->fState.fFont->hasGlyph(glyphID)) {
         int fontIndex = getFontResourceIndex(typeface, glyphID);
         if (fontIndex < 0) {
-            return false;
+            SkDebugf("SkPDF: Font error.");
+            return nullptr;
         }
         contentEntry->fContent.writeText("/");
         contentEntry->fContent.writeText(SkPDFResourceDict::getResourceName(
                 SkPDFResourceDict::kFont_ResourceType,
                 fontIndex).c_str());
         contentEntry->fContent.writeText(" ");
-        SkPDFUtils::AppendScalar(paint.getTextSize(), &contentEntry->fContent);
+        SkPDFUtils::AppendScalar(textSize, &contentEntry->fContent);
         contentEntry->fContent.writeText(" Tf\n");
         contentEntry->fState.fFont = fFontResources[fontIndex];
     }
-    return true;
+    return contentEntry->fState.fFont;
 }
 
 int SkPDFDevice::getFontResourceIndex(SkTypeface* typeface, uint16_t glyphID) {
diff --git a/src/pdf/SkPDFDevice.h b/src/pdf/SkPDFDevice.h
index b9ef501518..79b272d9ac 100644
--- a/src/pdf/SkPDFDevice.h
+++ b/src/pdf/SkPDFDevice.h
@@ -277,8 +277,11 @@ private:
     int addGraphicStateResource(SkPDFObject* gs);
     int addXObjectResource(SkPDFObject* xObject);
 
-    // returns false when a valid SkFont can not be produced
-    bool updateFont(const SkPaint& paint, uint16_t glyphID, ContentEntry* contentEntry);
+    // returns nullptr when a valid SkFont can not be produced
+    SkPDFFont* updateFont(SkTypeface* typeface,
+                          SkScalar textSize,
+                          uint16_t glyphID,
+                          ContentEntry* contentEntry);
     int getFontResourceIndex(SkTypeface* typeface, uint16_t glyphID);
 
 
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index 389df62fd5..d7ef8123a2 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -18,7 +18,6 @@
 #include "SkRefCnt.h"
 #include "SkScalar.h"
 #include "SkStream.h"
-#include "SkTypefacePriv.h"
 #include "SkTypes.h"
 #include "SkUtils.h"
 
@@ -148,63 +147,28 @@ static bool can_subset(const SkAdvancedTypefaceMetrics& metrics) {
 }
 #endif
 
-int SkPDFFont::glyphsToPDFFontEncoding(SkGlyphID* glyphIDs, int numGlyphs) const {
-    // A font with multibyte glyphs will support all glyph IDs in a single font.
-    if (this->multiByteGlyphs()) {
-        return numGlyphs;
-    }
-
-    for (int i = 0; i < numGlyphs; i++) {
-        if (glyphIDs[i] == 0) {
-            continue;
-        }
-        if (glyphIDs[i] < fFirstGlyphID || glyphIDs[i] > fLastGlyphID) {
-            return i;
-        }
-        glyphIDs[i] -= (fFirstGlyphID - 1);
-    }
-
-    return numGlyphs;
-}
-
-int SkPDFFont::glyphsToPDFFontEncodingCount(const SkGlyphID* glyphIDs,
-                                            int numGlyphs) const {
-    if (this->multiByteGlyphs()) {    // A font with multibyte glyphs will
-        return numGlyphs;             // support all glyph IDs in a single font.
-    }
-    for (int i = 0; i < numGlyphs; i++) {
-        if (glyphIDs[i] != 0 &&
-            (glyphIDs[i] < fFirstGlyphID || glyphIDs[i] > fLastGlyphID)) {
-            return i;
-        }
-    }
-    return numGlyphs;
-}
-
-
 const SkAdvancedTypefaceMetrics* SkPDFFont::GetMetrics(SkTypeface* typeface,
                                                        SkPDFCanon* canon) {
-    SkFontID id = SkTypeface::UniqueID(typeface);
+    SkASSERT(typeface);
+    SkFontID id = typeface->uniqueID();
     if (SkAdvancedTypefaceMetrics** ptr = canon->fTypefaceMetrics.find(id)) {
         return *ptr;
     }
-    sk_sp<SkTypeface> defaultFace;
-    if (!typeface) {
-        defaultFace = SkTypeface::MakeDefault();
-        typeface = defaultFace.get();
+    int count = typeface->countGlyphs();
+    if (count <= 0 || count > 1 + SK_MaxU16) {
+        // Cache nullptr to skip this check.  Use SkSafeUnref().
+        canon->fTypefaceMetrics.set(id, nullptr);
+        return nullptr;
     }
     sk_sp<SkAdvancedTypefaceMetrics> metrics(
             typeface->getAdvancedTypefaceMetrics(
                     SkTypeface::kGlyphNames_PerGlyphInfo | SkTypeface::kToUnicode_PerGlyphInfo,
                     nullptr, 0));
     if (!metrics) {
-        if (typeface->countGlyphs() > 0) {
-            metrics = sk_make_sp<SkAdvancedTypefaceMetrics>();
-        } else {
-            SkDEBUGF(("SkPDF: SkTypeface:getAdvancedTypefaceMetrics() returned null.\n"));
-        }
-    }
-    // May cache null to skip this check.  use SkSafeUnref.
+        metrics = sk_make_sp<SkAdvancedTypefaceMetrics>();
+        metrics->fLastGlyphID = SkToU16(count - 1);
+    } 
+    SkASSERT(metrics->fLastGlyphID == SkToU16(count - 1));
     return *canon->fTypefaceMetrics.set(id, metrics.release());
 }
 
@@ -216,7 +180,7 @@ SkAdvancedTypefaceMetrics::FontType font_type(const SkAdvancedTypefaceMetrics& m
     return metrics.fType;
 }
 
-static SkGlyphID first_glyph_for_single_byte_encoding(SkGlyphID gid) {
+static SkGlyphID first_nonzero_glyph_for_single_byte_encoding(SkGlyphID gid) {
     return gid != 0 ? gid - (gid - 1) % 255 : 1;
 }
 
@@ -224,40 +188,37 @@ SkPDFFont* SkPDFFont::GetFontResource(SkPDFCanon* canon,
                                       SkTypeface* face,
                                       SkGlyphID glyphID) {
     SkASSERT(canon);
+    SkASSERT(face);  // All SkPDFDevice::internalDrawText ensures this.
     const SkAdvancedTypefaceMetrics* fontMetrics = SkPDFFont::GetMetrics(face, canon);
-    if (!fontMetrics) {
-        return nullptr;  // bad font, return early.
-    }
+    SkASSERT(fontMetrics);  // SkPDFDevice::internalDrawText ensures the typeface is good.
+                            // GetMetrics only returns null to signify a bad typeface.
     const SkAdvancedTypefaceMetrics& metrics = *fontMetrics;
     SkAdvancedTypefaceMetrics::FontType type = font_type(metrics);
     bool multibyte = SkPDFFont::IsMultiByte(type);
-    SkGlyphID firstGlyph = multibyte ? 0 : first_glyph_for_single_byte_encoding(glyphID);
-    uint64_t fontID = (SkTypeface::UniqueID(face) << 16) | firstGlyph;
+    SkGlyphID subsetCode = multibyte ? 0 : first_nonzero_glyph_for_single_byte_encoding(glyphID);
+    uint64_t fontID = (SkTypeface::UniqueID(face) << 16) | subsetCode;
 
     if (SkPDFFont** found = canon->fFontMap.find(fontID)) {
-        SkASSERT(multibyte == (*found)->multiByteGlyphs());
-        return SkRef(*found);
+        SkPDFFont* foundFont = *found;
+        SkASSERT(foundFont && multibyte == foundFont->multiByteGlyphs());
+        return SkRef(foundFont);
     }
 
-    sk_sp<SkTypeface> typeface(face ? sk_ref_sp(face) : SkTypeface::MakeDefault());
+    sk_sp<SkTypeface> typeface(sk_ref_sp(face));
     SkASSERT(typeface);
-    int glyphCount = typeface->countGlyphs();
-    // Validate typeface + glyph;
-    if (glyphCount < 1 ||  // typeface lacks even a NOTDEF glyph.
-        glyphCount > 1 + SK_MaxU16 ||  // invalid glyphCount
-        glyphID >= glyphCount) {  // invalid glyph
-        return nullptr;
-    }
+
+    SkGlyphID lastGlyph = metrics.fLastGlyphID;
+    SkASSERT(typeface->countGlyphs() == SkToInt(1 + metrics.fLastGlyphID));
+
+    // should be caught by SkPDFDevice::internalDrawText
+    SkASSERT(glyphID <= lastGlyph);
 
     SkGlyphID firstNonZeroGlyph;
-    SkGlyphID lastGlyph;
     if (multibyte) {
         firstNonZeroGlyph = 1;
-        lastGlyph = SkToU16(glyphCount - 1);
     } else {
-        firstNonZeroGlyph = firstGlyph;
-        lastGlyph = SkToU16(SkTMin<int>(glyphCount - 1,
-                                        254 + (int)firstGlyph));
+        firstNonZeroGlyph = subsetCode;
+        lastGlyph = SkToU16(SkTMin<int>((int)lastGlyph, 254 + (int)subsetCode));
     }
     SkPDFFont::Info info = {std::move(typeface), firstNonZeroGlyph, lastGlyph, type};
     sk_sp<SkPDFFont> font;
diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h
index abf164c013..9f33819ed8 100644
--- a/src/pdf/SkPDFFont.h
+++ b/src/pdf/SkPDFFont.h
@@ -55,23 +55,37 @@ public:
         return (gid >= fFirstGlyphID && gid <= fLastGlyphID) || gid == 0;
     }
 
-    /** Convert (in place) the input glyph IDs into the font encoding.  If the
-     *  font has more glyphs than can be encoded (like a type 1 font with more
-     *  than 255 glyphs) this method only converts up to the first out of range
-     *  glyph ID.
-     *  @param glyphIDs       The input text as glyph IDs.
-     *  @param numGlyphs      The number of input glyphs.
-     *  @return               Returns the number of glyphs consumed.
-     */
-    int glyphsToPDFFontEncoding(SkGlyphID* glyphIDs, int numGlyphs) const;
-    /**
-     * Like above, but does not modify glyphIDs array.
-     */
-    int glyphsToPDFFontEncodingCount(const SkGlyphID* glyphIDs,
-                                     int numGlyphs) const;
+    /** Convert the input glyph ID into the font encoding.  */
+    SkGlyphID glyphToPDFFontEncoding(SkGlyphID gid) const {
+        if (this->multiByteGlyphs() || gid == 0) {
+            return gid;
+        }
+        SkASSERT(gid >= fFirstGlyphID && gid <= fLastGlyphID);
+        SkASSERT(fFirstGlyphID > 0);
+        return gid - fFirstGlyphID + 1;
+    }
 
-    void noteGlyphUsage(const SkGlyphID* glyphs, int count) {
-        fGlyphUsage.setAll(glyphs, count);
+    /** Count the number of glyphIDs that can be encoded with this font.
+     *  glyphIDs > maxGlyphID are considered okay. */
+    int countStretch(const SkGlyphID* glyphIDs,
+                     int numGlyphs,
+                     SkGlyphID maxGlyphID) const {
+        if (this->multiByteGlyphs()) {
+            return numGlyphs;
+        }
+        for (int i = 0; i < numGlyphs; i++) {
+            SkGlyphID gid = glyphIDs[i];
+            if (gid != 0 && gid <= maxGlyphID &&
+                (gid < fFirstGlyphID || gid > fLastGlyphID)) {
+                return i;
+            }
+        }
+        return numGlyphs;
+    }
+
+    void noteGlyphUsage(SkGlyphID glyph) {
+        SkASSERT(this->hasGlyph(glyph));
+        fGlyphUsage.set(glyph);
     }
 
     /** Get the font resource for the passed typeface and glyphID. The
@@ -79,14 +93,18 @@ public:
      *  responsibility to unreference it when done.  This is needed to
      *  accommodate the weak reference pattern used when the returned object
      *  is new and has no other references.
-     *  @param typeface  The typeface to find.
+     *  @param typeface  The typeface to find, not nullptr.
      *  @param glyphID   Specify which section of a large font is of interest.
      */
     static SkPDFFont* GetFontResource(SkPDFCanon* canon,
                                       SkTypeface* typeface,
                                       SkGlyphID glyphID);
 
-    // Uses (kGlyphNames_PerGlyphInfo | kToUnicode_PerGlyphInfo).
+    /** Uses (kGlyphNames_PerGlyphInfo | kToUnicode_PerGlyphInfo) to get 
+     *  SkAdvancedTypefaceMetrics, and caches the result.
+     *  @param typeface can not be nullptr.
+     *  @return nullptr only when typeface is bad.
+     */
     static const SkAdvancedTypefaceMetrics* GetMetrics(SkTypeface* typeface,
                                                        SkPDFCanon* canon);
 
@@ -97,7 +115,7 @@ public:
 
     /**
      *  Return false iff the typeface has its NotEmbeddable flag set.
-     *  If typeface is NULL, the default typeface is checked.
+     *  typeface is not nullptr
      */
     static bool CanEmbedTypeface(SkTypeface*, SkPDFCanon*);
 
@@ -124,9 +142,9 @@ private:
 
     // 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;
+    const SkGlyphID fFirstGlyphID;
+    const SkGlyphID fLastGlyphID;
+    const SkAdvancedTypefaceMetrics::FontType fFontType;
 
     typedef SkPDFDict INHERITED;
 };
diff --git a/src/pdf/SkScopeExit.h b/src/pdf/SkScopeExit.h
new file mode 100644
index 0000000000..5b7bcdc076
--- /dev/null
+++ b/src/pdf/SkScopeExit.h
@@ -0,0 +1,50 @@
+/*
+ * 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 SkScopeExit_DEFINED
+#define SkScopeExit_DEFINED
+
+#include "SkTypes.h"
+
+/**
+ * SK_AT_SCOPE_EXIT(stmt) evaluates stmt when the current scope ends.
+ *
+ * E.g.
+ *    {
+ *        int x = 5;
+ *        {
+ *           SK_AT_SCOPE_EXIT(x--);
+ *           SkASSERT(x == 5);
+ *        }
+ *        SkASSERT(x == 4);
+ *    }
+ */
+template <typename Fn>
+class SkScopeExit {
+public:
+    SkScopeExit(Fn f) : fFn(std::move(f)) {}
+    ~SkScopeExit() { fFn(); }
+
+private:
+    Fn fFn;
+
+    SkScopeExit(           const SkScopeExit& ) = delete;
+    SkScopeExit& operator=(const SkScopeExit& ) = delete;
+    SkScopeExit(                 SkScopeExit&&) = delete;
+    SkScopeExit& operator=(      SkScopeExit&&) = delete;
+};
+
+template <typename Fn>
+inline SkScopeExit<Fn> SkMakeScopeExit(Fn&& fn) {
+    return {std::move(fn)};
+}
+
+#define SK_AT_SCOPE_EXIT(stmt)                              \
+    SK_UNUSED auto&& SK_MACRO_APPEND_LINE(at_scope_exit_) = \
+        SkMakeScopeExit([&]() { stmt; });
+
+#endif  // SkScopeExit_DEFINED