Refactors Destroy to be upward calls instead of overriding.

- Fixes all class and implementations to call upwards when necessary.
- Makes DestroyImpl full virtual at ApiObjectBase.
- Fixes mock classes to call default DestroyImpl upwards on mock calls.
- Adds back D3D12 optimization for buffer destruction.

Bug: dawn:628, dawn:1189
Change-Id: Id2c2c6483dc7ed93daf8e4648af26ac1307e3d90
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/71243
Commit-Queue: Loko Kung <lokokung@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn_native/BindGroup.cpp

@@ -408,6 +408,9 @@ namespace dawn_native {
         }
     }
 
+    void BindGroupBase::DestroyImpl() {
+    }
+
     void BindGroupBase::DeleteThis() {
         // Add another ref to the layout so that if this is the last ref, the layout
         // is destroyed after the bind group. The bind group is slab-allocated inside

src/dawn_native/BindGroup.h

@@ -75,6 +75,7 @@ namespace dawn_native {
 
         // Constructor used only for mocking and testing.
         BindGroupBase(DeviceBase* device);
+        void DestroyImpl() override;
 
         ~BindGroupBase() override;
 

src/dawn_native/BindGroupLayout.cpp

@@ -418,13 +418,11 @@ namespace dawn_native {
 
     BindGroupLayoutBase::~BindGroupLayoutBase() = default;
 
-    bool BindGroupLayoutBase::Destroy() {
+    void BindGroupLayoutBase::DestroyImpl() {
-        bool wasDestroyed = ApiObjectBase::Destroy();
+        if (IsCachedReference()) {
-        if (wasDestroyed && IsCachedReference()) {
+            // Do not uncache the actual cached object if we are a blueprint.
-            // Do not uncache the actual cached object if we are a blueprint or already destroyed.
             GetDevice()->UncacheBindGroupLayout(this);
         }
-        return wasDestroyed;
     }
 
     // static

src/dawn_native/BindGroupLayout.h

@@ -53,7 +53,6 @@ namespace dawn_native {
 
         static BindGroupLayoutBase* MakeError(DeviceBase* device);
 
-        bool Destroy() override;
         ObjectType GetType() const override;
 
         // A map from the BindingNumber to its packed BindingIndex.
@@ -118,6 +117,7 @@ namespace dawn_native {
       protected:
         // Constructor used only for mocking and testing.
         BindGroupLayoutBase(DeviceBase* device);
+        void DestroyImpl() override;
 
         template <typename BindGroup>
         SlabAllocator<BindGroup> MakeFrontendBindGroupAllocator(size_t size) {

src/dawn_native/Buffer.cpp

@@ -91,10 +91,6 @@ namespace dawn_native {
                 mFakeMappedData.reset();
             }
 
-            void DestroyImpl() override {
-                mFakeMappedData.reset();
-            }
-
             std::unique_ptr<uint8_t[]> mFakeMappedData;
         };
 
@@ -183,12 +179,7 @@ namespace dawn_native {
         ASSERT(mState == BufferState::Unmapped || mState == BufferState::Destroyed);
     }
 
-    bool BufferBase::Destroy() {
+    void BufferBase::DestroyImpl() {
-        bool marked = MarkDestroyed();
-        if (!marked) {
-            return false;
-        }
-
         if (mState == BufferState::Mapped) {
             UnmapInternal(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
         } else if (mState == BufferState::MappedAtCreation) {
@@ -198,10 +189,7 @@ namespace dawn_native {
                 UnmapInternal(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
             }
         }
-
-        DestroyImpl();
         mState = BufferState::Destroyed;
-        return true;
     }
 
     // static

src/dawn_native/Buffer.h

@@ -52,7 +52,6 @@ namespace dawn_native {
 
         static BufferBase* MakeError(DeviceBase* device, const BufferDescriptor* descriptor);
 
-        bool Destroy() override;
         ObjectType GetType() const override;
 
         uint64_t GetSize() const;
@@ -89,6 +88,7 @@ namespace dawn_native {
 
         // Constructor used only for mocking and testing.
         BufferBase(DeviceBase* device, BufferState state);
+        void DestroyImpl() override;
 
         ~BufferBase() override;
 

src/dawn_native/CommandBuffer.h

@@ -47,14 +47,13 @@ namespace dawn_native {
       protected:
         // Constructor used only for mocking and testing.
         CommandBufferBase(DeviceBase* device);
+        void DestroyImpl() override;
 
         CommandIterator mCommands;
 
       private:
         CommandBufferBase(DeviceBase* device, ObjectBase::ErrorTag tag);
 
-        void DestroyImpl() override;
-
         CommandBufferResourceUsage mResourceUsages;
     };
 

src/dawn_native/ComputePassEncoder.cpp

@@ -133,7 +133,6 @@ namespace dawn_native {
     }
 
     void ComputePassEncoder::DestroyImpl() {
-        ApiObjectBase::DestroyImpl();
         // Ensure that the pass has exited. This is done for passes only since validation requires
         // they exit before destruction while bundles do not.
         mEncodingContext->EnsurePassExited(this);

src/dawn_native/ComputePipeline.cpp

@@ -60,13 +60,11 @@ namespace dawn_native {
 
     ComputePipelineBase::~ComputePipelineBase() = default;
 
-    bool ComputePipelineBase::Destroy() {
+    void ComputePipelineBase::DestroyImpl() {
-        bool wasDestroyed = ApiObjectBase::Destroy();
+        if (IsCachedReference()) {
-        if (wasDestroyed && IsCachedReference()) {
+            // Do not uncache the actual cached object if we are a blueprint.
-            // Do not uncache the actual cached object if we are a blueprint or already destroyed.
             GetDevice()->UncacheComputePipeline(this);
         }
-        return wasDestroyed;
     }
 
     // static

src/dawn_native/ComputePipeline.h

@@ -34,7 +34,6 @@ namespace dawn_native {
 
         static ComputePipelineBase* MakeError(DeviceBase* device);
 
-        bool Destroy() override;
         ObjectType GetType() const override;
 
         // Functors necessary for the unordered_set<ComputePipelineBase*>-based cache.
@@ -45,6 +44,7 @@ namespace dawn_native {
       protected:
         // Constructor used only for mocking and testing.
         ComputePipelineBase(DeviceBase* device);
+        void DestroyImpl() override;
 
       private:
         ComputePipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag);

src/dawn_native/ExternalTexture.cpp

@@ -122,9 +122,8 @@ namespace dawn_native {
         Destroy();
     }
 
-    bool ExternalTextureBase::Destroy() {
+    void ExternalTextureBase::DestroyImpl() {
         mState = ExternalTextureState::Destroyed;
-        return ApiObjectBase::Destroy();
     }
 
     // static

src/dawn_native/ExternalTexture.h

@@ -42,7 +42,6 @@ namespace dawn_native {
 
         static ExternalTextureBase* MakeError(DeviceBase* device);
 
-        bool Destroy() override;
         ObjectType GetType() const override;
 
         void APIDestroy();
@@ -50,6 +49,7 @@ namespace dawn_native {
       protected:
         // Constructor used only for mocking and testing.
         ExternalTextureBase(DeviceBase* device);
+        void DestroyImpl() override;
 
       private:
         enum class ExternalTextureState { Alive, Destroyed };

src/dawn_native/ObjectBase.cpp

@@ -80,20 +80,11 @@ namespace dawn_native {
         GetDevice()->TrackObject(this);
     }
 
-    bool ApiObjectBase::MarkDestroyed() {
+    void ApiObjectBase::Destroy() {
         const std::lock_guard<std::mutex> lock(*GetDevice()->GetObjectListMutex(GetType()));
-        return RemoveFromList();
+        if (RemoveFromList()) {
-    }
-
-    bool ApiObjectBase::Destroy() {
-        bool marked = MarkDestroyed();
-        if (marked) {
             DestroyImpl();
         }
-        return marked;
-    }
-
-    void ApiObjectBase::DestroyImpl() {
     }
 
 }  // namespace dawn_native

src/dawn_native/ObjectBase.h

@@ -60,12 +60,8 @@ namespace dawn_native {
         // by the owning device.
         bool IsAlive() const;
 
-        // Allow virtual overriding of actual destroy call in order to allow for re-using of base
+        // This needs to be public because it can be called from the device owning the object.
-        // destruction oerations. Classes that override this function should almost always call this
+        void Destroy();
-        // class's implementation in the override. This needs to be public because it can be called
-        // from the device owning the object. Returns true iff destruction occurs. Upon any re-calls
-        // of the function it will return false to indicate no further operations should be taken.
-        virtual bool Destroy();
 
         // Dawn API
         void APISetLabel(const char* label);
@@ -84,12 +80,11 @@ namespace dawn_native {
         void DeleteThis() override;
         void TrackInDevice();
 
-        // Thread-safe manner to mark an object as destroyed. Returns true iff the call was the
+        // Sub-classes may override this function multiple times. Whenever overriding this function,
-        // "winning" attempt to destroy the object. This is useful when sub-classes may have extra
+        // however, users should be sure to call their parent's version in the new override to make
-        // pre-destruction steps that need to occur only once, i.e. Buffer needs to be unmapped
+        // sure that all destroy functionality is kept. This function is guaranteed to only be
-        // before being destroyed.
+        // called once through the exposed Destroy function.
-        bool MarkDestroyed();
+        virtual void DestroyImpl() = 0;
-        virtual void DestroyImpl();
 
       private:
         virtual void SetLabelImpl();

src/dawn_native/PipelineLayout.cpp

@@ -85,13 +85,11 @@ namespace dawn_native {
 
     PipelineLayoutBase::~PipelineLayoutBase() = default;
 
-    bool PipelineLayoutBase::Destroy() {
+    void PipelineLayoutBase::DestroyImpl() {
-        bool wasDestroyed = ApiObjectBase::Destroy();
+        if (IsCachedReference()) {
-        if (wasDestroyed && IsCachedReference()) {
+            // Do not uncache the actual cached object if we are a blueprint.
-            // Do not uncache the actual cached object if we are a blueprint
             GetDevice()->UncachePipelineLayout(this);
         }
-        return wasDestroyed;
     }
 
     // static

src/dawn_native/PipelineLayout.h

@@ -61,7 +61,6 @@ namespace dawn_native {
             DeviceBase* device,
             std::vector<StageAndDescriptor> stages);
 
-        bool Destroy() override;
         ObjectType GetType() const override;
 
         const BindGroupLayoutBase* GetBindGroupLayout(BindGroupIndex group) const;
@@ -87,6 +86,7 @@ namespace dawn_native {
         // Constructor used only for mocking and testing.
         PipelineLayoutBase(DeviceBase* device);
         PipelineLayoutBase(DeviceBase* device, ObjectBase::ErrorTag tag);
+        void DestroyImpl() override;
 
         BindGroupLayoutArray mBindGroupLayouts;
         BindGroupLayoutMask mMask;

src/dawn_native/QuerySet.cpp

@@ -126,9 +126,8 @@ namespace dawn_native {
         ASSERT(mState == QuerySetState::Unavailable || mState == QuerySetState::Destroyed);
     }
 
-    bool QuerySetBase::Destroy() {
+    void QuerySetBase::DestroyImpl() {
         mState = QuerySetState::Destroyed;
-        return ApiObjectBase::Destroy();
     }
 
     // static

src/dawn_native/QuerySet.h

@@ -31,7 +31,6 @@ namespace dawn_native {
 
         static QuerySetBase* MakeError(DeviceBase* device);
 
-        bool Destroy() override;
         ObjectType GetType() const override;
 
         wgpu::QueryType GetQueryType() const;
@@ -50,6 +49,7 @@ namespace dawn_native {
 
         // Constructor used only for mocking and testing.
         QuerySetBase(DeviceBase* device);
+        void DestroyImpl() override;
 
         ~QuerySetBase() override;
 

src/dawn_native/Queue.cpp

@@ -173,6 +173,9 @@ namespace dawn_native {
         ASSERT(mTasksInFlight.Empty());
     }
 
+    void QueueBase::DestroyImpl() {
+    }
+
     // static
     QueueBase* QueueBase::MakeError(DeviceBase* device) {
         return new ErrorQueue(device);

src/dawn_native/Queue.h

@@ -69,6 +69,7 @@ namespace dawn_native {
       protected:
         QueueBase(DeviceBase* device);
         QueueBase(DeviceBase* device, ObjectBase::ErrorTag tag);
+        void DestroyImpl() override;
 
       private:
         MaybeError WriteTextureInternal(const ImageCopyTexture* destination,

src/dawn_native/RenderPipeline.cpp

@@ -683,17 +683,15 @@ namespace dawn_native {
 
     RenderPipelineBase::~RenderPipelineBase() = default;
 
-    bool RenderPipelineBase::Destroy() {
+    void RenderPipelineBase::DestroyImpl() {
-        bool wasDestroyed = ApiObjectBase::Destroy();
+        if (IsCachedReference()) {
-        if (wasDestroyed && IsCachedReference()) {
+            // Do not uncache the actual cached object if we are a blueprint.
-            // Do not uncache the actual cached object if we are a blueprint or already destroyed.
             GetDevice()->UncacheRenderPipeline(this);
         }
 
         // Remove reference to the attachment state so that we don't have lingering references to
         // it preventing it from being uncached in the device.
         mAttachmentState = nullptr;
-        return wasDestroyed;
     }
 
     // static

src/dawn_native/RenderPipeline.h

@@ -63,7 +63,6 @@ namespace dawn_native {
 
         static RenderPipelineBase* MakeError(DeviceBase* device);
 
-        bool Destroy() override;
         ObjectType GetType() const override;
 
         const ityp::bitset<VertexAttributeLocation, kMaxVertexAttributes>&
@@ -111,6 +110,7 @@ namespace dawn_native {
       protected:
         // Constructor used only for mocking and testing.
         RenderPipelineBase(DeviceBase* device);
+        void DestroyImpl() override;
 
       private:
         RenderPipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag);

src/dawn_native/Sampler.cpp

@@ -100,13 +100,11 @@ namespace dawn_native {
 
     SamplerBase::~SamplerBase() = default;
 
-    bool SamplerBase::Destroy() {
+    void SamplerBase::DestroyImpl() {
-        bool wasDestroyed = ApiObjectBase::Destroy();
+        if (IsCachedReference()) {
-        if (wasDestroyed && IsCachedReference()) {
+            // Do not uncache the actual cached object if we are a blueprint.
-            // Do not uncache the actual cached object if we are a blueprint or already destroyed.
             GetDevice()->UncacheSampler(this);
         }
-        return wasDestroyed;
     }
 
     // static

src/dawn_native/Sampler.h

@@ -38,7 +38,6 @@ namespace dawn_native {
 
         static SamplerBase* MakeError(DeviceBase* device);
 
-        bool Destroy() override;
         ObjectType GetType() const override;
 
         bool IsComparison() const;
@@ -58,6 +57,7 @@ namespace dawn_native {
       protected:
         // Constructor used only for mocking and testing.
         SamplerBase(DeviceBase* device);
+        void DestroyImpl() override;
 
       private:
         SamplerBase(DeviceBase* device, ObjectBase::ErrorTag tag);

src/dawn_native/ShaderModule.cpp

@@ -1204,13 +1204,11 @@ namespace dawn_native {
 
     ShaderModuleBase::~ShaderModuleBase() = default;
 
-    bool ShaderModuleBase::Destroy() {
+    void ShaderModuleBase::DestroyImpl() {
-        bool wasDestroyed = ApiObjectBase::Destroy();
+        if (IsCachedReference()) {
-        if (wasDestroyed && IsCachedReference()) {
+            // Do not uncache the actual cached object if we are a blueprint.
-            // Do not uncache the actual cached object if we are a blueprint or already destroyed.
             GetDevice()->UncacheShaderModule(this);
         }
-        return wasDestroyed;
     }
 
     // static

src/dawn_native/ShaderModule.h

@@ -250,7 +250,6 @@ namespace dawn_native {
 
         static Ref<ShaderModuleBase> MakeError(DeviceBase* device);
 
-        bool Destroy() override;
         ObjectType GetType() const override;
 
         // Return true iff the program has an entrypoint called `entryPoint`.
@@ -279,6 +278,7 @@ namespace dawn_native {
       protected:
         // Constructor used only for mocking and testing.
         ShaderModuleBase(DeviceBase* device);
+        void DestroyImpl() override;
 
         MaybeError InitializeBase(ShaderModuleParseResult* parseResult);
 

src/dawn_native/SwapChain.cpp

@@ -125,6 +125,9 @@ namespace dawn_native {
     SwapChainBase::~SwapChainBase() {
     }
 
+    void SwapChainBase::DestroyImpl() {
+    }
+
     // static
     SwapChainBase* SwapChainBase::MakeError(DeviceBase* device) {
         return new ErrorSwapChain(device);

src/dawn_native/SwapChain.h

@@ -49,6 +49,7 @@ namespace dawn_native {
       protected:
         SwapChainBase(DeviceBase* device, ObjectBase::ErrorTag tag);
         ~SwapChainBase() override;
+        void DestroyImpl() override;
     };
 
     // The base class for implementation-based SwapChains that are deprecated.

src/dawn_native/Texture.cpp

@@ -483,14 +483,8 @@ namespace dawn_native {
         : ApiObjectBase(device, tag), mFormat(kUnusedFormat) {
     }
 
-    bool TextureBase::Destroy() {
+    void TextureBase::DestroyImpl() {
-        // We need to run the destroy operations prior to setting the state to destroyed so that
-        // the state is both consistent, and implementations of the destroy that may check the
-        // state do not skip operations unintentionally. (Example in Vulkan backend, the destroy
-        // implementation will not be ran if we are already in the Destroyed state.)
-        bool wasDestroyed = ApiObjectBase::Destroy();
         mState = TextureState::Destroyed;
-        return wasDestroyed;
     }
 
     // static
@@ -718,6 +712,9 @@ namespace dawn_native {
         : ApiObjectBase(device, tag), mFormat(kUnusedFormat) {
     }
 
+    void TextureViewBase::DestroyImpl() {
+    }
+
     // static
     TextureViewBase* TextureViewBase::MakeError(DeviceBase* device) {
         return new TextureViewBase(device, ObjectBase::kError);

src/dawn_native/Texture.h

@@ -51,7 +51,6 @@ namespace dawn_native {
 
         static TextureBase* MakeError(DeviceBase* device);
 
-        bool Destroy() override;
         ObjectType GetType() const override;
 
         wgpu::TextureDimension GetDimension() const;
@@ -99,6 +98,7 @@ namespace dawn_native {
       protected:
         // Constructor used only for mocking and testing.
         TextureBase(DeviceBase* device, TextureState state);
+        void DestroyImpl() override;
 
       private:
         TextureBase(DeviceBase* device, ObjectBase::ErrorTag tag);
@@ -140,6 +140,7 @@ namespace dawn_native {
       protected:
         // Constructor used only for mocking and testing.
         TextureViewBase(TextureBase* texture);
+        void DestroyImpl() override;
 
       private:
         TextureViewBase(DeviceBase* device, ObjectBase::ErrorTag tag);

src/dawn_native/d3d12/BindGroupD3D12.cpp

@@ -222,6 +222,7 @@ namespace dawn_native { namespace d3d12 {
     BindGroup::~BindGroup() = default;
 
     void BindGroup::DestroyImpl() {
+        BindGroupBase::DestroyImpl();
         ToBackend(GetLayout())->DeallocateBindGroup(this, &mCPUViewAllocation);
         ASSERT(!mCPUViewAllocation.IsValid());
     }

src/dawn_native/d3d12/BufferD3D12.cpp

@@ -379,8 +379,14 @@ namespace dawn_native { namespace d3d12 {
     }
 
     void Buffer::DestroyImpl() {
-        // TODO(crbug.com/dawn/1189) Reintroduce optimization to skip flushing the writes to the GPU
+        if (mMappedData != nullptr) {
-        // memory when we unmap in destruction case since the buffer will be destroyed anyways.
+            // If the buffer is currently mapped, unmap without flushing the writes to the GPU
+            // since the buffer cannot be used anymore. UnmapImpl checks mWrittenRange to know
+            // which parts to flush, so we set it to an empty range to prevent flushes.
+            mWrittenMappedRange = {0, 0};
+        }
+        BufferBase::DestroyImpl();
+
         ToBackend(GetDevice())->DeallocateMemory(mResourceAllocation);
     }
 

src/dawn_native/d3d12/ComputePipelineD3D12.cpp

@@ -69,6 +69,7 @@ namespace dawn_native { namespace d3d12 {
     ComputePipeline::~ComputePipeline() = default;
 
     void ComputePipeline::DestroyImpl() {
+        ComputePipelineBase::DestroyImpl();
         ToBackend(GetDevice())->ReferenceUntilUnused(mPipelineState);
     }
 

src/dawn_native/d3d12/QuerySetD3D12.cpp

@@ -63,6 +63,7 @@ namespace dawn_native { namespace d3d12 {
     QuerySet::~QuerySet() = default;
 
     void QuerySet::DestroyImpl() {
+        QuerySetBase::DestroyImpl();
         ToBackend(GetDevice())->ReferenceUntilUnused(mQueryHeap);
         mQueryHeap = nullptr;
     }

src/dawn_native/d3d12/RenderPipelineD3D12.cpp

@@ -425,6 +425,7 @@ namespace dawn_native { namespace d3d12 {
     RenderPipeline::~RenderPipeline() = default;
 
     void RenderPipeline::DestroyImpl() {
+        RenderPipelineBase::DestroyImpl();
         ToBackend(GetDevice())->ReferenceUntilUnused(mPipelineState);
     }
 

src/dawn_native/d3d12/SwapChainD3D12.cpp

@@ -144,6 +144,7 @@ namespace dawn_native { namespace d3d12 {
     SwapChain::~SwapChain() = default;
 
     void SwapChain::DestroyImpl() {
+        SwapChainBase::DestroyImpl();
         DetachFromSurface();
     }
 

src/dawn_native/d3d12/TextureD3D12.cpp

@@ -658,6 +658,8 @@ namespace dawn_native { namespace d3d12 {
     }
 
     void Texture::DestroyImpl() {
+        TextureBase::DestroyImpl();
+
         Device* device = ToBackend(GetDevice());
 
         // In PIX's D3D12-only mode, there is no way to determine frame boundaries

src/dawn_native/metal/BindGroupMTL.mm

@@ -25,6 +25,7 @@ namespace dawn_native { namespace metal {
     BindGroup::~BindGroup() = default;
 
     void BindGroup::DestroyImpl() {
+        BindGroupBase::DestroyImpl();
         ToBackend(GetLayout())->DeallocateBindGroup(this);
     }
 

src/dawn_native/metal/BufferMTL.mm

@@ -172,6 +172,7 @@ namespace dawn_native { namespace metal {
     }
 
     void Buffer::DestroyImpl() {
+        BufferBase::DestroyImpl();
         mMtlBuffer = nullptr;
     }
 

src/dawn_native/metal/QuerySetMTL.mm

@@ -124,6 +124,8 @@ namespace dawn_native { namespace metal {
     QuerySet::~QuerySet() = default;
 
     void QuerySet::DestroyImpl() {
+        QuerySetBase::DestroyImpl();
+
         mVisibilityBuffer = nullptr;
 
         // mCounterSampleBuffer isn't an NSRef because API_AVAILABLE doesn't work will with

src/dawn_native/metal/SwapChainMTL.mm

@@ -76,6 +76,7 @@ namespace dawn_native { namespace metal {
     SwapChain::~SwapChain() = default;
 
     void SwapChain::DestroyImpl() {
+        SwapChainBase::DestroyImpl();
         DetachFromSurface();
     }
 

src/dawn_native/metal/TextureMTL.mm

@@ -500,6 +500,7 @@ namespace dawn_native { namespace metal {
     }
 
     void Texture::DestroyImpl() {
+        TextureBase::DestroyImpl();
         mMtlTexture = nullptr;
     }
 

src/dawn_native/null/DeviceNull.cpp

@@ -331,6 +331,7 @@ namespace dawn_native { namespace null {
     }
 
     void Buffer::DestroyImpl() {
+        BufferBase::DestroyImpl();
         ToBackend(GetDevice())->DecrementMemoryUsage(GetSize());
     }
 
@@ -346,9 +347,6 @@ namespace dawn_native { namespace null {
         : QuerySetBase(device, descriptor) {
     }
 
-    void QuerySet::DestroyImpl() {
-    }
-
     // Queue
 
     Queue::Queue(Device* device) : QueueBase(device) {

src/dawn_native/null/DeviceNull.h

@@ -246,9 +246,6 @@ namespace dawn_native { namespace null {
     class QuerySet final : public QuerySetBase {
       public:
         QuerySet(Device* device, const QuerySetDescriptor* descriptor);
-
-      private:
-        void DestroyImpl() override;
     };
 
     class Queue final : public QueueBase {

src/dawn_native/opengl/BindGroupGL.cpp

@@ -53,6 +53,7 @@ namespace dawn_native { namespace opengl {
     BindGroup::~BindGroup() = default;
 
     void BindGroup::DestroyImpl() {
+        BindGroupBase::DestroyImpl();
         ToBackend(GetLayout())->DeallocateBindGroup(this);
     }
 

src/dawn_native/opengl/BufferGL.cpp

@@ -175,6 +175,7 @@ namespace dawn_native { namespace opengl {
     }
 
     void Buffer::DestroyImpl() {
+        BufferBase::DestroyImpl();
         ToBackend(GetDevice())->gl.DeleteBuffers(1, &mBuffer);
         mBuffer = 0;
     }

src/dawn_native/opengl/ComputePipelineGL.cpp

@@ -28,6 +28,7 @@ namespace dawn_native { namespace opengl {
     ComputePipeline::~ComputePipeline() = default;
 
     void ComputePipeline::DestroyImpl() {
+        ComputePipelineBase::DestroyImpl();
         DeleteProgram(ToBackend(GetDevice())->gl);
     }
 

src/dawn_native/opengl/QuerySetGL.cpp

@@ -24,7 +24,4 @@ namespace dawn_native { namespace opengl {
 
     QuerySet::~QuerySet() = default;
 
-    void QuerySet::DestroyImpl() {
-    }
-
 }}  // namespace dawn_native::opengl

src/dawn_native/opengl/QuerySetGL.h

@@ -27,8 +27,6 @@ namespace dawn_native { namespace opengl {
 
       private:
         ~QuerySet() override;
-
-        void DestroyImpl() override;
     };
 
 }}  // namespace dawn_native::opengl

src/dawn_native/opengl/RenderPipelineGL.cpp

@@ -237,6 +237,7 @@ namespace dawn_native { namespace opengl {
     RenderPipeline::~RenderPipeline() = default;
 
     void RenderPipeline::DestroyImpl() {
+        RenderPipelineBase::DestroyImpl();
         const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
         gl.DeleteVertexArrays(1, &mVertexArrayObject);
         gl.BindVertexArray(0);

src/dawn_native/opengl/SamplerGL.cpp

@@ -79,6 +79,7 @@ namespace dawn_native { namespace opengl {
     Sampler::~Sampler() = default;
 
     void Sampler::DestroyImpl() {
+        SamplerBase::DestroyImpl();
         const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
         gl.DeleteSamplers(1, &mFilteringHandle);
         gl.DeleteSamplers(1, &mNonFilteringHandle);

src/dawn_native/opengl/TextureGL.cpp

@@ -191,6 +191,7 @@ namespace dawn_native { namespace opengl {
     }
 
     void Texture::DestroyImpl() {
+        TextureBase::DestroyImpl();
         if (GetTextureState() == TextureState::OwnedInternal) {
             ToBackend(GetDevice())->gl.DeleteTextures(1, &mHandle);
             mHandle = 0;
@@ -561,6 +562,7 @@ namespace dawn_native { namespace opengl {
     }
 
     void TextureView::DestroyImpl() {
+        TextureViewBase::DestroyImpl();
         if (mOwnsHandle) {
             ToBackend(GetDevice())->gl.DeleteTextures(1, &mHandle);
         }

src/dawn_native/vulkan/BindGroupLayoutVk.cpp

@@ -155,6 +155,8 @@ namespace dawn_native { namespace vulkan {
     BindGroupLayout::~BindGroupLayout() = default;
 
     void BindGroupLayout::DestroyImpl() {
+        BindGroupLayoutBase::DestroyImpl();
+
         Device* device = ToBackend(GetDevice());
 
         // DescriptorSetLayout aren't used by execution on the GPU and can be deleted at any time,

src/dawn_native/vulkan/BindGroupVk.cpp

@@ -164,6 +164,7 @@ namespace dawn_native { namespace vulkan {
     BindGroup::~BindGroup() = default;
 
     void BindGroup::DestroyImpl() {
+        BindGroupBase::DestroyImpl();
         ToBackend(GetLayout())->DeallocateBindGroup(this, &mDescriptorSetAllocation);
     }
 

src/dawn_native/vulkan/BufferVk.cpp

@@ -330,6 +330,8 @@ namespace dawn_native { namespace vulkan {
     }
 
     void Buffer::DestroyImpl() {
+        BufferBase::DestroyImpl();
+
         ToBackend(GetDevice())->GetResourceMemoryAllocator()->Deallocate(&mMemoryAllocation);
 
         if (mHandle != VK_NULL_HANDLE) {

src/dawn_native/vulkan/ComputePipelineVk.cpp

@@ -92,6 +92,8 @@ namespace dawn_native { namespace vulkan {
     ComputePipeline::~ComputePipeline() = default;
 
     void ComputePipeline::DestroyImpl() {
+        ComputePipelineBase::DestroyImpl();
+
         if (mHandle != VK_NULL_HANDLE) {
             ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(mHandle);
             mHandle = VK_NULL_HANDLE;

src/dawn_native/vulkan/PipelineLayoutVk.cpp

@@ -65,6 +65,7 @@ namespace dawn_native { namespace vulkan {
     PipelineLayout::~PipelineLayout() = default;
 
     void PipelineLayout::DestroyImpl() {
+        PipelineLayoutBase::DestroyImpl();
         if (mHandle != VK_NULL_HANDLE) {
             ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(mHandle);
             mHandle = VK_NULL_HANDLE;

src/dawn_native/vulkan/QuerySetVk.cpp

@@ -102,6 +102,7 @@ namespace dawn_native { namespace vulkan {
     QuerySet::~QuerySet() = default;
 
     void QuerySet::DestroyImpl() {
+        QuerySetBase::DestroyImpl();
         if (mHandle != VK_NULL_HANDLE) {
             ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(mHandle);
             mHandle = VK_NULL_HANDLE;

src/dawn_native/vulkan/RenderPipelineVk.cpp

@@ -602,6 +602,7 @@ namespace dawn_native { namespace vulkan {
     RenderPipeline::~RenderPipeline() = default;
 
     void RenderPipeline::DestroyImpl() {
+        RenderPipelineBase::DestroyImpl();
         if (mHandle != VK_NULL_HANDLE) {
             ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(mHandle);
             mHandle = VK_NULL_HANDLE;

src/dawn_native/vulkan/SamplerVk.cpp

@@ -112,6 +112,7 @@ namespace dawn_native { namespace vulkan {
     Sampler::~Sampler() = default;
 
     void Sampler::DestroyImpl() {
+        SamplerBase::DestroyImpl();
         if (mHandle != VK_NULL_HANDLE) {
             ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(mHandle);
             mHandle = VK_NULL_HANDLE;

src/dawn_native/vulkan/ShaderModuleVk.cpp

@@ -100,6 +100,7 @@ namespace dawn_native { namespace vulkan {
     }
 
     void ShaderModule::DestroyImpl() {
+        ShaderModuleBase::DestroyImpl();
         // Remove reference to internal cache to trigger cleanup.
         mTransformedShaderModuleCache = nullptr;
     }

src/dawn_native/vulkan/SwapChainVk.cpp

@@ -226,6 +226,7 @@ namespace dawn_native { namespace vulkan {
     SwapChain::~SwapChain() = default;
 
     void SwapChain::DestroyImpl() {
+        SwapChainBase::DestroyImpl();
         DetachFromSurface();
     }
 

src/dawn_native/vulkan/TextureVk.cpp

@@ -833,6 +833,9 @@ namespace dawn_native { namespace vulkan {
             // If a signal semaphore exists it should be requested before we delete the texture
             ASSERT(mSignalSemaphore == VK_NULL_HANDLE);
         }
+        // For Vulkan, we currently run the base destruction code after the internal changes because
+        // of the dependency on the texture state which the base code overwrites too early.
+        TextureBase::DestroyImpl();
     }
 
     VkImage Texture::GetHandle() const {

src/tests/unittests/native/DestroyObjectTests.cpp

@@ -168,8 +168,8 @@ namespace dawn_native { namespace {
             BufferMock bufferMock(&mDevice, BufferBase::BufferState::Mapped);
             {
                 InSequence seq;
-                EXPECT_CALL(bufferMock, UnmapImpl).Times(1);
                 EXPECT_CALL(bufferMock, DestroyImpl).Times(1);
+                EXPECT_CALL(bufferMock, UnmapImpl).Times(1);
             }
 
             EXPECT_TRUE(bufferMock.IsAlive());
@@ -198,8 +198,8 @@ namespace dawn_native { namespace {
             BufferMock* bufferMock = new BufferMock(&mDevice, BufferBase::BufferState::Mapped);
             {
                 InSequence seq;
-                EXPECT_CALL(*bufferMock, UnmapImpl).Times(1);
                 EXPECT_CALL(*bufferMock, DestroyImpl).Times(1);
+                EXPECT_CALL(*bufferMock, UnmapImpl).Times(1);
             }
             {
                 BufferDescriptor desc = {};

src/tests/unittests/native/mocks/BindGroupLayoutMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class BindGroupLayoutMock final : public BindGroupLayoutBase {
       public:
         BindGroupLayoutMock(DeviceBase* device) : BindGroupLayoutBase(device) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->BindGroupLayoutBase::DestroyImpl();
+            });
         }
         ~BindGroupLayoutMock() override = default;
 

src/tests/unittests/native/mocks/BindGroupMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class BindGroupMock : public BindGroupBase {
       public:
         BindGroupMock(DeviceBase* device) : BindGroupBase(device) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->BindGroupBase::DestroyImpl();
+            });
         }
         ~BindGroupMock() override = default;
 

src/tests/unittests/native/mocks/BufferMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class BufferMock : public BufferBase {
       public:
         BufferMock(DeviceBase* device, BufferBase::BufferState state) : BufferBase(device, state) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->BufferBase::DestroyImpl();
+            });
         }
         ~BufferMock() override = default;
 

src/tests/unittests/native/mocks/CommandBufferMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class CommandBufferMock : public CommandBufferBase {
       public:
         CommandBufferMock(DeviceBase* device) : CommandBufferBase(device) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->CommandBufferBase::DestroyImpl();
+            });
         }
         ~CommandBufferMock() override = default;
 

src/tests/unittests/native/mocks/ComputePipelineMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class ComputePipelineMock : public ComputePipelineBase {
       public:
         ComputePipelineMock(DeviceBase* device) : ComputePipelineBase(device) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->ComputePipelineBase::DestroyImpl();
+            });
         }
         ~ComputePipelineMock() override = default;
 

src/tests/unittests/native/mocks/ExternalTextureMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class ExternalTextureMock : public ExternalTextureBase {
       public:
         ExternalTextureMock(DeviceBase* device) : ExternalTextureBase(device) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->ExternalTextureBase::DestroyImpl();
+            });
         }
         ~ExternalTextureMock() override = default;
 

src/tests/unittests/native/mocks/PipelineLayoutMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class PipelineLayoutMock : public PipelineLayoutBase {
       public:
         PipelineLayoutMock(DeviceBase* device) : PipelineLayoutBase(device) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->PipelineLayoutBase::DestroyImpl();
+            });
         }
         ~PipelineLayoutMock() override = default;
 

src/tests/unittests/native/mocks/QuerySetMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class QuerySetMock : public QuerySetBase {
       public:
         QuerySetMock(DeviceBase* device) : QuerySetBase(device) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->QuerySetBase::DestroyImpl();
+            });
         }
         ~QuerySetMock() override = default;
 

src/tests/unittests/native/mocks/RenderPipelineMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class RenderPipelineMock : public RenderPipelineBase {
       public:
         RenderPipelineMock(DeviceBase* device) : RenderPipelineBase(device) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->RenderPipelineBase::DestroyImpl();
+            });
         }
         ~RenderPipelineMock() override = default;
 

src/tests/unittests/native/mocks/SamplerMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class SamplerMock : public SamplerBase {
       public:
         SamplerMock(DeviceBase* device) : SamplerBase(device) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->SamplerBase::DestroyImpl();
+            });
         }
         ~SamplerMock() override = default;
 

src/tests/unittests/native/mocks/ShaderModuleMock.cpp

@@ -16,6 +16,12 @@
 
 namespace dawn_native {
 
+    ShaderModuleMock::ShaderModuleMock(DeviceBase* device) : ShaderModuleBase(device) {
+        ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+            this->ShaderModuleBase::DestroyImpl();
+        });
+    }
+
     ResultOrError<Ref<ShaderModuleMock>> ShaderModuleMock::Create(DeviceBase* device,
                                                                   const char* source) {
         ShaderModuleMock* mock = new ShaderModuleMock(device);

src/tests/unittests/native/mocks/ShaderModuleMock.h

@@ -27,8 +27,7 @@ namespace dawn_native {
 
     class ShaderModuleMock : public ShaderModuleBase {
       public:
-        ShaderModuleMock(DeviceBase* device) : ShaderModuleBase(device) {
+        ShaderModuleMock(DeviceBase* device);
-        }
         ~ShaderModuleMock() override = default;
 
         MOCK_METHOD(void, DestroyImpl, (), (override));

src/tests/unittests/native/mocks/SwapChainMock.h

@@ -25,6 +25,9 @@ namespace dawn_native {
     class SwapChainMock : public SwapChainBase {
       public:
         SwapChainMock(DeviceBase* device) : SwapChainBase(device) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->SwapChainBase::DestroyImpl();
+            });
         }
         ~SwapChainMock() override = default;
 

src/tests/unittests/native/mocks/TextureMock.h

@@ -26,6 +26,9 @@ namespace dawn_native {
       public:
         TextureMock(DeviceBase* device, TextureBase::TextureState state)
             : TextureBase(device, state) {
+            ON_CALL(*this, DestroyImpl).WillByDefault([this]() {
+                this->TextureBase::DestroyImpl();
+            });
         }
         ~TextureMock() override = default;