commit f30e49310fb8927d1acbfd39f6bdc5513aa7daa1
author Chris Dalton <csmartdalton@google.com> Thu Oct 18 18:56:59 2018 -0600
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Oct 19 01:59:27 2018 +0000
tree 11c7b25456d8363ca22d02d3d8423899164bd90a
parent 0f7e55a818f4661309871f2db2fe2c2196135bda

ccpr: Unregister path listeners when their cache entries are evicted

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

src/core/SkPathRef.cpp

diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index f767323..85d1b9c 100644
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -707,19 +707,31 @@
     if (nullptr == listener || this == gEmpty) {
         return;
     }
+
     SkAutoMutexAcquire lock(fGenIDChangeListenersMutex);
-    *fGenIDChangeListeners.append() = listener.release();
+
+    // Clean out any stale listeners before we append the new one.
+    for (int i = 0; i < fGenIDChangeListeners.count(); ++i) {
+        if (fGenIDChangeListeners[i]->shouldUnregisterFromPath()) {
+            fGenIDChangeListeners.removeShuffle(i--);  // No need to preserve the order after i.
+        }
+    }
+
+    SkASSERT(!listener->shouldUnregisterFromPath());
+    fGenIDChangeListeners.push_back(std::move(listener));
 }
 
 // we need to be called *before* the genID gets changed or zerod
 void SkPathRef::callGenIDChangeListeners() {
     SkAutoMutexAcquire lock(fGenIDChangeListenersMutex);
-    for (int i = 0; i < fGenIDChangeListeners.count(); i++) {
-        fGenIDChangeListeners[i]->onChange();
+    for (const sk_sp<GenIDChangeListener>& listener : fGenIDChangeListeners) {
+        if (!listener->shouldUnregisterFromPath()) {
+            listener->onChange();
+        }
     }
 
     // Listeners get at most one shot, so whether these triggered or not, blow them away.
-    fGenIDChangeListeners.unrefAll();
+    fGenIDChangeListeners.reset();
 }
 
 SkRRect SkPathRef::getRRect() const {

src/gpu/ccpr/GrCCPathCache.cpp

diff --git a/src/gpu/ccpr/GrCCPathCache.cpp b/src/gpu/ccpr/GrCCPathCache.cpp
index db85641..32008c2 100644
--- a/src/gpu/ccpr/GrCCPathCache.cpp
+++ b/src/gpu/ccpr/GrCCPathCache.cpp
@@ -187,12 +187,15 @@
 }
 
 void GrCCPathCache::evict(GrCCPathCacheEntry* entry) {
-    bool isInCache = entry->fNext || (fLRU.tail() == entry);
-    SkASSERT(isInCache == fLRU.isInList(entry));
-    if (isInCache) {
-        fLRU.remove(entry);
-        fHashTable.remove(HashNode::GetKey(entry));  // Do this last, as it might delete the entry.
+    // Has the entry already been evicted? (SkPaths can post eviction messages from any thread.)
+    if (entry->shouldUnregisterFromPath()) {
+        SkASSERT(!fLRU.isInList(entry));
+        return;
     }
+
+    entry->markShouldUnregisterFromPath();
+    fLRU.remove(entry);
+    fHashTable.remove(HashNode::GetKey(entry));  // Do this last, as it might delete the entry.
 }
 
 void GrCCPathCache::purgeAsNeeded() {