SkPDF: Glyph validation change

src/pdf/SkPDFFont.h

Index: src/pdf/SkPDFFont.h
diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h
index abf164c0137b503fd09827fecce39eb16cab69f8..de2d46a570aea66cef5baa970e92cfc9115f72bf 100644
--- a/src/pdf/SkPDFFont.h
+++ b/src/pdf/SkPDFFont.h
@@ -35,6 +35,18 @@ public:
      */
     SkTypeface* typeface() const { return fTypeface.get(); }
 
+    struct TypefaceInfo {
+        SkGlyphID fMaxGlyphID;
+        bool      fGood;
+    };
+    static TypefaceInfo GetTypefaceInfo(SkTypeface* face) {
+        SkASSERT(face);
+        int count = face->countGlyphs();
+        return (count > 0 && count <= 1 + SK_MaxU16)
+               ? TypefaceInfo{SkToU16(count - 1), true}
+               : TypefaceInfo{0, false};
+    }
+
     /** Returns the font type represented in this font.  For Type0 fonts,
      *  returns the type of the decendant font.
      */
@@ -55,23 +67,35 @@ 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;
+    }
+
+    /** 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 {
+        SkASSERT(!this->multiByteGlyphs());

[bungeman-skia, 2016/08/25 19:28:44]

This seems like a really odd thing to put here. Why not just

if (this->multiByteGlyphs) { return numGlyphs; }

This would also mean you wouldn't need the get_stretch function in
SkPDFDevice.cpp. With this assert, you need to document a really constraining
pre-condition.

[hal.canary, 2016/08/26 15:27:04]

Done.

+        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(const SkGlyphID* glyphs, int count) {
-        fGlyphUsage.setAll(glyphs, count);
+    void noteGlyphUsage(SkGlyphID glyph) {
+        SkASSERT(this->hasGlyph(glyph));
+        fGlyphUsage.set(glyph);
     }
 
     /** Get the font resource for the passed typeface and glyphID. The
@@ -79,7 +103,7 @@ 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, non-NULL.

[bungeman-skia, 2016/08/25 19:28:44]

nit: what's NULL? Oh, it's that old name for nullptr.

[hal.canary, 2016/08/26 15:27:04]

Done.

      *  @param glyphID   Specify which section of a large font is of interest.
      */
     static SkPDFFont* GetFontResource(SkPDFCanon* canon,
@@ -87,6 +111,7 @@ public:
                                       SkGlyphID glyphID);
 
     // Uses (kGlyphNames_PerGlyphInfo | kToUnicode_PerGlyphInfo).
+    // typeface is non-NULL
     static const SkAdvancedTypefaceMetrics* GetMetrics(SkTypeface* typeface,
                                                        SkPDFCanon* canon);
 
@@ -97,7 +122,7 @@ public:
 
     /**
      *  Return false iff the typeface has its NotEmbeddable flag set.
-     *  If typeface is NULL, the default typeface is checked.
+     *  typeface is non-NULL
      */
     static bool CanEmbedTypeface(SkTypeface*, SkPDFCanon*);
 
@@ -124,9 +149,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;
 };