WrappedSkImage: Fix a use after release problem.

The backing (vkimage) of WrapperSkImage could be still used by some
submitted but not finished vulkan tasks, when the WrapperSkImage is
being destroyed. So we need use VulkanFenceHelper to release the backing.

Bug: 968727
Change-Id: I8750cb9e0ddc9daea5a04bba3335e81554e45f4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1651034
Commit-Queue: Peng Huang <penghuang@chromium.org>
Reviewed-by: Jonathan Backer <backer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#668567}

components/viz/common/gpu/vulkan_in_process_context_provider.cc

@@ -34,6 +34,14 @@ GrVkGetProc make_unified_getter(const PFN_vkGetInstanceProcAddr& iproc,
   };
 }
 
+VulkanInProcessContextProvider::VulkanInProcessContextProvider(
+    gpu::VulkanImplementation* vulkan_implementation)
+    : vulkan_implementation_(vulkan_implementation) {}
+
+VulkanInProcessContextProvider::~VulkanInProcessContextProvider() {
+  Destroy();
+}
+
 bool VulkanInProcessContextProvider::Initialize() {
   DCHECK(!device_queue_);
   const gfx::ExtensionSet& extensions =
@@ -89,8 +97,12 @@ bool VulkanInProcessContextProvider::Initialize() {
 }
 
 void VulkanInProcessContextProvider::Destroy() {
-  if (gr_context_)
+  if (gr_context_) {
+    // releaseResourcesAndAbandonContext() will wait on GPU to finish all works,
+    // execute pending flush done callbacks and release all resources.
+    gr_context_->releaseResourcesAndAbandonContext();
     gr_context_.reset();
+  }
 
   if (device_queue_) {
     device_queue_->Destroy();
@@ -126,12 +138,4 @@ void VulkanInProcessContextProvider::EnqueueSecondaryCBPostSubmitTask(
   NOTREACHED();
 }
 
-VulkanInProcessContextProvider::VulkanInProcessContextProvider(
-    gpu::VulkanImplementation* vulkan_implementation)
-    : vulkan_implementation_(vulkan_implementation) {}
-
-VulkanInProcessContextProvider::~VulkanInProcessContextProvider() {
-  Destroy();
-}
-
 }  // namespace viz

components/viz/common/gpu/vulkan_in_process_context_provider.h

@@ -27,7 +27,6 @@ class VIZ_VULKAN_CONTEXT_PROVIDER_EXPORT VulkanInProcessContextProvider
   static scoped_refptr<VulkanInProcessContextProvider> Create(
       gpu::VulkanImplementation* vulkan_implementation);
 
-  bool Initialize();
   void Destroy();
 
   // VulkanContextProvider implementation
@@ -39,12 +38,13 @@ class VIZ_VULKAN_CONTEXT_PROVIDER_EXPORT VulkanInProcessContextProvider
       std::vector<VkSemaphore> semaphores) override;
   void EnqueueSecondaryCBPostSubmitTask(base::OnceClosure closure) override;
 
- protected:
+ private:
   explicit VulkanInProcessContextProvider(
       gpu::VulkanImplementation* vulkan_implementation);
   ~VulkanInProcessContextProvider() override;
 
- private:
+  bool Initialize();
+
 #if BUILDFLAG(ENABLE_VULKAN)
   sk_sp<GrContext> gr_context_;
   gpu::VulkanImplementation* vulkan_implementation_;

gpu/command_buffer/service/skia_utils.cc

@@ -7,6 +7,7 @@
 #include "base/logging.h"
 #include "components/viz/common/gpu/vulkan_context_provider.h"
 #include "components/viz/common/resources/resource_format_utils.h"
+#include "gpu/command_buffer/service/shared_context_state.h"
 #include "third_party/skia/include/gpu/GrBackendSurface.h"
 #include "third_party/skia/include/gpu/gl/GrGLTypes.h"
 #include "ui/gfx/geometry/size.h"
@@ -90,4 +91,27 @@ void AddVulkanCleanupTaskForSkiaFlush(
 #endif
 }
 
+void DeleteGrBackendTexture(SharedContextState* context_state,
+                            GrBackendTexture* backend_texture) {
+  DCHECK(backend_texture && backend_texture->isValid());
+  if (!context_state->GrContextIsVulkan()) {
+    context_state->gr_context()->deleteBackendTexture(
+        std::move(*backend_texture));
+    return;
+  }
+
+#if BUILDFLAG(ENABLE_VULKAN)
+  auto* fence_helper =
+      context_state->vk_context_provider()->GetDeviceQueue()->GetFenceHelper();
+  fence_helper->EnqueueCleanupTaskForSubmittedWork(base::BindOnce(
+      [](const sk_sp<GrContext>& gr_context, GrBackendTexture backend_texture,
+         gpu::VulkanDeviceQueue* device_queue, bool is_lost) {
+        // If underlying Vulkan device is destroyed, gr_context should have been
+        // abandoned, the deleteBackendTexture() should be noop.
+        gr_context->deleteBackendTexture(std::move(backend_texture));
+      },
+      sk_ref_sp(context_state->gr_context()), std::move(*backend_texture)));
+#endif
+}
+
 }  // namespace gpu

gpu/command_buffer/service/skia_utils.h

@@ -31,6 +31,9 @@ class VulkanContextProvider;
 }  // namespace viz
 
 namespace gpu {
+
+class SharedContextState;
+
 // Creates a GrBackendTexture from a service ID. Skia does not take ownership.
 // Returns true on success.
 GPU_GLES2_EXPORT bool GetGrBackendTexture(const gl::GLVersionInfo* version_info,
@@ -50,6 +53,9 @@ GPU_GLES2_EXPORT void AddVulkanCleanupTaskForSkiaFlush(
     viz::VulkanContextProvider* context_provider,
     GrFlushInfo* flush_info);
 
+GPU_GLES2_EXPORT void DeleteGrBackendTexture(
+    SharedContextState* context_state,
+    GrBackendTexture* backend_textures);
 }  // namespace gpu
 
 #endif  // GPU_COMMAND_BUFFER_SERVICE_SKIA_UTILS_H_

gpu/command_buffer/service/wrapped_sk_image.cc

@@ -17,6 +17,7 @@
 #include "gpu/command_buffer/service/shared_context_state.h"
 #include "gpu/command_buffer/service/shared_image_backing.h"
 #include "gpu/command_buffer/service/shared_image_representation.h"
+#include "gpu/command_buffer/service/skia_utils.h"
 #include "third_party/skia/include/core/SkPromiseImageTexture.h"
 #include "third_party/skia/include/core/SkSurface.h"
 #include "third_party/skia/include/core/SkSurfaceProps.h"
@@ -47,7 +48,7 @@ class WrappedSkImage : public SharedImageBacking {
 
   void Destroy() override {
     DCHECK(backend_texture_.isValid());
-    context_state_->gr_context()->deleteBackendTexture(backend_texture_);
+    DeleteGrBackendTexture(context_state_, &backend_texture_);
   }
 
   bool IsCleared() const override { return cleared_; }