0

[cc][ClientSI] Move shared image destruction out of the worker thread

We intend to use a single ClientSharedImage to replace the two mailboxes
involved in OneCopyRasterBufferProvider::RasterBufferImpl. As a
pre-requisite of that effort, this CL moves a DestroySharedImage() call
out of the worker thread to avoid possible data race issues.

Bug: 1494911, 1499992
Change-Id: I45cc291684fdaa9cdc6f05f03f7e2f8ccea814cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5032830
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Mingjing Zhang <mjzhang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1226138}
This commit is contained in:
Mingjing Zhang
2023-11-17 16:12:05 +00:00
committed by Chromium LUCI CQ
parent 4dda9ec180
commit 2027c17aaa
2 changed files with 14 additions and 9 deletions

@ -111,6 +111,11 @@ OneCopyRasterBufferProvider::RasterBufferImpl::~RasterBufferImpl() {
// happened if the |after_raster_sync_token_| was set.
backing_->returned_sync_token = gpu::SyncToken();
}
if (should_destroy_shared_image_ && !mailbox_.IsZero()) {
backing_->worker_context_provider->SharedImageInterface()
->DestroySharedImage(before_raster_sync_token_, mailbox_);
mailbox_.SetZero();
}
backing_->mailbox = mailbox_;
}
@ -131,7 +136,8 @@ void OneCopyRasterBufferProvider::RasterBufferImpl::Playback(
&mailbox_, mailbox_texture_target_, mailbox_texture_is_overlay_candidate_,
before_raster_sync_token_, raster_source, raster_full_rect,
raster_dirty_rect, transform, resource_size_, format_, color_space_,
playback_settings, previous_content_id_, new_content_id);
playback_settings, previous_content_id_, new_content_id,
should_destroy_shared_image_);
}
bool OneCopyRasterBufferProvider::RasterBufferImpl::
@ -289,7 +295,8 @@ gpu::SyncToken OneCopyRasterBufferProvider::PlaybackAndCopyOnWorkerThread(
const gfx::ColorSpace& color_space,
const RasterSource::PlaybackSettings& playback_settings,
uint64_t previous_content_id,
uint64_t new_content_id) {
uint64_t new_content_id,
bool& should_destroy_shared_image) {
std::unique_ptr<StagingBuffer> staging_buffer =
staging_pool_.AcquireStagingBuffer(resource_size, format,
previous_content_id);
@ -308,16 +315,12 @@ gpu::SyncToken OneCopyRasterBufferProvider::PlaybackAndCopyOnWorkerThread(
staging_buffer.get(), raster_source, raster_full_rect, format,
resource_size, mailbox, mailbox_texture_target,
mailbox_texture_is_overlay_candidate, sync_token, color_space);
} else {
} else if (!mailbox->IsZero()) {
// If we failed to put data in the staging buffer
// (https://crbug.com/554541), then we don't have anything to give to copy
// into the resource. We report a zero mailbox that will result in
// checkerboarding, and be treated as OOM which should retry.
if (!mailbox->IsZero()) {
worker_context_provider_->SharedImageInterface()->DestroySharedImage(
sync_token, *mailbox);
mailbox->SetZero();
}
should_destroy_shared_image = true;
}
staging_pool_.ReleaseStagingBuffer(std::move(staging_buffer));

@ -86,7 +86,8 @@ class CC_EXPORT OneCopyRasterBufferProvider : public RasterBufferProvider {
const gfx::ColorSpace& color_space,
const RasterSource::PlaybackSettings& playback_settings,
uint64_t previous_content_id,
uint64_t new_content_id);
uint64_t new_content_id,
bool& should_destroy_shared_image);
protected:
void Flush() override;
@ -133,6 +134,7 @@ class CC_EXPORT OneCopyRasterBufferProvider : public RasterBufferProvider {
// A SyncToken to be returned from the worker thread, and waited on before
// using the rastered resource.
gpu::SyncToken after_raster_sync_token_;
bool should_destroy_shared_image_ = false;
};
// Returns true if data has successfully put in the staging buffer.