commit 185a3798db64c64d47ef89a5fd3d4c5c70f1e621
author xidachen <xidachen@chromium.org> Thu Jun 29 11:19:42 2017 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Thu Jun 29 15:51:17 2017 +0000
tree 2014d366f021406493e8fe035731d32e4cba7034
parent 9026fe13a751582e58e98f9bf735c18b4719d7fe

Fix memory leak in SkImageFilter

In our current implementation of SkImageFilterCache, when the
removeInternal() function is called, the Value is removed, but their
corresponding keys are not always removed in SkImageFilter. That could
result in memory leak.

In this CL, we made changes such that the Value structure now keeps
a pointer to the SkImageFilter. Each time when the removeInternal()
is called, we ask the SkImageFilter to remove the associated keys.

Bug: 689740
Change-Id: I0807fa3581881ad1530536df5289e3976792281f
Reviewed-on: https://skia-review.googlesource.com/20960
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>

tests/ImageFilterCacheTest.cpp

diff --git a/tests/ImageFilterCacheTest.cpp b/tests/ImageFilterCacheTest.cpp
index 7088570..d718181 100644
--- a/tests/ImageFilterCacheTest.cpp
+++ b/tests/ImageFilterCacheTest.cpp
@@ -37,7 +37,7 @@
     SkImageFilterCacheKey key2(0, SkMatrix::I(), clip, subset->uniqueID(), subset->subset());
 
     SkIPoint offset = SkIPoint::Make(3, 4);
-    cache->set(key1, image.get(), offset);
+    cache->set(key1, image.get(), offset, nullptr);
 
     SkIPoint foundOffset;
 
@@ -66,7 +66,7 @@
     SkImageFilterCacheKey key4(0, SkMatrix::I(), clip1, subset->uniqueID(), subset->subset());
 
     SkIPoint offset = SkIPoint::Make(3, 4);
-    cache->set(key0, image.get(), offset);
+    cache->set(key0, image.get(), offset, nullptr);
 
     SkIPoint foundOffset;
     REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset));
@@ -86,20 +86,20 @@
     SkImageFilterCacheKey key2(1, SkMatrix::I(), clip, image->uniqueID(), image->subset());
 
     SkIPoint offset = SkIPoint::Make(3, 4);
-    cache->set(key1, image.get(), offset);
+    cache->set(key1, image.get(), offset, nullptr);
 
     SkIPoint foundOffset;
 
     REPORTER_ASSERT(reporter, cache->get(key1, &foundOffset));
 
     // This should knock the first one out of the cache
-    cache->set(key2, image.get(), offset);
+    cache->set(key2, image.get(), offset, nullptr);
 
     REPORTER_ASSERT(reporter, cache->get(key2, &foundOffset));
     REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset));
 }
 
-// Exercise the purgeByKeys and purge methods
+// Exercise the purgeByKey and purge methods
 static void test_explicit_purging(skiatest::Reporter* reporter,
                                   const sk_sp<SkSpecialImage>& image,
                                   const sk_sp<SkSpecialImage>& subset) {
@@ -111,8 +111,8 @@
     SkImageFilterCacheKey key2(1, SkMatrix::I(), clip, subset->uniqueID(), image->subset());
 
     SkIPoint offset = SkIPoint::Make(3, 4);
-    cache->set(key1, image.get(), offset);
-    cache->set(key2, image.get(), offset);
+    cache->set(key1, image.get(), offset, nullptr);
+    cache->set(key2, image.get(), offset, nullptr);
     SkDEBUGCODE(REPORTER_ASSERT(reporter, 2 == cache->count());)
 
     SkIPoint foundOffset;