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);