Remove ability for Release code to call getRefCnt() or getWeakRefCnt().

These getRefCnt() methods are not thread safe, so Skia code should not
be calling them.  unique() is fine.

SkDEBUG code (SkASSERTs) can still call getRefCnt() / getWeakRefCnt().

This adds tools/RefCntIs.{h,cpp}, which lets tests make their assertions in
both debug and release modes.

BUG=skia:2726
R=senorblanco@chromium.org, mtklein@google.com, reed@google.com

Author: mtklein@chromium.org

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

diff --git a/gyp/tests.gypi b/gyp/tests.gypi
index 3b67ebd..6935614 100644 (file)
--- a/gyp/tests.gypi
+++ b/gyp/tests.gypi
@@ -24,6 +24,7 @@
     'tools.gyp:picture_utils',
     'tools.gyp:resources',
     'tools.gyp:sk_tool_utils',
+    'tools.gyp:ref_cnt_is',
   ],
   'sources': [
     '../tests/Test.cpp',

diff --git a/gyp/tools.gyp b/gyp/tools.gyp
index e16fa4b..30d014c 100644 (file)
--- a/gyp/tools.gyp
+++ b/gyp/tools.gyp
@@ -44,6 +44,17 @@
         ],
       ],
     },
+    {
+      'target_name': 'ref_cnt_is',
+      'type': 'static_library',
+      'sources': [ '../tools/RefCntIs.cpp' ],
+      'direct_dependent_settings': {
+        'include_dirs': [ '../tools' ],
+      },
+      'dependencies': [
+        'skia_lib.gyp:skia_lib',
+      ],
+    },
     {  # This would go in gm.gyp, but it's also used by skimage below.
       'target_name': 'gm_expectations',
       'type': 'static_library',

diff --git a/include/core/SkImageFilter.h b/include/core/SkImageFilter.h
index 9cdd275..15e9a2f 100644 (file)
--- a/include/core/SkImageFilter.h
+++ b/include/core/SkImageFilter.h
@@ -51,6 +51,7 @@ public:
     class SK_API Cache : public SkRefCnt {
     public:
         // By default, we cache only image filters with 2 or more children.
+        // Values less than 2 mean always cache; values greater than 2 are not supported.
         static Cache* Create(int minChildren = 2);
         virtual ~Cache() {}
         virtual bool get(const SkImageFilter* key, SkBitmap* result, SkIPoint* offset) = 0;

diff --git a/include/core/SkRefCnt.h b/include/core/SkRefCnt.h
index 1724c77..d3a26df 100644 (file)
--- a/include/core/SkRefCnt.h
+++ b/include/core/SkRefCnt.h
@@ -42,8 +42,10 @@ public:
 #endif
     }
 
+#ifdef SK_DEBUG
     /** Return the reference count. Use only for debugging. */
     int32_t getRefCnt() const { return fRefCnt; }
+#endif
 
     /** May return true if the caller is the only owner.
      *  Ensures that all previous owner's actions are complete.
@@ -118,6 +120,9 @@ private:
 
     mutable int32_t fRefCnt;
 
+    // Used by tests.
+    friend bool RefCntIs(const SkRefCntBase&, int32_t);
+
     typedef SkNoncopyable INHERITED;
 };
 

diff --git a/include/core/SkWeakRefCnt.h b/include/core/SkWeakRefCnt.h
index 210dcc9..d9f0dc8 100644 (file)
--- a/include/core/SkWeakRefCnt.h
+++ b/include/core/SkWeakRefCnt.h
@@ -60,20 +60,18 @@ public:
     */
     SkWeakRefCnt() : SkRefCnt(), fWeakCnt(1) {}
 
+#ifdef SK_DEBUG
     /** Destruct, asserting that the weak reference count is 1.
     */
     virtual ~SkWeakRefCnt() {
-#ifdef SK_DEBUG
         SkASSERT(fWeakCnt == 1);
         fWeakCnt = 0;
-#endif
     }
 
     /** Return the weak reference count.
     */
     int32_t getWeakCnt() const { return fWeakCnt; }
 
-#ifdef SK_DEBUG
     void validate() const {
         this->INHERITED::validate();
         SkASSERT(fWeakCnt > 0);
@@ -155,6 +153,9 @@ private:
     /* Invariant: fWeakCnt = #weak + (fRefCnt > 0 ? 1 : 0) */
     mutable int32_t fWeakCnt;
 
+    // Used by tests.
+    friend bool WeakRefCntIs(const SkWeakRefCnt&, int32_t);
+
     typedef SkRefCnt INHERITED;
 };
 

diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index 4c4b56b..b67fbe0 100644 (file)
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -379,7 +379,10 @@ static uint32_t compute_hash(const uint32_t* data, int count) {
 
 class CacheImpl : public SkImageFilter::Cache {
 public:
-    explicit CacheImpl(int minChildren) : fMinChildren(minChildren) {}
+    explicit CacheImpl(int minChildren) : fMinChildren(minChildren) {
+        SkASSERT(fMinChildren <= 2);
+    }
+
     virtual ~CacheImpl();
     bool get(const SkImageFilter* key, SkBitmap* result, SkIPoint* offset) SK_OVERRIDE;
     void set(const SkImageFilter* key, const SkBitmap& result, const SkIPoint& offset) SK_OVERRIDE;
@@ -422,7 +425,10 @@ void CacheImpl::remove(const SkImageFilter* key) {
 }
 
 void CacheImpl::set(const SkImageFilter* key, const SkBitmap& result, const SkIPoint& offset) {
-    if (key->getRefCnt() >= fMinChildren) {
+    if (fMinChildren < 2 || !key->unique()) {
+        // We take !key->unique() as a signal that there are probably at least 2 refs on the key,
+        // meaning this filter probably has at least two children and is worth caching when
+        // fMinChildren is 2.  If fMinChildren is less than two, we'll just always cache.
         fData.add(new Value(key, result, offset));
     }
 }

diff --git a/tests/ClipCacheTest.cpp b/tests/ClipCacheTest.cpp
index 77f0137..c6f33c7 100644 (file)
--- a/tests/ClipCacheTest.cpp
+++ b/tests/ClipCacheTest.cpp
@@ -6,6 +6,7 @@
  */
 
 #include "Test.h"
+#include "RefCntIs.h"
 // This is a GR test
 #if SK_SUPPORT_GPU
 #include "../../src/gpu/GrClipMaskManager.h"
@@ -164,14 +165,14 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) {
 
     // check that the set took
     check_state(reporter, cache, clip1, texture1, bound1);
-    REPORTER_ASSERT(reporter, texture1->getRefCnt());
+    REPORTER_ASSERT(reporter, !RefCntIs(*texture1, 0));
 
     // push the state
     cache.push();
 
     // verify that the pushed state is initially empty
     check_empty_state(reporter, cache);
-    REPORTER_ASSERT(reporter, texture1->getRefCnt());
+    REPORTER_ASSERT(reporter, !RefCntIs(*texture1, 0));
 
     // modify the new state
     SkIRect bound2;
@@ -189,8 +190,8 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) {
 
     // check that the changes took
     check_state(reporter, cache, clip2, texture2, bound2);
-    REPORTER_ASSERT(reporter, texture1->getRefCnt());
-    REPORTER_ASSERT(reporter, texture2->getRefCnt());
+    REPORTER_ASSERT(reporter, !RefCntIs(*texture1, 0));
+    REPORTER_ASSERT(reporter, !RefCntIs(*texture2, 0));
 
     // check to make sure canReuse works
     REPORTER_ASSERT(reporter, cache.canReuse(clip2.getTopmostGenID(), bound2));
@@ -201,7 +202,7 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) {
 
     // verify that the old state is restored
     check_state(reporter, cache, clip1, texture1, bound1);
-    REPORTER_ASSERT(reporter, texture1->getRefCnt());
+    REPORTER_ASSERT(reporter, !RefCntIs(*texture1, 0));
 
     // manually clear the state
     cache.reset();

diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp
index 3f71b01..d6af244 100644 (file)
--- a/tests/ImageFilterTest.cpp
+++ b/tests/ImageFilterTest.cpp
@@ -262,7 +262,7 @@ static void test_crop_rects(SkBaseDevice* device, skiatest::Reporter* reporter)
         SkIPoint offset;
         SkString str;
         str.printf("filter %d", static_cast<int>(i));
-        SkAutoTUnref<SkImageFilter::Cache> cache(SkImageFilter::Cache::Create(2));
+        SkAutoTUnref<SkImageFilter::Cache> cache(SkImageFilter::Cache::Create());
         SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeLargest(), cache.get());
         REPORTER_ASSERT_MESSAGE(reporter, filter->filterImage(&proxy, bitmap, ctx,
                                 &result, &offset), str.c_str());

diff --git a/tests/MetaDataTest.cpp b/tests/MetaDataTest.cpp
index eb7eae9..293e12a 100644 (file)
--- a/tests/MetaDataTest.cpp
+++ b/tests/MetaDataTest.cpp
@@ -6,11 +6,12 @@
  */
 
 #include "SkMetaData.h"
+#include "RefCntIs.h"
 #include "Test.h"
 
 static void test_ptrs(skiatest::Reporter* reporter) {
     SkRefCnt ref;
-    REPORTER_ASSERT(reporter, 1 == ref.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(ref, 1));
 
     {
         SkMetaData md0, md1;
@@ -19,19 +20,19 @@ static void test_ptrs(skiatest::Reporter* reporter) {
         md0.setRefCnt(name, &ref);
         REPORTER_ASSERT(reporter, md0.findRefCnt(name));
         REPORTER_ASSERT(reporter, md0.hasRefCnt(name, &ref));
-        REPORTER_ASSERT(reporter, 2 == ref.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(ref, 2));
 
         md1 = md0;
         REPORTER_ASSERT(reporter, md1.findRefCnt(name));
         REPORTER_ASSERT(reporter, md1.hasRefCnt(name, &ref));
-        REPORTER_ASSERT(reporter, 3 == ref.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(ref, 3));
 
         REPORTER_ASSERT(reporter, md0.removeRefCnt(name));
         REPORTER_ASSERT(reporter, !md0.findRefCnt(name));
         REPORTER_ASSERT(reporter, !md0.hasRefCnt(name, &ref));
-        REPORTER_ASSERT(reporter, 2 == ref.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(ref, 2));
     }
-    REPORTER_ASSERT(reporter, 1 == ref.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(ref, 1));
 }
 
 DEF_TEST(MetaData, reporter) {

diff --git a/tests/RefCntTest.cpp b/tests/RefCntTest.cpp
index bd4f348..5f2cbe4 100644 (file)
--- a/tests/RefCntTest.cpp
+++ b/tests/RefCntTest.cpp
@@ -10,6 +10,7 @@
 #include "SkThreadUtils.h"
 #include "SkTypes.h"
 #include "SkWeakRefCnt.h"
+#include "RefCntIs.h"
 #include "Test.h"
 
 class InstCounterClass {
@@ -33,7 +34,7 @@ static void test_refarray(skiatest::Reporter* reporter) {
     const int N = 10;
     SkTRefArray<InstCounterClass>* array = SkTRefArray<InstCounterClass>::Create(N);
 
-    REPORTER_ASSERT(reporter, 1 == array->getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(*array, 1));
     REPORTER_ASSERT(reporter, N == array->count());
 
     REPORTER_ASSERT(reporter, N == InstCounterClass::gInstCounter);
@@ -50,7 +51,7 @@ static void test_refarray(skiatest::Reporter* reporter) {
     }
 
     array = SkTRefArray<InstCounterClass>::Create(src, N);
-    REPORTER_ASSERT(reporter, 1 == array->getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(*array, 1));
     REPORTER_ASSERT(reporter, N == array->count());
 
     REPORTER_ASSERT(reporter, 2*N == InstCounterClass::gInstCounter);
@@ -91,7 +92,7 @@ static void test_refCnt(skiatest::Reporter* reporter) {
     thing1.join();
     thing2.join();
 
-    REPORTER_ASSERT(reporter, ref->getRefCnt() == 1);
+    REPORTER_ASSERT(reporter, RefCntIs(*ref, 1));
     ref->unref();
 }
 
@@ -135,8 +136,8 @@ static void test_weakRefCnt(skiatest::Reporter* reporter) {
     thing3.join();
     thing4.join();
 
-    REPORTER_ASSERT(reporter, ref->getRefCnt() == 1);
-    REPORTER_ASSERT(reporter, ref->getWeakCnt() == 1);
+    REPORTER_ASSERT(reporter, RefCntIs(*ref, 1));
+    REPORTER_ASSERT(reporter, WeakRefCntIs(*ref, 1));
     ref->unref();
 }
 

diff --git a/tests/RefDictTest.cpp b/tests/RefDictTest.cpp
index 1e18a68..5b9987e 100644 (file)
--- a/tests/RefDictTest.cpp
+++ b/tests/RefDictTest.cpp
@@ -6,6 +6,7 @@
  */
 
 #include "SkRefDict.h"
+#include "RefCntIs.h"
 #include "Test.h"
 
 class TestRC : public SkRefCnt {
@@ -25,50 +26,50 @@ DEF_TEST(RefDict, reporter) {
 
     dict.set("foo", &data0);
     REPORTER_ASSERT(reporter, &data0 == dict.find("foo"));
-    REPORTER_ASSERT(reporter, 2 == data0.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(data0, 2));
 
     dict.set("foo", &data0);
     REPORTER_ASSERT(reporter, &data0 == dict.find("foo"));
-    REPORTER_ASSERT(reporter, 2 == data0.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(data0, 2));
 
     dict.set("foo", &data1);
     REPORTER_ASSERT(reporter, &data1 == dict.find("foo"));
-    REPORTER_ASSERT(reporter, 1 == data0.getRefCnt());
-    REPORTER_ASSERT(reporter, 2 == data1.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(data0, 1));
+    REPORTER_ASSERT(reporter, RefCntIs(data1, 2));
 
     dict.set("foo", NULL);
     REPORTER_ASSERT(reporter, NULL == dict.find("foo"));
-    REPORTER_ASSERT(reporter, 1 == data0.getRefCnt());
-    REPORTER_ASSERT(reporter, 1 == data1.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(data0, 1));
+    REPORTER_ASSERT(reporter, RefCntIs(data1, 1));
 
     dict.set("foo", &data0);
     dict.set("bar", &data1);
     REPORTER_ASSERT(reporter, &data0 == dict.find("foo"));
     REPORTER_ASSERT(reporter, &data1 == dict.find("bar"));
-    REPORTER_ASSERT(reporter, 2 == data0.getRefCnt());
-    REPORTER_ASSERT(reporter, 2 == data1.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(data0, 2));
+    REPORTER_ASSERT(reporter, RefCntIs(data1, 2));
 
     dict.set("foo", &data1);
     REPORTER_ASSERT(reporter, &data1 == dict.find("foo"));
     REPORTER_ASSERT(reporter, &data1 == dict.find("bar"));
-    REPORTER_ASSERT(reporter, 1 == data0.getRefCnt());
-    REPORTER_ASSERT(reporter, 3 == data1.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(data0, 1));
+    REPORTER_ASSERT(reporter, RefCntIs(data1, 3));
 
     dict.removeAll();
     REPORTER_ASSERT(reporter, NULL == dict.find("foo"));
     REPORTER_ASSERT(reporter, NULL == dict.find("bar"));
-    REPORTER_ASSERT(reporter, 1 == data0.getRefCnt());
-    REPORTER_ASSERT(reporter, 1 == data1.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(data0, 1));
+    REPORTER_ASSERT(reporter, RefCntIs(data1, 1));
 
     {
         SkRefDict d;
         REPORTER_ASSERT(reporter, NULL == d.find("foo"));
-        REPORTER_ASSERT(reporter, 1 == data0.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(data0, 1));
         d.set("foo", &data0);
         REPORTER_ASSERT(reporter, &data0 == d.find("foo"));
-        REPORTER_ASSERT(reporter, 2 == data0.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(data0, 2));
         // let d go out of scope still with a ref on data0
     }
     // be sure d's destructor lowered data0's owner count back to 1
-    REPORTER_ASSERT(reporter, 1 == data0.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(data0, 1));
 }

diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp
index 610e337..93ba3c2 100644 (file)
--- a/tests/SurfaceTest.cpp
+++ b/tests/SurfaceTest.cpp
@@ -11,6 +11,7 @@
 #include "SkRRect.h"
 #include "SkSurface.h"
 #include "SkUtils.h"
+#include "RefCntIs.h"
 #include "Test.h"
 
 #if SK_SUPPORT_GPU
@@ -77,11 +78,11 @@ static void test_image(skiatest::Reporter* reporter) {
     void* addr = sk_malloc_throw(size);
     SkData* data = SkData::NewFromMalloc(addr, size);
 
-    REPORTER_ASSERT(reporter, 1 == data->getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(*data, 1));
     SkImage* image = SkImage::NewRasterData(info, data, rowBytes);
-    REPORTER_ASSERT(reporter, 2 == data->getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(*data, 2));
     image->unref();
-    REPORTER_ASSERT(reporter, 1 == data->getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(*data, 1));
     data->unref();
 }
 

diff --git a/tests/UtilsTest.cpp b/tests/UtilsTest.cpp
index 438a5cc..9454b54 100644 (file)
--- a/tests/UtilsTest.cpp
+++ b/tests/UtilsTest.cpp
@@ -10,6 +10,7 @@
 #include "SkTSearch.h"
 #include "SkTSort.h"
 #include "SkUtils.h"
+#include "RefCntIs.h"
 #include "Test.h"
 
 class RefClass : public SkRefCnt {
@@ -27,30 +28,30 @@ private:
 
 static void test_autounref(skiatest::Reporter* reporter) {
     RefClass obj(0);
-    REPORTER_ASSERT(reporter, 1 == obj.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(obj, 1));
 
     SkAutoTUnref<RefClass> tmp(&obj);
     REPORTER_ASSERT(reporter, &obj == tmp.get());
-    REPORTER_ASSERT(reporter, 1 == obj.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(obj, 1));
 
     REPORTER_ASSERT(reporter, &obj == tmp.detach());
-    REPORTER_ASSERT(reporter, 1 == obj.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(obj, 1));
     REPORTER_ASSERT(reporter, NULL == tmp.detach());
     REPORTER_ASSERT(reporter, NULL == tmp.get());
 
     obj.ref();
-    REPORTER_ASSERT(reporter, 2 == obj.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(obj, 2));
     {
         SkAutoTUnref<RefClass> tmp2(&obj);
     }
-    REPORTER_ASSERT(reporter, 1 == obj.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(obj, 1));
 }
 
 static void test_autostarray(skiatest::Reporter* reporter) {
     RefClass obj0(0);
     RefClass obj1(1);
-    REPORTER_ASSERT(reporter, 1 == obj0.getRefCnt());
-    REPORTER_ASSERT(reporter, 1 == obj1.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(obj0, 1));
+    REPORTER_ASSERT(reporter, RefCntIs(obj1, 1));
 
     {
         SkAutoSTArray<2, SkRefPtr<RefClass> > tmp;
@@ -61,14 +62,14 @@ static void test_autostarray(skiatest::Reporter* reporter) {
         REPORTER_ASSERT(reporter, 4 == tmp.count());
         tmp[0] = &obj0;
         tmp[1] = &obj1;
-        REPORTER_ASSERT(reporter, 2 == obj0.getRefCnt());
-        REPORTER_ASSERT(reporter, 2 == obj1.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(obj0, 2));
+        REPORTER_ASSERT(reporter, RefCntIs(obj1, 2));
 
         // test out reset with data in the array (and a new allocation)
         tmp.reset(0);
         REPORTER_ASSERT(reporter, 0 == tmp.count());
-        REPORTER_ASSERT(reporter, 1 == obj0.getRefCnt());
-        REPORTER_ASSERT(reporter, 1 == obj1.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(obj0, 1));
+        REPORTER_ASSERT(reporter, RefCntIs(obj1, 1));
 
         tmp.reset(2);   // this should use the preexisting allocation
         REPORTER_ASSERT(reporter, 2 == tmp.count());
@@ -77,8 +78,8 @@ static void test_autostarray(skiatest::Reporter* reporter) {
     }
 
     // test out destructor with data in the array (and using existing allocation)
-    REPORTER_ASSERT(reporter, 1 == obj0.getRefCnt());
-    REPORTER_ASSERT(reporter, 1 == obj1.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(obj0, 1));
+    REPORTER_ASSERT(reporter, RefCntIs(obj1, 1));
 
     {
         // test out allocating ctor (this should allocate new memory)
@@ -87,32 +88,32 @@ static void test_autostarray(skiatest::Reporter* reporter) {
 
         tmp[0] = &obj0;
         tmp[1] = &obj1;
-        REPORTER_ASSERT(reporter, 2 == obj0.getRefCnt());
-        REPORTER_ASSERT(reporter, 2 == obj1.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(obj0, 2));
+        REPORTER_ASSERT(reporter, RefCntIs(obj1, 2));
 
         // Test out resut with data in the array and malloced storage
         tmp.reset(0);
-        REPORTER_ASSERT(reporter, 1 == obj0.getRefCnt());
-        REPORTER_ASSERT(reporter, 1 == obj1.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(obj0, 1));
+        REPORTER_ASSERT(reporter, RefCntIs(obj1, 1));
 
         tmp.reset(2);   // this should use the preexisting storage
         tmp[0] = &obj0;
         tmp[1] = &obj1;
-        REPORTER_ASSERT(reporter, 2 == obj0.getRefCnt());
-        REPORTER_ASSERT(reporter, 2 == obj1.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(obj0, 2));
+        REPORTER_ASSERT(reporter, RefCntIs(obj1, 2));
 
         tmp.reset(4);   // this should force a new malloc
-        REPORTER_ASSERT(reporter, 1 == obj0.getRefCnt());
-        REPORTER_ASSERT(reporter, 1 == obj1.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(obj0, 1));
+        REPORTER_ASSERT(reporter, RefCntIs(obj1, 1));
 
         tmp[0] = &obj0;
         tmp[1] = &obj1;
-        REPORTER_ASSERT(reporter, 2 == obj0.getRefCnt());
-        REPORTER_ASSERT(reporter, 2 == obj1.getRefCnt());
+        REPORTER_ASSERT(reporter, RefCntIs(obj0, 2));
+        REPORTER_ASSERT(reporter, RefCntIs(obj1, 2));
     }
 
-    REPORTER_ASSERT(reporter, 1 == obj0.getRefCnt());
-    REPORTER_ASSERT(reporter, 1 == obj1.getRefCnt());
+    REPORTER_ASSERT(reporter, RefCntIs(obj0, 1));
+    REPORTER_ASSERT(reporter, RefCntIs(obj1, 1));
 }
 
 /////////////////////////////////////////////////////////////////////////////

diff --git a/tools/RefCntIs.cpp b/tools/RefCntIs.cpp
new file mode 100644 (file)
index 0000000..48154b0
--- /dev/null
+++ b/tools/RefCntIs.cpp
@@ -0,0 +1,9 @@
+#include "RefCntIs.h"
+
+bool RefCntIs(const SkRefCntBase& obj, int32_t n) {
+    return obj.fRefCnt == n;
+}
+
+bool WeakRefCntIs(const SkWeakRefCnt& obj, int32_t n) {
+    return obj.fWeakCnt == n;
+}

diff --git a/tools/RefCntIs.h b/tools/RefCntIs.h
new file mode 100644 (file)
index 0000000..86d3cc3
--- /dev/null
+++ b/tools/RefCntIs.h
@@ -0,0 +1,13 @@
+#ifndef RefCntIs_DEFINED
+#define RefCntIs_DEFINED
+
+// Allows tests to assert a particular value for a ref count,
+// without letting Skia library code look at that value.
+
+#include "SkRefCnt.h"
+#include "SkWeakRefCnt.h"
+
+bool RefCntIs(const SkRefCntBase&, int32_t);
+bool WeakRefCntIs(const SkWeakRefCnt&, int32_t);
+
+#endif//RefCntIs_DEFINED

diff --git a/tools/tsan.supp b/tools/tsan.supp
index 179adc9..6c2b090 100644 (file)
--- a/tools/tsan.supp
+++ b/tools/tsan.supp
@@ -31,9 +31,6 @@ race:SkPDF
 race:SkPixelRef::callGenIDChangeListeners
 race:SkPixelRef::needsNewGenID
 
-# This calls SkRefCnt::getRefCnt(), which is not thread safe.  skia:2726
-race:SkImageFilter::filterImage
-
 # SkPathRef caches its bounding box the first time it's needed.
 # This will be fixed naturally once we create (from a single thread) a
 # bounding-box hierarchy for SkRecord-based SkPictures; all bounds will come pre-cached.