src/pdf: code cleanup

  * SkPDFCanon: remove unnecessary abstraction
  * Make use of SkTHashMap<K, sk_sp<T>>.
  * Remove unncessary struct constructors.
  * More factory fns return sk_sp<T>
  * SkPDFUtility::GetCachedT<T> factored out.

Change-Id: I4055a131b43fe2588fd042b769cd09fff8a3466c
Reviewed-on: https://skia-review.googlesource.com/13655
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Hal Canary <halcanary@google.com>

13 files changed, 83 insertions, 170 deletions

diff --git a/src/pdf/SkPDFCanon.cpp b/src/pdf/SkPDFCanon.cpp
index a804d6b47b..5e8a2cb0c2 100644
--- a/src/pdf/SkPDFCanon.cpp
+++ b/src/pdf/SkPDFCanon.cpp
@@ -12,25 +12,8 @@
 
 ////////////////////////////////////////////////////////////////////////////////
 
-namespace {
-template <typename K, typename V> struct UnrefValue {
-    void operator()(K, V** v) { SkSafeUnref(*v); }
-};
-}
-
 SkPDFCanon::~SkPDFCanon() {
-    // TODO(halcanary): make SkTHashSet work nicely with sk_sp<>,
-    // or use std::unordered_set<>
     fGraphicStateRecords.foreach ([](WrapGS w) { w.fPtr->unref(); });
-    fPDFBitmapMap.foreach(UnrefValue<SkBitmapKey, SkPDFObject>());
-    fTypefaceMetrics.foreach(UnrefValue<uint32_t, SkAdvancedTypefaceMetrics>());
-    fFontDescriptors.foreach(UnrefValue<uint32_t, SkPDFDict>());
-    fFontMap.foreach(UnrefValue<uint64_t, SkPDFFont>());
-}
-
-void SkPDFCanon::reset() {
-    this->~SkPDFCanon();
-    new (this)SkPDFCanon;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -52,7 +35,7 @@ sk_sp<SkPDFObject> SkPDFCanon::findFunctionShader(
 }
 void SkPDFCanon::addFunctionShader(sk_sp<SkPDFObject> pdfShader,
                                    SkPDFShader::State state) {
-    fFunctionShaderRecords.emplace_back(std::move(state), std::move(pdfShader));
+    fFunctionShaderRecords.emplace_back(ShaderRec{std::move(state), std::move(pdfShader)});
 }
 
 sk_sp<SkPDFObject> SkPDFCanon::findAlphaShader(
@@ -61,7 +44,7 @@ sk_sp<SkPDFObject> SkPDFCanon::findAlphaShader(
 }
 void SkPDFCanon::addAlphaShader(sk_sp<SkPDFObject> pdfShader,
                                 SkPDFShader::State state) {
-    fAlphaShaderRecords.emplace_back(std::move(state), std::move(pdfShader));
+    fAlphaShaderRecords.emplace_back(ShaderRec{std::move(state), std::move(pdfShader)});
 }
 
 sk_sp<SkPDFObject> SkPDFCanon::findImageShader(
@@ -71,7 +54,7 @@ sk_sp<SkPDFObject> SkPDFCanon::findImageShader(
 
 void SkPDFCanon::addImageShader(sk_sp<SkPDFObject> pdfShader,
                                 SkPDFShader::State state) {
-    fImageShaderRecords.emplace_back(std::move(state), std::move(pdfShader));
+    fImageShaderRecords.emplace_back(ShaderRec{std::move(state), std::move(pdfShader)});
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -89,37 +72,3 @@ void SkPDFCanon::addGraphicState(const SkPDFGraphicState* state) {
     fGraphicStateRecords.add(w);
 }
 
-////////////////////////////////////////////////////////////////////////////////
-
-sk_sp<SkPDFObject> SkPDFCanon::findPDFBitmap(SkBitmapKey key) const {
-    SkPDFObject** ptr = fPDFBitmapMap.find(key);
-    return ptr ? sk_ref_sp(*ptr) : sk_sp<SkPDFObject>();
-}
-
-void SkPDFCanon::addPDFBitmap(SkBitmapKey key, sk_sp<SkPDFObject> pdfBitmap) {
-    fPDFBitmapMap.set(key, pdfBitmap.release());
-}
-
-////////////////////////////////////////////////////////////////////////////////
-
-sk_sp<SkPDFStream> SkPDFCanon::makeInvertFunction() {
-    if (fInvertFunction) {
-        return fInvertFunction;
-    }
-    fInvertFunction = SkPDFGraphicState::MakeInvertFunction();
-    return fInvertFunction;
-}
-sk_sp<SkPDFDict> SkPDFCanon::makeNoSmaskGraphicState() {
-    if (fNoSmaskGraphicState) {
-        return fNoSmaskGraphicState;
-    }
-    fNoSmaskGraphicState = SkPDFGraphicState::MakeNoSmaskGraphicState();
-    return fNoSmaskGraphicState;
-}
-sk_sp<SkPDFArray> SkPDFCanon::makeRangeObject() {
-    if (fRangeObject) {
-        return fRangeObject;
-    }
-    fRangeObject = SkPDFShader::MakeRangeObject();
-    return fRangeObject;
-}
diff --git a/src/pdf/SkPDFCanon.h b/src/pdf/SkPDFCanon.h
index 2da9e52f70..248e17833f 100644
--- a/src/pdf/SkPDFCanon.h
+++ b/src/pdf/SkPDFCanon.h
@@ -38,9 +38,6 @@ class SkPDFCanon : SkNoncopyable {
 public:
     ~SkPDFCanon();
 
-    // reset to original setting, unrefs all objects.
-    void reset();
-
     sk_sp<SkPDFObject> findFunctionShader(const SkPDFShader::State&) const;
     void addFunctionShader(sk_sp<SkPDFObject>, SkPDFShader::State);
 
@@ -53,28 +50,21 @@ public:
     const SkPDFGraphicState* findGraphicState(const SkPDFGraphicState&) const;
     void addGraphicState(const SkPDFGraphicState*);
 
-    sk_sp<SkPDFObject> findPDFBitmap(SkBitmapKey key) const;
-    void addPDFBitmap(SkBitmapKey key, sk_sp<SkPDFObject>);
-
-    SkTHashMap<uint32_t, SkAdvancedTypefaceMetrics*> fTypefaceMetrics;
-    SkTHashMap<uint32_t, SkPDFDict*> fFontDescriptors;
-    SkTHashMap<uint64_t, SkPDFFont*> fFontMap;
+    SkTHashMap<SkBitmapKey, sk_sp<SkPDFObject>> fPDFBitmapMap;
 
-    SkPixelSerializer* getPixelSerializer() const { return fPixelSerializer.get(); }
-    void setPixelSerializer(sk_sp<SkPixelSerializer> ps) {
-        fPixelSerializer = std::move(ps);
-    }
+    SkTHashMap<uint32_t, sk_sp<SkAdvancedTypefaceMetrics>> fTypefaceMetrics;
+    SkTHashMap<uint32_t, sk_sp<SkPDFDict>> fFontDescriptors;
+    SkTHashMap<uint64_t, sk_sp<SkPDFFont>> fFontMap;
 
-    sk_sp<SkPDFStream> makeInvertFunction();
-    sk_sp<SkPDFDict> makeNoSmaskGraphicState();
-    sk_sp<SkPDFArray> makeRangeObject();
+    sk_sp<SkPixelSerializer> fPixelSerializer;
+    sk_sp<SkPDFStream> fInvertFunction;
+    sk_sp<SkPDFDict> fNoSmaskGraphicState;
+    sk_sp<SkPDFArray> fRangeObject;
 
 private:
     struct ShaderRec {
         SkPDFShader::State fShaderState;
         sk_sp<SkPDFObject> fShaderObject;
-        ShaderRec(SkPDFShader::State s, sk_sp<SkPDFObject> o)
-            : fShaderState(std::move(s)), fShaderObject(std::move(o)) {}
     };
     SkTArray<ShaderRec> fFunctionShaderRecords;
     SkTArray<ShaderRec> fAlphaShaderRecords;
@@ -96,13 +86,5 @@ private:
         };
     };
     SkTHashSet<WrapGS, WrapGS::Hash> fGraphicStateRecords;
-
-    // TODO(halcanary): make SkTHashMap<K, sk_sp<V>> work correctly.
-    SkTHashMap<SkBitmapKey, SkPDFObject*> fPDFBitmapMap;
-
-    sk_sp<SkPixelSerializer> fPixelSerializer;
-    sk_sp<SkPDFStream> fInvertFunction;
-    sk_sp<SkPDFDict> fNoSmaskGraphicState;
-    sk_sp<SkPDFArray> fRangeObject;
 };
 #endif  // SkPDFCanon_DEFINED
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index aa8f7ae23f..781e2c7543 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -519,7 +519,7 @@ void SkPDFDevice::drawAnnotation(const SkRect& rect, const char key[], SkData* v
         if (!strcmp(SkAnnotationKeys::Define_Named_Dest_Key(), key)) {
             SkPoint transformedPoint;
             this->ctm().mapXY(rect.x(), rect.y(), &transformedPoint);
-            fNamedDestinations.emplace_back(value, transformedPoint);
+            fNamedDestinations.emplace_back(NamedDestination{sk_ref_sp(value), transformedPoint});
         }
         return;
     }
@@ -536,9 +536,9 @@ void SkPDFDevice::drawAnnotation(const SkRect& rect, const char key[], SkData* v
         return;
     }
     if (!strcmp(SkAnnotationKeys::URL_Key(), key)) {
-        fLinkToURLs.emplace_back(transformedRect, value);
+        fLinkToURLs.emplace_back(RectWithData{transformedRect, sk_ref_sp(value)});
     } else if (!strcmp(SkAnnotationKeys::Link_Named_Dest_Key(), key)) {
-        fLinkToDestinations.emplace_back(transformedRect, value);
+        fLinkToDestinations.emplace_back(RectWithData{transformedRect, sk_ref_sp(value)});
     }
 }
 
@@ -1480,15 +1480,15 @@ void SkPDFDevice::drawDevice(SkBaseDevice* device, int x, int y, const SkPaint&
     SkScalar scalarY = SkIntToScalar(y);
     for (const RectWithData& l : pdfDevice->fLinkToURLs) {
         SkRect r = l.rect.makeOffset(scalarX, scalarY);
-        fLinkToURLs.emplace_back(r, l.data.get());
+        fLinkToURLs.emplace_back(RectWithData{r, l.data});
     }
     for (const RectWithData& l : pdfDevice->fLinkToDestinations) {
         SkRect r = l.rect.makeOffset(scalarX, scalarY);
-        fLinkToDestinations.emplace_back(r, l.data.get());
+        fLinkToDestinations.emplace_back(RectWithData{r, l.data});
     }
     for (const NamedDestination& d : pdfDevice->fNamedDestinations) {
         SkPoint p = d.point + SkPoint::Make(scalarX, scalarY);
-        fNamedDestinations.emplace_back(d.nameData.get(), p);
+        fNamedDestinations.emplace_back(NamedDestination{d.nameData, p});
     }
 
     if (pdfDevice->isContentEmpty()) {
@@ -1704,10 +1704,8 @@ void SkPDFDevice::drawFormXObjectWithMask(int xObjectIndex,
                                   &content.entry()->fContent);
     SkPDFUtils::DrawFormXObject(xObjectIndex, &content.entry()->fContent);
 
-    // Call makeNoSmaskGraphicState() instead of
-    // SkPDFGraphicState::MakeNoSmaskGraphicState so that the canon
-    // can deduplicate.
-    sMaskGS = fDocument->canon()->makeNoSmaskGraphicState();
+    sMaskGS = SkPDFUtils::GetCachedT(&fDocument->canon()->fNoSmaskGraphicState,
+                                     &SkPDFGraphicState::MakeNoSmaskGraphicState);
     SkPDFUtils::ApplyGraphicState(addGraphicStateResource(sMaskGS.get()),
                                   &content.entry()->fContent);
 }
@@ -1984,13 +1982,11 @@ void SkPDFDevice::populateGraphicStateEntryFromPaint(
 
     sk_sp<SkPDFGraphicState> newGraphicState;
     if (color == paint.getColor()) {
-        newGraphicState.reset(
-                SkPDFGraphicState::GetGraphicStateForPaint(fDocument->canon(), paint));
+        newGraphicState = SkPDFGraphicState::GetGraphicStateForPaint(fDocument->canon(), paint);
     } else {
         SkPaint newPaint = paint;
         newPaint.setColor(color);
-        newGraphicState.reset(
-                SkPDFGraphicState::GetGraphicStateForPaint(fDocument->canon(), newPaint));
+        newGraphicState = SkPDFGraphicState::GetGraphicStateForPaint(fDocument->canon(), newPaint);
     }
     int resourceIndex = addGraphicStateResource(newGraphicState.get());
     entry->fGraphicStateIndex = resourceIndex;
@@ -2028,8 +2024,7 @@ int SkPDFDevice::addXObjectResource(SkPDFObject* xObject) {
 }
 
 int SkPDFDevice::getFontResourceIndex(SkTypeface* typeface, uint16_t glyphID) {
-    sk_sp<SkPDFFont> newFont(
-            SkPDFFont::GetFontResource(fDocument->canon(), typeface, glyphID));
+    sk_sp<SkPDFFont> newFont = SkPDFFont::GetFontResource(fDocument->canon(), typeface, glyphID);
     if (!newFont) {
         return -1;
     }
@@ -2178,19 +2173,20 @@ void SkPDFDevice::internalDrawImage(const SkMatrix& origMatrix,
     }
 
     SkBitmapKey key = imageSubset.getKey();
-    sk_sp<SkPDFObject> pdfimage = fDocument->canon()->findPDFBitmap(key);
+    sk_sp<SkPDFObject>* pdfimagePtr = fDocument->canon()->fPDFBitmapMap.find(key);
+    sk_sp<SkPDFObject> pdfimage = pdfimagePtr ? *pdfimagePtr : nullptr;
     if (!pdfimage) {
         sk_sp<SkImage> img = imageSubset.makeImage();
         if (!img) {
             return;
         }
-        pdfimage = SkPDFCreateBitmapObject(
-                std::move(img), fDocument->canon()->getPixelSerializer());
+        pdfimage =
+                SkPDFCreateBitmapObject(std::move(img), fDocument->canon()->fPixelSerializer.get());
         if (!pdfimage) {
             return;
         }
         fDocument->serialize(pdfimage);  // serialize images early.
-        fDocument->canon()->addPDFBitmap(key, pdfimage);
+        fDocument->canon()->fPDFBitmapMap.set(key, pdfimage);
     }
     // TODO(halcanary): addXObjectResource() should take a sk_sp<SkPDFObject>
     SkPDFUtils::DrawFormXObject(this->addXObjectResource(pdfimage.get()),
diff --git a/src/pdf/SkPDFDevice.h b/src/pdf/SkPDFDevice.h
index f4318e489d..1906bb37a7 100644
--- a/src/pdf/SkPDFDevice.h
+++ b/src/pdf/SkPDFDevice.h
@@ -181,19 +181,11 @@ private:
     struct RectWithData {
         SkRect rect;
         sk_sp<SkData> data;
-        RectWithData(const SkRect& rect, SkData* data)
-            : rect(rect), data(SkRef(data)) {}
-        RectWithData(RectWithData&&) = default;
-        RectWithData& operator=(RectWithData&& other) = default;
     };
 
     struct NamedDestination {
         sk_sp<SkData> nameData;
         SkPoint point;
-        NamedDestination(SkData* nameData, const SkPoint& point)
-            : nameData(SkRef(nameData)), point(point) {}
-        NamedDestination(NamedDestination&&) = default;
-        NamedDestination& operator=(NamedDestination&&) = default;
     };
 
     // TODO(vandebo): push most of SkPDFDevice's state into a core object in
diff --git a/src/pdf/SkPDFDocument.cpp b/src/pdf/SkPDFDocument.cpp
index 92e82fc018..a83e246945 100644
--- a/src/pdf/SkPDFDocument.cpp
+++ b/src/pdf/SkPDFDocument.cpp
@@ -180,7 +180,7 @@ SkPDFDocument::SkPDFDocument(SkWStream* stream,
     , fRasterDpi(rasterDpi)
     , fMetadata(metadata)
     , fPDFA(pdfa) {
-    fCanon.setPixelSerializer(std::move(jpegEncoder));
+    fCanon.fPixelSerializer = std::move(jpegEncoder);
 }
 
 SkPDFDocument::~SkPDFDocument() {
@@ -251,7 +251,7 @@ void SkPDFDocument::onAbort() {
 void SkPDFDocument::reset() {
     fCanvas.reset(nullptr);
     fPages.reset();
-    fCanon.reset();
+    renew(&fCanon);
     renew(&fObjectSerializer);
     fFonts.reset();
 }
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index 09d133ece7..b373514808 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -142,8 +142,8 @@ const SkAdvancedTypefaceMetrics* SkPDFFont::GetMetrics(SkTypeface* typeface,
                                                        SkPDFCanon* canon) {
     SkASSERT(typeface);
     SkFontID id = typeface->uniqueID();
-    if (SkAdvancedTypefaceMetrics** ptr = canon->fTypefaceMetrics.find(id)) {
-        return *ptr;
+    if (sk_sp<SkAdvancedTypefaceMetrics>* ptr = canon->fTypefaceMetrics.find(id)) {
+        return ptr->get();  // canon retains ownership.
     }
     int count = typeface->countGlyphs();
     if (count <= 0 || count > 1 + SK_MaxU16) {
@@ -158,7 +158,7 @@ const SkAdvancedTypefaceMetrics* SkPDFFont::GetMetrics(SkTypeface* typeface,
     if (!metrics) {
         metrics = sk_make_sp<SkAdvancedTypefaceMetrics>();
     }
-    return *canon->fTypefaceMetrics.set(id, metrics.release());
+    return canon->fTypefaceMetrics.set(id, std::move(metrics))->get();
 }
 
 SkAdvancedTypefaceMetrics::FontType SkPDFFont::FontType(const SkAdvancedTypefaceMetrics& metrics) {
@@ -174,9 +174,9 @@ static SkGlyphID first_nonzero_glyph_for_single_byte_encoding(SkGlyphID gid) {
     return gid != 0 ? gid - (gid - 1) % 255 : 1;
 }
 
-SkPDFFont* SkPDFFont::GetFontResource(SkPDFCanon* canon,
-                                      SkTypeface* face,
-                                      SkGlyphID glyphID) {
+sk_sp<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);
@@ -188,10 +188,10 @@ SkPDFFont* SkPDFFont::GetFontResource(SkPDFCanon* canon,
     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)) {
-        SkPDFFont* foundFont = *found;
+    if (sk_sp<SkPDFFont>* found = canon->fFontMap.find(fontID)) {
+        SkDEBUGCODE(SkPDFFont* foundFont = found->get());
         SkASSERT(foundFont && multibyte == foundFont->multiByteGlyphs());
-        return SkRef(foundFont);
+        return *found;
     }
 
     sk_sp<SkTypeface> typeface(sk_ref_sp(face));
@@ -227,8 +227,8 @@ SkPDFFont* SkPDFFont::GetFontResource(SkPDFCanon* canon,
             font = sk_make_sp<SkPDFType3Font>(std::move(info), metrics);
             break;
     }
-    canon->fFontMap.set(fontID, SkRef(font.get()));
-    return font.release();  // TODO(halcanary) return sk_sp<SkPDFFont>.
+    canon->fFontMap.set(fontID, font);
+    return font;
 }
 
 SkPDFFont::SkPDFFont(SkPDFFont::Info info)
@@ -544,11 +544,11 @@ SkPDFType1Font::SkPDFType1Font(SkPDFFont::Info info,
 {
     SkFontID fontID = this->typeface()->uniqueID();
     sk_sp<SkPDFDict> fontDescriptor;
-    if (SkPDFDict** ptr = canon->fFontDescriptors.find(fontID)) {
-        fontDescriptor = sk_ref_sp(*ptr);
+    if (sk_sp<SkPDFDict>* ptr = canon->fFontDescriptors.find(fontID)) {
+        fontDescriptor = *ptr;
     } else {
         fontDescriptor = make_type1_font_descriptor(this->typeface(), metrics);
-        canon->fFontDescriptors.set(fontID, SkRef(fontDescriptor.get()));
+        canon->fFontDescriptors.set(fontID, fontDescriptor);
     }
     this->insertObjRef("FontDescriptor", std::move(fontDescriptor));
     // TODO(halcanary): subset this (advances and names).
diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h
index 67786f33fc..6151279248 100644
--- a/src/pdf/SkPDFFont.h
+++ b/src/pdf/SkPDFFont.h
@@ -83,9 +83,9 @@ public:
      *  @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);
+    static sk_sp<SkPDFFont> GetFontResource(SkPDFCanon* canon,
+                                            SkTypeface* typeface,
+                                            SkGlyphID glyphID);
 
     /** Uses (kGlyphNames_PerGlyphInfo | kToUnicode_PerGlyphInfo) to get 
      *  SkAdvancedTypefaceMetrics, and caches the result.
diff --git a/src/pdf/SkPDFGraphicState.cpp b/src/pdf/SkPDFGraphicState.cpp
index d60526c11a..068f9bd950 100644
--- a/src/pdf/SkPDFGraphicState.cpp
+++ b/src/pdf/SkPDFGraphicState.cpp
@@ -104,9 +104,8 @@ SkPDFGraphicState::SkPDFGraphicState(const SkPaint& p)
     , fStrokeJoin(SkToU8(p.getStrokeJoin()))
     , fMode(SkToU8((unsigned)mode_for_pdf(p.getBlendMode()))) {}
 
-// static
-SkPDFGraphicState* SkPDFGraphicState::GetGraphicStateForPaint(
-        SkPDFCanon* canon, const SkPaint& paint) {
+sk_sp<SkPDFGraphicState> SkPDFGraphicState::GetGraphicStateForPaint(SkPDFCanon* canon,
+                                                                    const SkPaint& paint) {
     SkASSERT(canon);
     SkPDFGraphicState key(paint);
     if (const SkPDFGraphicState* canonGS = canon->findGraphicState(key)) {
@@ -114,14 +113,14 @@ SkPDFGraphicState* SkPDFGraphicState::GetGraphicStateForPaint(
         // since the emitObject() interface is non-const.  But We
         // promise that there is no way to mutate this object from
         // here on out.
-        return SkRef(const_cast<SkPDFGraphicState*>(canonGS));
+        return sk_sp<SkPDFGraphicState>(SkRef(const_cast<SkPDFGraphicState*>(canonGS)));
     }
-    SkPDFGraphicState* pdfGraphicState = new SkPDFGraphicState(paint);
-    canon->addGraphicState(pdfGraphicState);
+    sk_sp<SkPDFGraphicState> pdfGraphicState(new SkPDFGraphicState(paint));
+    canon->addGraphicState(pdfGraphicState.get());
     return pdfGraphicState;
 }
 
-sk_sp<SkPDFStream> SkPDFGraphicState::MakeInvertFunction() {
+static sk_sp<SkPDFStream> make_invert_function() {
     // Acrobat crashes if we use a type 0 function, kpdf crashes if we use
     // a type 2 function, so we use a type 4 function.
     auto domainAndRange = sk_make_sp<SkPDFArray>();
@@ -156,7 +155,8 @@ sk_sp<SkPDFDict> SkPDFGraphicState::GetSMaskGraphicState(
     if (invert) {
         // Instead of calling SkPDFGraphicState::MakeInvertFunction,
         // let the canon deduplicate this object.
-        sMaskDict->insertObjRef("TR", canon->makeInvertFunction());
+        sMaskDict->insertObjRef(
+                "TR", SkPDFUtils::GetCachedT(&canon->fInvertFunction, &make_invert_function));
     }
 
     auto result = sk_make_sp<SkPDFDict>("ExtGState");
diff --git a/src/pdf/SkPDFGraphicState.h b/src/pdf/SkPDFGraphicState.h
index 8ee6728f56..310c1cfb06 100644
--- a/src/pdf/SkPDFGraphicState.h
+++ b/src/pdf/SkPDFGraphicState.h
@@ -33,15 +33,11 @@ public:
     void emitObject(SkWStream* stream,
                     const SkPDFObjNumMap& objNumMap) const override;
 
-    /** Get the graphic state for the passed SkPaint. The reference count of
-     *  the object is incremented and it is the caller's 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.
+    /** Get the graphic state for the passed SkPaint.
      *  @param paint  The SkPaint to emulate.
      */
-    static SkPDFGraphicState* GetGraphicStateForPaint(SkPDFCanon* canon,
-                                                      const SkPaint& paint);
+    static sk_sp<SkPDFGraphicState> GetGraphicStateForPaint(SkPDFCanon* canon,
+                                                            const SkPaint& paint);
 
     /** Make a graphic state that only sets the passed soft mask.
      *  @param sMask     The form xobject to use as a soft mask.
diff --git a/src/pdf/SkPDFShader.cpp b/src/pdf/SkPDFShader.cpp
index 13a9ecd1d5..b6b096d22c 100644
--- a/src/pdf/SkPDFShader.cpp
+++ b/src/pdf/SkPDFShader.cpp
@@ -737,7 +737,7 @@ static bool split_perspective(const SkMatrix in, SkMatrix* affine,
     return true;
 }
 
-sk_sp<SkPDFArray> SkPDFShader::MakeRangeObject() {
+static sk_sp<SkPDFArray> make_range_object() {
     auto range = sk_make_sp<SkPDFArray>();
     range->reserve(6);
     range->appendInt(0);
@@ -933,14 +933,11 @@ static sk_sp<SkPDFDict> make_function_shader(SkPDFCanon* canon,
         
         pdfShader->insertObject("Domain", domain);
 
-        // Call canon->makeRangeObject() instead of
-        // SkPDFShader::MakeRangeObject() so that the canon can
-        // deduplicate.
-        std::unique_ptr<SkStreamAsset> functionStream(
-                functionCode.detachAsStream());
-        sk_sp<SkPDFStream> function = make_ps_function(std::move(functionStream),
-                                                       std::move(domain),
-                                                       canon->makeRangeObject());
+        std::unique_ptr<SkStreamAsset> functionStream(functionCode.detachAsStream());
+        sk_sp<SkPDFArray> rangeObject =
+                SkPDFUtils::GetCachedT(&canon->fRangeObject, &make_range_object);
+        sk_sp<SkPDFStream> function = make_ps_function(std::move(functionStream), std::move(domain),
+                                                       std::move(rangeObject));
         pdfShader->insertObjRef("Function", std::move(function));
     }
 
diff --git a/src/pdf/SkPDFTypes.cpp b/src/pdf/SkPDFTypes.cpp
index 42b4ed4953..c66c80a6bc 100644
--- a/src/pdf/SkPDFTypes.cpp
+++ b/src/pdf/SkPDFTypes.cpp
@@ -383,32 +383,29 @@ void SkPDFDict::addResources(SkPDFObjNumMap* catalog) const {
     }
 }
 
-SkPDFDict::Record::Record(SkPDFUnion&& k, SkPDFUnion&& v)
-    : fKey(std::move(k)), fValue(std::move(v)) {}
-
 int SkPDFDict::size() const { return fRecords.count(); }
 
 void SkPDFDict::insertObjRef(const char key[], sk_sp<SkPDFObject> objSp) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::ObjRef(std::move(objSp)));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::ObjRef(std::move(objSp))});
 }
 
 void SkPDFDict::insertObjRef(const SkString& key, sk_sp<SkPDFObject> objSp) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::ObjRef(std::move(objSp)));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::ObjRef(std::move(objSp))});
 }
 
 void SkPDFDict::insertObject(const char key[], sk_sp<SkPDFObject> objSp) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Object(std::move(objSp)));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::Object(std::move(objSp))});
 }
 void SkPDFDict::insertObject(const SkString& key, sk_sp<SkPDFObject> objSp) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Object(std::move(objSp)));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::Object(std::move(objSp))});
 }
 
 void SkPDFDict::insertBool(const char key[], bool value) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Bool(value));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::Bool(value)});
 }
 
 void SkPDFDict::insertInt(const char key[], int32_t value) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Int(value));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::Int(value)});
 }
 
 void SkPDFDict::insertInt(const char key[], size_t value) {
@@ -416,23 +413,23 @@ void SkPDFDict::insertInt(const char key[], size_t value) {
 }
 
 void SkPDFDict::insertScalar(const char key[], SkScalar value) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Scalar(value));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::Scalar(value)});
 }
 
 void SkPDFDict::insertName(const char key[], const char name[]) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Name(name));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::Name(name)});
 }
 
 void SkPDFDict::insertName(const char key[], const SkString& name) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Name(name));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::Name(name)});
 }
 
 void SkPDFDict::insertString(const char key[], const char value[]) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::String(value));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::String(value)});
 }
 
 void SkPDFDict::insertString(const char key[], const SkString& value) {
-    fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::String(value));
+    fRecords.emplace_back(Record{SkPDFUnion::Name(key), SkPDFUnion::String(value)});
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/src/pdf/SkPDFTypes.h b/src/pdf/SkPDFTypes.h
index 0be20f13a1..06e4858fbe 100644
--- a/src/pdf/SkPDFTypes.h
+++ b/src/pdf/SkPDFTypes.h
@@ -278,11 +278,6 @@ private:
     struct Record {
         SkPDFUnion fKey;
         SkPDFUnion fValue;
-        Record(SkPDFUnion&&, SkPDFUnion&&);
-        Record(Record&&) = default;
-        Record& operator=(Record&&) = default;
-        Record(const Record&) = delete;
-        Record& operator=(const Record&) = delete;
     };
     SkTArray<Record> fRecords;
     SkDEBUGCODE(bool fDumped;)
diff --git a/src/pdf/SkPDFUtils.h b/src/pdf/SkPDFUtils.h
index 5c9e46b381..00c4b72b7b 100644
--- a/src/pdf/SkPDFUtils.h
+++ b/src/pdf/SkPDFUtils.h
@@ -102,6 +102,15 @@ inline void WriteUTF16beHex(SkDynamicMemoryWStream* wStream, SkUnichar utf32) {
         SkPDFUtils::WriteUInt16BE(wStream, utf16[1]);
     }
 }
+
+template <class T>
+static sk_sp<T> GetCachedT(sk_sp<T>* cachedT, sk_sp<T> (*makeNewT)()) {
+    if (*cachedT) {
+        return *cachedT;
+    }
+    *cachedT = (*makeNewT)();
+    return *cachedT;
+}
 }  // namespace SkPDFUtils
 
 #endif