Have lazy proxies keep their callbacks around and clean up their lambdas in the dtor

I believe after this CL we will be at a place where we just have to null out the
fTarget of a lazy proxy and it will reinstantiate itself.

Bug: skia:
Change-Id: I88fdc70e149eba4514a0823da99383583394005c
Reviewed-on: https://skia-review.googlesource.com/102021
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>

7 files changed, 33 insertions, 21 deletions

diff --git a/include/private/GrSurfaceProxy.h b/include/private/GrSurfaceProxy.h
index 5fef077088..d05d3ad1cb 100644
--- a/include/private/GrSurfaceProxy.h
+++ b/include/private/GrSurfaceProxy.h
@@ -183,13 +183,13 @@ private:
 class GrSurfaceProxy : public GrIORefProxy {
 public:
     enum class LazyState {
-        kNot,       // The proxy has no lazy callback that must be made.
+        kNot,       // The proxy is instantiated or does not have a lazy callback
         kPartially, // The proxy has a lazy callback but knows basic information about itself.
         kFully,     // The proxy has a lazy callback and also doesn't know its width, height, etc.
     };
 
     LazyState lazyInstantiationState() const {
-        if (!SkToBool(fLazyInstantiateCallback)) {
+        if (fTarget || !SkToBool(fLazyInstantiateCallback)) {
             return LazyState::kNot;
         } else {
             if (fWidth <= 0) {
diff --git a/src/gpu/GrProxyProvider.cpp b/src/gpu/GrProxyProvider.cpp
index 604dd1bb32..ec05243d18 100644
--- a/src/gpu/GrProxyProvider.cpp
+++ b/src/gpu/GrProxyProvider.cpp
@@ -217,6 +217,8 @@ sk_sp<GrTextureProxy> GrProxyProvider::createTextureProxy(sk_sp<SkImage> srcImag
             [desc, budgeted, srcImage]
             (GrResourceProvider* resourceProvider, GrSurfaceOrigin* /*outOrigin*/) {
                 if (!resourceProvider) {
+                    // Nothing to clean up here. Once the proxy (and thus lambda) is deleted the ref
+                    // on srcImage will be released.
                     return sk_sp<GrTexture>();
                 }
                 SkPixmap pixMap;
@@ -396,9 +398,9 @@ sk_sp<GrTextureProxy> GrProxyProvider::createWrappedTextureProxy(
             [backendTex, ownership, releaseHelper]
             (GrResourceProvider* resourceProvider, GrSurfaceOrigin* /*outOrigin*/) {
                 if (!resourceProvider) {
-                    // This lazy proxy was never initialized. If this had a releaseHelper it will
-                    // get unrefed when we delete this lambda and will call the release proc so that
-                    // the client knows they can free the underlying backend object.
+                    // If this had a releaseHelper it will get unrefed when we delete this lambda
+                    // and will call the release proc so that the client knows they can free the
+                    // underlying backend object.
                     return sk_sp<GrTexture>();
                 }
 
@@ -408,9 +410,8 @@ sk_sp<GrTextureProxy> GrProxyProvider::createWrappedTextureProxy(
                     return sk_sp<GrTexture>();
                 }
                 if (releaseHelper) {
-                    // DDL TODO: once we are reusing lazy proxies, remove this move and hold onto to
-                    // the ref till the lambda goes away.
-                    tex->setRelease(std::move(releaseHelper));
+                    // This gives the texture a ref on the releaseHelper
+                    tex->setRelease(releaseHelper);
                 }
                 SkASSERT(!tex->asRenderTarget());   // Strictly a GrTexture
                 // Make sure we match how we created the proxy with SkBudgeted::kNo
@@ -491,7 +492,6 @@ sk_sp<GrTextureProxy> GrProxyProvider::createLazyProxy(LazyInstantiateCallback&&
                                                                 mipMapped, fit, budgeted, flags) :
                                  new GrTextureProxy(std::move(callback), desc, mipMapped, fit,
                                                     budgeted, flags));
-
 }
 
 sk_sp<GrTextureProxy> GrProxyProvider::createFullyLazyProxy(LazyInstantiateCallback&& callback,
diff --git a/src/gpu/GrRenderTargetProxy.cpp b/src/gpu/GrRenderTargetProxy.cpp
index d1b77d2354..9eb37a0b2f 100644
--- a/src/gpu/GrRenderTargetProxy.cpp
+++ b/src/gpu/GrRenderTargetProxy.cpp
@@ -61,6 +61,9 @@ int GrRenderTargetProxy::maxWindowRectangles(const GrCaps& caps) const {
 }
 
 bool GrRenderTargetProxy::instantiate(GrResourceProvider* resourceProvider) {
+    if (LazyState::kNot != this->lazyInstantiationState()) {
+        return false;
+    }
     static constexpr GrSurfaceFlags kFlags = kRenderTarget_GrSurfaceFlag;
 
     if (!this->instantiateImpl(resourceProvider, fSampleCnt, fNeedsStencil, kFlags,
diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp
index 2ad28f8ac4..64c47b56b1 100644
--- a/src/gpu/GrSurfaceProxy.cpp
+++ b/src/gpu/GrSurfaceProxy.cpp
@@ -84,8 +84,8 @@ GrSurfaceProxy::GrSurfaceProxy(sk_sp<GrSurface> surface, GrSurfaceOrigin origin,
 
 GrSurfaceProxy::~GrSurfaceProxy() {
     if (fLazyInstantiateCallback) {
-        // We have an uninstantiated lazy proxy. Call fLazyInstantiateCallback with a nullptr for
-        // the GrResourceProvider to signal the callback should clean itself up.
+        // We call the callback with a null GrResourceProvider to signal that the lambda should
+        // clean itself up if it is holding onto any captured objects.
         this->fLazyInstantiateCallback(nullptr, nullptr);
     }
     // For this to be deleted the opList that held a ref on it (if there was one) must have been
@@ -148,7 +148,6 @@ sk_sp<GrSurface> GrSurfaceProxy::createSurfaceImpl(
 }
 
 void GrSurfaceProxy::assign(sk_sp<GrSurface> surface) {
-    SkASSERT(LazyState::kNot == this->lazyInstantiationState());
     SkASSERT(!fTarget && surface);
     fTarget = surface.release();
     this->INHERITED::transferRefs();
@@ -348,8 +347,7 @@ void GrSurfaceProxyPriv::exactify() {
 }
 
 bool GrSurfaceProxyPriv::doLazyInstantiation(GrResourceProvider* resourceProvider) {
-    SkASSERT(fProxy->fLazyInstantiateCallback);
-    SkASSERT(!fProxy->fTarget);
+    SkASSERT(GrSurfaceProxy::LazyState::kNot != fProxy->lazyInstantiationState());
 
     GrSurfaceOrigin* outOrigin;
     if (GrSurfaceProxy::LazyState::kPartially == fProxy->lazyInstantiationState()) {
@@ -364,10 +362,6 @@ bool GrSurfaceProxyPriv::doLazyInstantiation(GrResourceProvider* resourceProvide
 
     sk_sp<GrTexture> texture = fProxy->fLazyInstantiateCallback(resourceProvider, outOrigin);
 
-
-    // Indicate we are no longer pending lazy instantiation.
-    fProxy->fLazyInstantiateCallback = nullptr;
-
     if (!texture) {
         fProxy->fWidth = 0;
         fProxy->fHeight = 0;
diff --git a/src/gpu/GrTextureProxy.cpp b/src/gpu/GrTextureProxy.cpp
index 03b810823e..8e6afbf0a7 100644
--- a/src/gpu/GrTextureProxy.cpp
+++ b/src/gpu/GrTextureProxy.cpp
@@ -61,6 +61,9 @@ GrTextureProxy::~GrTextureProxy() {
 }
 
 bool GrTextureProxy::instantiate(GrResourceProvider* resourceProvider) {
+    if (LazyState::kNot != this->lazyInstantiationState()) {
+        return false;
+    }
     if (!this->instantiateImpl(resourceProvider, 0, /* needsStencil = */ false,
                                kNone_GrSurfaceFlags, fMipMapped, fMipColorMode,
                                fUniqueKey.isValid() ? &fUniqueKey : nullptr)) {
diff --git a/src/gpu/GrTextureRenderTargetProxy.cpp b/src/gpu/GrTextureRenderTargetProxy.cpp
index fe12a0a975..da8e48307d 100644
--- a/src/gpu/GrTextureRenderTargetProxy.cpp
+++ b/src/gpu/GrTextureRenderTargetProxy.cpp
@@ -61,6 +61,9 @@ size_t GrTextureRenderTargetProxy::onUninstantiatedGpuMemorySize() const {
 }
 
 bool GrTextureRenderTargetProxy::instantiate(GrResourceProvider* resourceProvider) {
+    if (LazyState::kNot != this->lazyInstantiationState()) {
+        return false;
+    }
     static constexpr GrSurfaceFlags kFlags = kRenderTarget_GrSurfaceFlag;
 
     const GrUniqueKey& key = this->getUniqueKey();
diff --git a/tests/LazyProxyTest.cpp b/tests/LazyProxyTest.cpp
index 029a5a5ed1..f1bffa527b 100644
--- a/tests/LazyProxyTest.cpp
+++ b/tests/LazyProxyTest.cpp
@@ -59,6 +59,9 @@ public:
                     : GrDrawOp(ClassID()), fTest(test) {
             fProxy = proxyProvider->createFullyLazyProxy([this, nullTexture](
                                         GrResourceProvider* rp, GrSurfaceOrigin* origin) {
+                if (!rp) {
+                    return sk_sp<GrTexture>();
+                }
                 REPORTER_ASSERT(fTest->fReporter, !fTest->fHasOpTexture);
                 fTest->fHasOpTexture = true;
                 *origin = kTopLeft_GrSurfaceOrigin;
@@ -111,6 +114,9 @@ public:
                 , fAtlas(atlas) {
             fLazyProxy = proxyProvider->createFullyLazyProxy([this](GrResourceProvider* rp,
                                                                     GrSurfaceOrigin* origin) {
+                if (!rp) {
+                    return sk_sp<GrTexture>();
+                }
                 REPORTER_ASSERT(fTest->fReporter, !fTest->fHasClipTexture);
                 fTest->fHasClipTexture = true;
                 *origin = kBottomLeft_GrSurfaceOrigin;
@@ -225,7 +231,7 @@ DEF_GPUTEST(LazyProxyReleaseTest, reporter, /* options */) {
             proxy->priv().doLazyInstantiation(ctx->contextPriv().resourceProvider());
             REPORTER_ASSERT(reporter, 1 == testCount);
             proxy.reset();
-            REPORTER_ASSERT(reporter, 1 == testCount);
+            REPORTER_ASSERT(reporter, -1 == testCount);
         } else {
             proxy.reset();
             REPORTER_ASSERT(reporter, -1 == testCount);
@@ -249,8 +255,11 @@ public:
         fLazyProxy = proxyProvider->createLazyProxy(
                 [testExecuteValue, shouldFailInstantiation, desc] (
                         GrResourceProvider* rp, GrSurfaceOrigin* /*origin*/) {
-                    *testExecuteValue = 1;
-                    if (shouldFailInstantiation || !rp) {
+                    if (!rp) {
+                        return sk_sp<GrTexture>();
+                    }
+                    if (shouldFailInstantiation) {
+                        *testExecuteValue = 1;
                         return sk_sp<GrTexture>();
                     }
                     return rp->createTexture(desc, SkBudgeted::kNo);