New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 776050 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 732572



Sign in to add a comment

viz: Handle context loss correctly

Project Member Reported by kylec...@chromium.org, Oct 18 2017

Issue description

We need to handle context loss correctly for viz. There is the case when the GPU processes crashes and all contexts are lost. We can also have individual contexts lost without the GPU process crashing, for example when an OOM error occurs. 
 
Another concern that fsamuel raised is the order around context loss. There are a lot of display compositor related connections from browser to gpu:
1. HostFrameSinkManager <-> FrameSinkManagerImpl
2. GpuChannel/ContextProvider
3. ClientLayerTreeFrameSink <-> [Root]CompositorFrameSinkImpl

The order in which they find out about about the connection loss could be problematic.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c5cbe85f310704b191dd6b6ad61e75c45c337fee

commit c5cbe85f310704b191dd6b6ad61e75c45c337fee
Author: kylechar <kylechar@chromium.org>
Date: Mon Oct 23 16:55:02 2017

Make HostFrameSinkManager handle GPU crash.

This CL adds code to HostFrameSinkManager to handle the case of the GPU
crashing. The mojom::FrameSinkManager connection will be lost when this
happens. CompositorFrameSinkImpl will be destroyed for all clients so
change state internally.

Add code to reregister FrameSinkIds and FrameSink hierarchy when
HostFrameSinkManager reconnects. VizProcessTransportFactory installs a
callback so it can also handle GPU crash.

Add test for HostFrameSinkManager handling of GPU crash and
reconnection.

Bug:  770833 ,  776050 
Change-Id: I400c658b62d09a6d469c37124a5c3aed5ce7f028
Reviewed-on: https://chromium-review.googlesource.com/731266
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510823}
[modify] https://crrev.com/c5cbe85f310704b191dd6b6ad61e75c45c337fee/components/viz/host/BUILD.gn
[modify] https://crrev.com/c5cbe85f310704b191dd6b6ad61e75c45c337fee/components/viz/host/DEPS
[modify] https://crrev.com/c5cbe85f310704b191dd6b6ad61e75c45c337fee/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/c5cbe85f310704b191dd6b6ad61e75c45c337fee/components/viz/host/host_frame_sink_manager.h
[rename] https://crrev.com/c5cbe85f310704b191dd6b6ad61e75c45c337fee/components/viz/host/host_frame_sink_manager_unittest.cc
[modify] https://crrev.com/c5cbe85f310704b191dd6b6ad61e75c45c337fee/components/viz/test/mock_compositor_frame_sink_client.cc
[modify] https://crrev.com/c5cbe85f310704b191dd6b6ad61e75c45c337fee/components/viz/test/mock_compositor_frame_sink_client.h
[modify] https://crrev.com/c5cbe85f310704b191dd6b6ad61e75c45c337fee/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/c5cbe85f310704b191dd6b6ad61e75c45c337fee/content/browser/compositor/viz_process_transport_factory.h

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71aadccf16627d5b2451756a551d5c2012f6f95f

commit 71aadccf16627d5b2451756a551d5c2012f6f95f
Author: Jamie Madill <jmadill@chromium.org>
Date: Mon Oct 23 19:38:42 2017

Revert "Make HostFrameSinkManager handle GPU crash."

This reverts commit c5cbe85f310704b191dd6b6ad61e75c45c337fee.

Reason for revert:

Seems to cause a reproducible crash on the GPU.FYI Windows bots:

https://build.chromium.org/p/chromium.gpu.fyi/builders/Win7%20Release%20%28NVIDIA%29/builds/31825
https://build.chromium.org/p/tryserver.chromium.angle/builders/win_angle_rel_ng/builds/7359

Failing test:

context_lost_tests on NVIDIA GPU

Crash text:

Received fatal exception EXCEPTION_ACCESS_VIOLATION
Backtrace:
	viz::GpuServiceImpl::Crash [0x67B0E67F+223]
	viz::mojom::GpuServiceStubDispatch::Accept [0x6614277A+1056]
	viz::mojom::GpuServiceStub<mojo::RawPtrImplRefTraits<viz::mojom::GpuService> >::Accept [0x67B111D3+19]
	mojo::InterfaceEndpointClient::HandleValidatedMessage [0x66DAAC98+608]
	mojo::FilterChain::Accept [0x66DADFC3+129]
	mojo::InterfaceEndpointClient::HandleIncomingMessage [0x66DAB9F6+104]
	mojo::internal::MultiplexRouter::ProcessIncomingMessage [0x66DA27A8+694]
	mojo::internal::MultiplexRouter::Accept [0x66DA22B9+295]
	mojo::FilterChain::Accept [0x66DADFC3+129]
	mojo::Connector::ReadSingleMessage [0x66DA85E6+376]
	mojo::Connector::ReadAllAvailableMessages [0x66DA8C99+85]
	mojo::Connector::OnHandleReadyInternal [0x66DA8B6F+135]
<snip>

Bug:  770833 ,  776050 

Original change's description:
> Make HostFrameSinkManager handle GPU crash.
> 
> This CL adds code to HostFrameSinkManager to handle the case of the GPU
> crashing. The mojom::FrameSinkManager connection will be lost when this
> happens. CompositorFrameSinkImpl will be destroyed for all clients so
> change state internally.
> 
> Add code to reregister FrameSinkIds and FrameSink hierarchy when
> HostFrameSinkManager reconnects. VizProcessTransportFactory installs a
> callback so it can also handle GPU crash.
> 
> Add test for HostFrameSinkManager handling of GPU crash and
> reconnection.
> 
> Bug:  770833 ,  776050 
> Change-Id: I400c658b62d09a6d469c37124a5c3aed5ce7f028
> Reviewed-on: https://chromium-review.googlesource.com/731266
> Commit-Queue: kylechar <kylechar@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#510823}

TBR=danakj@chromium.org,kylechar@chromium.org

Change-Id: I4f180251de60bf7bdaa4d729dc8257ba9b385b8d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  770833 ,  776050 
Reviewed-on: https://chromium-review.googlesource.com/734103
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510875}
[modify] https://crrev.com/71aadccf16627d5b2451756a551d5c2012f6f95f/components/viz/host/BUILD.gn
[modify] https://crrev.com/71aadccf16627d5b2451756a551d5c2012f6f95f/components/viz/host/DEPS
[modify] https://crrev.com/71aadccf16627d5b2451756a551d5c2012f6f95f/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/71aadccf16627d5b2451756a551d5c2012f6f95f/components/viz/host/host_frame_sink_manager.h
[rename] https://crrev.com/71aadccf16627d5b2451756a551d5c2012f6f95f/components/viz/host/host_frame_sink_manager_unittests.cc
[modify] https://crrev.com/71aadccf16627d5b2451756a551d5c2012f6f95f/components/viz/test/mock_compositor_frame_sink_client.cc
[modify] https://crrev.com/71aadccf16627d5b2451756a551d5c2012f6f95f/components/viz/test/mock_compositor_frame_sink_client.h
[modify] https://crrev.com/71aadccf16627d5b2451756a551d5c2012f6f95f/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/71aadccf16627d5b2451756a551d5c2012f6f95f/content/browser/compositor/viz_process_transport_factory.h

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69f228125ab1819575f4dfa1f61b7ac0d6312eee

commit 69f228125ab1819575f4dfa1f61b7ac0d6312eee
Author: Jamie Madill <jmadill@chromium.org>
Date: Mon Oct 23 20:54:11 2017

Revert "Revert "Make HostFrameSinkManager handle GPU crash.""

This reverts commit 71aadccf16627d5b2451756a551d5c2012f6f95f.

Reason for revert: Did not seem to clear up the crash.

Original change's description:
> Revert "Make HostFrameSinkManager handle GPU crash."
> 
> This reverts commit c5cbe85f310704b191dd6b6ad61e75c45c337fee.
> 
> Reason for revert:
> 
> Seems to cause a reproducible crash on the GPU.FYI Windows bots:
> 
> https://build.chromium.org/p/chromium.gpu.fyi/builders/Win7%20Release%20%28NVIDIA%29/builds/31825
> https://build.chromium.org/p/tryserver.chromium.angle/builders/win_angle_rel_ng/builds/7359
> 
> Failing test:
> 
> context_lost_tests on NVIDIA GPU
> 
> Crash text:
> 
> Received fatal exception EXCEPTION_ACCESS_VIOLATION
> Backtrace:
> 	viz::GpuServiceImpl::Crash [0x67B0E67F+223]
> 	viz::mojom::GpuServiceStubDispatch::Accept [0x6614277A+1056]
> 	viz::mojom::GpuServiceStub<mojo::RawPtrImplRefTraits<viz::mojom::GpuService> >::Accept [0x67B111D3+19]
> 	mojo::InterfaceEndpointClient::HandleValidatedMessage [0x66DAAC98+608]
> 	mojo::FilterChain::Accept [0x66DADFC3+129]
> 	mojo::InterfaceEndpointClient::HandleIncomingMessage [0x66DAB9F6+104]
> 	mojo::internal::MultiplexRouter::ProcessIncomingMessage [0x66DA27A8+694]
> 	mojo::internal::MultiplexRouter::Accept [0x66DA22B9+295]
> 	mojo::FilterChain::Accept [0x66DADFC3+129]
> 	mojo::Connector::ReadSingleMessage [0x66DA85E6+376]
> 	mojo::Connector::ReadAllAvailableMessages [0x66DA8C99+85]
> 	mojo::Connector::OnHandleReadyInternal [0x66DA8B6F+135]
> <snip>
> 
> Bug:  770833 ,  776050 
> 
> Original change's description:
> > Make HostFrameSinkManager handle GPU crash.
> > 
> > This CL adds code to HostFrameSinkManager to handle the case of the GPU
> > crashing. The mojom::FrameSinkManager connection will be lost when this
> > happens. CompositorFrameSinkImpl will be destroyed for all clients so
> > change state internally.
> > 
> > Add code to reregister FrameSinkIds and FrameSink hierarchy when
> > HostFrameSinkManager reconnects. VizProcessTransportFactory installs a
> > callback so it can also handle GPU crash.
> > 
> > Add test for HostFrameSinkManager handling of GPU crash and
> > reconnection.
> > 
> > Bug:  770833 ,  776050 
> > Change-Id: I400c658b62d09a6d469c37124a5c3aed5ce7f028
> > Reviewed-on: https://chromium-review.googlesource.com/731266
> > Commit-Queue: kylechar <kylechar@chromium.org>
> > Reviewed-by: danakj <danakj@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#510823}
> 
> TBR=danakj@chromium.org,kylechar@chromium.org
> 
> Change-Id: I4f180251de60bf7bdaa4d729dc8257ba9b385b8d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  770833 ,  776050 
> Reviewed-on: https://chromium-review.googlesource.com/734103
> Reviewed-by: Jamie Madill <jmadill@chromium.org>
> Commit-Queue: Jamie Madill <jmadill@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#510875}

TBR=danakj@chromium.org,jmadill@chromium.org,kylechar@chromium.org

Change-Id: I91c25fb0ee253606e3035b90bec3447924b26d7d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  770833 ,  776050 
Reviewed-on: https://chromium-review.googlesource.com/734083
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510910}
[modify] https://crrev.com/69f228125ab1819575f4dfa1f61b7ac0d6312eee/components/viz/host/BUILD.gn
[modify] https://crrev.com/69f228125ab1819575f4dfa1f61b7ac0d6312eee/components/viz/host/DEPS
[modify] https://crrev.com/69f228125ab1819575f4dfa1f61b7ac0d6312eee/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/69f228125ab1819575f4dfa1f61b7ac0d6312eee/components/viz/host/host_frame_sink_manager.h
[rename] https://crrev.com/69f228125ab1819575f4dfa1f61b7ac0d6312eee/components/viz/host/host_frame_sink_manager_unittest.cc
[modify] https://crrev.com/69f228125ab1819575f4dfa1f61b7ac0d6312eee/components/viz/test/mock_compositor_frame_sink_client.cc
[modify] https://crrev.com/69f228125ab1819575f4dfa1f61b7ac0d6312eee/components/viz/test/mock_compositor_frame_sink_client.h
[modify] https://crrev.com/69f228125ab1819575f4dfa1f61b7ac0d6312eee/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/69f228125ab1819575f4dfa1f61b7ac0d6312eee/content/browser/compositor/viz_process_transport_factory.h

Comment 5 by laforge@google.com, Nov 8 2017

Components: -Internals>Viz Internals>Services>Viz
Migrating from Internals>Viz to Internals>Services>Viz.
Blocking: -601863 732572
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc8a048f8c1d5398ac84c6583192466f8877764b

commit dc8a048f8c1d5398ac84c6583192466f8877764b
Author: kylechar <kylechar@chromium.org>
Date: Wed Jan 03 17:27:57 2018

viz: Add shared main thread context provider code.

In VizProcessTransportFactory we already have a context provider that is
shared on the main thread. It's used for all ui::Compositors already.
Implement VizProcessTransportFactory::SharedMainThreadContextProvider()
to return that context provider.

Bug:  776050 
Change-Id: Ie113fab9d2b49042f173138e4f9f143b191ae848
Reviewed-on: https://chromium-review.googlesource.com/827687
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526721}
[modify] https://crrev.com/dc8a048f8c1d5398ac84c6583192466f8877764b/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/dc8a048f8c1d5398ac84c6583192466f8877764b/content/browser/compositor/viz_process_transport_factory.h

Owner: kylec...@chromium.org
Status: Started (was: Available)
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e0f8ad9539d9b2bd8d027a3a89ec39efa7734cf

commit 1e0f8ad9539d9b2bd8d027a3a89ec39efa7734cf
Author: kylechar <kylechar@chromium.org>
Date: Wed Feb 07 19:11:15 2018

viz: Recover from lost GL context.

If HostFrameSinkManager gets a request to create a
[Root]CompositorFrameSink but has already created one, then first send
an IPC to destroy the old before sending the new request. When GL
context is lost without the GPU process crashing the
ClientLayerTreeFrameSink will be recreated and a new request will be
sent.

Add tests for HostFrameSinkManager to make sure this works as expected.
Also add RootCompositorFrameSinkData in the tests to contain the four
interfaces needed when creating a RootCompositorFrameSink. This reduces
a lot of test boilerplate.

TBR: fsamuel@chromium.org
Bug:  776050 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I059c802443669fc7e3b6f2a2489632299aae1512
Reviewed-on: https://chromium-review.googlesource.com/904859
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535090}
[modify] https://crrev.com/1e0f8ad9539d9b2bd8d027a3a89ec39efa7734cf/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/1e0f8ad9539d9b2bd8d027a3a89ec39efa7734cf/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/1e0f8ad9539d9b2bd8d027a3a89ec39efa7734cf/components/viz/host/host_frame_sink_manager_unittest.cc
[modify] https://crrev.com/1e0f8ad9539d9b2bd8d027a3a89ec39efa7734cf/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/1e0f8ad9539d9b2bd8d027a3a89ec39efa7734cf/content/browser/renderer_host/render_widget_host_impl.cc

Status: Fixed (was: Started)
Status: Started (was: Fixed)
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ab7fb0d9584d0eb8be71f11194d81bd1f6885d5

commit 0ab7fb0d9584d0eb8be71f11194d81bd1f6885d5
Author: kylechar <kylechar@chromium.org>
Date: Fri Apr 27 22:39:14 2018

viz: Fix software compositing fallback.

Add some missing cases where we want to fallback on software compositing
to VizProcessTransportFactory.

1. When GPU_FEATURE_TYPE_GPU_COMPOSITING is blacklisted in
   GpuFeatureInfo.
2. When ContextProvider creation fatally fails.

This CL also cleans up the logic to make it easier to follow and removes
an unnecessary subclass from VizProcessTransportFactory.

Bug:  776050 
Change-Id: I7d0e65ac224e649cad786246486fc40c9c841f27
Reviewed-on: https://chromium-review.googlesource.com/1030829
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554538}
[modify] https://crrev.com/0ab7fb0d9584d0eb8be71f11194d81bd1f6885d5/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/0ab7fb0d9584d0eb8be71f11194d81bd1f6885d5/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/0ab7fb0d9584d0eb8be71f11194d81bd1f6885d5/content/browser/compositor/viz_process_transport_factory.h

Status: Fixed (was: Started)

Sign in to add a comment