Revert "Hold sk_sp<GrAtlasTextBlob> refs in GrTextBlobCache instead of raw ptrs"

This reverts commit db3ceb86421fb9da86bb920e3a1f0957beec08d9.

Reason for revert: observing some strange budget behavior w/ instrumented Chromium builds;  need to investigate.

Original change's description:
> Hold sk_sp<GrAtlasTextBlob> refs in GrTextBlobCache instead of raw ptrs
> 
> Refactor to store sk_sps, and minimize explicit ref manipulation.
> 
> Change-Id: Ie3d18e5fe1cefbbc5c2f3c4941287a24038522a6
> Reviewed-on: https://skia-review.googlesource.com/9490
> Commit-Queue: Florin Malita <fmalita@chromium.org>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> 

TBR=bsalomon@google.com,robertphillips@google.com,fmalita@chromium.org,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: I8ca9862ad1519a9ec69ad1ce8e4d129b0dae7b0a
Reviewed-on: https://skia-review.googlesource.com/9524
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>

1 files changed, 35 insertions, 33 deletions

diff --git a/src/gpu/text/GrTextBlobCache.h b/src/gpu/text/GrTextBlobCache.h
index af6c7926fa..886a0914ca 100644
--- a/src/gpu/text/GrTextBlobCache.h
+++ b/src/gpu/text/GrTextBlobCache.h
@@ -9,7 +9,6 @@
 #define GrTextBlobCache_DEFINED
 
 #include "GrAtlasTextContext.h"
-#include "SkRefCnt.h"
 #include "SkTArray.h"
 #include "SkTextBlobRunIterator.h"
 #include "SkTHash.h"
@@ -32,28 +31,31 @@ public:
     ~GrTextBlobCache();
 
     // creates an uncached blob
-    sk_sp<GrAtlasTextBlob> makeBlob(int glyphCount, int runCount) {
-        return GrAtlasTextBlob::Make(&fPool, glyphCount, runCount);
+    GrAtlasTextBlob* createBlob(int glyphCount, int runCount) {
+        return GrAtlasTextBlob::Create(&fPool, glyphCount, runCount);
     }
-
-    sk_sp<GrAtlasTextBlob> makeBlob(const SkTextBlob* blob) {
+    GrAtlasTextBlob* createBlob(const SkTextBlob* blob) {
         int glyphCount = 0;
         int runCount = 0;
         BlobGlyphCount(&glyphCount, &runCount, blob);
-        return GrAtlasTextBlob::Make(&fPool, glyphCount, runCount);
+        GrAtlasTextBlob* cacheBlob = GrAtlasTextBlob::Create(&fPool, glyphCount, runCount);
+        return cacheBlob;
     }
 
-    sk_sp<GrAtlasTextBlob> makeCachedBlob(const SkTextBlob* blob,
-                                          const GrAtlasTextBlob::Key& key,
-                                          const SkMaskFilter::BlurRec& blurRec,
-                                          const SkPaint& paint) {
-        sk_sp<GrAtlasTextBlob> cacheBlob(this->makeBlob(blob));
+    GrAtlasTextBlob* createCachedBlob(const SkTextBlob* blob,
+                                      const GrAtlasTextBlob::Key& key,
+                                      const SkMaskFilter::BlurRec& blurRec,
+                                      const SkPaint& paint) {
+        int glyphCount = 0;
+        int runCount = 0;
+        BlobGlyphCount(&glyphCount, &runCount, blob);
+        GrAtlasTextBlob* cacheBlob = GrAtlasTextBlob::Create(&fPool, glyphCount, runCount);
         cacheBlob->setupKey(key, blurRec, paint);
         this->add(cacheBlob);
         return cacheBlob;
     }
 
-    sk_sp<GrAtlasTextBlob> find(const GrAtlasTextBlob::Key& key) const {
+    GrAtlasTextBlob* find(const GrAtlasTextBlob::Key& key) const {
         const auto* idEntry = fBlobIDCache.find(key.fUniqueID);
         return idEntry ? idEntry->find(key) : nullptr;
     }
@@ -63,11 +65,26 @@ public:
         auto* idEntry = fBlobIDCache.find(id);
         SkASSERT(idEntry);
 
-        fBlobList.remove(blob);
         idEntry->removeBlob(blob);
         if (idEntry->fBlobs.empty()) {
             fBlobIDCache.remove(id);
         }
+
+        fBlobList.remove(blob);
+        blob->unref();
+    }
+
+    void add(GrAtlasTextBlob* blob) {
+        auto  id      = GrAtlasTextBlob::GetKey(*blob).fUniqueID;
+        auto* idEntry = fBlobIDCache.find(id);
+        if (!idEntry) {
+            idEntry = fBlobIDCache.set(id, BlobIDCacheEntry(id));
+        }
+
+        idEntry->addBlob(blob);
+        fBlobList.addToHead(blob);
+
+        this->checkPurge(blob);
     }
 
     void makeMRU(GrAtlasTextBlob* blob) {
@@ -105,12 +122,12 @@ private:
             return entry.fID;
         }
 
-        void addBlob(sk_sp<GrAtlasTextBlob> blob) {
+        void addBlob(GrAtlasTextBlob* blob) {
             SkASSERT(blob);
             SkASSERT(GrAtlasTextBlob::GetKey(*blob).fUniqueID == fID);
             SkASSERT(!this->find(GrAtlasTextBlob::GetKey(*blob)));
 
-            fBlobs.emplace_back(std::move(blob));
+            fBlobs.push_back(blob);
         }
 
         void removeBlob(GrAtlasTextBlob* blob) {
@@ -123,7 +140,7 @@ private:
             fBlobs.removeShuffle(index);
         }
 
-        sk_sp<GrAtlasTextBlob> find(const GrAtlasTextBlob::Key& key) const {
+        GrAtlasTextBlob* find(const GrAtlasTextBlob::Key& key) const {
             auto index = this->findBlobIndex(key);
             return index < 0 ? nullptr : fBlobs[index];
         }
@@ -140,24 +157,9 @@ private:
         uint32_t                             fID;
         // Current clients don't generate multiple GrAtlasTextBlobs per SkTextBlob, so an array w/
         // linear search is acceptable.  If usage changes, we should re-evaluate this structure.
-        SkSTArray<1, sk_sp<GrAtlasTextBlob>, true> fBlobs;
+        SkSTArray<1, GrAtlasTextBlob*, true> fBlobs;
     };
 
-    void add(sk_sp<GrAtlasTextBlob> blob) {
-        auto  id      = GrAtlasTextBlob::GetKey(*blob).fUniqueID;
-        auto* idEntry = fBlobIDCache.find(id);
-        if (!idEntry) {
-            idEntry = fBlobIDCache.set(id, BlobIDCacheEntry(id));
-        }
-
-        // Safe to retain a raw ptr temporarily here, because the cache will hold a ref.
-        GrAtlasTextBlob* rawBlobPtr = blob.get();
-        fBlobList.addToHead(rawBlobPtr);
-        idEntry->addBlob(std::move(blob));
-
-        this->checkPurge(rawBlobPtr);
-    }
-
     void checkPurge(GrAtlasTextBlob* blob = nullptr) {
         // If we are overbudget, then unref until we are below budget again
         if (fPool.size() > fBudget) {
@@ -191,9 +193,9 @@ private:
     static const int kPreAllocSize = 1 << 17;
     static const int kMinGrowthSize = 1 << 17;
     static const int kDefaultBudget = 1 << 22;
-    GrMemoryPool fPool;
     BitmapBlobList fBlobList;
     SkTHashMap<uint32_t, BlobIDCacheEntry> fBlobIDCache;
+    GrMemoryPool fPool;
     PFOverBudgetCB fCallback;
     void* fData;
     size_t fBudget;