Use Ref<TextureBase> instead of TextureBase* in more places

To avoid accidental memory leaks on account of using raw pointers,
use Ref<TextureBase> as method return type except at Dawn interface
boundaries.

Change-Id: I6459062ee28984de2cb1d5a2059bc70cf82b2faf
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19580
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Rafael Cintron <rafael.cintron@microsoft.com>

src/common/Result.h

@@ -177,11 +177,14 @@ class DAWN_NO_DISCARD Result<Ref<T>, E> {
     static_assert(alignof_if_defined_else_default<E, 4> >= 4,
                   "Result<Ref<T>, E> reserves two bits for tagging pointers");
 
-    Result(Ref<T>&& success);
+    template <typename U>
+    Result(Ref<U>&& success);
     Result(std::unique_ptr<E> error);
 
-    Result(Result<Ref<T>, E>&& other);
+    template <typename U>
-    Result<Ref<T>, E>& operator=(Result<Ref<T>, E>&& other);
+    Result(Result<Ref<U>, E>&& other);
+    template <typename U>
+    Result<Ref<U>, E>& operator=(Result<Ref<U>, E>&& other);
 
     ~Result();
 
@@ -192,6 +195,9 @@ class DAWN_NO_DISCARD Result<Ref<T>, E> {
     std::unique_ptr<E> AcquireError();
 
   private:
+    template <typename T2, typename E2>
+    friend class Result;
+
     intptr_t mPayload = detail::kEmptyPayload;
 };
 
@@ -399,8 +405,10 @@ std::unique_ptr<E> Result<const T*, E>::AcquireError() {
 
 // Implementation of Result<Ref<T>, E>
 template <typename T, typename E>
-Result<Ref<T>, E>::Result(Ref<T>&& success)
+template <typename U>
+Result<Ref<T>, E>::Result(Ref<U>&& success)
     : mPayload(detail::MakePayload(success.Detach(), detail::Success)) {
+    static_assert(std::is_convertible<U*, T*>::value, "");
 }
 
 template <typename T, typename E>
@@ -409,12 +417,16 @@ Result<Ref<T>, E>::Result(std::unique_ptr<E> error)
 }
 
 template <typename T, typename E>
-Result<Ref<T>, E>::Result(Result<Ref<T>, E>&& other) : mPayload(other.mPayload) {
+template <typename U>
+Result<Ref<T>, E>::Result(Result<Ref<U>, E>&& other) : mPayload(other.mPayload) {
+    static_assert(std::is_convertible<U*, T*>::value, "");
     other.mPayload = detail::kEmptyPayload;
 }
 
 template <typename T, typename E>
-Result<Ref<T>, E>& Result<Ref<T>, E>::operator=(Result<Ref<T>, E>&& other) {
+template <typename U>
+Result<Ref<U>, E>& Result<Ref<T>, E>::operator=(Result<Ref<U>, E>&& other) {
+    static_assert(std::is_convertible<U*, T*>::value, "");
     ASSERT(mPayload == detail::kEmptyPayload);
     mPayload = other.mPayload;
     other.mPayload = detail::kEmptyPayload;

src/dawn_native/Device.cpp

@@ -628,13 +628,13 @@ namespace dawn_native {
         return result;
     }
     TextureBase* DeviceBase::CreateTexture(const TextureDescriptor* descriptor) {
-        TextureBase* result = nullptr;
+        Ref<TextureBase> result;
 
-        if (ConsumedError(CreateTextureInternal(&result, descriptor))) {
+        if (ConsumedError(CreateTextureInternal(descriptor), &result)) {
             return TextureBase::MakeError(this);
         }
 
-        return result;
+        return result.Detach();
     }
     TextureViewBase* DeviceBase::CreateTextureView(TextureBase* texture,
                                                    const TextureViewDescriptor* descriptor) {
@@ -918,14 +918,13 @@ namespace dawn_native {
         return {};
     }
 
-    MaybeError DeviceBase::CreateTextureInternal(TextureBase** result,
+    ResultOrError<Ref<TextureBase>> DeviceBase::CreateTextureInternal(
         const TextureDescriptor* descriptor) {
         DAWN_TRY(ValidateIsAlive());
         if (IsValidationEnabled()) {
             DAWN_TRY(ValidateTextureDescriptor(this, descriptor));
         }
-        DAWN_TRY_ASSIGN(*result, CreateTextureImpl(descriptor));
+        return CreateTextureImpl(descriptor);
-        return {};
     }
 
     MaybeError DeviceBase::CreateTextureViewInternal(TextureViewBase** result,

src/dawn_native/Device.h

@@ -243,7 +243,7 @@ namespace dawn_native {
             Surface* surface,
             NewSwapChainBase* previousSwapChain,
             const SwapChainDescriptor* descriptor) = 0;
-        virtual ResultOrError<TextureBase*> CreateTextureImpl(
+        virtual ResultOrError<Ref<TextureBase>> CreateTextureImpl(
             const TextureDescriptor* descriptor) = 0;
         virtual ResultOrError<TextureViewBase*> CreateTextureViewImpl(
             TextureBase* texture,
@@ -269,7 +269,7 @@ namespace dawn_native {
         MaybeError CreateSwapChainInternal(SwapChainBase** result,
                                            Surface* surface,
                                            const SwapChainDescriptor* descriptor);
-        MaybeError CreateTextureInternal(TextureBase** result, const TextureDescriptor* descriptor);
+        ResultOrError<Ref<TextureBase>> CreateTextureInternal(const TextureDescriptor* descriptor);
         MaybeError CreateTextureViewInternal(TextureViewBase** result,
                                              TextureBase* texture,
                                              const TextureViewDescriptor* descriptor);

src/dawn_native/d3d12/D3D12Backend.cpp

@@ -63,10 +63,10 @@ namespace dawn_native { namespace d3d12 {
     WGPUTexture WrapSharedHandle(WGPUDevice device,
                                  const ExternalImageDescriptorDXGISharedHandle* descriptor) {
         Device* backendDevice = reinterpret_cast<Device*>(device);
-        TextureBase* texture = backendDevice->WrapSharedHandle(descriptor, descriptor->sharedHandle,
+        Ref<TextureBase> texture = backendDevice->WrapSharedHandle(
-                                                               descriptor->acquireMutexKey,
+            descriptor, descriptor->sharedHandle, descriptor->acquireMutexKey,
             descriptor->isSwapChainTexture);
-        return reinterpret_cast<WGPUTexture>(texture);
+        return reinterpret_cast<WGPUTexture>(texture.Detach());
     }
 
 }}  // namespace dawn_native::d3d12

src/dawn_native/d3d12/DeviceD3D12.cpp

@@ -292,7 +292,7 @@ namespace dawn_native { namespace d3d12 {
         const SwapChainDescriptor* descriptor) {
         return DAWN_VALIDATION_ERROR("New swapchains not implemented.");
     }
-    ResultOrError<TextureBase*> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
+    ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
         return Texture::Create(this, descriptor);
     }
     ResultOrError<TextureViewBase*> Device::CreateTextureViewImpl(
@@ -339,11 +339,11 @@ namespace dawn_native { namespace d3d12 {
                                                          initialUsage);
     }
 
-    TextureBase* Device::WrapSharedHandle(const ExternalImageDescriptor* descriptor,
+    Ref<TextureBase> Device::WrapSharedHandle(const ExternalImageDescriptor* descriptor,
                                               HANDLE sharedHandle,
                                               uint64_t acquireMutexKey,
                                               bool isSwapChainTexture) {
-        TextureBase* dawnTexture;
+        Ref<TextureBase> dawnTexture;
         if (ConsumedError(Texture::Create(this, descriptor, sharedHandle, acquireMutexKey,
                                           isSwapChainTexture),
                           &dawnTexture))

src/dawn_native/d3d12/DeviceD3D12.h

@@ -113,7 +113,7 @@ namespace dawn_native { namespace d3d12 {
 
         StagingDescriptorAllocator* GetDepthStencilViewAllocator() const;
 
-        TextureBase* WrapSharedHandle(const ExternalImageDescriptor* descriptor,
+        Ref<TextureBase> WrapSharedHandle(const ExternalImageDescriptor* descriptor,
                                           HANDLE sharedHandle,
                                           uint64_t acquireMutexKey,
                                           bool isSwapChainTexture);
@@ -146,7 +146,8 @@ namespace dawn_native { namespace d3d12 {
             Surface* surface,
             NewSwapChainBase* previousSwapChain,
             const SwapChainDescriptor* descriptor) override;
-        ResultOrError<TextureBase*> CreateTextureImpl(const TextureDescriptor* descriptor) override;
+        ResultOrError<Ref<TextureBase>> CreateTextureImpl(
+            const TextureDescriptor* descriptor) override;
         ResultOrError<TextureViewBase*> CreateTextureViewImpl(
             TextureBase* texture,
             const TextureViewDescriptor* descriptor) override;

src/dawn_native/d3d12/TextureD3D12.cpp

@@ -273,15 +273,15 @@ namespace dawn_native { namespace d3d12 {
         return {};
     }
 
-    ResultOrError<TextureBase*> Texture::Create(Device* device,
+    ResultOrError<Ref<TextureBase>> Texture::Create(Device* device,
                                                     const TextureDescriptor* descriptor) {
         Ref<Texture> dawnTexture =
             AcquireRef(new Texture(device, descriptor, TextureState::OwnedInternal));
         DAWN_TRY(dawnTexture->InitializeAsInternalTexture());
-        return dawnTexture.Detach();
+        return std::move(dawnTexture);
     }
 
-    ResultOrError<TextureBase*> Texture::Create(Device* device,
+    ResultOrError<Ref<TextureBase>> Texture::Create(Device* device,
                                                     const ExternalImageDescriptor* descriptor,
                                                     HANDLE sharedHandle,
                                                     uint64_t acquireMutexKey,
@@ -297,7 +297,7 @@ namespace dawn_native { namespace d3d12 {
         dawnTexture->SetIsSubresourceContentInitialized(descriptor->isCleared, 0,
                                                         textureDescriptor->mipLevelCount, 0,
                                                         textureDescriptor->arrayLayerCount);
-        return dawnTexture.Detach();
+        return std::move(dawnTexture);
     }
 
     MaybeError Texture::InitializeAsExternalTexture(const TextureDescriptor* descriptor,

src/dawn_native/d3d12/TextureD3D12.h

@@ -34,9 +34,9 @@ namespace dawn_native { namespace d3d12 {
 
     class Texture final : public TextureBase {
       public:
-        static ResultOrError<TextureBase*> Create(Device* device,
+        static ResultOrError<Ref<TextureBase>> Create(Device* device,
                                                       const TextureDescriptor* descriptor);
-        static ResultOrError<TextureBase*> Create(Device* device,
+        static ResultOrError<Ref<TextureBase>> Create(Device* device,
                                                       const ExternalImageDescriptor* descriptor,
                                                       HANDLE sharedHandle,
                                                       uint64_t acquireMutexKey,

src/dawn_native/metal/DeviceMTL.h

@@ -94,7 +94,8 @@ namespace dawn_native { namespace metal {
             Surface* surface,
             NewSwapChainBase* previousSwapChain,
             const SwapChainDescriptor* descriptor) override;
-        ResultOrError<TextureBase*> CreateTextureImpl(const TextureDescriptor* descriptor) override;
+        ResultOrError<Ref<TextureBase>> CreateTextureImpl(
+            const TextureDescriptor* descriptor) override;
         ResultOrError<TextureViewBase*> CreateTextureViewImpl(
             TextureBase* texture,
             const TextureViewDescriptor* descriptor) override;

src/dawn_native/metal/DeviceMTL.mm

@@ -145,8 +145,8 @@ namespace dawn_native { namespace metal {
         const SwapChainDescriptor* descriptor) {
         return new SwapChain(this, surface, previousSwapChain, descriptor);
     }
-    ResultOrError<TextureBase*> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
+    ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
-        return new Texture(this, descriptor);
+        return AcquireRef(new Texture(this, descriptor));
     }
     ResultOrError<TextureViewBase*> Device::CreateTextureViewImpl(
         TextureBase* texture,

src/dawn_native/null/DeviceNull.cpp

@@ -159,8 +159,8 @@ namespace dawn_native { namespace null {
         const SwapChainDescriptor* descriptor) {
         return new SwapChain(this, surface, previousSwapChain, descriptor);
     }
-    ResultOrError<TextureBase*> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
+    ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
-        return new Texture(this, descriptor, TextureBase::TextureState::OwnedInternal);
+        return AcquireRef(new Texture(this, descriptor, TextureBase::TextureState::OwnedInternal));
     }
     ResultOrError<TextureViewBase*> Device::CreateTextureViewImpl(
         TextureBase* texture,

src/dawn_native/null/DeviceNull.h

@@ -132,7 +132,8 @@ namespace dawn_native { namespace null {
             Surface* surface,
             NewSwapChainBase* previousSwapChain,
             const SwapChainDescriptor* descriptor) override;
-        ResultOrError<TextureBase*> CreateTextureImpl(const TextureDescriptor* descriptor) override;
+        ResultOrError<Ref<TextureBase>> CreateTextureImpl(
+            const TextureDescriptor* descriptor) override;
         ResultOrError<TextureViewBase*> CreateTextureViewImpl(
             TextureBase* texture,
             const TextureViewDescriptor* descriptor) override;

src/dawn_native/opengl/DeviceGL.cpp

@@ -136,8 +136,8 @@ namespace dawn_native { namespace opengl {
         const SwapChainDescriptor* descriptor) {
         return DAWN_VALIDATION_ERROR("New swapchains not implemented.");
     }
-    ResultOrError<TextureBase*> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
+    ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
-        return new Texture(this, descriptor);
+        return AcquireRef(new Texture(this, descriptor));
     }
     ResultOrError<TextureViewBase*> Device::CreateTextureViewImpl(
         TextureBase* texture,

src/dawn_native/opengl/DeviceGL.h

@@ -89,7 +89,8 @@ namespace dawn_native { namespace opengl {
             Surface* surface,
             NewSwapChainBase* previousSwapChain,
             const SwapChainDescriptor* descriptor) override;
-        ResultOrError<TextureBase*> CreateTextureImpl(const TextureDescriptor* descriptor) override;
+        ResultOrError<Ref<TextureBase>> CreateTextureImpl(
+            const TextureDescriptor* descriptor) override;
         ResultOrError<TextureViewBase*> CreateTextureViewImpl(
             TextureBase* texture,
             const TextureViewDescriptor* descriptor) override;

src/dawn_native/vulkan/DeviceVk.cpp

@@ -148,7 +148,7 @@ namespace dawn_native { namespace vulkan {
         const SwapChainDescriptor* descriptor) {
         return DAWN_VALIDATION_ERROR("New swapchains not implemented.");
     }
-    ResultOrError<TextureBase*> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
+    ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
         return Texture::Create(this, descriptor);
     }
     ResultOrError<TextureViewBase*> Device::CreateTextureViewImpl(

src/dawn_native/vulkan/DeviceVk.h

@@ -123,7 +123,8 @@ namespace dawn_native { namespace vulkan {
             Surface* surface,
             NewSwapChainBase* previousSwapChain,
             const SwapChainDescriptor* descriptor) override;
-        ResultOrError<TextureBase*> CreateTextureImpl(const TextureDescriptor* descriptor) override;
+        ResultOrError<Ref<TextureBase>> CreateTextureImpl(
+            const TextureDescriptor* descriptor) override;
         ResultOrError<TextureViewBase*> CreateTextureViewImpl(
             TextureBase* texture,
             const TextureViewDescriptor* descriptor) override;

src/dawn_native/vulkan/TextureVk.cpp

@@ -406,11 +406,12 @@ namespace dawn_native { namespace vulkan {
     }
 
     // static
-    ResultOrError<Texture*> Texture::Create(Device* device, const TextureDescriptor* descriptor) {
+    ResultOrError<Ref<TextureBase>> Texture::Create(Device* device,
+                                                    const TextureDescriptor* descriptor) {
         Ref<Texture> texture =
             AcquireRef(new Texture(device, descriptor, TextureState::OwnedInternal));
         DAWN_TRY(texture->InitializeAsInternalTexture());
-        return texture.Detach();
+        return std::move(texture);
     }
 
     // static

src/dawn_native/vulkan/TextureVk.h

@@ -40,7 +40,8 @@ namespace dawn_native { namespace vulkan {
     class Texture final : public TextureBase {
       public:
         // Used to create a regular texture from a descriptor.
-        static ResultOrError<Texture*> Create(Device* device, const TextureDescriptor* descriptor);
+        static ResultOrError<Ref<TextureBase>> Create(Device* device,
+                                                      const TextureDescriptor* descriptor);
 
         // Creates a texture and initializes it with a VkImage that references an external memory
         // object. Before the texture can be used, the VkDeviceMemory associated with the external

src/tests/unittests/ResultTests.cpp

@@ -296,6 +296,42 @@ TEST(ResultRefT, ReturningSuccess) {
     TestSuccess(&result, &success);
 }
 
+class OtherClass {
+  public:
+    int a = 0;
+};
+class Base : public RefCounted {};
+class Child : public OtherClass, public Base {};
+
+// Test constructing a Result<Ref<TChild>, E>
+TEST(ResultRefT, ConversionFromChildConstructor) {
+    Child child;
+    Ref<Child> refChild(&child);
+
+    Result<Ref<Base>, int> result(std::move(refChild));
+    TestSuccess<Base>(&result, &child);
+}
+
+// Test copy constructing Result<Ref<TChild>, E>
+TEST(ResultRefT, ConversionFromChildCopyConstructor) {
+    Child child;
+    Ref<Child> refChild(&child);
+
+    Result<Ref<Child>, int> resultChild(std::move(refChild));
+    Result<Ref<Base>, int> result(std::move(resultChild));
+    TestSuccess<Base>(&result, &child);
+}
+
+// Test assignment operator for Result<Ref<TChild>, E>
+TEST(ResultRefT, ConversionFromChildAssignmentOperator) {
+    Child child;
+    Ref<Child> refChild(&child);
+
+    Result<Ref<Child>, int> resultChild(std::move(refChild));
+    Result<Ref<Base>, int> result = std::move(resultChild);
+    TestSuccess<Base>(&result, &child);
+}
+
 // Result<T, E>
 
 // Test constructing an error Result<T, E>