M110: cc: fix crashes with shared image decode cache tasks

Adds additional image ref for a client if it has never had a decode/
upload task requested. Otherwise, the client will try to unref the
image more times than the image's ref count's number is and the process
will crash as the image will not be presented in the cache.

(cherry picked from commit c7aa039)

Bug: 1400712
Change-Id: Ia9268a7621b2860ca776cbd6c43880c7607ff379
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4110588
Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Original-Commit-Position: refs/heads/main@{#1084396}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4114024
Cr-Commit-Position: refs/branch-heads/5481@{#14}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}

cc/tiles/gpu_image_decode_cache.cc

@@ -1125,6 +1125,11 @@ ImageDecodeCache::TaskResult GpuImageDecodeCache::GetTaskForImageAndRefInternal(
     scoped_refptr<TileTask> task =
         GetTaskFromMapForClientId(client_id, image_data->upload.task_map);
     if (!task) {
+      // Given it's a new task for this |client_id|, the image must be reffed
+      // before creating a task - this ref is owned by the caller, and it is
+      // their responsibility to release it by calling UnrefImage.
+      RefImage(draw_image, cache_key);
+
       task = base::MakeRefCounted<ImageUploadTaskImpl>(
           this, draw_image,
           GetImageDecodeTaskAndRef(client_id, draw_image, tracing_info,
@@ -1151,6 +1156,10 @@ ImageDecodeCache::TaskResult GpuImageDecodeCache::GetTaskForImageAndRefInternal(
     scoped_refptr<TileTask> task = GetTaskFromMapForClientId(
         client_id, image_data->decode.stand_alone_task_map);
     if (!task) {
+      // Even though it's a new task for this client, we don't need to have
+      // additional reference here (which the caller is responsible for) as
+      // GetImageDecodeTaskAndRef does that for us.
+
       task = GetImageDecodeTaskAndRef(client_id, draw_image, tracing_info,
                                       task_type);
 #if DCHECK_IS_ON()

cc/tiles/gpu_image_decode_cache_unittest.cc

@@ -783,20 +783,19 @@ TEST_P(GpuImageDecodeCacheTest, GetTaskForImageSameImageDifferentClients) {
     // Ensure each client gets own task.
     EXPECT_NE(result1.task, result2.task);
 
-    DrawImage another_draw_image =
+    DrawImage draw_image2 =
         CreateDrawImageInternal(image, CreateMatrix(SkSize::Make(1.5f, 1.5f)));
-    EXPECT_EQ(another_draw_image.frame_index(), PaintImage::kDefaultFrameIndex);
+    EXPECT_EQ(draw_image2.frame_index(), PaintImage::kDefaultFrameIndex);
     ImageDecodeCache::TaskResult another_result = cache->GetTaskForImageAndRef(
-        kClientId1, another_draw_image, ImageDecodeCache::TracingInfo());
+        kClientId1, draw_image2, ImageDecodeCache::TracingInfo());
     EXPECT_TRUE(another_result.need_unref);
     EXPECT_TRUE(result1.task.get() == another_result.task.get());
 
-    DrawImage another_draw_image2 =
+    DrawImage draw_image3 =
         CreateDrawImageInternal(image, CreateMatrix(SkSize::Make(1.5f, 1.5f)));
-    EXPECT_EQ(another_draw_image2.frame_index(),
-              PaintImage::kDefaultFrameIndex);
+    EXPECT_EQ(draw_image3.frame_index(), PaintImage::kDefaultFrameIndex);
     ImageDecodeCache::TaskResult another_result2 = cache->GetTaskForImageAndRef(
-        kClientId2, another_draw_image, ImageDecodeCache::TracingInfo());
+        kClientId2, draw_image3, ImageDecodeCache::TracingInfo());
     EXPECT_TRUE(another_result2.need_unref);
     EXPECT_TRUE(result2.task.get() == another_result2.task.get());
 
@@ -844,12 +843,19 @@ TEST_P(GpuImageDecodeCacheTest, GetTaskForImageSameImageDifferentClients) {
       EXPECT_EQ(transfer_cache_helper_.num_of_entries(), 0u);
     }
 
+    EXPECT_TRUE(cache->IsInInUseCacheForTesting(draw_image));
+    EXPECT_TRUE(cache->IsInInUseCacheForTesting(draw_image2));
+    EXPECT_TRUE(cache->IsInInUseCacheForTesting(draw_image3));
+    cache->UnrefImage(draw_image);
+    EXPECT_TRUE(cache->IsInInUseCacheForTesting(draw_image));
     cache->UnrefImage(draw_image);
     EXPECT_TRUE(cache->IsInInUseCacheForTesting(draw_image));
     cache->UnrefImage(draw_image);
     EXPECT_TRUE(cache->IsInInUseCacheForTesting(draw_image));
     cache->UnrefImage(draw_image);
     EXPECT_FALSE(cache->IsInInUseCacheForTesting(draw_image));
+    EXPECT_FALSE(cache->IsInInUseCacheForTesting(draw_image2));
+    EXPECT_FALSE(cache->IsInInUseCacheForTesting(draw_image3));
 
     cache->ClearCache();
   }
@@ -2933,6 +2939,43 @@ TEST_P(GpuImageDecodeCacheTest, NonLazyImageUploadTaskCancelled) {
   cache->UnrefImage(draw_image);
 }
 
+TEST_P(GpuImageDecodeCacheTest,
+       NonLazyImageUploadTaskCancelledMultipleClients) {
+  if (do_yuv_decode_) {
+    // YUV bitmap images do not happen, so this test will always skip for YUV.
+    GTEST_SKIP();
+  }
+
+  auto cache = CreateCache();
+  const uint32_t client_id = cache->GenerateClientId();
+  const uint32_t client_id2 = cache->GenerateClientId();
+
+  PaintImage image = CreateBitmapImageInternal(GetNormalImageSize());
+  DrawImage draw_image = CreateDrawImageInternal(image);
+  auto result = cache->GetTaskForImageAndRef(client_id, draw_image,
+                                             ImageDecodeCache::TracingInfo());
+  EXPECT_TRUE(result.need_unref);
+  EXPECT_TRUE(result.task);
+  EXPECT_TRUE(result.task->dependencies().empty());
+
+  DrawImage draw_image2 = CreateDrawImageInternal(image);
+  auto result2 = cache->GetTaskForImageAndRef(client_id2, draw_image2,
+                                              ImageDecodeCache::TracingInfo());
+
+  EXPECT_TRUE(result2.need_unref);
+  EXPECT_TRUE(result2.task);
+  EXPECT_TRUE(result2.task->dependencies().empty());
+
+  TestTileTaskRunner::CancelTask(result.task.get());
+  TestTileTaskRunner::CompleteTask(result.task.get());
+
+  TestTileTaskRunner::CancelTask(result2.task.get());
+  TestTileTaskRunner::CompleteTask(result2.task.get());
+
+  cache->UnrefImage(draw_image);
+  cache->UnrefImage(draw_image2);
+}
+
 TEST_P(GpuImageDecodeCacheTest, NonLazyImageLargeImageNotColorConverted) {
   if (do_yuv_decode_) {
     // YUV bitmap images do not happen, so this test will always skip for YUV.