[go: nahoru, domu]

Revert "Revert "Refactor DestroySharedImage() for raster buffer providers""

This reverts commit 27e5288576bc4c4d226dff17d26fb6a9f1e38cdb.

Reason for revert:
LUCI Bisection has identified this change as the culprit of a build failure. See the analysis: https://ci.chromium.org/ui/p/chromium/bisection/compile-analysis/b/8764795698775102353

Sample failed build: https://ci.chromium.org/b/8764795698775102353

If this is a false positive, please report it at https://bugs.chromium.org/p/chromium/issues/entry?comment=Analysis%3A+https%3A%2F%2Fci.chromium.org%2Fui%2Fp%2Fchromium%2Fbisection%2Fcompile-analysis%2Fb%2F8764795698775102353&components=Tools%3ETest%3EFindit&labels=LUCI-Bisection-Wrong%2CPri-3%2CType-Bug&status=Available&summary=Wrongly+blamed+https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fsrc%2F%2B%2F5017748

Original change's description:
> Revert "Refactor DestroySharedImage() for raster buffer providers"
>
> This reverts commit f685881ef6cbe4cece342b7d2a8c81358e5b5d8c.
>
> Reason for revert: Causing crashes on Windows GPU bots in https://crbug.com/1501374
>
> Original change's description:
> > Refactor DestroySharedImage() for raster buffer providers
> >
> > As part of a large scale refactorization that aims at introducing
> > ClientSharedImage, this CL refactors several raster buffer providers
> > to 1. use the new version of DestroySharedImage() (that accepts a
> > ClientSharedImage parameter instead of a mailbox), and 2. make sure
> > the ClientSharedImage created by CreateSharedImage() could persist
> > until the calling of DestroySharedImage().
> >
> > Bug: 1494911, 1499992
> > Change-Id: Ib1de7766fc30fd45516b22ec590711a6f34da31c
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5003181
> > Commit-Queue: Mingjing Zhang <mjzhang@chromium.org>
> > Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1222586}
>
> Bug: 1494911, 1499992, 1501374
> Change-Id: I4d90425fc1aa1c057f1b865d14a1f1f1d32ce0d2
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017748
> Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Kyle Charbonneau <kylechar@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1222991}
>

Bug: 1494911, 1499992, 1501374
Change-Id: I8d1160e02fbea140a7c306f0d9de8879d10ae9bc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5021207
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Owners-Override: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Bot-Commit: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1222996}
diff --git a/cc/raster/zero_copy_raster_buffer_provider.cc b/cc/raster/zero_copy_raster_buffer_provider.cc
index 9506ac53..7b832cb 100644
--- a/cc/raster/zero_copy_raster_buffer_provider.cc
+++ b/cc/raster/zero_copy_raster_buffer_provider.cc
@@ -42,12 +42,15 @@
 class ZeroCopyGpuBacking : public ResourcePool::GpuBacking {
  public:
   ~ZeroCopyGpuBacking() override {
-    if (mailbox.IsZero())
+    if (!shared_image) {
       return;
+    }
     if (returned_sync_token.HasData())
-      shared_image_interface->DestroySharedImage(returned_sync_token, mailbox);
+      shared_image_interface->DestroySharedImage(returned_sync_token,
+                                                 std::move(shared_image));
     else if (mailbox_sync_token.HasData())
-      shared_image_interface->DestroySharedImage(mailbox_sync_token, mailbox);
+      shared_image_interface->DestroySharedImage(mailbox_sync_token,
+                                                 std::move(shared_image));
   }
 
   void OnMemoryDump(
@@ -56,10 +59,11 @@
       uint64_t tracing_process_id,
       int importance) const override {
     if (base::FeatureList::IsEnabled(kAlwaysUseMappableSIForZeroCopyRaster)) {
-      if (mailbox.IsZero()) {
+      if (!shared_image) {
         return;
       }
-      auto mapping = shared_image_interface->MapSharedImage(mailbox);
+      auto mapping =
+          shared_image_interface->MapSharedImage(shared_image->mailbox());
       if (!mapping) {
         return;
       }
@@ -106,12 +110,12 @@
     // checkerboarding.
     if (base::FeatureList::IsEnabled(kAlwaysUseMappableSIForZeroCopyRaster)) {
       CHECK(!gpu_memory_buffer_);
-      if (backing_->mailbox.IsZero()) {
+      if (!backing_->shared_image) {
         return;
       }
     } else {
       if (!gpu_memory_buffer_) {
-        DCHECK(backing_->mailbox.IsZero());
+        DCHECK(!backing_->shared_image);
         return;
       }
     }
@@ -122,21 +126,21 @@
     // TODO(danakj): This could be done with the worker context in Playback. Do
     // we need to do things in IsResourceReadyToDraw() and OrderingBarrier then?
     gpu::SharedImageInterface* sii = backing_->shared_image_interface;
-    if (backing_->mailbox.IsZero()) {
+    if (!backing_->shared_image) {
       CHECK(
           !base::FeatureList::IsEnabled(kAlwaysUseMappableSIForZeroCopyRaster));
       uint32_t usage = gpu::SHARED_IMAGE_USAGE_DISPLAY_READ |
                        gpu::SHARED_IMAGE_USAGE_SCANOUT;
       // Make a mailbox for export of the GpuMemoryBuffer to the display
       // compositor.
-      auto client_shared_image = sii->CreateSharedImage(
+      backing_->shared_image = sii->CreateSharedImage(
           format_, resource_size_, resource_color_space_,
           kTopLeft_GrSurfaceOrigin, kPremul_SkAlphaType, usage,
           "ZeroCopyRasterTile", gpu_memory_buffer_->CloneHandle());
-      CHECK(client_shared_image);
-      backing_->mailbox = client_shared_image->mailbox();
+      CHECK(backing_->shared_image);
     } else {
-      sii->UpdateSharedImage(backing_->returned_sync_token, backing_->mailbox);
+      sii->UpdateSharedImage(backing_->returned_sync_token,
+                             backing_->shared_image->mailbox());
     }
 
     backing_->mailbox_sync_token = sii->GenUnverifiedSyncToken();
@@ -165,25 +169,24 @@
       gpu::SharedImageInterface* sii = backing_->shared_image_interface;
 
       // Create a MappableSI if necessary.
-      if (backing_->mailbox.IsZero()) {
+      if (!backing_->shared_image) {
         uint32_t usage = gpu::SHARED_IMAGE_USAGE_DISPLAY_READ |
                          gpu::SHARED_IMAGE_USAGE_SCANOUT;
-        auto client_shared_image = sii->CreateSharedImage(
+        backing_->shared_image = sii->CreateSharedImage(
             format_, resource_size_, resource_color_space_,
             kTopLeft_GrSurfaceOrigin, kPremul_SkAlphaType, usage,
             "ZeroCopyRasterTile", gpu::kNullSurfaceHandle, kBufferUsage);
-        if (!client_shared_image) {
+        if (!backing_->shared_image) {
           LOG(ERROR) << "Creation of MappableSharedImage failed.";
           return;
         }
-        backing_->mailbox = client_shared_image->mailbox();
       }
 
-      mapping = sii->MapSharedImage(backing_->mailbox);
+      mapping = sii->MapSharedImage(backing_->shared_image->mailbox());
       if (!mapping) {
         LOG(ERROR) << "MapSharedImage Failed.";
-        sii->DestroySharedImage(gpu::SyncToken(), backing_->mailbox);
-        backing_->mailbox.SetZero();
+        sii->DestroySharedImage(gpu::SyncToken(),
+                                std::move(backing_->shared_image));
         return;
       }
       memory = mapping->Memory(0);