Commit 834dd8f9 authored by Brian Ho's avatar Brian Ho Committed by Chromium LUCI CQ

viz: Clear current_image_ if there is no primary plane

Promoting a quad using the fullscreen overlay strategy results in a
segfault when using SkiaRenderer. This is because for fullscreen
overlays, we remove the frame's OutputSurfaceOverlayPlane [1]. In
turn, this means we call SchedulePrimaryPlane with a null frame which
sets a flag [2] that prevents |current_image_| from being cleared
later in PostSubBuffer [3].

On the next frame, there is no damage on the primary plane because
the entire contents have been promoted to a fullscreen overlay. Thus,
we never render the root render pass [4] which results in
|SkiaOutputDeviceBufferQueue::current_image_| never being set [5].
Since we never set a new |current_image_| and |current_image_| wasn't
cleared as mentioned in the first paragraph, we end up re-using the
|current_image_| from the last frame. This is where we get our
segfault because various |current_image_| members are reset after
their first use [6].

This CL fixes this issue by explicitly clearing |current_image_| (and
adding it back to the |available_images_| queue) if the output surface
plane is null. This makes sense because we shouldn't be presenting an
image anyways since the primary plane is empty.

[1] https://source.chromium.org/chromium/chromium/src/+/master:components/viz/service/display/overlay_processor_using_strategy.cc;l=232;drc=c6b95e6511da9afc58debb10a67fceb6b230341a
[2] https://source.chromium.org/chromium/chromium/src/+/master:components/viz/service/display_embedder/skia_output_device_buffer_queue.cc;l=218;drc=58ff9e17fba96d56e68842b570b37d88a70d7f73
[3] https://source.chromium.org/chromium/chromium/src/+/master:components/viz/service/display_embedder/skia_output_device_buffer_queue.cc;l=332;drc=c6b95e6511da9afc58debb10a67fceb6b230341a
[4] https://source.chromium.org/chromium/chromium/src/+/master:components/viz/service/display/direct_renderer.cc;l=383;drc=0348a1421914f3609d287cb5645a856f5eb2542c
[5] https://source.chromium.org/chromium/chromium/src/+/master:components/viz/service/display_embedder/skia_output_device_buffer_queue.cc;l=506;drc=619ddf10544160ce22caec007a31873b2e28f691
[6] https://source.chromium.org/chromium/chromium/src/+/master:components/viz/service/display_embedder/output_presenter.cc;l=103;drc=b621c956cfd2712c38f05b2240b0a5780066a07c

Bug: 1156158
Change-Id: I2f030652166212cf6e28073c660e07526e67dcce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2585811Reviewed-by: default avatarPeng Huang <penghuang@chromium.org>
Commit-Queue: Brian Ho <hob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836700}
parent e2eec62f
......@@ -217,6 +217,16 @@ void SkiaOutputDeviceBufferQueue::SchedulePrimaryPlane(
image == submitted_image_);
} else {
current_frame_has_no_primary_plane_ = true;
// Even if there is no primary plane, |current_image_| may be non-null if
// an overlay just transitioned from an underlay strategy to a fullscreen
// strategy (e.g. a the media controls disappearing on a fullscreen video).
// In this case, there is still damage which triggers a render pass, but
// since we promote via fullscreen, we remove the primary plane in the end.
// We need to recycle |current_image_| to avoid a use-after-free error.
if (current_image_) {
available_images_.push_back(current_image_);
current_image_ = nullptr;
}
}
}
......
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