Remove GrSurfaceProxy::MakeWrapped (take 2)

TBR=bsalomon@google.com
Change-Id: I26fd911da502fb00addacb8b2c1a263efc5aa4ec
Reviewed-on: https://skia-review.googlesource.com/95881
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>

8 files changed, 75 insertions, 84 deletions

diff --git a/include/private/GrSurfaceProxy.h b/include/private/GrSurfaceProxy.h
index 55b30a5d5b..5fef077088 100644
--- a/include/private/GrSurfaceProxy.h
+++ b/include/private/GrSurfaceProxy.h
@@ -182,9 +182,6 @@ private:
 
 class GrSurfaceProxy : public GrIORefProxy {
 public:
-    // DDL TODO: remove this entry point
-    static sk_sp<GrTextureProxy> MakeWrapped(sk_sp<GrTexture>, GrSurfaceOrigin);
-
     enum class LazyState {
         kNot,       // The proxy has no lazy callback that must be made.
         kPartially, // The proxy has a lazy callback but knows basic information about itself.
diff --git a/src/gpu/GrAHardwareBufferImageGenerator.cpp b/src/gpu/GrAHardwareBufferImageGenerator.cpp
index 66ba484198..f9208b144b 100644
--- a/src/gpu/GrAHardwareBufferImageGenerator.cpp
+++ b/src/gpu/GrAHardwareBufferImageGenerator.cpp
@@ -17,6 +17,7 @@
 #include "GrBackendSurface.h"
 #include "GrContext.h"
 #include "GrContextPriv.h"
+#include "GrProxyProvider.h"
 #include "GrResourceCache.h"
 #include "GrResourceProvider.h"
 #include "GrTexture.h"
@@ -151,9 +152,12 @@ sk_sp<GrTextureProxy> GrAHardwareBufferImageGenerator::makeProxy(GrContext* cont
         return nullptr;
     }
 
+    auto proxyProvider = context->contextPriv().proxyProvider();
+
     // return a cached GrTexture if invoked with the same context
     if (fOriginalTexture && fOwningContextID == context->uniqueID()) {
-        return GrSurfaceProxy::MakeWrapped(sk_ref_sp(fOriginalTexture), kTopLeft_GrSurfaceOrigin);
+        return proxyProvider->createWrapped(sk_ref_sp(fOriginalTexture),
+                                            kTopLeft_GrSurfaceOrigin);
     }
 
     while (GL_NO_ERROR != glGetError()) {} //clear GL errors
@@ -241,7 +245,7 @@ sk_sp<GrTextureProxy> GrAHardwareBufferImageGenerator::makeProxy(GrContext* cont
     //TODO: GrResourceCache should delete GrTexture, when GrContext is deleted. Currently
     //TODO: SkMessageBus ignores messages for deleted contexts and GrTexture will leak.
     context->contextPriv().getResourceCache()->insertCrossContextGpuResource(fOriginalTexture);
-    return GrSurfaceProxy::MakeWrapped(std::move(tex), kTopLeft_GrSurfaceOrigin);
+    return proxyProvider->createWrapped(std::move(tex), kTopLeft_GrSurfaceOrigin);
 }
 
 bool GrAHardwareBufferImageGenerator::onIsValid(GrContext* context) const {
diff --git a/src/gpu/GrProxyProvider.cpp b/src/gpu/GrProxyProvider.cpp
index bf5caeb26a..f60fc3eba6 100644
--- a/src/gpu/GrProxyProvider.cpp
+++ b/src/gpu/GrProxyProvider.cpp
@@ -40,11 +40,11 @@ GrProxyProvider::~GrProxyProvider() {
     SkASSERT(!fUniquelyKeyedProxies.count());
 }
 
-void GrProxyProvider::assignUniqueKeyToProxy(const GrUniqueKey& key, GrTextureProxy* proxy) {
+bool GrProxyProvider::assignUniqueKeyToProxy(const GrUniqueKey& key, GrTextureProxy* proxy) {
     ASSERT_SINGLE_OWNER
     SkASSERT(key.isValid());
     if (this->isAbandoned() || !proxy) {
-        return;
+        return false;
     }
 
     // If there is already a GrResource with this key then the caller has violated the normal
@@ -59,7 +59,7 @@ void GrProxyProvider::assignUniqueKeyToProxy(const GrUniqueKey& key, GrTexturePr
     if (SkBudgeted::kNo == proxy->isBudgeted() &&
                     (!proxy->priv().isInstantiated() ||
                      !proxy->priv().peekSurface()->resourcePriv().refsWrappedObjects())) {
-        return;
+        return false;
     }
 
     SkASSERT(!fUniquelyKeyedProxies.find(key));     // multiple proxies can't get the same key
@@ -67,6 +67,7 @@ void GrProxyProvider::assignUniqueKeyToProxy(const GrUniqueKey& key, GrTexturePr
     proxy->cacheAccess().setUniqueKey(this, key);
     SkASSERT(proxy->getUniqueKey() == key);
     fUniquelyKeyedProxies.add(proxy);
+    return true;
 }
 
 void GrProxyProvider::adoptUniqueKeyFromSurface(GrTextureProxy* proxy, const GrSurface* surf) {
@@ -101,6 +102,20 @@ sk_sp<GrTextureProxy> GrProxyProvider::findProxyByUniqueKey(const GrUniqueKey& k
     return result;
 }
 
+sk_sp<GrTextureProxy> GrProxyProvider::createWrapped(sk_sp<GrTexture> tex, GrSurfaceOrigin origin) {
+#ifdef SK_DEBUG
+    if (tex->getUniqueKey().isValid()) {
+        SkASSERT(!this->findProxyByUniqueKey(tex->getUniqueKey(), origin));
+    }
+#endif
+
+    if (tex->asRenderTarget()) {
+        return sk_sp<GrTextureProxy>(new GrTextureRenderTargetProxy(std::move(tex), origin));
+    } else {
+        return sk_sp<GrTextureProxy>(new GrTextureProxy(std::move(tex), origin));
+    }
+}
+
 sk_sp<GrTextureProxy> GrProxyProvider::findOrCreateProxyByUniqueKey(const GrUniqueKey& key,
                                                                     GrSurfaceOrigin origin) {
     ASSERT_SINGLE_OWNER
@@ -122,9 +137,9 @@ sk_sp<GrTextureProxy> GrProxyProvider::findOrCreateProxyByUniqueKey(const GrUniq
     sk_sp<GrTexture> texture(static_cast<GrSurface*>(resource)->asTexture());
     SkASSERT(texture);
 
-    result = GrSurfaceProxy::MakeWrapped(std::move(texture), origin);
+    result = this->createWrapped(std::move(texture), origin);
     SkASSERT(result->getUniqueKey() == key);
-    // MakeWrapped should've added this for us
+    // createWrapped should've added this for us
     SkASSERT(fUniquelyKeyedProxies.find(key));
     return result;
 }
@@ -144,13 +159,7 @@ sk_sp<GrTextureProxy> GrProxyProvider::createInstantiatedProxy(const GrSurfaceDe
         return nullptr;
     }
 
-    SkASSERT(!tex->getUniqueKey().isValid());
-
-    if (tex->asRenderTarget()) {
-        return sk_sp<GrTextureProxy>(new GrTextureRenderTargetProxy(std::move(tex), desc.fOrigin));
-    }
-
-    return sk_sp<GrTextureProxy>(new GrTextureProxy(std::move(tex), desc.fOrigin));
+    return this->createWrapped(std::move(tex), desc.fOrigin);
 }
 
 sk_sp<GrTextureProxy> GrProxyProvider::createTextureProxy(const GrSurfaceDesc& desc,
@@ -170,7 +179,7 @@ sk_sp<GrTextureProxy> GrProxyProvider::createTextureProxy(const GrSurfaceDesc& d
             return nullptr;
         }
 
-        return GrSurfaceProxy::MakeWrapped(std::move(tex), desc.fOrigin);
+        return this->createWrapped(std::move(tex), desc.fOrigin);
     }
 
     return this->createProxy(desc, SkBackingFit::kExact, budgeted);
@@ -225,7 +234,7 @@ sk_sp<GrTextureProxy> GrProxyProvider::createMipMapProxy(
         return nullptr;
     }
 
-    return GrSurfaceProxy::MakeWrapped(std::move(tex), desc.fOrigin);
+    return this->createWrapped(std::move(tex), desc.fOrigin);
 }
 
 sk_sp<GrTextureProxy> GrProxyProvider::createMipMapProxy(const GrSurfaceDesc& desc,
@@ -310,19 +319,27 @@ sk_sp<GrTextureProxy> GrProxyProvider::createProxy(const GrSurfaceDesc& desc,
 #endif
 }
 
-sk_sp<GrTextureProxy> GrProxyProvider::createWrappedTextureProxy(const GrBackendTexture& backendTex,
-                                                                 GrSurfaceOrigin origin) {
+sk_sp<GrTextureProxy> GrProxyProvider::createWrappedTextureProxy(
+                                                         const GrBackendTexture& backendTex,
+                                                         GrSurfaceOrigin origin,
+                                                         GrWrapOwnership ownership,
+                                                         ReleaseProc releaseProc,
+                                                         ReleaseContext releaseCtx) {
     if (this->isAbandoned()) {
         return nullptr;
     }
 
-    sk_sp<GrTexture> texture(fResourceProvider->wrapBackendTexture(backendTex));
+    sk_sp<GrTexture> texture(fResourceProvider->wrapBackendTexture(backendTex, ownership));
     if (!texture) {
         return nullptr;
     }
+    if (releaseProc) {
+        texture->setRelease(releaseProc, releaseCtx);
+    }
+
     SkASSERT(!texture->asRenderTarget());   // Strictly a GrTexture
 
-    return GrSurfaceProxy::MakeWrapped(std::move(texture), origin);
+    return this->createWrapped(std::move(texture), origin);
 }
 
 sk_sp<GrTextureProxy> GrProxyProvider::createWrappedTextureProxy(const GrBackendTexture& tex,
@@ -338,7 +355,7 @@ sk_sp<GrTextureProxy> GrProxyProvider::createWrappedTextureProxy(const GrBackend
     }
     SkASSERT(texture->asRenderTarget());  // A GrTextureRenderTarget
 
-    return GrSurfaceProxy::MakeWrapped(std::move(texture), origin);
+    return this->createWrapped(std::move(texture), origin);
 }
 
 sk_sp<GrSurfaceProxy> GrProxyProvider::createWrappedRenderTargetProxy(
diff --git a/src/gpu/GrProxyProvider.h b/src/gpu/GrProxyProvider.h
index 4c93886da6..f0b1bbd1ed 100644
--- a/src/gpu/GrProxyProvider.h
+++ b/src/gpu/GrProxyProvider.h
@@ -32,7 +32,7 @@ public:
      * Assigns a unique key to a proxy. The proxy will be findable via this key using
      * findProxyByUniqueKey(). It is an error if an existing proxy already has a key.
      */
-    void assignUniqueKeyToProxy(const GrUniqueKey&, GrTextureProxy*);
+    bool assignUniqueKeyToProxy(const GrUniqueKey&, GrTextureProxy*);
 
     /*
      * Sets the unique key of the provided proxy to the unique key of the surface. The surface must
@@ -102,10 +102,17 @@ public:
     sk_sp<GrTextureProxy> createProxy(const GrSurfaceDesc&, SkBackingFit, SkBudgeted,
                                       uint32_t flags = 0);
 
+    // These match the definitions in SkImage & GrTexture.h, for whence they came
+    typedef void* ReleaseContext;
+    typedef void (*ReleaseProc)(ReleaseContext);
+
     /*
      * Create a texture proxy that wraps a (non-renderable) backend texture.
      */
-    sk_sp<GrTextureProxy> createWrappedTextureProxy(const GrBackendTexture&, GrSurfaceOrigin);
+    sk_sp<GrTextureProxy> createWrappedTextureProxy(const GrBackendTexture&, GrSurfaceOrigin,
+                                                    GrWrapOwnership = kBorrow_GrWrapOwnership,
+                                                    ReleaseProc = nullptr,
+                                                    ReleaseContext = nullptr);
 
     /*
      * Create a texture proxy that wraps a backend texture and is both texture-able and renderable
@@ -188,6 +195,10 @@ public:
     void removeAllUniqueKeys();
 
 private:
+    friend class GrAHardwareBufferImageGenerator; // for createWrapped
+
+    sk_sp<GrTextureProxy> createWrapped(sk_sp<GrTexture> tex, GrSurfaceOrigin origin);
+
     struct UniquelyKeyedProxyHashTraits {
         static const GrUniqueKey& GetKey(const GrTextureProxy& p) { return p.getUniqueKey(); }
 
diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp
index 828b936c8b..9dadaca506 100644
--- a/src/gpu/GrSurfaceProxy.cpp
+++ b/src/gpu/GrSurfaceProxy.cpp
@@ -228,28 +228,6 @@ GrTextureOpList* GrSurfaceProxy::getLastTextureOpList() {
     return fLastOpList ? fLastOpList->asTextureOpList() : nullptr;
 }
 
-sk_sp<GrTextureProxy> GrSurfaceProxy::MakeWrapped(sk_sp<GrTexture> tex, GrSurfaceOrigin origin) {
-    if (!tex) {
-        return nullptr;
-    }
-
-    if (tex->getUniqueKey().isValid()) {
-        // The proxy may already be in the hash. Thus we need to look for it first before creating
-        // new one.
-        GrProxyProvider* provider = tex->getContext()->contextPriv().proxyProvider();
-        sk_sp<GrTextureProxy> proxy = provider->findProxyByUniqueKey(tex->getUniqueKey(), origin);
-        if (proxy) {
-            return proxy;
-        }
-    }
-
-    if (tex->asRenderTarget()) {
-        return sk_sp<GrTextureProxy>(new GrTextureRenderTargetProxy(std::move(tex), origin));
-    } else {
-        return sk_sp<GrTextureProxy>(new GrTextureProxy(std::move(tex), origin));
-    }
-}
-
 int GrSurfaceProxy::worstCaseWidth() const {
     SkASSERT(LazyState::kFully != this->lazyInstantiationState());
     if (fTarget) {
diff --git a/src/gpu/GrTextureProxy.cpp b/src/gpu/GrTextureProxy.cpp
index 9eca5d9876..03b810823e 100644
--- a/src/gpu/GrTextureProxy.cpp
+++ b/src/gpu/GrTextureProxy.cpp
@@ -131,8 +131,10 @@ void GrTextureProxy::setUniqueKey(GrProxyProvider* proxyProvider, const GrUnique
     SkASSERT(key.isValid());
     SkASSERT(!fUniqueKey.isValid()); // proxies can only ever get one uniqueKey
 
-    if (fTarget && !fTarget->getUniqueKey().isValid()) {
-        fTarget->resourcePriv().setUniqueKey(key);
+    if (fTarget) {
+        if (!fTarget->getUniqueKey().isValid()) {
+            fTarget->resourcePriv().setUniqueKey(key);
+        }
         SkASSERT(fTarget->getUniqueKey() == key);
     }
 
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index d08f0a4eeb..d078de1f20 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -284,17 +284,12 @@ static sk_sp<SkImage> new_wrapped_texture_common(GrContext* ctx,
         return nullptr;
     }
 
-    GrResourceProvider* resourceProvider = ctx->contextPriv().resourceProvider();
-
-    sk_sp<GrTexture> tex = resourceProvider->wrapBackendTexture(backendTex, ownership);
-    if (!tex) {
+    GrProxyProvider* proxyProvider = ctx->contextPriv().proxyProvider();
+    sk_sp<GrTextureProxy> proxy = proxyProvider->createWrappedTextureProxy(
+                                        backendTex, origin, ownership, releaseProc, releaseCtx);
+    if (!proxy) {
         return nullptr;
     }
-    if (releaseProc) {
-        tex->setRelease(releaseProc, releaseCtx);
-    }
-
-    sk_sp<GrTextureProxy> proxy(GrSurfaceProxy::MakeWrapped(std::move(tex), origin));
 
     return sk_make_sp<SkImage_Gpu>(ctx, kNeedNewImageUniqueID,
                                    at, std::move(proxy), std::move(colorSpace), SkBudgeted::kNo);
diff --git a/tests/TextureProxyTest.cpp b/tests/TextureProxyTest.cpp
index 914e83e7df..d422b076ad 100644
--- a/tests/TextureProxyTest.cpp
+++ b/tests/TextureProxyTest.cpp
@@ -43,8 +43,7 @@ static GrSurfaceDesc make_desc(GrSurfaceFlags flags) {
 // Basic test
 
 static sk_sp<GrTextureProxy> deferred_tex(skiatest::Reporter* reporter,
-                                          GrContext* context, SkBackingFit fit) {
-    GrProxyProvider* proxyProvider = context->contextPriv().proxyProvider();
+                                          GrProxyProvider* proxyProvider, SkBackingFit fit) {
     const GrSurfaceDesc desc = make_desc(kNone_GrSurfaceFlags);
 
     sk_sp<GrTextureProxy> proxy = proxyProvider->createProxy(desc, fit, SkBudgeted::kYes);
@@ -54,8 +53,7 @@ static sk_sp<GrTextureProxy> deferred_tex(skiatest::Reporter* reporter,
 }
 
 static sk_sp<GrTextureProxy> deferred_texRT(skiatest::Reporter* reporter,
-                                            GrContext* context, SkBackingFit fit) {
-    GrProxyProvider* proxyProvider = context->contextPriv().proxyProvider();
+                                            GrProxyProvider* proxyProvider, SkBackingFit fit) {
     const GrSurfaceDesc desc = make_desc(kRenderTarget_GrSurfaceFlag);
 
     sk_sp<GrTextureProxy> proxy = proxyProvider->createProxy(desc, fit, SkBudgeted::kYes);
@@ -65,8 +63,7 @@ static sk_sp<GrTextureProxy> deferred_texRT(skiatest::Reporter* reporter,
 }
 
 static sk_sp<GrTextureProxy> wrapped(skiatest::Reporter* reporter,
-                                     GrContext* context, SkBackingFit fit) {
-    GrProxyProvider* proxyProvider = context->contextPriv().proxyProvider();
+                                     GrProxyProvider* proxyProvider, SkBackingFit fit) {
     const GrSurfaceDesc desc = make_desc(kNone_GrSurfaceFlags);
 
     sk_sp<GrTextureProxy> proxy = proxyProvider->createInstantiatedProxy(desc, fit,
@@ -77,9 +74,7 @@ static sk_sp<GrTextureProxy> wrapped(skiatest::Reporter* reporter,
 }
 
 static sk_sp<GrTextureProxy> wrapped_with_key(skiatest::Reporter* reporter,
-                                              GrContext* context, SkBackingFit fit) {
-    GrResourceProvider* resourceProvider = context->contextPriv().resourceProvider();
-
+                                              GrProxyProvider* proxyProvider, SkBackingFit fit) {
     static GrUniqueKey::Domain d = GrUniqueKey::GenerateDomain();
     static int kUniqueKeyData = 0;
 
@@ -91,18 +86,10 @@ static sk_sp<GrTextureProxy> wrapped_with_key(skiatest::Reporter* reporter,
 
     const GrSurfaceDesc desc = make_desc(kNone_GrSurfaceFlags);
 
-    sk_sp<GrTexture> tex;
-    if (SkBackingFit::kApprox == fit) {
-        tex = sk_sp<GrTexture>(resourceProvider->createApproxTexture(desc, 0));
-    } else {
-        // Only budgeted & wrapped external proxies get to carry uniqueKeys
-        tex = resourceProvider->createTexture(desc, SkBudgeted::kYes);
-    }
-
-    tex->resourcePriv().setUniqueKey(key);
-
-    sk_sp<GrTextureProxy> proxy = GrSurfaceProxy::MakeWrapped(std::move(tex),
-                                                              kBottomLeft_GrSurfaceOrigin);
+    // Only budgeted & wrapped external proxies get to carry uniqueKeys
+    sk_sp<GrTextureProxy> proxy = proxyProvider->createInstantiatedProxy(desc, fit,
+                                                                         SkBudgeted::kYes, 0);
+    SkAssertResult(proxyProvider->assignUniqueKeyToProxy(key, proxy.get()));
     REPORTER_ASSERT(reporter, proxy->getUniqueKey().isValid());
     return proxy;
 }
@@ -147,7 +134,7 @@ static void basic_test(GrContext* context,
 
         // Assigning the uniqueKey adds the proxy to the hash but doesn't force instantiation
         REPORTER_ASSERT(reporter, !proxyProvider->numUniqueKeyProxies_TestOnly());
-        proxyProvider->assignUniqueKeyToProxy(key, proxy.get());
+        SkAssertResult(proxyProvider->assignUniqueKeyToProxy(key, proxy.get()));
     }
 
     REPORTER_ASSERT(reporter, 1 == proxyProvider->numUniqueKeyProxies_TestOnly());
@@ -265,8 +252,8 @@ static void invalidation_and_instantiation_test(GrContext* context, skiatest::Re
     builder.finish();
 
     // Create proxy, assign unique key
-    sk_sp<GrTextureProxy> proxy = deferred_tex(reporter, context, SkBackingFit::kExact);
-    proxyProvider->assignUniqueKeyToProxy(key, proxy.get());
+    sk_sp<GrTextureProxy> proxy = deferred_tex(reporter, proxyProvider, SkBackingFit::kExact);
+    SkAssertResult(proxyProvider->assignUniqueKeyToProxy(key, proxy.get()));
 
     // Send an invalidation message, which will be sitting in the cache's inbox
     SkMessageBus<GrUniqueKeyInvalidatedMessage>::Post(GrUniqueKeyInvalidatedMessage(key));
@@ -302,7 +289,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(TextureProxyTest, reporter, ctxInfo) {
     for (auto fit : { SkBackingFit::kExact, SkBackingFit::kApprox }) {
         for (auto create : { deferred_tex, deferred_texRT, wrapped, wrapped_with_key }) {
             REPORTER_ASSERT(reporter, 0 == cache->getResourceCount());
-            basic_test(context, reporter, create(reporter, context, fit), true);
+            basic_test(context, reporter, create(reporter, proxyProvider, fit), true);
         }
 
         REPORTER_ASSERT(reporter, 0 == cache->getResourceCount());