Revert of Pass effects directly to fontcache (patchset #8 id:140001 of https://codereview.chromium.org/1863013003/ )

Reason for revert:
Seems to have broken the DEPS roll.

Original issue's description:
> Pass effects directly to fontcache
>
> BUG=skia:5176
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1863013003
>
> Committed: https://skia.googlesource.com/skia/+/c79172857c3f69cc46837e1beeae0c1ead377bb2

TBR=djsollen@google.com,bungeman@google.com,mtklein@google.com,fmalita@chromium.org,msarett@google.com,reed@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:5176

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

6 files changed, 68 insertions, 71 deletions

diff --git a/src/core/SkGlyphCache.cpp b/src/core/SkGlyphCache.cpp
index 309f494828..241854eb5a 100644
--- a/src/core/SkGlyphCache.cpp
+++ b/src/core/SkGlyphCache.cpp
@@ -508,10 +508,9 @@ void SkGlyphCache_Globals::purgeAll() {
     - call a fontscaler (which might call into the cache)
 */
 SkGlyphCache* SkGlyphCache::VisitCache(SkTypeface* typeface,
-                                       const SkScalerContextEffects& effects,
-                                       const SkDescriptor* desc,
-                                       bool (*proc)(const SkGlyphCache*, void*),
-                                       void* context) {
+                              const SkDescriptor* desc,
+                              bool (*proc)(const SkGlyphCache*, void*),
+                              void* context) {
     if (!typeface) {
         typeface = SkTypeface::GetDefaultTypeface();
     }
@@ -543,10 +542,10 @@ SkGlyphCache* SkGlyphCache::VisitCache(SkTypeface* typeface,
     {
         // pass true the first time, to notice if the scalercontext failed,
         // so we can try the purge.
-        SkScalerContext* ctx = typeface->createScalerContext(effects, desc, true);
+        SkScalerContext* ctx = typeface->createScalerContext(desc, true);
         if (!ctx) {
             get_globals().purgeAll();
-            ctx = typeface->createScalerContext(effects, desc, false);
+            ctx = typeface->createScalerContext(desc, false);
             SkASSERT(ctx);
         }
         cache = new SkGlyphCache(typeface, desc, ctx);
diff --git a/src/core/SkGlyphCache.h b/src/core/SkGlyphCache.h
index fd3ef66028..9ae47c6207 100644
--- a/src/core/SkGlyphCache.h
+++ b/src/core/SkGlyphCache.h
@@ -129,7 +129,7 @@ public:
         If the proc() returns true, detach the cache and return it, otherwise leave it and return
         nullptr.
     */
-    static SkGlyphCache* VisitCache(SkTypeface*, const SkScalerContextEffects&, const SkDescriptor*,
+    static SkGlyphCache* VisitCache(SkTypeface*, const SkDescriptor* desc,
                                     bool (*proc)(const SkGlyphCache*, void*),
                                     void* context);
 
@@ -146,9 +146,8 @@ public:
         more than 1 strike for the same descriptor, but that will eventually get purged, and the
         win is that different thread will never block each other while a strike is being used.
     */
-    static SkGlyphCache* DetachCache(SkTypeface* typeface, const SkScalerContextEffects& effects,
-                                     const SkDescriptor* desc) {
-        return VisitCache(typeface, effects, desc, DetachProc, nullptr);
+    static SkGlyphCache* DetachCache(SkTypeface* typeface, const SkDescriptor* desc) {
+        return VisitCache(typeface, desc, DetachProc, nullptr);
     }
 
     static void Dump();
@@ -279,9 +278,8 @@ public:
     SkGlyphCache* getCache() const { return this->get(); }
 
     SkAutoGlyphCache(SkGlyphCache* cache) : INHERITED(cache) {}
-    SkAutoGlyphCache(SkTypeface* typeface, const SkScalerContextEffects& effects,
-                     const SkDescriptor* desc)
-        : INHERITED(SkGlyphCache::DetachCache(typeface, effects, desc))
+    SkAutoGlyphCache(SkTypeface* typeface, const SkDescriptor* desc)
+        : INHERITED(SkGlyphCache::DetachCache(typeface, desc))
     {}
     /** deprecated: always enables fake gamma */
     SkAutoGlyphCache(const SkPaint& paint,
diff --git a/src/core/SkPaint.cpp b/src/core/SkPaint.cpp
index 123f20365e..c3bf1ba9ac 100644
--- a/src/core/SkPaint.cpp
+++ b/src/core/SkPaint.cpp
@@ -430,12 +430,13 @@ bool SkPaint::TooBigToUseCache(const SkMatrix& ctm, const SkMatrix& textM) {
 #include "SkGlyphCache.h"
 #include "SkUtils.h"
 
-static void DetachDescProc(SkTypeface* typeface, const SkScalerContextEffects& effects,
-                           const SkDescriptor* desc, void* context) {
-    *((SkGlyphCache**)context) = SkGlyphCache::DetachCache(typeface, effects, desc);
+static void DetachDescProc(SkTypeface* typeface, const SkDescriptor* desc,
+                           void* context) {
+    *((SkGlyphCache**)context) = SkGlyphCache::DetachCache(typeface, desc);
 }
 
-int SkPaint::textToGlyphs(const void* textData, size_t byteLength, uint16_t glyphs[]) const {
+int SkPaint::textToGlyphs(const void* textData, size_t byteLength,
+                          uint16_t glyphs[]) const {
     if (byteLength == 0) {
         return 0;
     }
@@ -937,9 +938,9 @@ static bool FontMetricsCacheProc(const SkGlyphCache* cache, void* context) {
     return false;   // don't detach the cache
 }
 
-static void FontMetricsDescProc(SkTypeface* typeface, const SkScalerContextEffects& effects,
-                                const SkDescriptor* desc, void* context) {
-    SkGlyphCache::VisitCache(typeface, effects, desc, FontMetricsCacheProc, context);
+static void FontMetricsDescProc(SkTypeface* typeface, const SkDescriptor* desc,
+                                void* context) {
+    SkGlyphCache::VisitCache(typeface, desc, FontMetricsCacheProc, context);
 }
 
 SkScalar SkPaint::getFontMetrics(FontMetrics* metrics, SkScalar zoom) const {
@@ -1647,8 +1648,7 @@ static void test_desc(const SkScalerContext::Rec& rec,
 #endif
 
 /* see the note on ignoreGamma on descriptorProc */
-void SkPaint::getScalerContextDescriptor(SkScalerContextEffects* effects,
-                                         SkAutoDescriptor* ad,
+void SkPaint::getScalerContextDescriptor(SkAutoDescriptor* ad,
                                          const SkSurfaceProps& surfaceProps,
                                          uint32_t scalerContextFlags,
                                          const SkMatrix* deviceMatrix) const {
@@ -1674,10 +1674,6 @@ void SkPaint::getScalerContextDescriptor(SkScalerContextEffects* effects,
 #ifdef TEST_DESC
     test_desc(rec, pe, &peBuffer, mf, &mfBuffer, ra, &raBuffer, desc, descSize);
 #endif
-
-    effects->fPathEffect = pe;
-    effects->fMaskFilter = mf;
-    effects->fRasterizer = ra;
 }
 
 /*
@@ -1688,8 +1684,7 @@ void SkPaint::getScalerContextDescriptor(SkScalerContextEffects* effects,
 void SkPaint::descriptorProc(const SkSurfaceProps* surfaceProps,
                              uint32_t scalerContextFlags,
                              const SkMatrix* deviceMatrix,
-                             void (*proc)(SkTypeface*, const SkScalerContextEffects&,
-                                          const SkDescriptor*, void*),
+                             void (*proc)(SkTypeface*, const SkDescriptor*, void*),
                              void* context) const {
     SkScalerContext::Rec    rec;
 
@@ -1714,7 +1709,7 @@ void SkPaint::descriptorProc(const SkSurfaceProps* surfaceProps,
     test_desc(rec, pe, &peBuffer, mf, &mfBuffer, ra, &raBuffer, desc, descSize);
 #endif
 
-    proc(fTypeface.get(), { pe, mf, ra }, desc, context);
+    proc(fTypeface.get(), desc, context);
 }
 
 SkGlyphCache* SkPaint::detachCache(const SkSurfaceProps* surfaceProps,
diff --git a/src/core/SkScalerContext.cpp b/src/core/SkScalerContext.cpp
index ad753dfd21..cf4098bbdd 100644
--- a/src/core/SkScalerContext.cpp
+++ b/src/core/SkScalerContext.cpp
@@ -62,14 +62,30 @@ void SkGlyph::zeroMetrics() {
     #define DUMP_RECx
 #endif
 
-SkScalerContext::SkScalerContext(SkTypeface* typeface, const SkScalerContextEffects& effects,
-                                 const SkDescriptor* desc)
+static SkFlattenable* load_flattenable(const SkDescriptor* desc, uint32_t tag,
+                                       SkFlattenable::Type ft) {
+    SkFlattenable*  obj = nullptr;
+    uint32_t        len;
+    const void*     data = desc->findEntry(tag, &len);
+
+    if (data) {
+        SkReadBuffer buffer(data, len);
+        obj = buffer.readFlattenable(ft);
+        SkASSERT(buffer.offset() == buffer.size());
+    }
+    return obj;
+}
+
+SkScalerContext::SkScalerContext(SkTypeface* typeface, const SkDescriptor* desc)
     : fRec(*static_cast<const Rec*>(desc->findEntry(kRec_SkDescriptorTag, nullptr)))
 
-    , fTypeface(sk_ref_sp(typeface))
-    , fPathEffect(sk_ref_sp(effects.fPathEffect))
-    , fMaskFilter(sk_ref_sp(effects.fMaskFilter))
-    , fRasterizer(sk_ref_sp(effects.fRasterizer))
+    , fTypeface(SkRef(typeface))
+    , fPathEffect(static_cast<SkPathEffect*>(load_flattenable(desc, kPathEffect_SkDescriptorTag,
+                                             SkFlattenable::kSkPathEffect_Type)))
+    , fMaskFilter(static_cast<SkMaskFilter*>(load_flattenable(desc, kMaskFilter_SkDescriptorTag,
+                                             SkFlattenable::kSkMaskFilter_Type)))
+    , fRasterizer(static_cast<SkRasterizer*>(load_flattenable(desc, kRasterizer_SkDescriptorTag,
+                                             SkFlattenable::kSkRasterizer_Type)))
       // Initialize based on our settings. Subclasses can also force this.
     , fGenerateImageFromPath(fRec.fFrameWidth > 0 || fPathEffect != nullptr || fRasterizer != nullptr)
 
@@ -93,7 +109,11 @@ SkScalerContext::SkScalerContext(SkTypeface* typeface, const SkScalerContextEffe
 #endif
 }
 
-SkScalerContext::~SkScalerContext() {}
+SkScalerContext::~SkScalerContext() {
+    SkSafeUnref(fPathEffect);
+    SkSafeUnref(fMaskFilter);
+    SkSafeUnref(fRasterizer);
+}
 
 void SkScalerContext::getAdvance(SkGlyph* glyph) {
     // mark us as just having a valid advance
@@ -136,7 +156,7 @@ void SkScalerContext::getMetrics(SkGlyph* glyph) {
             SkMask  mask;
 
             if (fRasterizer->rasterize(fillPath, fillToDevMatrix, nullptr,
-                                       fMaskFilter.get(), &mask,
+                                       fMaskFilter, &mask,
                                        SkMask::kJustComputeBounds_CreateMode)) {
                 glyph->fLeft    = mask.fBounds.fLeft;
                 glyph->fTop     = mask.fBounds.fTop;
@@ -465,9 +485,10 @@ void SkScalerContext::getImage(const SkGlyph& origGlyph) {
         tmpGlyph.initGlyphIdFrom(origGlyph);
 
         // need the original bounds, sans our maskfilter
-        SkMaskFilter* mf = fMaskFilter.release();   // temp disable
+        SkMaskFilter* mf = fMaskFilter;
+        fMaskFilter = nullptr;             // temp disable
         this->getMetrics(&tmpGlyph);
-        fMaskFilter = sk_sp<SkMaskFilter>(mf);      // restore
+        fMaskFilter = mf;               // restore
 
         // we need the prefilter bounds to be <= filter bounds
         SkASSERT(tmpGlyph.fWidth <= origGlyph.fWidth);
@@ -495,7 +516,7 @@ void SkScalerContext::getImage(const SkGlyph& origGlyph) {
             sk_bzero(glyph->fImage, mask.computeImageSize());
 
             if (!fRasterizer->rasterize(fillPath, fillToDevMatrix, nullptr,
-                                        fMaskFilter.get(), &mask,
+                                        fMaskFilter, &mask,
                                         SkMask::kJustRenderImage_CreateMode)) {
                 return;
             }
@@ -830,9 +851,8 @@ SkAxisAlignment SkScalerContext::computeAxisAlignmentForHText() {
 
 class SkScalerContext_Empty : public SkScalerContext {
 public:
-    SkScalerContext_Empty(SkTypeface* typeface, const SkScalerContextEffects& effects,
-                          const SkDescriptor* desc)
-        : SkScalerContext(typeface, effects, desc) {}
+    SkScalerContext_Empty(SkTypeface* face, const SkDescriptor* desc)
+        : SkScalerContext(face, desc) {}
 
 protected:
     unsigned generateGlyphCount() override {
@@ -858,13 +878,12 @@ protected:
 
 extern SkScalerContext* SkCreateColorScalerContext(const SkDescriptor* desc);
 
-SkScalerContext* SkTypeface::createScalerContext(const SkScalerContextEffects& effects,
-                                                 const SkDescriptor* desc,
+SkScalerContext* SkTypeface::createScalerContext(const SkDescriptor* desc,
                                                  bool allowFailure) const {
-    SkScalerContext* c = this->onCreateScalerContext(effects, desc);
+    SkScalerContext* c = this->onCreateScalerContext(desc);
 
     if (!c && !allowFailure) {
-        c = new SkScalerContext_Empty(const_cast<SkTypeface*>(this), effects, desc);
+        c = new SkScalerContext_Empty(const_cast<SkTypeface*>(this), desc);
     }
     return c;
 }
diff --git a/src/core/SkScalerContext.h b/src/core/SkScalerContext.h
index ffde83cb0a..96674cd500 100644
--- a/src/core/SkScalerContext.h
+++ b/src/core/SkScalerContext.h
@@ -20,16 +20,6 @@ class SkMaskFilter;
 class SkPathEffect;
 class SkRasterizer;
 
-struct SkScalerContextEffects {
-    SkScalerContextEffects() : fPathEffect(nullptr), fMaskFilter(nullptr), fRasterizer(nullptr) {}
-    SkScalerContextEffects(SkPathEffect* pe, SkMaskFilter* mf, SkRasterizer* ra)
-        : fPathEffect(pe), fMaskFilter(mf), fRasterizer(ra) {}
-
-    SkPathEffect*   fPathEffect;
-    SkMaskFilter*   fMaskFilter;
-    SkRasterizer*   fRasterizer;
-};
-
 enum SkAxisAlignment {
     kNone_SkAxisAlignment,
     kX_SkAxisAlignment,
@@ -210,7 +200,8 @@ public:
         kHinting_Mask   = kHintingBit1_Flag | kHintingBit2_Flag,
     };
 
-    SkScalerContext(SkTypeface*, const SkScalerContextEffects&, const SkDescriptor*);
+
+    SkScalerContext(SkTypeface*, const SkDescriptor*);
     virtual ~SkScalerContext();
 
     SkTypeface* getTypeface() const { return fTypeface.get(); }
@@ -269,16 +260,13 @@ public:
 
     const Rec& getRec() const { return fRec; }
 
-    SkScalerContextEffects getEffects() const {
-        return { fPathEffect.get(), fMaskFilter.get(), fRasterizer.get() };
-    }
-
     /**
     *  Return the axis (if any) that the baseline for horizontal text should land on.
     *  As an example, the identity matrix will return kX_SkAxisAlignment
     */
     SkAxisAlignment computeAxisAlignmentForHText();
 
+
 protected:
     Rec         fRec;
 
@@ -338,12 +326,12 @@ private:
     friend class SkRandomScalerContext; // For debug purposes
 
     // never null
-    sk_sp<SkTypeface> fTypeface;
+    SkAutoTUnref<SkTypeface> fTypeface;
 
-    // optional objects, which may be null
-    sk_sp<SkPathEffect> fPathEffect;
-    sk_sp<SkMaskFilter> fMaskFilter;
-    sk_sp<SkRasterizer> fRasterizer;
+    // optional object, which may be null
+    SkPathEffect*   fPathEffect;
+    SkMaskFilter*   fMaskFilter;
+    SkRasterizer*   fRasterizer;
 
     // if this is set, we draw the image from a path, rather than
     // calling generateImage.
diff --git a/src/core/SkTypeface.cpp b/src/core/SkTypeface.cpp
index dc5306331c..6024c81a17 100644
--- a/src/core/SkTypeface.cpp
+++ b/src/core/SkTypeface.cpp
@@ -40,8 +40,7 @@ protected:
     SkEmptyTypeface() : SkTypeface(SkFontStyle(), 0, true) { }
 
     SkStreamAsset* onOpenStream(int* ttcIndex) const override { return nullptr; }
-    SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
-                                           const SkDescriptor*) const override {
+    SkScalerContext* onCreateScalerContext(const SkDescriptor*) const override {
         return nullptr;
     }
     void onFilterRec(SkScalerContextRec*) const override { }
@@ -349,8 +348,7 @@ bool SkTypeface::onComputeBounds(SkRect* bounds) const {
     desc->init();
     desc->addEntry(kRec_SkDescriptorTag, sizeof(rec), &rec);
 
-    SkScalerContextEffects noeffects;
-    SkAutoTDelete<SkScalerContext> ctx(this->createScalerContext(noeffects, desc, true));
+    SkAutoTDelete<SkScalerContext> ctx(this->createScalerContext(desc, true));
     if (ctx.get()) {
         SkPaint::FontMetrics fm;
         ctx->getFontMetrics(&fm);