commit 8cabb327e70ab70ec9a7bab6a4533e4f41060faa
author Brian Salomon <bsalomon@google.com> Fri Feb 22 10:44:19 2019 -0500
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Feb 22 16:12:35 2019 +0000
tree fad8b7fe3e829c0e2bfef0006c3365fa55cfe8a2
parent aa76bc13887ae9e33e24125e3afed89b0c62a388

Make it safe to enter the cache from a GrTexture idle proc.

This fixes an issue in Chrome where Skia is calling a promise SkImage
texture release proc from ~SkImage that in turn flushes a SkSurface.
Prior to this change this caused an assert because we had already
decremented the GrTexture's ref count priot to calling the release
proc. This made the GrTexture purgeable, but the cache had not yet
been notified that the texture was purgeable and still had it in its
array of non-purgeable resources. This triggered an assert in the
cache's self-validation checks during the flush.

Now we call the release proc just prior to decrementing the ref count.
This also makes it legal to re-ref the resources from the release proc.

Bug: chromium:933526
Change-Id: I8cd921b77ca3dfe112089f9a553c1a625160d16d
Reviewed-on: https://skia-review.googlesource.com/c/194000
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

src/gpu/vk/GrVkImage.cpp

diff --git a/src/gpu/vk/GrVkImage.cpp b/src/gpu/vk/GrVkImage.cpp
index 6ce5d02..f1446ac 100644
--- a/src/gpu/vk/GrVkImage.cpp
+++ b/src/gpu/vk/GrVkImage.cpp
@@ -246,7 +246,7 @@
 }
 
 void GrVkImage::Resource::freeGPUData(GrVkGpu* gpu) const {
-    SkASSERT(!fReleaseHelper);
+    this->invokeReleaseProc();
     VK_CALL(gpu, DestroyImage(gpu->device(), fImage, nullptr));
     bool isLinear = (VK_IMAGE_TILING_LINEAR == fImageTiling);
     GrVkMemory::FreeImageMemory(gpu, isLinear, fAlloc);

src/gpu/vk/GrVkImage.h

diff --git a/src/gpu/vk/GrVkImage.h b/src/gpu/vk/GrVkImage.h
index c19621b..e5b4ce1 100644
--- a/src/gpu/vk/GrVkImage.h
+++ b/src/gpu/vk/GrVkImage.h
@@ -198,9 +198,18 @@
     protected:
         mutable sk_sp<GrReleaseProcHelper> fReleaseHelper;
 
+        void invokeReleaseProc() const {
+            if (fReleaseHelper) {
+                // Depending on the ref count of fReleaseHelper this may or may not actually trigger
+                // the ReleaseProc to be called.
+                fReleaseHelper.reset();
+            }
+        }
+
     private:
         void freeGPUData(GrVkGpu* gpu) const override;
         void abandonGPUData() const override {
+            this->invokeReleaseProc();
             SkASSERT(!fReleaseHelper);
         }
 
@@ -222,14 +231,6 @@
             : Resource(image, alloc, tiling) {
         }
     private:
-        void invokeReleaseProc() const {
-            if (fReleaseHelper) {
-                // Depending on the ref count of fReleaseHelper this may or may not actually trigger
-                // the ReleaseProc to be called.
-                fReleaseHelper.reset();
-            }
-        }
-
         void freeGPUData(GrVkGpu* gpu) const override;
         void abandonGPUData() const override;
     };

src/gpu/vk/GrVkTexture.cpp

diff --git a/src/gpu/vk/GrVkTexture.cpp b/src/gpu/vk/GrVkTexture.cpp
index 6e8fe67..cdb9351 100644
--- a/src/gpu/vk/GrVkTexture.cpp
+++ b/src/gpu/vk/GrVkTexture.cpp
@@ -179,7 +179,7 @@
     }
 }
 
-void GrVkTexture::removedLastRefOrPendingIO() {
+void GrVkTexture::willRemoveLastRefOrPendingIO() {
     if (!fIdleProc) {
         return;
     }

src/gpu/vk/GrVkTexture.h

diff --git a/src/gpu/vk/GrVkTexture.h b/src/gpu/vk/GrVkTexture.h
index be84229..0cfce86 100644
--- a/src/gpu/vk/GrVkTexture.h
+++ b/src/gpu/vk/GrVkTexture.h
@@ -69,7 +69,7 @@
         this->setResourceRelease(std::move(releaseHelper));
     }
 
-    void removedLastRefOrPendingIO() override;
+    void willRemoveLastRefOrPendingIO() override;
 
     const GrVkImageView* fTextureView;
     GrTexture::IdleProc* fIdleProc = nullptr;