Fix explicit allocation bug

Change-Id: I9866f563e02b2ab290cc46ede05f8eda21f6d3b2
Reviewed-on: https://skia-review.googlesource.com/142163
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

3 files changed, 69 insertions, 37 deletions

diff --git a/tests/ProcessorTest.cpp b/tests/ProcessorTest.cpp
index 8263615115..1d0e70cef0 100644
--- a/tests/ProcessorTest.cpp
+++ b/tests/ProcessorTest.cpp
@@ -199,7 +199,7 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(ProcessorRefTest, reporter, ctxInfo) {
                 testingOnly_getIORefCnts(proxy1.get(), &refCnt, &readCnt, &writeCnt);
                 // IO counts should be double if there is a clone of the FP.
                 int ioRefMul = makeClone ? 2 : 1;
-                REPORTER_ASSERT(reporter, 1 == refCnt);
+                REPORTER_ASSERT(reporter, -1 == refCnt);
                 REPORTER_ASSERT(reporter, ioRefMul * 1 == readCnt);
                 REPORTER_ASSERT(reporter, ioRefMul * 0 == writeCnt);
 
diff --git a/tests/ProxyRefTest.cpp b/tests/ProxyRefTest.cpp
index 6a7dfbdcf0..8161827c2d 100644
--- a/tests/ProxyRefTest.cpp
+++ b/tests/ProxyRefTest.cpp
@@ -18,16 +18,12 @@
 #include "GrTexture.h"
 #include "GrTextureProxy.h"
 
-int32_t GrIORefProxy::getProxyRefCnt_TestOnly() const {
-    return fRefCnt;
-}
-
 int32_t GrIORefProxy::getBackingRefCnt_TestOnly() const {
     if (fTarget) {
         return fTarget->fRefCnt;
     }
 
-    return fRefCnt;
+    return -1; // no backing GrSurface
 }
 
 int32_t GrIORefProxy::getPendingReadCnt_TestOnly() const {
@@ -54,12 +50,12 @@ static void check_refs(skiatest::Reporter* reporter,
                        int32_t expectedBackingRefs,
                        int32_t expectedNumReads,
                        int32_t expectedNumWrites) {
-    REPORTER_ASSERT(reporter, proxy->getProxyRefCnt_TestOnly() == expectedProxyRefs);
+    REPORTER_ASSERT(reporter, proxy->priv().getProxyRefCnt() == expectedProxyRefs);
     REPORTER_ASSERT(reporter, proxy->getBackingRefCnt_TestOnly() == expectedBackingRefs);
     REPORTER_ASSERT(reporter, proxy->getPendingReadCnt_TestOnly() == expectedNumReads);
     REPORTER_ASSERT(reporter, proxy->getPendingWriteCnt_TestOnly() == expectedNumWrites);
 
-    SkASSERT(proxy->getProxyRefCnt_TestOnly() == expectedProxyRefs);
+    SkASSERT(proxy->priv().getProxyRefCnt() == expectedProxyRefs);
     SkASSERT(proxy->getBackingRefCnt_TestOnly() == expectedBackingRefs);
     SkASSERT(proxy->getPendingReadCnt_TestOnly() == expectedNumReads);
     SkASSERT(proxy->getPendingWriteCnt_TestOnly() == expectedNumWrites);
@@ -101,7 +97,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ProxyRefTest, reporter, ctxInfo) {
                 static const int kExpectedReads = 0;
                 static const int kExpectedWrites = 1;
 
-                check_refs(reporter, proxy.get(), 1, 1, kExpectedReads, kExpectedWrites);
+                int backingRefs = proxy->isWrapped_ForTesting() ? 1 : -1;
+
+                check_refs(reporter, proxy.get(), 1, backingRefs, kExpectedReads, kExpectedWrites);
 
                 proxy->instantiate(resourceProvider);
 
@@ -119,7 +117,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ProxyRefTest, reporter, ctxInfo) {
                 static const int kExpectedReads = 1;
                 static const int kExpectedWrites = 0;
 
-                check_refs(reporter, proxy.get(), 1, 1, kExpectedReads, kExpectedWrites);
+                int backingRefs = proxy->isWrapped_ForTesting() ? 1 : -1;
+
+                check_refs(reporter, proxy.get(), 1, backingRefs, kExpectedReads, kExpectedWrites);
 
                 proxy->instantiate(resourceProvider);
 
@@ -137,7 +137,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ProxyRefTest, reporter, ctxInfo) {
                 static const int kExpectedReads = 1;
                 static const int kExpectedWrites = 1;
 
-                check_refs(reporter, proxy.get(), 1, 1, kExpectedReads, kExpectedWrites);
+                int backingRefs = proxy->isWrapped_ForTesting() ? 1 : -1;
+
+                check_refs(reporter, proxy.get(), 1, backingRefs, kExpectedReads, kExpectedWrites);
 
                 proxy->instantiate(resourceProvider);
 
@@ -156,7 +158,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ProxyRefTest, reporter, ctxInfo) {
                 static const int kExpectedReads = 0;
                 static const int kExpectedWrites = 0;
 
-                check_refs(reporter, proxy.get(), 3, 3,kExpectedReads, kExpectedWrites);
+                int backingRefs = proxy->isWrapped_ForTesting() ? 3 : -1;
+
+                check_refs(reporter, proxy.get(), 3, backingRefs, kExpectedReads, kExpectedWrites);
 
                 proxy->instantiate(resourceProvider);
 
@@ -178,7 +182,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ProxyRefTest, reporter, ctxInfo) {
 
                 static const int kExpectedWrites = 1;
 
-                check_refs(reporter, proxy.get(), 2, 2, 0, kExpectedWrites);
+                int backingRefs = proxy->isWrapped_ForTesting() ? 2 : -1;
+
+                check_refs(reporter, proxy.get(), 2, backingRefs, 0, kExpectedWrites);
 
                 proxy->instantiate(resourceProvider);
 
diff --git a/tests/ResourceAllocatorTest.cpp b/tests/ResourceAllocatorTest.cpp
index 51ecdb7ed0..852fe5aa33 100644
--- a/tests/ResourceAllocatorTest.cpp
+++ b/tests/ResourceAllocatorTest.cpp
@@ -30,7 +30,7 @@ struct ProxyParams {
     // TODO: do we care about mipmapping
 };
 
-static sk_sp<GrSurfaceProxy> make_deferred(GrProxyProvider* proxyProvider, const ProxyParams& p) {
+static GrSurfaceProxy* make_deferred(GrProxyProvider* proxyProvider, const ProxyParams& p) {
     GrSurfaceDesc desc;
     desc.fFlags = p.fIsRT ? kRenderTarget_GrSurfaceFlag : kNone_GrSurfaceFlags;
     desc.fWidth  = p.fSize;
@@ -38,11 +38,20 @@ static sk_sp<GrSurfaceProxy> make_deferred(GrProxyProvider* proxyProvider, const
     desc.fConfig = p.fConfig;
     desc.fSampleCnt = p.fSampleCnt;
 
-    return proxyProvider->createProxy(desc, p.fOrigin, p.fFit, SkBudgeted::kNo);
+    auto tmp = proxyProvider->createProxy(desc, p.fOrigin, p.fFit, SkBudgeted::kNo);
+    if (!tmp) {
+        return nullptr;
+    }
+    GrSurfaceProxy* ret = tmp.release();
+
+    // Add a read to keep the proxy around but unref it so its backing surfaces can be recycled
+    ret->addPendingRead();
+    ret->unref();
+    return ret;
 }
 
-static sk_sp<GrSurfaceProxy> make_backend(GrContext* context, const ProxyParams& p,
-                                          GrBackendTexture* backendTex) {
+static GrSurfaceProxy* make_backend(GrContext* context, const ProxyParams& p,
+                                    GrBackendTexture* backendTex) {
     GrProxyProvider* proxyProvider = context->contextPriv().proxyProvider();
     GrGpu* gpu = context->contextPriv().getGpu();
 
@@ -50,7 +59,16 @@ static sk_sp<GrSurfaceProxy> make_backend(GrContext* context, const ProxyParams&
                                                        p.fConfig, false,
                                                        GrMipMapped::kNo);
 
-    return proxyProvider->wrapBackendTexture(*backendTex, p.fOrigin);
+    auto tmp = proxyProvider->wrapBackendTexture(*backendTex, p.fOrigin);
+    if (!tmp) {
+        return nullptr;
+    }
+    GrSurfaceProxy* ret = tmp.release();
+
+    // Add a read to keep the proxy around but unref it so its backing surfaces can be recycled
+    ret->addPendingRead();
+    ret->unref();
+    return ret;
 }
 
 static void cleanup_backend(GrContext* context, const GrBackendTexture& backendTex) {
@@ -60,12 +78,11 @@ static void cleanup_backend(GrContext* context, const GrBackendTexture& backendT
 // Basic test that two proxies with overlapping intervals and compatible descriptors are
 // assigned different GrSurfaces.
 static void overlap_test(skiatest::Reporter* reporter, GrResourceProvider* resourceProvider,
-                         sk_sp<GrSurfaceProxy> p1, sk_sp<GrSurfaceProxy> p2,
-                         bool expectedResult) {
+                         GrSurfaceProxy* p1, GrSurfaceProxy* p2, bool expectedResult) {
     GrResourceAllocator alloc(resourceProvider);
 
-    alloc.addInterval(p1.get(), 0, 4);
-    alloc.addInterval(p2.get(), 1, 2);
+    alloc.addInterval(p1, 0, 4);
+    alloc.addInterval(p2, 1, 2);
     alloc.markEndOfOpList(0);
 
     int startIndex, stopIndex;
@@ -83,12 +100,12 @@ static void overlap_test(skiatest::Reporter* reporter, GrResourceProvider* resou
 // Test various cases when two proxies do not have overlapping intervals.
 // This mainly acts as a test of the ResourceAllocator's free pool.
 static void non_overlap_test(skiatest::Reporter* reporter, GrResourceProvider* resourceProvider,
-                             sk_sp<GrSurfaceProxy> p1, sk_sp<GrSurfaceProxy> p2,
+                             GrSurfaceProxy* p1, GrSurfaceProxy* p2,
                              bool expectedResult) {
     GrResourceAllocator alloc(resourceProvider);
 
-    alloc.addInterval(p1.get(), 0, 2);
-    alloc.addInterval(p2.get(), 3, 5);
+    alloc.addInterval(p1, 0, 2);
+    alloc.addInterval(p2, 3, 5);
     alloc.markEndOfOpList(0);
 
     int startIndex, stopIndex;
@@ -149,10 +166,11 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ResourceAllocatorTest, reporter, ctxInfo) {
     };
 
     for (auto test : gOverlappingTests) {
-        sk_sp<GrSurfaceProxy> p1 = make_deferred(proxyProvider, test.fP1);
-        sk_sp<GrSurfaceProxy> p2 = make_deferred(proxyProvider, test.fP2);
-        overlap_test(reporter, resourceProvider,
-                     std::move(p1), std::move(p2), test.fExpectation);
+        GrSurfaceProxy* p1 = make_deferred(proxyProvider, test.fP1);
+        GrSurfaceProxy* p2 = make_deferred(proxyProvider, test.fP2);
+        overlap_test(reporter, resourceProvider, p1, p2, test.fExpectation);
+        p1->completedRead();
+        p2->completedRead();
     }
 
     int k2 = ctxInfo.grContext()->contextPriv().caps()->getRenderTargetSampleCount(2, kRGBA);
@@ -188,13 +206,17 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ResourceAllocatorTest, reporter, ctxInfo) {
     };
 
     for (auto test : gNonOverlappingTests) {
-        sk_sp<GrSurfaceProxy> p1 = make_deferred(proxyProvider, test.fP1);
-        sk_sp<GrSurfaceProxy> p2 = make_deferred(proxyProvider, test.fP2);
+        GrSurfaceProxy* p1 = make_deferred(proxyProvider, test.fP1);
+        GrSurfaceProxy* p2 = make_deferred(proxyProvider, test.fP2);
+
         if (!p1 || !p2) {
             continue; // creation can fail (i.e., for msaa4 on iOS)
         }
-        non_overlap_test(reporter, resourceProvider,
-                         std::move(p1), std::move(p2), test.fExpectation);
+
+        non_overlap_test(reporter, resourceProvider, p1, p2, test.fExpectation);
+
+        p1->completedRead();
+        p2->completedRead();
     }
 
     {
@@ -204,10 +226,14 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ResourceAllocatorTest, reporter, ctxInfo) {
         };
 
         GrBackendTexture backEndTex;
-        sk_sp<GrSurfaceProxy> p1 = make_backend(ctxInfo.grContext(), t[0].fP1, &backEndTex);
-        sk_sp<GrSurfaceProxy> p2 = make_deferred(proxyProvider, t[0].fP2);
-        non_overlap_test(reporter, resourceProvider,
-                         std::move(p1), std::move(p2), t[0].fExpectation);
+        GrSurfaceProxy* p1 = make_backend(ctxInfo.grContext(), t[0].fP1, &backEndTex);
+        GrSurfaceProxy* p2 = make_deferred(proxyProvider, t[0].fP2);
+
+        non_overlap_test(reporter, resourceProvider, p1, p2, t[0].fExpectation);
+
+        p1->completedRead();
+        p2->completedRead();
+
         cleanup_backend(ctxInfo.grContext(), backEndTex);
     }