Missing surfaces on gpu crash |
|||||||||
Issue descriptionIf the GPU crashes we can embed a SurfaceId for a Surface that has been deleted. Here is what happens. When the GPU crashes the DirectLayerTreeFrameSink is destroyed along with the CompositorFrameSinkSupport it owns. This deletes the display Surface. Any renderer Surfaces that were referenced by the display Surface become unreachable at this point. After this DelegatedFrameHost::DidCreateNewRendererCompositorFrameSink() is called which destroys the renderer CompositorFrameSinkSupport. The renderer Surface is unreachable and so it's deleted. However, the renderers SurfaceLayer is still set to the now deleted SurfaceId. When ui::Compositor submits the next display CompositorFrame, it will try to embed the deleted SurfaceId. This increments the missing surfaces UMA stat and obviously nothing shows up. I'm not sure on if this is a problem beyond that, since even if the Surface wasn't deleted the textures would be missing? The screen flickers white briefly the GPU restarts (1) with surface sequences on (2) with surface references on and this problem and (3) with surface references on and this problem fixed. There is DelegatedFrameHost::OnLostResources() which would reset the renderer SurfaceLayer. This is called on gpu crash for Chrome OS but not Linux desktop. I don't know why this is different? Alternatively, we could call DelegatedFrameHost::EvictDelegatedFrame() in DelegatedFrameHost::DidCreateNewRendererCompositorFrameSink(). This also fixes the problem. piman: Any ideas on what the right approach is here? Should OnLostResources() get called when the GPU crashes on all platforms and not just Chrome OS?
,
Sep 5 2017
,
Sep 11 2017
I just realized this landed after the branch point. Without this change Compositing.SurfaceAggregator.SurfaceDrawQuad.MissingSurface won't be a useful metric.
,
Sep 11 2017
Thanks for the fix - have you already verified and confirmed the fix in Canary? Can you briefly mention why it's critical to have this in M62 (what are issues if we don't have MissingSurface for M62) and is this already present in M60 and M61?
,
Sep 11 2017
I've verified the fix in canary. We changed the way surface lifetime management works for M62 and I added some UMA stats to see if any regressions occur. As such, this isn't applicable to anything before M62. A Surface owns a CompositorFrame of a specific size from a specific client. If a surface gets deleted too early then we can't draw that area anymore and it will be blank. This typically manifests as a flash of the background color when switching tabs, resizing, etc. The metric tracks how often this happens. There was a bug where we weren't clearing the SurfaceLayer on GPU crash and we were embedding a deleted surface. From a user standpoint it wouldn't be noticable, because when the GPU crashes we can't draw the old surface anyways and it flashes the background color. The problem is that the missing surface metric gets incremented. We can't compare the metric pre vs post change anymore, which might indicate some other problem exists, without this fix.
,
Sep 12 2017
Your change meets the bar and is auto-approved for M62. Please go ahead and merge the CL to branch 3202 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/827fd7409fd61868f5357b673b910c50d1b365dc commit 827fd7409fd61868f5357b673b910c50d1b365dc Author: kylechar <kylechar@chromium.org> Date: Tue Sep 12 18:41:01 2017 Evict renderer Surface on gpu restart. The renderer SurfaceLayer and some other Surface related information wasn't always being cleared when the gpu restarted. Ensure that happens. Bug: 761131 Change-Id: I4c53d4f0ec288bf8bf9cee32af1174caa8256f4b Reviewed-on: https://chromium-review.googlesource.com/646515 Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: kylechar <kylechar@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#499256}(cherry picked from commit ac4bbf4d312142ceb4824a46dcec1ae53961054d) Reviewed-on: https://chromium-review.googlesource.com/663478 Reviewed-by: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#175} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/827fd7409fd61868f5357b673b910c50d1b365dc/content/browser/renderer_host/delegated_frame_host.cc
,
Sep 14 2017
,
Oct 25 2017
It looks like https://crrev.com/c/654198 stopped clearing the SurfaceLayer when we destroy the CompositorFrameSinkSupport.
,
Oct 25 2017
,
Jul 16
This got fixed again at some point. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by bugdroid1@chromium.org
, Sep 1 2017