SkPDFCanon

SkPDFCanon's fields and methods will eventually become part of
SkPDFDocument/SkDocument_PDF.  For now, it exists as a singleton to
preflight that transition.  This replaces three global arrays in
SkPDFFont, SkPDFShader, and SkPDFGraphicsContext.  This code is still
thread-unsafe (http://skbug.com/2683), but moving this functionality
into SkPDFDocument will fix that.

This CL does not change pdf output from either GMs or SKPs.

This change also simplifies some code around the SkPDFCanon methods.

BUG=skia:2683

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

9 files changed, 433 insertions, 339 deletions

diff --git a/gyp/pdf.gypi b/gyp/pdf.gypi
index e4cb958e1c..a819e38164 100644
--- a/gyp/pdf.gypi
+++ b/gyp/pdf.gypi
@@ -12,6 +12,8 @@
 
         '<(skia_src_path)/doc/SkDocument_PDF.cpp',
 
+        '<(skia_src_path)/pdf/SkPDFCanon.cpp',
+        '<(skia_src_path)/pdf/SkPDFCanon.h',
         '<(skia_src_path)/pdf/SkPDFCatalog.cpp',
         '<(skia_src_path)/pdf/SkPDFCatalog.h',
         '<(skia_src_path)/pdf/SkPDFDevice.cpp',
diff --git a/src/pdf/SkPDFCanon.cpp b/src/pdf/SkPDFCanon.cpp
new file mode 100644
index 0000000000..31212f8c91
--- /dev/null
+++ b/src/pdf/SkPDFCanon.cpp
@@ -0,0 +1,136 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkLazyPtr.h"
+#include "SkPDFCanon.h"
+#include "SkPDFFont.h"
+#include "SkPDFGraphicState.h"
+#include "SkPDFShader.h"
+
+////////////////////////////////////////////////////////////////////////////////
+
+SK_DECLARE_STATIC_MUTEX(gSkPDFCanonFontMutex);
+SK_DECLARE_STATIC_MUTEX(gSkPDFCanonShaderMutex);
+SK_DECLARE_STATIC_MUTEX(gSkPDFCanonPaintMutex);
+
+SkBaseMutex& SkPDFCanon::GetFontMutex() { return gSkPDFCanonFontMutex; }
+SkBaseMutex& SkPDFCanon::GetShaderMutex() { return gSkPDFCanonShaderMutex; }
+SkBaseMutex& SkPDFCanon::GetPaintMutex() { return gSkPDFCanonPaintMutex; }
+
+SkPDFCanon::SkPDFCanon() {}
+
+SkPDFCanon::~SkPDFCanon() {}
+
+SK_DECLARE_STATIC_LAZY_PTR(SkPDFCanon, singleton);
+
+SkPDFCanon& SkPDFCanon::GetCanon() { return *singleton.get(); }
+
+////////////////////////////////////////////////////////////////////////////////
+
+static void assert_mutex_held(const SkPDFCanon* canon, SkBaseMutex& mutex) {
+    if (canon == singleton.get()) {
+        mutex.assertHeld();
+    }
+}
+
+SkPDFFont* SkPDFCanon::findFont(uint32_t fontID,
+                                uint16_t glyphID,
+                                SkPDFFont** relatedFontPtr) const {
+    assert_mutex_held(this, gSkPDFCanonFontMutex);
+    SkASSERT(relatedFontPtr);
+
+    SkPDFFont* relatedFont = NULL;
+    for (int i = 0; i < fFontRecords.count(); ++i) {
+        SkPDFFont::Match match = SkPDFFont::IsMatch(
+                fFontRecords[i].fFont, fFontRecords[i].fFontID,
+                fFontRecords[i].fGlyphID, fontID, glyphID);
+        if (SkPDFFont::kExact_Match == match) {
+            return fFontRecords[i].fFont;
+        } else if (!relatedFont && SkPDFFont::kRelated_Match == match) {
+            relatedFont = fFontRecords[i].fFont;
+        }
+    }
+    *relatedFontPtr = relatedFont;  // May still be NULL.
+    return NULL;
+}
+
+void SkPDFCanon::addFont(SkPDFFont* font, uint32_t fontID, uint16_t fGlyphID) {
+    assert_mutex_held(this, gSkPDFCanonFontMutex);
+    SkPDFCanon::FontRec* rec = fFontRecords.push();
+    rec->fFont = font;
+    rec->fFontID = fontID;
+    rec->fGlyphID = fGlyphID;
+}
+
+void SkPDFCanon::removeFont(SkPDFFont* pdfFont) {
+    assert_mutex_held(this, gSkPDFCanonFontMutex);
+    for (int i = 0; i < fFontRecords.count(); i++) {
+        if (fFontRecords[i].fFont == pdfFont) {
+            fFontRecords.removeShuffle(i);
+            return;
+        }
+    }
+    // Not all SkPDFFonts are added to the Canon.
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+SkPDFShader* SkPDFCanon::findShader(const SkPDFShader::State& state) const {
+    assert_mutex_held(this, gSkPDFCanonShaderMutex);
+    for (int i = 0; i < fShaderRecords.count(); ++i) {
+        if (fShaderRecords[i]->equals(state)) {
+            return fShaderRecords[i];
+        }
+    }
+    return NULL;
+}
+
+void SkPDFCanon::addShader(SkPDFShader* shader) {
+    assert_mutex_held(this, gSkPDFCanonShaderMutex);
+    SkASSERT(shader);
+    fShaderRecords.push(shader);
+}
+
+void SkPDFCanon::removeShader(SkPDFShader* pdfShader) {
+    assert_mutex_held(this, gSkPDFCanonShaderMutex);
+    for (int i = 0; i < fShaderRecords.count(); ++i) {
+        if (fShaderRecords[i] == pdfShader) {
+            fShaderRecords.removeShuffle(i);
+            return;
+        }
+    }
+    SkDEBUGFAIL("pdfShader not found");
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+SkPDFGraphicState* SkPDFCanon::findGraphicState(const SkPaint& paint) const {
+    assert_mutex_held(this, gSkPDFCanonPaintMutex);
+    for (int i = 0; i < fGraphicStateRecords.count(); ++i) {
+        if (fGraphicStateRecords[i]->equals(paint)) {
+            return fGraphicStateRecords[i];
+        }
+    }
+    return NULL;
+}
+
+void SkPDFCanon::addGraphicState(SkPDFGraphicState* state) {
+    assert_mutex_held(this, gSkPDFCanonPaintMutex);
+    SkASSERT(state);
+    fGraphicStateRecords.push(state);
+}
+
+void SkPDFCanon::removeGraphicState(SkPDFGraphicState* pdfGraphicState) {
+    assert_mutex_held(this, gSkPDFCanonPaintMutex);
+    for (int i = 0; i < fGraphicStateRecords.count(); ++i) {
+        if (fGraphicStateRecords[i] == pdfGraphicState) {
+            fGraphicStateRecords.removeShuffle(i);
+            return;
+        }
+    }
+    SkDEBUGFAIL("pdfGraphicState not found");
+}
diff --git a/src/pdf/SkPDFCanon.h b/src/pdf/SkPDFCanon.h
new file mode 100644
index 0000000000..5a06a465dd
--- /dev/null
+++ b/src/pdf/SkPDFCanon.h
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+#ifndef SkPDFCanon_DEFINED
+#define SkPDFCanon_DEFINED
+
+#include "SkPDFShader.h"
+#include "SkThread.h"
+#include "SkTDArray.h"
+
+struct SkIRect;
+class SkMatrix;
+class SkPDFFont;
+class SkPDFGraphicState;
+class SkPaint;
+class SkShader;
+
+// This class's fields and methods will eventually become part of
+// SkPDFDocument/SkDocument_PDF.  For now, it exists as a singleton to
+// preflight that transition.  This replaces three global arrays in
+// SkPDFFont, SkPDFShader, and SkPDFGraphicsContext.
+//
+// IF YOU ARE LOOKING AT THIS API PLEASE DO NOT WRITE THE CHANGE
+// YOU ARE ABOUT TO WRITE WITHOUT TALKING TO HALCANARY@.
+//
+// Note that this class does not create, delete, reference or
+// dereference the SkPDFObject objects that it indexes.  It is up to
+// the caller to manage the lifetime of these objects.
+class SkPDFCanon : SkNoncopyable {
+public:
+    SkPDFCanon();
+    ~SkPDFCanon();
+
+    static SkPDFCanon& GetCanon();
+
+    // This mutexes will be removed once this class is subsumed into
+    // SkPDFDocument.
+    static SkBaseMutex& GetFontMutex();
+    static SkBaseMutex& GetShaderMutex();
+    static SkBaseMutex& GetPaintMutex();
+
+    // Returns exact match if there is one.  If not, it returns NULL.
+    // If there is no exact match, but there is a related font, we
+    // still return NULL, but also set *relatedFont.
+    SkPDFFont* findFont(uint32_t fontID,
+                        uint16_t glyphID,
+                        SkPDFFont** relatedFont) const;
+    void addFont(SkPDFFont* font, uint32_t fontID, uint16_t fGlyphID);
+    void removeFont(SkPDFFont*);
+
+    SkPDFShader* findShader(const SkPDFShader::State&) const;
+    void addShader(SkPDFShader*);
+    void removeShader(SkPDFShader*);
+
+    SkPDFGraphicState* findGraphicState(const SkPaint&) const;
+    void addGraphicState(SkPDFGraphicState*);
+    void removeGraphicState(SkPDFGraphicState*);
+
+private:
+    struct FontRec {
+        SkPDFFont* fFont;
+        uint32_t fFontID;
+        uint16_t fGlyphID;
+    };
+    SkTDArray<FontRec> fFontRecords;
+
+    SkTDArray<SkPDFShader*> fShaderRecords;
+
+    SkTDArray<SkPDFGraphicState*> fGraphicStateRecords;
+};
+#endif  // SkPDFCanon_DEFINED
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index 08c1289ce3..a093f620ca 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -11,6 +11,7 @@
 #include "SkGlyphCache.h"
 #include "SkPaint.h"
 #include "SkPDFCatalog.h"
+#include "SkPDFCanon.h"
 #include "SkPDFDevice.h"
 #include "SkPDFFont.h"
 #include "SkPDFFontImpl.h"
@@ -741,22 +742,9 @@ SkPDFGlyphSet* SkPDFGlyphSetMap::getGlyphSetForFont(SkPDFFont* font) {
  */
 
 SkPDFFont::~SkPDFFont() {
-    SkAutoMutexAcquire lock(CanonicalFontsMutex());
-    int index = -1;
-    for (int i = 0 ; i < CanonicalFonts().count() ; i++) {
-        if (CanonicalFonts()[i].fFont == this) {
-            index = i;
-        }
-    }
-
-    SkDEBUGCODE(int indexFound;)
-    SkASSERT(index == -1 ||
-             (Find(fTypeface->uniqueID(),
-                   fFirstGlyphID,
-                   &indexFound) &&
-             index == indexFound));
-    if (index >= 0) {
-        CanonicalFonts().removeShuffle(index);
+    {
+        SkAutoMutexAcquire lock(SkPDFCanon::GetFontMutex());
+        SkPDFCanon::GetCanon().removeFont(this);
     }
     fResources.unrefAll();
 }
@@ -817,24 +805,22 @@ int SkPDFFont::glyphsToPDFFontEncoding(uint16_t* glyphIDs, int numGlyphs) {
 
 // static
 SkPDFFont* SkPDFFont::GetFontResource(SkTypeface* typeface, uint16_t glyphID) {
-    SkAutoMutexAcquire lock(CanonicalFontsMutex());
-
     SkAutoResolveDefaultTypeface autoResolve(typeface);
     typeface = autoResolve.get();
-
     const uint32_t fontID = typeface->uniqueID();
-    int relatedFontIndex;
-    if (Find(fontID, glyphID, &relatedFontIndex)) {
-        CanonicalFonts()[relatedFontIndex].fFont->ref();
-        return CanonicalFonts()[relatedFontIndex].fFont;
+
+    SkAutoMutexAcquire lock(SkPDFCanon::GetFontMutex());
+    SkPDFFont* relatedFont;
+    SkPDFFont* pdfFont =
+            SkPDFCanon::GetCanon().findFont(fontID, glyphID, &relatedFont);
+    if (pdfFont) {
+        return SkRef(pdfFont);
     }
 
     SkAutoTUnref<const SkAdvancedTypefaceMetrics> fontMetrics;
     SkPDFDict* relatedFontDescriptor = NULL;
-    if (relatedFontIndex >= 0) {
-        SkPDFFont* relatedFont = CanonicalFonts()[relatedFontIndex].fFont;
-        fontMetrics.reset(relatedFont->fontInfo());
-        SkSafeRef(fontMetrics.get());
+    if (relatedFont) {
+        fontMetrics.reset(SkSafeRef(relatedFont->fontInfo()));
         relatedFontDescriptor = relatedFont->getFontDescriptor();
 
         // This only is to catch callers who pass invalid glyph ids.
@@ -846,8 +832,7 @@ SkPDFFont* SkPDFFont::GetFontResource(SkTypeface* typeface, uint16_t glyphID) {
 
         if (fontType == SkAdvancedTypefaceMetrics::kType1CID_Font ||
             fontType == SkAdvancedTypefaceMetrics::kTrueType_Font) {
-            CanonicalFonts()[relatedFontIndex].fFont->ref();
-            return CanonicalFonts()[relatedFontIndex].fFont;
+            return SkRef(relatedFont);
         }
     } else {
         SkAdvancedTypefaceMetrics::PerGlyphInfo info;
@@ -874,8 +859,7 @@ SkPDFFont* SkPDFFont::GetFontResource(SkTypeface* typeface, uint16_t glyphID) {
 
     SkPDFFont* font = Create(fontMetrics.get(), typeface, glyphID,
                              relatedFontDescriptor);
-    FontRec newEntry(font, fontID, font->fFirstGlyphID);
-    CanonicalFonts().push(newEntry);
+    SkPDFCanon::GetCanon().addFont(font, fontID, font->fFirstGlyphID);
     return font;  // Return the reference new SkPDFFont() created.
 }
 
@@ -883,32 +867,6 @@ SkPDFFont* SkPDFFont::getFontSubset(const SkPDFGlyphSet*) {
     return NULL;  // Default: no support.
 }
 
-// static
-SkTDArray<SkPDFFont::FontRec>& SkPDFFont::CanonicalFonts() {
-    SkPDFFont::CanonicalFontsMutex().assertHeld();
-    static SkTDArray<FontRec> gCanonicalFonts;
-    return gCanonicalFonts;
-}
-
-SK_DECLARE_STATIC_MUTEX(gCanonicalFontsMutex);
-// static
-SkBaseMutex& SkPDFFont::CanonicalFontsMutex() {
-    return gCanonicalFontsMutex;
-}
-
-// static
-bool SkPDFFont::Find(uint32_t fontID, uint16_t glyphID, int* index) {
-    // TODO(vandebo): Optimize this, do only one search?
-    FontRec search(NULL, fontID, glyphID);
-    *index = CanonicalFonts().find(search);
-    if (*index >= 0) {
-        return true;
-    }
-    search.fGlyphID = 0;
-    *index = CanonicalFonts().find(search);
-    return false;
-}
-
 SkPDFFont::SkPDFFont(const SkAdvancedTypefaceMetrics* info,
                      SkTypeface* typeface,
                      SkPDFDict* relatedFontDescriptor)
@@ -1034,34 +992,6 @@ void SkPDFFont::adjustGlyphRangeForSingleByteEncoding(uint16_t glyphID) {
     }
 }
 
-bool SkPDFFont::FontRec::operator==(const SkPDFFont::FontRec& b) const {
-    if (fFontID != b.fFontID) {
-        return false;
-    }
-    if (fFont != NULL && b.fFont != NULL) {
-        return fFont->fFirstGlyphID == b.fFont->fFirstGlyphID &&
-            fFont->fLastGlyphID == b.fFont->fLastGlyphID;
-    }
-    if (fGlyphID == 0 || b.fGlyphID == 0) {
-        return true;
-    }
-
-    if (fFont != NULL) {
-        return fFont->fFirstGlyphID <= b.fGlyphID &&
-            b.fGlyphID <= fFont->fLastGlyphID;
-    } else if (b.fFont != NULL) {
-        return b.fFont->fFirstGlyphID <= fGlyphID &&
-            fGlyphID <= b.fFont->fLastGlyphID;
-    }
-    return fGlyphID == b.fGlyphID;
-}
-
-SkPDFFont::FontRec::FontRec(SkPDFFont* font, uint32_t fontID, uint16_t glyphID)
-    : fFont(font),
-      fFontID(fontID),
-      fGlyphID(glyphID) {
-}
-
 void SkPDFFont::populateToUnicodeTable(const SkPDFGlyphSet* subset) {
     if (fFontInfo == NULL || fFontInfo->fGlyphToUnicode.begin() == NULL) {
         return;
@@ -1494,3 +1424,24 @@ bool SkPDFType3Font::populate(uint16_t glyphID) {
     populateToUnicodeTable(NULL);
     return true;
 }
+
+SkPDFFont::Match SkPDFFont::IsMatch(SkPDFFont* existingFont,
+                                    uint32_t existingFontID,
+                                    uint16_t existingGlyphID,
+                                    uint32_t searchFontID,
+                                    uint16_t searchGlyphID) {
+    if (existingFontID != searchFontID) {
+        return SkPDFFont::kNot_Match;
+    }
+    if (existingGlyphID == 0 || searchGlyphID == 0) {
+        return SkPDFFont::kExact_Match;
+    }
+    if (existingFont != NULL) {
+        return (existingFont->fFirstGlyphID <= searchGlyphID &&
+                searchGlyphID <= existingFont->fLastGlyphID)
+                       ? SkPDFFont::kExact_Match
+                       : SkPDFFont::kRelated_Match;
+    }
+    return (existingGlyphID == searchGlyphID) ? SkPDFFont::kExact_Match
+                                              : SkPDFFont::kRelated_Match;
+}
diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h
index 27f1b5bfe3..cf2daa0fdf 100644
--- a/src/pdf/SkPDFFont.h
+++ b/src/pdf/SkPDFFont.h
@@ -139,6 +139,17 @@ public:
      */
     virtual SkPDFFont* getFontSubset(const SkPDFGlyphSet* usage);
 
+    enum Match {
+        kExact_Match,
+        kRelated_Match,
+        kNot_Match,
+    };
+    static Match IsMatch(SkPDFFont* existingFont,
+                         uint32_t existingFontID,
+                         uint16_t existingGlyphID,
+                         uint32_t searchFontID,
+                         uint16_t searchGlyphID);
+
 protected:
     // Common constructor to handle common members.
     SkPDFFont(const SkAdvancedTypefaceMetrics* fontInfo, SkTypeface* typeface,
@@ -178,17 +189,6 @@ protected:
     static bool Find(uint32_t fontID, uint16_t glyphID, int* index);
 
 private:
-    class FontRec {
-    public:
-        SkPDFFont* fFont;
-        uint32_t fFontID;
-        uint16_t fGlyphID;
-
-        // A fGlyphID of 0 with no fFont always matches.
-        bool operator==(const FontRec& b) const;
-        FontRec(SkPDFFont* font, uint32_t fontID, uint16_t fGlyphID);
-    };
-
     SkAutoTUnref<SkTypeface> fTypeface;
 
     // The glyph IDs accessible with this font.  For Type1 (non CID) fonts,
@@ -201,9 +201,6 @@ private:
 
     SkAdvancedTypefaceMetrics::FontType fFontType;
 
-    // This should be made a hash table if performance is a problem.
-    static SkTDArray<FontRec>& CanonicalFonts();
-    static SkBaseMutex& CanonicalFontsMutex();
     typedef SkPDFDict INHERITED;
 };
 
diff --git a/src/pdf/SkPDFGraphicState.cpp b/src/pdf/SkPDFGraphicState.cpp
index f4cb1b227b..c50a34994a 100644
--- a/src/pdf/SkPDFGraphicState.cpp
+++ b/src/pdf/SkPDFGraphicState.cpp
@@ -6,29 +6,46 @@
  */
 
 #include "SkData.h"
+#include "SkPDFCanon.h"
 #include "SkPDFFormXObject.h"
 #include "SkPDFGraphicState.h"
 #include "SkPDFUtils.h"
 #include "SkTypes.h"
 
-static const char* blend_mode_from_xfermode(SkXfermode::Mode mode) {
+static const char* as_blend_mode(SkXfermode::Mode mode) {
     switch (mode) {
-        case SkXfermode::kSrcOver_Mode:    return "Normal";
-        case SkXfermode::kMultiply_Mode:   return "Multiply";
-        case SkXfermode::kScreen_Mode:     return "Screen";
-        case SkXfermode::kOverlay_Mode:    return "Overlay";
-        case SkXfermode::kDarken_Mode:     return "Darken";
-        case SkXfermode::kLighten_Mode:    return "Lighten";
-        case SkXfermode::kColorDodge_Mode: return "ColorDodge";
-        case SkXfermode::kColorBurn_Mode:  return "ColorBurn";
-        case SkXfermode::kHardLight_Mode:  return "HardLight";
-        case SkXfermode::kSoftLight_Mode:  return "SoftLight";
-        case SkXfermode::kDifference_Mode: return "Difference";
-        case SkXfermode::kExclusion_Mode:  return "Exclusion";
-        case SkXfermode::kHue_Mode:        return "Hue";
-        case SkXfermode::kSaturation_Mode: return "Saturation";
-        case SkXfermode::kColor_Mode:      return "Color";
-        case SkXfermode::kLuminosity_Mode: return "Luminosity";
+        case SkXfermode::kSrcOver_Mode:
+            return "Normal";
+        case SkXfermode::kMultiply_Mode:
+            return "Multiply";
+        case SkXfermode::kScreen_Mode:
+            return "Screen";
+        case SkXfermode::kOverlay_Mode:
+            return "Overlay";
+        case SkXfermode::kDarken_Mode:
+            return "Darken";
+        case SkXfermode::kLighten_Mode:
+            return "Lighten";
+        case SkXfermode::kColorDodge_Mode:
+            return "ColorDodge";
+        case SkXfermode::kColorBurn_Mode:
+            return "ColorBurn";
+        case SkXfermode::kHardLight_Mode:
+            return "HardLight";
+        case SkXfermode::kSoftLight_Mode:
+            return "SoftLight";
+        case SkXfermode::kDifference_Mode:
+            return "Difference";
+        case SkXfermode::kExclusion_Mode:
+            return "Exclusion";
+        case SkXfermode::kHue_Mode:
+            return "Hue";
+        case SkXfermode::kSaturation_Mode:
+            return "Saturation";
+        case SkXfermode::kColor_Mode:
+            return "Color";
+        case SkXfermode::kLuminosity_Mode:
+            return "Luminosity";
 
         // These are handled in SkPDFDevice::setUpContentEntry.
         case SkXfermode::kClear_Mode:
@@ -52,13 +69,52 @@ static const char* blend_mode_from_xfermode(SkXfermode::Mode mode) {
     return NULL;
 }
 
+static bool equivalent(const SkPaint& a, const SkPaint& b) {
+    // We're only interested in some fields of the SkPaint, so we have
+    // a custom equality function.
+    if (SkColorGetA(a.getColor()) != SkColorGetA(b.getColor()) ||
+        a.getStrokeCap() != b.getStrokeCap() ||
+        a.getStrokeJoin() != b.getStrokeJoin() ||
+        a.getStrokeWidth() != b.getStrokeWidth() ||
+        a.getStrokeMiter() != b.getStrokeMiter()) {
+        return false;
+    }
+
+    SkXfermode::Mode aXfermodeName = SkXfermode::kSrcOver_Mode;
+    SkXfermode* aXfermode = a.getXfermode();
+    if (aXfermode) {
+        aXfermode->asMode(&aXfermodeName);
+    }
+    if (aXfermodeName < 0 || aXfermodeName > SkXfermode::kLastMode ||
+        as_blend_mode(aXfermodeName) == NULL) {
+        aXfermodeName = SkXfermode::kSrcOver_Mode;
+    }
+    const char* aXfermodeString = as_blend_mode(aXfermodeName);
+    SkASSERT(aXfermodeString != NULL);
+
+    SkXfermode::Mode bXfermodeName = SkXfermode::kSrcOver_Mode;
+    SkXfermode* bXfermode = b.getXfermode();
+    if (bXfermode) {
+        bXfermode->asMode(&bXfermodeName);
+    }
+    if (bXfermodeName < 0 || bXfermodeName > SkXfermode::kLastMode ||
+        as_blend_mode(bXfermodeName) == NULL) {
+        bXfermodeName = SkXfermode::kSrcOver_Mode;
+    }
+    const char* bXfermodeString = as_blend_mode(bXfermodeName);
+    SkASSERT(bXfermodeString != NULL);
+
+    return strcmp(aXfermodeString, bXfermodeString) == 0;
+}
+
+bool SkPDFGraphicState::equals(const SkPaint& paint) const {
+    return equivalent(paint, fPaint);
+}
+
 SkPDFGraphicState::~SkPDFGraphicState() {
-    SkAutoMutexAcquire lock(CanonicalPaintsMutex());
+    SkAutoMutexAcquire lock(SkPDFCanon::GetPaintMutex());
     if (!fSMask) {
-        int index = Find(fPaint);
-        SkASSERT(index >= 0);
-        SkASSERT(CanonicalPaints()[index].fGraphicState == this);
-        CanonicalPaints().removeShuffle(index);
+        SkPDFCanon::GetCanon().removeGraphicState(this);
     }
     fResources.unrefAll();
 }
@@ -75,35 +131,23 @@ void SkPDFGraphicState::emitObject(SkWStream* stream, SkPDFCatalog* catalog) {
 }
 
 // static
-SkTDArray<SkPDFGraphicState::GSCanonicalEntry>& SkPDFGraphicState::CanonicalPaints() {
-    CanonicalPaintsMutex().assertHeld();
-    static SkTDArray<SkPDFGraphicState::GSCanonicalEntry> gCanonicalPaints;
-    return gCanonicalPaints;
-}
-
-SK_DECLARE_STATIC_MUTEX(gCanonicalPaintsMutex);
-// static
-SkBaseMutex& SkPDFGraphicState::CanonicalPaintsMutex() {
-    return gCanonicalPaintsMutex;
-}
-
-// static
-SkPDFGraphicState* SkPDFGraphicState::GetGraphicStateForPaint(const SkPaint& paint) {
-    SkAutoMutexAcquire lock(CanonicalPaintsMutex());
-    int index = Find(paint);
-    if (index >= 0) {
-        CanonicalPaints()[index].fGraphicState->ref();
-        return CanonicalPaints()[index].fGraphicState;
+SkPDFGraphicState* SkPDFGraphicState::GetGraphicStateForPaint(
+        const SkPaint& paint) {
+    SkAutoMutexAcquire lock(SkPDFCanon::GetPaintMutex());
+    SkPDFGraphicState* pdfGraphicState =
+            SkPDFCanon::GetCanon().findGraphicState(paint);
+    if (pdfGraphicState) {
+        return SkRef(pdfGraphicState);
     }
-    GSCanonicalEntry newEntry(new SkPDFGraphicState(paint));
-    CanonicalPaints().push(newEntry);
-    return newEntry.fGraphicState;
+    pdfGraphicState = new SkPDFGraphicState(paint);
+    SkPDFCanon::GetCanon().addGraphicState(pdfGraphicState);
+    return pdfGraphicState;
 }
 
 // static
 SkPDFObject* SkPDFGraphicState::GetInvertFunction() {
     // This assumes that canonicalPaintsMutex is held.
-    CanonicalPaintsMutex().assertHeld();
+    SkPDFCanon::GetPaintMutex().assertHeld();
     static SkPDFStream* invertFunction = NULL;
     if (!invertFunction) {
         // Acrobat crashes if we use a type 0 function, kpdf crashes if we use
@@ -131,8 +175,7 @@ SkPDFGraphicState* SkPDFGraphicState::GetSMaskGraphicState(
         SkPDFFormXObject* sMask, bool invert, SkPDFSMaskMode sMaskMode) {
     // The practical chances of using the same mask more than once are unlikely
     // enough that it's not worth canonicalizing.
-    SkAutoMutexAcquire lock(CanonicalPaintsMutex());
-
+    SkAutoMutexAcquire lock(SkPDFCanon::GetPaintMutex());
     SkAutoTUnref<SkPDFDict> sMaskDict(new SkPDFDict("Mask"));
     if (sMaskMode == kAlpha_SMaskMode) {
         sMaskDict->insertName("S", "Alpha");
@@ -161,7 +204,7 @@ SkPDFGraphicState* SkPDFGraphicState::GetSMaskGraphicState(
 
 // static
 SkPDFGraphicState* SkPDFGraphicState::GetNoSMaskGraphicState() {
-    SkAutoMutexAcquire lock(CanonicalPaintsMutex());
+    SkAutoMutexAcquire lock(SkPDFCanon::GetPaintMutex());
     static SkPDFGraphicState* noSMaskGS = NULL;
     if (!noSMaskGS) {
         noSMaskGS = new SkPDFGraphicState;
@@ -174,13 +217,6 @@ SkPDFGraphicState* SkPDFGraphicState::GetNoSMaskGraphicState() {
     return noSMaskGS;
 }
 
-// static
-int SkPDFGraphicState::Find(const SkPaint& paint) {
-    CanonicalPaintsMutex().assertHeld();
-    GSCanonicalEntry search(&paint);
-    return CanonicalPaints().find(search);
-}
-
 SkPDFGraphicState::SkPDFGraphicState()
     : fPopulated(false),
       fSMask(false) {
@@ -227,54 +263,11 @@ void SkPDFGraphicState::populateDict() {
             fPaint.getXfermode()->asMode(&xfermode);
         // If we don't support the mode, just use kSrcOver_Mode.
         if (xfermode < 0 || xfermode > SkXfermode::kLastMode ||
-                blend_mode_from_xfermode(xfermode) == NULL) {
+            as_blend_mode(xfermode) == NULL) {
             xfermode = SkXfermode::kSrcOver_Mode;
             NOT_IMPLEMENTED("unsupported xfermode", false);
         }
-        insertName("BM", blend_mode_from_xfermode(xfermode));
+        insertName("BM", as_blend_mode(xfermode));
     }
 }
 
-// We're only interested in some fields of the SkPaint, so we have a custom
-// operator== function.
-bool SkPDFGraphicState::GSCanonicalEntry::operator==(
-        const SkPDFGraphicState::GSCanonicalEntry& gs) const {
-    const SkPaint* a = fPaint;
-    const SkPaint* b = gs.fPaint;
-    SkASSERT(a != NULL);
-    SkASSERT(b != NULL);
-
-    if (SkColorGetA(a->getColor()) != SkColorGetA(b->getColor()) ||
-           a->getStrokeCap() != b->getStrokeCap() ||
-           a->getStrokeJoin() != b->getStrokeJoin() ||
-           a->getStrokeWidth() != b->getStrokeWidth() ||
-           a->getStrokeMiter() != b->getStrokeMiter()) {
-        return false;
-    }
-
-    SkXfermode::Mode aXfermodeName = SkXfermode::kSrcOver_Mode;
-    SkXfermode* aXfermode = a->getXfermode();
-    if (aXfermode) {
-        aXfermode->asMode(&aXfermodeName);
-    }
-    if (aXfermodeName < 0 || aXfermodeName > SkXfermode::kLastMode ||
-            blend_mode_from_xfermode(aXfermodeName) == NULL) {
-        aXfermodeName = SkXfermode::kSrcOver_Mode;
-    }
-    const char* aXfermodeString = blend_mode_from_xfermode(aXfermodeName);
-    SkASSERT(aXfermodeString != NULL);
-
-    SkXfermode::Mode bXfermodeName = SkXfermode::kSrcOver_Mode;
-    SkXfermode* bXfermode = b->getXfermode();
-    if (bXfermode) {
-        bXfermode->asMode(&bXfermodeName);
-    }
-    if (bXfermodeName < 0 || bXfermodeName > SkXfermode::kLastMode ||
-            blend_mode_from_xfermode(bXfermodeName) == NULL) {
-        bXfermodeName = SkXfermode::kSrcOver_Mode;
-    }
-    const char* bXfermodeString = blend_mode_from_xfermode(bXfermodeName);
-    SkASSERT(bXfermodeString != NULL);
-
-    return strcmp(aXfermodeString, bXfermodeString) == 0;
-}
diff --git a/src/pdf/SkPDFGraphicState.h b/src/pdf/SkPDFGraphicState.h
index 246f1db26e..cfc80ff548 100644
--- a/src/pdf/SkPDFGraphicState.h
+++ b/src/pdf/SkPDFGraphicState.h
@@ -71,30 +71,14 @@ public:
      */
     static SkPDFGraphicState* GetNoSMaskGraphicState();
 
+    bool equals(const SkPaint&) const;
+
 private:
     const SkPaint fPaint;
     SkTDArray<SkPDFObject*> fResources;
     bool fPopulated;
     bool fSMask;
 
-    class GSCanonicalEntry {
-    public:
-        SkPDFGraphicState* fGraphicState;
-        const SkPaint* fPaint;
-
-        bool operator==(const GSCanonicalEntry& b) const;
-        explicit GSCanonicalEntry(SkPDFGraphicState* gs)
-            : fGraphicState(gs),
-              fPaint(&gs->fPaint) {}
-        explicit GSCanonicalEntry(const SkPaint* paint)
-            : fGraphicState(NULL),
-              fPaint(paint) {}
-    };
-
-    // This should be made a hash table if performance is a problem.
-    static SkTDArray<GSCanonicalEntry>& CanonicalPaints();
-    static SkBaseMutex& CanonicalPaintsMutex();
-
     SkPDFGraphicState();
     explicit SkPDFGraphicState(const SkPaint& paint);
 
@@ -102,7 +86,6 @@ private:
 
     static SkPDFObject* GetInvertFunction();
 
-    static int Find(const SkPaint& paint);
     typedef SkPDFDict INHERITED;
 };
 
diff --git a/src/pdf/SkPDFShader.cpp b/src/pdf/SkPDFShader.cpp
index 890bf447d5..2a144a6926 100644
--- a/src/pdf/SkPDFShader.cpp
+++ b/src/pdf/SkPDFShader.cpp
@@ -10,6 +10,7 @@
 #include "SkPDFShader.h"
 
 #include "SkData.h"
+#include "SkPDFCanon.h"
 #include "SkPDFCatalog.h"
 #include "SkPDFDevice.h"
 #include "SkPDFFormXObject.h"
@@ -509,13 +510,12 @@ class SkPDFFunctionShader : public SkPDFDict, public SkPDFShader {
 public:
     explicit SkPDFFunctionShader(SkPDFShader::State* state);
     virtual ~SkPDFFunctionShader() {
-        if (isValid()) {
-            RemoveShader(this);
-        }
+        SkPDFShader::RemoveFromCanonIfValid(this);
         fResources.unrefAll();
     }
 
     bool isValid() SK_OVERRIDE { return fResources.count() > 0; }
+    SkPDFObject* toPDFObject() SK_OVERRIDE { return this; }
 
     void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
                       SkTSet<SkPDFObject*>* newResourceObjects) SK_OVERRIDE {
@@ -528,7 +528,6 @@ private:
     static SkPDFObject* RangeObject();
 
     SkTDArray<SkPDFObject*> fResources;
-    SkAutoTDelete<const SkPDFShader::State> fState;
 
     SkPDFStream* makePSFunction(const SkString& psCode, SkPDFArray* domain);
     typedef SkPDFDict INHERITED;
@@ -543,18 +542,15 @@ class SkPDFAlphaFunctionShader : public SkPDFStream, public SkPDFShader {
 public:
     explicit SkPDFAlphaFunctionShader(SkPDFShader::State* state);
     virtual ~SkPDFAlphaFunctionShader() {
-        if (isValid()) {
-            RemoveShader(this);
-        }
+        SkPDFShader::RemoveFromCanonIfValid(this);
     }
 
     bool isValid() SK_OVERRIDE {
         return fColorShader.get() != NULL;
     }
+    SkPDFObject* toPDFObject() SK_OVERRIDE { return this; }
 
 private:
-    SkAutoTDelete<const SkPDFShader::State> fState;
-
     SkPDFGraphicState* CreateSMaskGraphicState();
 
     void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
@@ -572,13 +568,12 @@ class SkPDFImageShader : public SkPDFStream, public SkPDFShader {
 public:
     explicit SkPDFImageShader(SkPDFShader::State* state);
     virtual ~SkPDFImageShader() {
-        if (isValid()) {
-            RemoveShader(this);
-        }
+        SkPDFShader::RemoveFromCanonIfValid(this);
         fResources.unrefAll();
     }
 
     bool isValid() SK_OVERRIDE { return size() > 0; }
+    SkPDFObject* toPDFObject() SK_OVERRIDE { return this; }
 
     void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
                       SkTSet<SkPDFObject*>* newResourceObjects) SK_OVERRIDE {
@@ -589,18 +584,37 @@ public:
 
 private:
     SkTSet<SkPDFObject*> fResources;
-    SkAutoTDelete<const SkPDFShader::State> fState;
 };
 
-SkPDFShader::SkPDFShader() {}
+SkPDFShader::SkPDFShader(SkPDFShader::State* s) : fShaderState(s) {}
+
+SkPDFShader::~SkPDFShader() {}
+
+void SkPDFShader::RemoveFromCanonIfValid(SkPDFShader* shader) {
+    if (shader->isValid()) {
+        SkAutoMutexAcquire lock(SkPDFCanon::GetShaderMutex());
+        SkPDFCanon::GetCanon().removeShader(shader);
+    }
+}
+
+bool SkPDFShader::equals(const SkPDFShader::State& state) const {
+    return state == *fShaderState.get();
+}
+
+SkPDFObject* SkPDFShader::AddToCanonIfValid(SkPDFShader* shader) {
+    if (!shader->isValid()) {
+        SkDELETE(shader);
+        return NULL;
+    }
+    SkPDFCanon::GetCanon().addShader(shader);
+    return shader->toPDFObject();
+}
 
 // static
 SkPDFObject* SkPDFShader::GetPDFShaderByState(State* inState) {
-    SkPDFObject* result;
-
-    SkAutoTDelete<State> shaderState(inState);
-    if (shaderState.get()->fType == SkShader::kNone_GradientType &&
-            shaderState.get()->fImage.isNull()) {
+    SkAutoTDelete<State> state(inState);
+    if (state->fType == SkShader::kNone_GradientType &&
+        state->fImage.isNull()) {
         // TODO(vandebo) This drops SKComposeShader on the floor.  We could
         // handle compose shader by pulling things up to a layer, drawing with
         // the first shader, applying the xfer mode and drawing again with the
@@ -608,50 +622,23 @@ SkPDFObject* SkPDFShader::GetPDFShaderByState(State* inState) {
         return NULL;
     }
 
-    ShaderCanonicalEntry entry(NULL, shaderState.get());
-    int index = CanonicalShaders().find(entry);
-    if (index >= 0) {
-        result = CanonicalShaders()[index].fPDFShader;
-        result->ref();
-        return result;
+    SkPDFShader* pdfShader = SkPDFCanon::GetCanon().findShader(*state);
+    if (pdfShader) {
+        SkASSERT(pdfShader->isValid());
+        return SkRef(pdfShader->toPDFObject());
     }
 
-    bool valid = false;
     // The PDFShader takes ownership of the shaderSate.
-    if (shaderState.get()->fType == SkShader::kNone_GradientType) {
-        SkPDFImageShader* imageShader =
-            new SkPDFImageShader(shaderState.detach());
-        valid = imageShader->isValid();
-        result = imageShader;
+    if (state->fType == SkShader::kNone_GradientType) {
+        return SkPDFShader::AddToCanonIfValid(
+                SkNEW_ARGS(SkPDFImageShader, (state.detach())));
+    } else if (state->GradientHasAlpha()) {
+        return SkPDFShader::AddToCanonIfValid(
+                SkNEW_ARGS(SkPDFAlphaFunctionShader, (state.detach())));
     } else {
-        if (shaderState.get()->GradientHasAlpha()) {
-            SkPDFAlphaFunctionShader* gradientShader =
-                SkNEW_ARGS(SkPDFAlphaFunctionShader, (shaderState.detach()));
-            valid = gradientShader->isValid();
-            result = gradientShader;
-        } else {
-            SkPDFFunctionShader* functionShader =
-                SkNEW_ARGS(SkPDFFunctionShader, (shaderState.detach()));
-            valid = functionShader->isValid();
-            result = functionShader;
-        }
-    }
-    if (!valid) {
-        delete result;
-        return NULL;
+        return SkPDFShader::AddToCanonIfValid(
+                SkNEW_ARGS(SkPDFFunctionShader, (state.detach())));
     }
-    entry.fPDFShader = result;
-    CanonicalShaders().push(entry);
-    return result;  // return the reference that came from new.
-}
-
-// static
-void SkPDFShader::RemoveShader(SkPDFObject* shader) {
-    SkAutoMutexAcquire lock(CanonicalShadersMutex());
-    ShaderCanonicalEntry entry(shader, NULL);
-    int index = CanonicalShaders().find(entry);
-    SkASSERT(index >= 0);
-    CanonicalShaders().removeShuffle(index);
 }
 
 // static
@@ -659,27 +646,15 @@ SkPDFObject* SkPDFShader::GetPDFShader(const SkShader& shader,
                                        const SkMatrix& matrix,
                                        const SkIRect& surfaceBBox,
                                        SkScalar rasterScale) {
-    SkAutoMutexAcquire lock(CanonicalShadersMutex());
+    SkAutoMutexAcquire lock(SkPDFCanon::GetShaderMutex());
     return GetPDFShaderByState(
             SkNEW_ARGS(State, (shader, matrix, surfaceBBox, rasterScale)));
 }
 
-// static
-SkTDArray<SkPDFShader::ShaderCanonicalEntry>& SkPDFShader::CanonicalShaders() {
-    SkPDFShader::CanonicalShadersMutex().assertHeld();
-    static SkTDArray<ShaderCanonicalEntry> gCanonicalShaders;
-    return gCanonicalShaders;
-}
-
-SK_DECLARE_STATIC_MUTEX(gCanonicalShadersMutex);
-// static
-SkBaseMutex& SkPDFShader::CanonicalShadersMutex() {
-    return gCanonicalShadersMutex;
-}
 
 // static
 SkPDFObject* SkPDFFunctionShader::RangeObject() {
-    SkPDFShader::CanonicalShadersMutex().assertHeld();
+    SkPDFCanon::GetShaderMutex().assertHeld();
     static SkPDFArray* range = NULL;
     // This method is only used with CanonicalShadersMutex, so it's safe to
     // populate domain.
@@ -757,11 +732,10 @@ static SkStream* create_pattern_fill_content(int gsIndex, SkRect& bounds) {
  */
 SkPDFGraphicState* SkPDFAlphaFunctionShader::CreateSMaskGraphicState() {
     SkRect bbox;
-    bbox.set(fState.get()->fBBox);
+    bbox.set(fShaderState->fBBox);
 
-    SkAutoTUnref<SkPDFObject> luminosityShader(
-            SkPDFShader::GetPDFShaderByState(
-                 fState->CreateAlphaToLuminosityState()));
+    SkAutoTUnref<SkPDFObject> luminosityShader(SkPDFShader::GetPDFShaderByState(
+            fShaderState->CreateAlphaToLuminosityState()));
 
     SkAutoTUnref<SkStream> alphaStream(create_pattern_fill_content(-1, bbox));
 
@@ -777,9 +751,9 @@ SkPDFGraphicState* SkPDFAlphaFunctionShader::CreateSMaskGraphicState() {
 }
 
 SkPDFAlphaFunctionShader::SkPDFAlphaFunctionShader(SkPDFShader::State* state)
-        : fState(state) {
+    : SkPDFShader(state) {
     SkRect bbox;
-    bbox.set(fState.get()->fBBox);
+    bbox.set(fShaderState->fBBox);
 
     fColorShader.reset(
             SkPDFShader::GetPDFShaderByState(state->CreateOpaqueState()));
@@ -837,18 +811,17 @@ static bool split_perspective(const SkMatrix in, SkMatrix* affine,
 }
 
 SkPDFFunctionShader::SkPDFFunctionShader(SkPDFShader::State* state)
-        : SkPDFDict("Pattern"),
-          fState(state) {
+    : SkPDFDict("Pattern"), SkPDFShader(state) {
     SkString (*codeFunction)(const SkShader::GradientInfo& info,
                              const SkMatrix& perspectiveRemover) = NULL;
     SkPoint transformPoints[2];
 
     // Depending on the type of the gradient, we want to transform the
     // coordinate space in different ways.
-    const SkShader::GradientInfo* info = &fState.get()->fInfo;
+    const SkShader::GradientInfo* info = &fShaderState->fInfo;
     transformPoints[0] = info->fPoint[0];
     transformPoints[1] = info->fPoint[1];
-    switch (fState.get()->fType) {
+    switch (fShaderState->fType) {
         case SkShader::kLinear_GradientType:
             codeFunction = &linearCode;
             break;
@@ -893,8 +866,8 @@ SkPDFFunctionShader::SkPDFFunctionShader(SkPDFShader::State* state)
     SkMatrix mapperMatrix;
     unitToPointsMatrix(transformPoints, &mapperMatrix);
 
-    SkMatrix finalMatrix = fState.get()->fCanvasTransform;
-    finalMatrix.preConcat(fState.get()->fShaderTransform);
+    SkMatrix finalMatrix = fShaderState->fCanvasTransform;
+    finalMatrix.preConcat(fShaderState->fShaderTransform);
     finalMatrix.preConcat(mapperMatrix);
 
     // Preserves as much as posible in the final matrix, and only removes
@@ -911,7 +884,7 @@ SkPDFFunctionShader::SkPDFFunctionShader(SkPDFShader::State* state)
     }
 
     SkRect bbox;
-    bbox.set(fState.get()->fBBox);
+    bbox.set(fShaderState->fBBox);
     if (!inverseTransformBBox(finalMatrix, &bbox)) {
         return;
     }
@@ -924,10 +897,11 @@ SkPDFFunctionShader::SkPDFFunctionShader(SkPDFShader::State* state)
     domain->appendScalar(bbox.fBottom);
 
     SkString functionCode;
-    // The two point radial gradient further references fState.get()->fInfo
+    // The two point radial gradient further references
+    // fShaderState->fInfo
     // in translating from x, y coordinates to the t parameter. So, we have
     // to transform the points and radii according to the calculated matrix.
-    if (fState.get()->fType == SkShader::kRadial2_GradientType) {
+    if (fShaderState->fType == SkShader::kRadial2_GradientType) {
         SkShader::GradientInfo twoPointRadialInfo = *info;
         SkMatrix inverseMapperMatrix;
         if (!mapperMatrix.invert(&inverseMapperMatrix)) {
@@ -957,8 +931,9 @@ SkPDFFunctionShader::SkPDFFunctionShader(SkPDFShader::State* state)
     insert("Shading", pdfShader.get());
 }
 
-SkPDFImageShader::SkPDFImageShader(SkPDFShader::State* state) : fState(state) {
-    fState.get()->fImage.lockPixels();
+SkPDFImageShader::SkPDFImageShader(SkPDFShader::State* state)
+    : SkPDFShader(state) {
+    fShaderState->fImage.lockPixels();
 
     // The image shader pattern cell will be drawn into a separate device
     // in pattern cell space (no scaling on the bitmap, though there may be
@@ -966,15 +941,15 @@ SkPDFImageShader::SkPDFImageShader(SkPDFShader::State* state) : fState(state) {
 
     // Map clip bounds to shader space to ensure the device is large enough
     // to handle fake clamping.
-    SkMatrix finalMatrix = fState.get()->fCanvasTransform;
-    finalMatrix.preConcat(fState.get()->fShaderTransform);
+    SkMatrix finalMatrix = fShaderState->fCanvasTransform;
+    finalMatrix.preConcat(fShaderState->fShaderTransform);
     SkRect deviceBounds;
-    deviceBounds.set(fState.get()->fBBox);
+    deviceBounds.set(fShaderState->fBBox);
     if (!inverseTransformBBox(finalMatrix, &deviceBounds)) {
         return;
     }
 
-    const SkBitmap* image = &fState.get()->fImage;
+    const SkBitmap* image = &fShaderState->fImage;
     SkRect bitmapBounds;
     image->getBounds(&bitmapBounds);
 
@@ -983,8 +958,8 @@ SkPDFImageShader::SkPDFImageShader(SkPDFShader::State* state) : fState(state) {
     // For clamp modes, we're only interested in the clip region, whether
     // or not the main bitmap is in it.
     SkShader::TileMode tileModes[2];
-    tileModes[0] = fState.get()->fImageTileModes[0];
-    tileModes[1] = fState.get()->fImageTileModes[1];
+    tileModes[0] = fShaderState->fImageTileModes[0];
+    tileModes[1] = fShaderState->fImageTileModes[1];
     if (tileModes[0] != SkShader::kClamp_TileMode ||
             tileModes[1] != SkShader::kClamp_TileMode) {
         deviceBounds.join(bitmapBounds);
@@ -1164,7 +1139,7 @@ SkPDFImageShader::SkPDFImageShader(SkPDFShader::State* state) : fState(state) {
     populate_tiling_pattern_dict(this, patternBBox,
                                  pattern.getResourceDict(), finalMatrix);
 
-    fState.get()->fImage.unlockPixels();
+    fShaderState->fImage.unlockPixels();
 }
 
 SkPDFStream* SkPDFFunctionShader::makePSFunction(const SkString& psCode, SkPDFArray* domain) {
@@ -1176,16 +1151,6 @@ SkPDFStream* SkPDFFunctionShader::makePSFunction(const SkString& psCode, SkPDFAr
     return result;
 }
 
-SkPDFShader::ShaderCanonicalEntry::ShaderCanonicalEntry(SkPDFObject* pdfShader, const State* state)
-    : fPDFShader(pdfShader)
-    , fState(state)
-{}
-
-bool SkPDFShader::ShaderCanonicalEntry::operator==(const ShaderCanonicalEntry& b) const {
-    return fPDFShader == b.fPDFShader ||
-           (fState != NULL && b.fState != NULL && *fState == *b.fState);
-}
-
 bool SkPDFShader::State::operator==(const SkPDFShader::State& b) const {
     if (fType != b.fType ||
             fCanvasTransform != b.fCanvasTransform ||
diff --git a/src/pdf/SkPDFShader.h b/src/pdf/SkPDFShader.h
index 9c92d77d92..e0abff8f68 100644
--- a/src/pdf/SkPDFShader.h
+++ b/src/pdf/SkPDFShader.h
@@ -45,31 +45,24 @@ public:
                                      const SkIRect& surfaceBBox,
                                      SkScalar rasterScale);
 
-protected:
     class State;
+    bool equals(const SkPDFShader::State&) const;
 
-    class ShaderCanonicalEntry {
-    public:
-        ShaderCanonicalEntry(SkPDFObject* pdfShader, const State* state);
-        bool operator==(const ShaderCanonicalEntry& b) const;
-
-        SkPDFObject* fPDFShader;
-        const State* fState;
-    };
-    // This should be made a hash table if performance is a problem.
-    static SkTDArray<ShaderCanonicalEntry>& CanonicalShaders();
-    static SkBaseMutex& CanonicalShadersMutex();
+protected:
+    SkAutoTDelete<const State> fShaderState;
 
     // This is an internal method.
     // CanonicalShadersMutex() should already be acquired.
     // This also takes ownership of shaderState.
     static SkPDFObject* GetPDFShaderByState(State* shaderState);
-    static void RemoveShader(SkPDFObject* shader);
+    static SkPDFObject* AddToCanonIfValid(SkPDFShader*);
+    static void RemoveFromCanonIfValid(SkPDFShader*);
 
-    SkPDFShader();
-    virtual ~SkPDFShader() {};
+    SkPDFShader(State*);
+    virtual ~SkPDFShader();
 
     virtual bool isValid() = 0;
+    virtual SkPDFObject* toPDFObject() = 0;
 };
 
 #endif