Commit a7a1388b authored by Sean Gilhuly's avatar Sean Gilhuly Committed by Commit Bot

Check before removing render pass resource

Replce the DCHECK in SkiaOutputSurfaceImpl::RemoveRenderPassResource()
with a test to see if the render pass has an entry in
|render_pass_image_cache_| before trying to remove it.

There isn't a known repro for this crash, but I suspect that using a
bypass to draw the quad in SkiaRenderer::DrawRenderPassQuad() can cause
the circumstances to arise. An entry for |render_pass_image_cache_| is
created in MakePromiseSkImageFromRenderPass(), but this is skipped if
a bypass quad is used, causing a difference between the render pass maps
in SkiaRenderer and SkiaOutputSurfaceImpl.

Bug: 959071
Change-Id: I4765f57eb30a7b4f14a559722e419ae526f79572
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1648326Reviewed-by: default avatarRobert Kroeger <rjkroege@chromium.org>
Commit-Queue: Sean Gilhuly <sgilhuly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667291}
parent 2ce55ee9
......@@ -449,7 +449,6 @@ sk_sp<SkImage> SkiaOutputSurfaceImpl::MakePromiseSkImageFromRenderPass(
}
void SkiaOutputSurfaceImpl::RemoveRenderPassResource(
std::vector<RenderPassId> ids) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
DCHECK(!ids.empty());
......@@ -458,18 +457,23 @@ void SkiaOutputSurfaceImpl::RemoveRenderPassResource(
image_contexts.reserve(ids.size());
for (const auto id : ids) {
auto it = render_pass_image_cache_.find(id);
DCHECK(it != render_pass_image_cache_.end());
it->second->image = nullptr;
image_contexts.push_back(std::move(it->second));
render_pass_image_cache_.erase(it);
// TODO(sgilhuly): This is a speculative fix for https://crbug.com/926194.
// Find out the cause of the crash and create a test that would repro it.
if (it != render_pass_image_cache_.end()) {
it->second->image = nullptr;
image_contexts.push_back(std::move(it->second));
render_pass_image_cache_.erase(it);
}
}
// impl_on_gpu_ is released on the GPU thread by a posted task from
// SkiaOutputSurfaceImpl::dtor. So it is safe to use base::Unretained.
auto callback = base::BindOnce(
&SkiaOutputSurfaceImplOnGpu::RemoveRenderPassResource,
base::Unretained(impl_on_gpu_.get()), std::move(image_contexts));
ScheduleGpuTask(std::move(callback), std::vector<gpu::SyncToken>());
if (!image_contexts.empty()) {
auto callback = base::BindOnce(
&SkiaOutputSurfaceImplOnGpu::RemoveRenderPassResource,
base::Unretained(impl_on_gpu_.get()), std::move(image_contexts));
ScheduleGpuTask(std::move(callback), std::vector<gpu::SyncToken>());
}
}
void SkiaOutputSurfaceImpl::CopyOutput(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment