commit 2163e410d1ad1b1a9af962f2565f676c3dbb8cb6
author Nolan Scobie <nscobie@google.com> Mon Oct 24 19:57:43 2022 -0400
committer Nolan Scobie <nscobie@google.com> Mon Oct 31 17:34:15 2022 -0400
tree 673bc94dcb0d2bf010e2a6e94048ffcb4037fe72
parent 38b3e5585802378cebd48b60ece70ba6edd79586

Fix erroneous self deletion on SkImage creation failure

TL;DR: Skia should always call releaseProc, and maybe sooner than we thought.

There are multiple scenarios where SkImage:MakeFromTexture will fail,
returning a nullptr and calling releaseProc due to a RefCntedCallback
falling out of scope. Previously this could cause mUsageCount to fall to
0, resulting in the AutoBackendTextureRelease deleting itself even
though DeferredLayerUpdater owned a ref and expected it to still exist.

Also added logging for some reasons that could cause the later call to
MakeFromTexture to fail.

Bug: b/246831853
Test: hwui_unit_tests
Change-Id: I7fd2566b9a85fe286f72b0fc42eba5450cac69b0

libs/hwui/AutoBackendTextureRelease.cpp

diff --git a/libs/hwui/AutoBackendTextureRelease.cpp b/libs/hwui/AutoBackendTextureRelease.cpp
index ef5eacb..b656b6a 100644
--- a/libs/hwui/AutoBackendTextureRelease.cpp
+++ b/libs/hwui/AutoBackendTextureRelease.cpp
@@ -32,9 +32,17 @@
     bool createProtectedImage = 0 != (desc.usage & AHARDWAREBUFFER_USAGE_PROTECTED_CONTENT);
     GrBackendFormat backendFormat =
             GrAHardwareBufferUtils::GetBackendFormat(context, buffer, desc.format, false);
+    LOG_ALWAYS_FATAL_IF(!backendFormat.isValid(),
+                        __FILE__ " Invalid GrBackendFormat. GrBackendApi==%" PRIu32
+                                 ", AHardwareBuffer_Format==%" PRIu32 ".",
+                        static_cast<int>(context->backend()), desc.format);
     mBackendTexture = GrAHardwareBufferUtils::MakeBackendTexture(
             context, buffer, desc.width, desc.height, &mDeleteProc, &mUpdateProc, &mImageCtx,
             createProtectedImage, backendFormat, false);
+    LOG_ALWAYS_FATAL_IF(!mBackendTexture.isValid(),
+                        __FILE__ " Invalid GrBackendTexture. Width==%" PRIu32 ", height==%" PRIu32
+                                 ", protected==%d",
+                        desc.width, desc.height, createProtectedImage);
 }
 
 void AutoBackendTextureRelease::unref(bool releaseImage) {
@@ -74,13 +82,13 @@
     AHardwareBuffer_Desc desc;
     AHardwareBuffer_describe(buffer, &desc);
     SkColorType colorType = GrAHardwareBufferUtils::GetSkColorTypeFromBufferFormat(desc.format);
+    // The following ref will be counteracted by Skia calling releaseProc, either during
+    // MakeFromTexture if there is a failure, or later when SkImage is discarded. It must
+    // be called before MakeFromTexture, otherwise Skia may remove HWUI's ref on failure.
+    ref();
     mImage = SkImage::MakeFromTexture(
             context, mBackendTexture, kTopLeft_GrSurfaceOrigin, colorType, kPremul_SkAlphaType,
             uirenderer::DataSpaceToColorSpace(dataspace), releaseProc, this);
-    if (mImage.get()) {
-        // The following ref will be counteracted by releaseProc, when SkImage is discarded.
-        ref();
-    }
 }
 
 void AutoBackendTextureRelease::newBufferContent(GrDirectContext* context) {