Make the SkPathRef GenIDChangeListener ref counted

Bug: skia:
Change-Id: I2780e3fc76153373b4efca6059ded82f4f749325
Reviewed-on: https://skia-review.googlesource.com/133502
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>

8 files changed, 20 insertions, 22 deletions

diff --git a/include/private/SkPathRef.h b/include/private/SkPathRef.h
index 77d031f1d0..9ce1aeef2c 100644
--- a/include/private/SkPathRef.h
+++ b/include/private/SkPathRef.h
@@ -306,12 +306,12 @@ public:
      */
     uint32_t genID() const;
 
-    struct GenIDChangeListener {
+    struct GenIDChangeListener : SkRefCnt {
         virtual ~GenIDChangeListener() {}
         virtual void onChange() = 0;
     };
 
-    void addGenIDChangeListener(GenIDChangeListener* listener);
+    void addGenIDChangeListener(sk_sp<GenIDChangeListener>);
 
     bool isValid() const;
     SkDEBUGCODE(void validate() const { SkASSERT(this->isValid()); } )
@@ -538,7 +538,7 @@ private:
     mutable uint32_t    fGenerationID;
     SkDEBUGCODE(int32_t fEditorsAttached;) // assert that only one editor in use at any time.
 
-    SkTDArray<GenIDChangeListener*> fGenIDChangeListeners;  // pointers are owned
+    SkTDArray<GenIDChangeListener*> fGenIDChangeListeners;  // pointers are reffed
 
     mutable uint8_t  fBoundsIsDirty;
     mutable SkBool8  fIsFinite;    // only meaningful if bounds are valid
diff --git a/src/core/SkPathPriv.h b/src/core/SkPathPriv.h
index 9190962962..bcf85a138d 100644
--- a/src/core/SkPathPriv.h
+++ b/src/core/SkPathPriv.h
@@ -86,8 +86,9 @@ public:
         return false;
     }
 
-    static void AddGenIDChangeListener(const SkPath& path, SkPathRef::GenIDChangeListener* listener) {
-        path.fPathRef->addGenIDChangeListener(listener);
+    static void AddGenIDChangeListener(const SkPath& path,
+                                       sk_sp<SkPathRef::GenIDChangeListener> listener) {
+        path.fPathRef->addGenIDChangeListener(std::move(listener));
     }
 
     /**
diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index cc6152a93d..58782df260 100644
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -669,12 +669,11 @@ uint32_t SkPathRef::genID() const {
     return fGenerationID;
 }
 
-void SkPathRef::addGenIDChangeListener(GenIDChangeListener* listener) {
+void SkPathRef::addGenIDChangeListener(sk_sp<GenIDChangeListener> listener) {
     if (nullptr == listener || this == gEmpty) {
-        delete listener;
         return;
     }
-    *fGenIDChangeListeners.append() = listener;
+    *fGenIDChangeListeners.append() = listener.release();
 }
 
 // we need to be called *before* the genID gets changed or zerod
@@ -684,7 +683,7 @@ void SkPathRef::callGenIDChangeListeners() {
     }
 
     // Listeners get at most one shot, so whether these triggered or not, blow them away.
-    fGenIDChangeListeners.deleteAll();
+    fGenIDChangeListeners.unrefAll();
 }
 
 SkRRect SkPathRef::getRRect() const {
diff --git a/src/gpu/GrShape.cpp b/src/gpu/GrShape.cpp
index 4cab0188d9..d74b144741 100644
--- a/src/gpu/GrShape.cpp
+++ b/src/gpu/GrShape.cpp
@@ -363,11 +363,9 @@ const SkPath* GrShape::originalPathForListeners() const {
     return nullptr;
 }
 
-void GrShape::addGenIDChangeListener(SkPathRef::GenIDChangeListener* listener) const {
+void GrShape::addGenIDChangeListener(sk_sp<SkPathRef::GenIDChangeListener> listener) const {
     if (const auto* lp = this->originalPathForListeners()) {
-        SkPathPriv::AddGenIDChangeListener(*lp, listener);
-    } else {
-        delete listener;
+        SkPathPriv::AddGenIDChangeListener(*lp, std::move(listener));
     }
 }
 
diff --git a/src/gpu/GrShape.h b/src/gpu/GrShape.h
index 36402330ce..c333168b53 100644
--- a/src/gpu/GrShape.h
+++ b/src/gpu/GrShape.h
@@ -396,9 +396,9 @@ public:
     /**
      * Adds a listener to the *original* path. Typically used to invalidate cached resources when
      * a path is no longer in-use. If the shape started out as something other than a path, this
-     * does nothing (but will delete the listener).
+     * does nothing.
      */
-    void addGenIDChangeListener(SkPathRef::GenIDChangeListener* listener) const;
+    void addGenIDChangeListener(sk_sp<SkPathRef::GenIDChangeListener>) const;
 
     /**
      * Helpers that are only exposed for unit tests, to determine if the shape is a path, and get
diff --git a/src/gpu/GrSoftwarePathRenderer.cpp b/src/gpu/GrSoftwarePathRenderer.cpp
index 15ea55d4f2..bcd9874807 100644
--- a/src/gpu/GrSoftwarePathRenderer.cpp
+++ b/src/gpu/GrSoftwarePathRenderer.cpp
@@ -363,7 +363,7 @@ bool GrSoftwarePathRenderer::onDrawPath(const DrawPathArgs& args) {
         if (useCache) {
             SkASSERT(proxy->origin() == kTopLeft_GrSurfaceOrigin);
             fProxyProvider->assignUniqueKeyToProxy(maskKey, proxy.get());
-            args.fShape->addGenIDChangeListener(new PathInvalidator(maskKey));
+            args.fShape->addGenIDChangeListener(sk_make_sp<PathInvalidator>(maskKey));
         }
     }
     if (inverseFilled) {
diff --git a/src/gpu/ops/GrTessellatingPathRenderer.cpp b/src/gpu/ops/GrTessellatingPathRenderer.cpp
index 651d34fae7..1918f666c9 100644
--- a/src/gpu/ops/GrTessellatingPathRenderer.cpp
+++ b/src/gpu/ops/GrTessellatingPathRenderer.cpp
@@ -284,7 +284,7 @@ private:
         info.fCount = count;
         key.setCustomData(SkData::MakeWithCopy(&info, sizeof(info)));
         rp->assignUniqueKeyToResource(key, allocator.vertexBuffer());
-        fShape.addGenIDChangeListener(new PathInvalidator(key));
+        fShape.addGenIDChangeListener(sk_make_sp<PathInvalidator>(key));
     }
 
     void drawAA(Target* target, const GrGeometryProcessor* gp) {
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index 6cf5dfba13..565e8d817b 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -4276,19 +4276,19 @@ public:
 
         // Check that listener is notified on moveTo().
 
-        SkPathPriv::AddGenIDChangeListener(p, new ChangeListener(&changed));
+        SkPathPriv::AddGenIDChangeListener(p, sk_make_sp<ChangeListener>(&changed));
         REPORTER_ASSERT(reporter, !changed);
         p.moveTo(10, 0);
         REPORTER_ASSERT(reporter, changed);
 
         // Check that listener is notified on lineTo().
-        SkPathPriv::AddGenIDChangeListener(p, new ChangeListener(&changed));
+        SkPathPriv::AddGenIDChangeListener(p, sk_make_sp<ChangeListener>(&changed));
         REPORTER_ASSERT(reporter, !changed);
         p.lineTo(20, 0);
         REPORTER_ASSERT(reporter, changed);
 
         // Check that listener is notified on reset().
-        SkPathPriv::AddGenIDChangeListener(p, new ChangeListener(&changed));
+        SkPathPriv::AddGenIDChangeListener(p, sk_make_sp<ChangeListener>(&changed));
         REPORTER_ASSERT(reporter, !changed);
         p.reset();
         REPORTER_ASSERT(reporter, changed);
@@ -4296,7 +4296,7 @@ public:
         p.moveTo(0, 0);
 
         // Check that listener is notified on rewind().
-        SkPathPriv::AddGenIDChangeListener(p, new ChangeListener(&changed));
+        SkPathPriv::AddGenIDChangeListener(p, sk_make_sp<ChangeListener>(&changed));
         REPORTER_ASSERT(reporter, !changed);
         p.rewind();
         REPORTER_ASSERT(reporter, changed);
@@ -4305,7 +4305,7 @@ public:
         {
             SkPath q;
             q.moveTo(10, 10);
-            SkPathPriv::AddGenIDChangeListener(q, new ChangeListener(&changed));
+            SkPathPriv::AddGenIDChangeListener(q, sk_make_sp<ChangeListener>(&changed));
             REPORTER_ASSERT(reporter, !changed);
         }
         // q went out of scope.