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

Issue 770833 link

Starred by 7 users

Issue metadata

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


Sign in to add a comment

Get VizDisplayCompositor working behind flag in Linux desktop Chrome

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

Issue description

We want to have a flag, something like --enable-viz, that enables viz on Linux desktop Chrome. Enabling viz would be the following:
1. viz process has a display compositor thread. FrameSinkManagerImpl is created here instead of in the browser UI thread.
2. ui::Compositor LayerTreeFrameSink connects to viz process over Mojo to submit CompositorFrames.
3. Renderer connects to viz process to submit CompositorFrames.
 
Description: Show this description
Project Member

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

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

commit 060896478f5b1bf77603d47845508547b99541c9
Author: kylechar <kylechar@chromium.org>
Date: Wed Oct 04 00:24:37 2017

Simplify DisplayPrivate interface.

There is no need to send IPC messages for ResizeDisplay() or
SetLocalSurfaceId() on viz::mojom::DisplayPrivate. This information is
already contained in the CompositorFrame that gets submitted on the
associated viz::mojom::CompositorFrameSink interface.

Bug:  770833 
Change-Id: Ia8d5f98b8feb90ac5d54a4d4b9a744e96f8092ae
Reviewed-on: https://chromium-review.googlesource.com/699231
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506234}
[modify] https://crrev.com/060896478f5b1bf77603d47845508547b99541c9/components/viz/service/frame_sinks/compositor_frame_sink_support.h
[modify] https://crrev.com/060896478f5b1bf77603d47845508547b99541c9/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/060896478f5b1bf77603d47845508547b99541c9/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h
[modify] https://crrev.com/060896478f5b1bf77603d47845508547b99541c9/services/ui/ws/compositor_frame_sink_client_binding.cc
[modify] https://crrev.com/060896478f5b1bf77603d47845508547b99541c9/services/ui/ws/compositor_frame_sink_client_binding.h
[modify] https://crrev.com/060896478f5b1bf77603d47845508547b99541c9/services/viz/privileged/interfaces/compositing/frame_sink_manager.mojom

Blockedon: 732900
Project Member

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

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

commit b82991b2aac88ca30d4de98ced305665181e682b
Author: kylechar <kylechar@chromium.org>
Date: Wed Oct 04 22:40:08 2017

Allow ClientLayerTreeFrameSink to use associated interfaces.

Root CompositorFrameSinks don't use mojom::CompositorFrameSinkPtr, they
use mojom::CompositorFrameSinkAssociatedPtr because it's paired with
mojom::DisplayPrivateAssociatedPtr. In order to reuse
ClientLayerTreeFrameSink for roots it needs to work with both.

Bug:  770833 
Change-Id: Iba0f020e787009d91213c071e2eb78a5a402a8c8
Reviewed-on: https://chromium-review.googlesource.com/700421
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506547}
[modify] https://crrev.com/b82991b2aac88ca30d4de98ced305665181e682b/components/viz/client/client_layer_tree_frame_sink.cc
[modify] https://crrev.com/b82991b2aac88ca30d4de98ced305665181e682b/components/viz/client/client_layer_tree_frame_sink.h
[modify] https://crrev.com/b82991b2aac88ca30d4de98ced305665181e682b/content/renderer/mus/renderer_window_tree_client.cc
[modify] https://crrev.com/b82991b2aac88ca30d4de98ced305665181e682b/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/b82991b2aac88ca30d4de98ced305665181e682b/ui/aura/mus/window_port_mus.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 10 2017

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

commit 34f44db294662fd8a8bec001244bac266c0445c1
Author: kylechar <kylechar@chromium.org>
Date: Tue Oct 10 18:21:49 2017

Improve DisplayPrivate interface.

The SetDisplayColorSpace() IPC message needs both blending and device
color spaces. Also use this opportunity to move DisplayPrivate into it's
own mojom file.

Bug:  770833 
Change-Id: I60ec214a6e7505c65f43936dd34826014018f14c
Reviewed-on: https://chromium-review.googlesource.com/703514
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507724}
[modify] https://crrev.com/34f44db294662fd8a8bec001244bac266c0445c1/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/34f44db294662fd8a8bec001244bac266c0445c1/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h
[modify] https://crrev.com/34f44db294662fd8a8bec001244bac266c0445c1/services/ui/ws/server_window.h
[modify] https://crrev.com/34f44db294662fd8a8bec001244bac266c0445c1/services/viz/privileged/interfaces/compositing/BUILD.gn
[add] https://crrev.com/34f44db294662fd8a8bec001244bac266c0445c1/services/viz/privileged/interfaces/compositing/display_private.mojom
[modify] https://crrev.com/34f44db294662fd8a8bec001244bac266c0445c1/services/viz/privileged/interfaces/compositing/frame_sink_manager.mojom

Comment 6 by fsamuel@google.com, Oct 10 2017

Blocking: -730193 601863
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 12 2017

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

commit 801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1
Author: kylechar <kylechar@chromium.org>
Date: Thu Oct 12 20:22:25 2017

Change how ImageTransportFactory is set.

Implementing an --enable-viz flag will require an ImageTransportFactory
that isn't GpuProcessTransportFactory. There is already a function to
set an arbitrary test factory.

Add ImageTransportFactory::SetFactory() and just use that everywhere
instead. Move the test only code into the test factory. Have
BrowserMainLoop pass in GpuProcessTransportFactory and tests pass in
NoTransportImageTransportFactory.

Also cleanup forward declerations and includes on ImageTransportFactory.

Bug:  770833 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I15c4d128fa8840c06d04f336dde2d0d075a00380
Reviewed-on: https://chromium-review.googlesource.com/716696
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508440}
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/browser_main_loop.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/compositor/image_transport_factory.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/compositor/reflector_impl.h
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/compositor/reflector_impl_unittest.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/compositor/test/no_transport_image_transport_factory.h
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/context_factory.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/frame_host/render_widget_host_view_guest_unittest.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/media/capture/desktop_capture_device_aura_unittest.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper_unittest.mm
[modify] https://crrev.com/801c940a54c5ddb497c1d3b6a8ce8ce3db4b15e1/content/public/test/test_renderer_host.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 13 2017

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

commit c9cb719f63b4fd535b0c5232244b96f5f366bf86
Author: kylechar <kylechar@chromium.org>
Date: Fri Oct 13 17:25:05 2017

Change GpuChannelHostFactory initialization.

There is a some initialization code in the BrowserMainLoop with regards
to GpuChannelHostFactory and ImageTransportCode that at some point
became unnecessary.

1. GetGpuChannelEstablishFactory() is never actually used. It only
   returns something when running with mus. We never use the value
   if we are running with mus though. Just delete this code.
2. There is a #ifdef !defined(OS_ANDROID) inside of another #ifdef
   !defined(OS_ANDROID). Delete extra #ifdef.
3. Pass GpuChannelHostFactory into GpuProcessTransportFactory
   constructor and simplify ImageTransportFactory interface.
4. Don't intialize BrowserGpuChannelHostFactory for mus.

Bug:  770833 
Change-Id: I5163cbbc3a7baf525ff7a3d7aad213e7210c8f7c
Reviewed-on: https://chromium-review.googlesource.com/718989
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508738}
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/content/browser/browser_main_loop.cc
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/content/browser/compositor/test/no_transport_image_transport_factory.h
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/content/public/browser/content_browser_client.h
[modify] https://crrev.com/c9cb719f63b4fd535b0c5232244b96f5f366bf86/ui/aura/mus/window_tree_client.h

Blockedon: 776052
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 19 2017

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

commit 150e09e16f6db3594a10593ea46af1e9f717761e
Author: kylechar <kylechar@chromium.org>
Date: Thu Oct 19 13:57:52 2017

Add VizProcessTransportFactory.

The VizProcessTransportFactory is a replacement for
GpuProcessTransportFactory for viz mode. In this mode the display
compositor runs in the viz process instead of browser process.

The --enable-viz flag won't work yet since there are changes required in
content/browser/renderer_host that still need to land.

TBR: mpearson@chromium.org
Bug:  770833 ,  732900 
Change-Id: I0aaf9ef369366838e2335447358fd6dfaaa5bf0a
Reviewed-on: https://chromium-review.googlesource.com/703274
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510076}
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/components/viz/common/switches.cc
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/components/viz/common/switches.h
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/content/browser/BUILD.gn
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/content/browser/browser_main_loop.cc
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/content/browser/compositor/OWNERS
[add] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/content/browser/compositor/viz_process_transport_factory.cc
[add] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/content/browser/compositor/viz_process_transport_factory.h
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/content/browser/gpu/gpu_process_host.h
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/services/ui/public/cpp/gpu/command_buffer_metrics.cc
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/services/ui/public/cpp/gpu/command_buffer_metrics.h
[modify] https://crrev.com/150e09e16f6db3594a10593ea46af1e9f717761e/tools/metrics/histograms/histograms.xml

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 19 2017

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

commit 883e99d9896c0dbcaf1ce4da26a1a3aa2fe2cdbe
Author: kylechar <kylechar@chromium.org>
Date: Thu Oct 19 15:07:36 2017

Update content/browser/renderer_host for --enable-viz.

With --enable-viz the host portion of the renderer cannot access
FrameSinkManagerImpl or create CompositorFrameSinkSupports. Update so
that viz doesn't immediately crash.

The --enable-viz flag should work for basic websites on Linux desktop
Chrome now. OOPIF and PDF Viewer are not implemented however.

Bug:  770833 ,  732903 
Change-Id: I373e02a221361b362f0eb35cc91d7d1a6b7c73a5
Reviewed-on: https://chromium-review.googlesource.com/724222
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510087}
[modify] https://crrev.com/883e99d9896c0dbcaf1ce4da26a1a3aa2fe2cdbe/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/883e99d9896c0dbcaf1ce4da26a1a3aa2fe2cdbe/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/883e99d9896c0dbcaf1ce4da26a1a3aa2fe2cdbe/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/883e99d9896c0dbcaf1ce4da26a1a3aa2fe2cdbe/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/883e99d9896c0dbcaf1ce4da26a1a3aa2fe2cdbe/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/883e99d9896c0dbcaf1ce4da26a1a3aa2fe2cdbe/content/browser/renderer_host/render_widget_host_view_aura.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 19 2017

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

commit c2c27cf05d34c670adcdaf9f9f3431016fffa152
Author: kylechar <kylechar@chromium.org>
Date: Thu Oct 19 21:52:49 2017

Support surface sync in VizProcessTransportFactory.

Needed to set |enable_surface_synchronization| correctly in the
ClientLayerTreeFrameSink::InitParams.

Bug:  770833 
Change-Id: Ibfbb810a1d787e6736e0522998175252977e7bee
Reviewed-on: https://chromium-review.googlesource.com/728706
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510221}
[modify] https://crrev.com/c2c27cf05d34c670adcdaf9f9f3431016fffa152/content/browser/compositor/viz_process_transport_factory.cc

Project Member

Comment 13 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 14 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

Blocking: -601863 730193
Project Member

Comment 16 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

Blockedon: 777594
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 25 2017

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

commit c36cea26b3a83d09028c1edc077e81dcdc386de2
Author: kylechar <kylechar@chromium.org>
Date: Wed Oct 25 21:42:56 2017

Change context lost from callback to observer.

We want to have multiple LayerTreeFrameSinks share a ContextProvider.
The current ContextProvider implementation holds a single callback from
the LayerTreeFrameSink which won't work.

Add ContextLostObserver to observer ContextProvider. Update all
ContextProvider implementations to use the observer.

Bug:  770833 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I646468e3708c3953acb16d94ae798bef7646bef8
Reviewed-on: https://chromium-review.googlesource.com/736259
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511598}
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/android_webview/browser/aw_render_thread_context_provider.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/android_webview/browser/aw_render_thread_context_provider.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/cc/raster/raster_buffer_provider_perftest.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/cc/test/test_context_provider.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/cc/test/test_context_provider.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/cc/test/test_in_process_context_provider.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/cc/test/test_in_process_context_provider.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/cc/trees/layer_tree_frame_sink.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/cc/trees/layer_tree_frame_sink.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/components/viz/common/BUILD.gn
[add] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/components/viz/common/gpu/context_lost_observer.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/components/viz/common/gpu/context_provider.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/components/viz/common/gpu/in_process_context_provider.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/components/viz/common/gpu/in_process_context_provider.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/components/viz/service/display/display.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/components/viz/service/display/display.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/components/viz/test/test_layer_tree_frame_sink.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/content/browser/gpu/gpu_ipc_browsertests.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/content/renderer/webgraphicscontext3d_provider_impl.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/content/renderer/webgraphicscontext3d_provider_impl.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/services/ui/public/cpp/gpu/context_provider_command_buffer.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/services/ui/public/cpp/gpu/context_provider_command_buffer.h
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/ui/compositor/test/in_process_context_provider.cc
[modify] https://crrev.com/c36cea26b3a83d09028c1edc077e81dcdc386de2/ui/compositor/test/in_process_context_provider.h

Project Member

Comment 19 by bugdroid1@chromium.org, Oct 26 2017

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

commit b311d7bf3bc03d9706c0d43624afdb56a7978610
Author: kylechar <kylechar@chromium.org>
Date: Thu Oct 26 01:16:47 2017

viz: Use one ContextProvider for all ui::Compositors.

The ui::Compositor doesn't share a ContextProvider with the viz::Display
when running with --enable-viz. All ui::Compositors can use the same
ContextProvider as a result.

Bug:  776052 ,  770833 
Change-Id: I4c5df37ce73d4aefbbb6991f027103635cb15729
Reviewed-on: https://chromium-review.googlesource.com/736331
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511680}
[modify] https://crrev.com/b311d7bf3bc03d9706c0d43624afdb56a7978610/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/b311d7bf3bc03d9706c0d43624afdb56a7978610/content/browser/compositor/viz_process_transport_factory.h

Blockedon: 778749
Blockedon: 778751
Blockedon: 778753
Blockedon: 778754
Blockedon: -778753
Blockedon: -764732
Components: -Internals>Viz Internals>Services>Viz
Migrating from Internals>Viz to Internals>Services>Viz.
Blockedon: 754872
Cc: m...@chromium.org
This will also require tab capture in viz.
Blockedon: 787097
Blockedon: -787097
Blocking: -730193 787097
Blockedon: 787589
Blockedon: 730660
Blockedon: 796700
Blockedon: 799478
Blockedon: -799478
Blockedon: -778749
Blockedon: -754872
Project Member

Comment 37 by bugdroid1@chromium.org, Feb 8 2018

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

commit 0e610ff740a0875d44277eda30348c6009c73761
Author: kylechar <kylechar@chromium.org>
Date: Thu Feb 08 18:19:32 2018

Cleanup VizProcessTransportFactory.

1. We will always have a GpuProessHost with VizDisplayCompositor,
   because we always need a GPU process. Remove comment.
2. Context loss is fixed. Remove comment.
3. We always use surface synchronization. Remove check.

Bug:  770833 
Change-Id: Ib5e4fd526aa94c8133c5aa660e8e0b2ca4b5df24
Reviewed-on: https://chromium-review.googlesource.com/908570
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535445}
[modify] https://crrev.com/0e610ff740a0875d44277eda30348c6009c73761/content/browser/compositor/viz_process_transport_factory.cc

Summary: Get VizDisplayCompositor working behind flag in Linux desktop Chrome (was: Get viz working behind flag in Linux desktop Chrome)
Blockedon: 813606
Blockedon: -730660
Status: Fixed (was: Available)

Sign in to add a comment