Use VizCompositorThreadRunner in VizMainImpl.

Add missing code VizCompositorThreadRunner to support GPU compositing
and GPU process force shutdown. Remove code in VizMainImpl and just use
VizCompositorThreadRunner there.

Bug: 849639
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I34ae6adec13d8be11e563f2c32de36504b6711b4
Reviewed-on: https://chromium-review.googlesource.com/1165842
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583827}

components/viz/service/main/viz_compositor_thread_runner.cc

@@ -14,8 +14,12 @@
 #include "build/build_config.h"
 #include "components/viz/common/switches.h"
 #include "components/viz/service/display_embedder/gpu_display_provider.h"
+#include "components/viz/service/display_embedder/in_process_gpu_memory_buffer_manager.h"
 #include "components/viz/service/display_embedder/server_shared_bitmap_manager.h"
 #include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
+#include "components/viz/service/gl/gpu_service_impl.h"
+#include "gpu/ipc/command_buffer_task_executor.h"
+#include "gpu/ipc/service/gpu_memory_buffer_factory.h"
 #include "ui/gfx/switches.h"
 
 namespace viz {
@@ -23,7 +27,13 @@ namespace {
 
 const char kThreadName[] = "VizCompositorThread";
 
-std::unique_ptr<base::Thread> CreateAndStartCompositorThread() {
+std::unique_ptr<VizCompositorThreadType> CreateAndStartCompositorThread() {
+#if defined(OS_ANDROID)
+  auto thread = std::make_unique<base::android::JavaHandlerThread>(
+      kThreadName, base::ThreadPriority::DISPLAY);
+  thread->Start();
+  return thread;
+#else  // !defined(OS_ANDROID)
   auto thread = std::make_unique<base::Thread>(kThreadName);
 
   base::Thread::Options thread_options;
@@ -37,6 +47,7 @@ std::unique_ptr<base::Thread> CreateAndStartCompositorThread() {
 
   CHECK(thread->StartWithOptions(thread_options));
   return thread;
+#endif  // !defined(OS_ANDROID)
 }
 
 }  // namespace
@@ -59,27 +70,72 @@ void VizCompositorThreadRunner::CreateFrameSinkManager(
       FROM_HERE,
       base::BindOnce(
           &VizCompositorThreadRunner::CreateFrameSinkManagerOnCompositorThread,
-          base::Unretained(this), std::move(params)));
+          base::Unretained(this), std::move(params), nullptr, nullptr, nullptr,
+          nullptr));
+}
+
+void VizCompositorThreadRunner::CreateFrameSinkManager(
+    mojom::FrameSinkManagerParamsPtr params,
+    scoped_refptr<gpu::CommandBufferTaskExecutor> task_executor,
+    GpuServiceImpl* gpu_service) {
+  auto* gpu_channel_manager = gpu_service->gpu_channel_manager();
+  auto* image_factory = gpu_service->gpu_image_factory();
+
+  // All of the unretained objects are owned on the GPU thread and destroyed
+  // after VizCompositorThread has been shutdown.
+  task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &VizCompositorThreadRunner::CreateFrameSinkManagerOnCompositorThread,
+          base::Unretained(this), std::move(params), std::move(task_executor),
+          base::Unretained(gpu_service), base::Unretained(image_factory),
+          base::Unretained(gpu_channel_manager)));
+}
+
+void VizCompositorThreadRunner::CleanupForShutdown(
+    base::OnceClosure cleanup_finished_callback) {
+  task_runner_->PostTaskAndReply(
+      FROM_HERE,
+      base::BindOnce(
+          &VizCompositorThreadRunner::CleanupForShutdownOnCompositorThread,
+          base::Unretained(this)),
+      std::move(cleanup_finished_callback));
 }
 
 void VizCompositorThreadRunner::CreateFrameSinkManagerOnCompositorThread(
-    mojom::FrameSinkManagerParamsPtr params) {
+    mojom::FrameSinkManagerParamsPtr params,
+    scoped_refptr<gpu::CommandBufferTaskExecutor> task_executor,
+    GpuServiceImpl* gpu_service,
+    gpu::ImageFactory* image_factory,
+    gpu::GpuChannelManager* gpu_channel_manager) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(!frame_sink_manager_);
 
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-
   server_shared_bitmap_manager_ = std::make_unique<ServerSharedBitmapManager>();
   base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
       server_shared_bitmap_manager_.get(), "ServerSharedBitmapManager",
-      base::ThreadTaskRunnerHandle::Get());
+      task_runner_);
 
-  // Create GpuDisplayProvider usable for software compositing only.
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  const bool headless = command_line->HasSwitch(switches::kHeadless);
+  const bool run_all_compositor_stages_before_draw =
+      command_line->HasSwitch(switches::kRunAllCompositorStagesBeforeDraw);
+
+  if (task_executor) {
+    // Create DisplayProvider usable for GPU + software compositing.
+    display_provider_ = std::make_unique<GpuDisplayProvider>(
+        params->restart_id, gpu_service, std::move(task_executor), gpu_service,
+        std::make_unique<InProcessGpuMemoryBufferManager>(gpu_channel_manager),
+        image_factory, server_shared_bitmap_manager_.get(), headless,
+        run_all_compositor_stages_before_draw);
+  } else {
+    // Create DisplayProvider usable for software compositing only.
     display_provider_ = std::make_unique<GpuDisplayProvider>(
-      params->restart_id, server_shared_bitmap_manager_.get(),
-      command_line->HasSwitch(switches::kHeadless),
-      command_line->HasSwitch(switches::kRunAllCompositorStagesBeforeDraw));
+        params->restart_id, server_shared_bitmap_manager_.get(), headless,
+        run_all_compositor_stages_before_draw);
+  }
 
+  // Create FrameSinkManagerImpl.
   base::Optional<uint32_t> activation_deadline_in_frames;
   if (params->use_activation_deadline)
     activation_deadline_in_frames = params->activation_deadline_in_frames;
@@ -92,6 +148,13 @@ void VizCompositorThreadRunner::CreateFrameSinkManagerOnCompositorThread(
           std::move(params->frame_sink_manager_client)));
 }
 
+void VizCompositorThreadRunner::CleanupForShutdownOnCompositorThread() {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
+  if (frame_sink_manager_)
+    frame_sink_manager_->ForceShutdown();
+}
+
 void VizCompositorThreadRunner::TearDownOnCompositorThread() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 

components/viz/service/main/viz_compositor_thread_runner.h

@@ -9,37 +9,79 @@
 
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
+#include "build/build_config.h"
 #include "components/viz/service/viz_service_export.h"
 #include "services/viz/privileged/interfaces/viz_main.mojom.h"
 
+#if defined(OS_ANDROID)
+#include "base/android/java_handler_thread.h"
+#endif
+
 namespace base {
 class SingleThreadTaskRunner;
 class Thread;
 }  // namespace base
 
+namespace gpu {
+class CommandBufferTaskExecutor;
+class GpuChannelManager;
+class ImageFactory;
+}  // namespace gpu
+
 namespace viz {
 class DisplayProvider;
 class FrameSinkManagerImpl;
+class GpuServiceImpl;
 class ServerSharedBitmapManager;
 
+#if defined(OS_ANDROID)
+using VizCompositorThreadType = base::android::JavaHandlerThread;
+#else
+using VizCompositorThreadType = base::Thread;
+#endif
+
 // Starts and runs the VizCompositorThread. The thread will be started when this
 // object is constructed. Objects on the thread will be initialized after
 // calling CreateFrameSinkManager(). Destructor will teardown objects on thread
 // and then stop the thread.
-// TODO(kylechar): Convert VizMainImpl to use VizCompositorThreadRunner.
 class VIZ_SERVICE_EXPORT VizCompositorThreadRunner {
  public:
   VizCompositorThreadRunner();
   // Performs teardown on thread and then stops thread.
   ~VizCompositorThreadRunner();
 
-  // Create FrameSinkManager from |params|. This can be called from the thread
-  // that owns |this| to initialize state on VizCompositorThreadRunner.
+  // Returns the TaskRunner for VizCompositorThread.
+  base::SingleThreadTaskRunner* task_runner() { return task_runner_.get(); }
+
+  // Creates FrameSinkManager from |params|. The version with |gpu_service| and
+  // |task_executor| supports both GPU and software compositing, while the
+  // version without supports only software compositing. Should be called from
+  // the thread that owns |this| to initialize state on VizCompositorThread.
   void CreateFrameSinkManager(mojom::FrameSinkManagerParamsPtr params);
+  void CreateFrameSinkManager(
+      mojom::FrameSinkManagerParamsPtr params,
+      scoped_refptr<gpu::CommandBufferTaskExecutor> task_executor,
+      GpuServiceImpl* gpu_service);
+
+  // Performs cleanup on VizCompositorThread needed before forcing thread to
+  // shut down. Ensures VizCompositorThread teardown during the destructor
+  // doesn't block on PostTasks back to the GPU thread. After cleanup has
+  // finished |cleanup_finished_callback| will be run. Should be called from the
+  // thread that owns |this|.
+  //
+  // This is intended to be used when the GPU thread wants to force restart. The
+  // cleanup is normally handled by the browser process before GPU process
+  // shutdown, except if the GPU thread is forcing restart.
+  void CleanupForShutdown(base::OnceClosure cleanup_finished_callback);
 
  private:
   void CreateFrameSinkManagerOnCompositorThread(
-      mojom::FrameSinkManagerParamsPtr params);
+      mojom::FrameSinkManagerParamsPtr params,
+      scoped_refptr<gpu::CommandBufferTaskExecutor> task_executor,
+      GpuServiceImpl* gpu_service,
+      gpu::ImageFactory* image_factory,
+      gpu::GpuChannelManager* gpu_channel_manager);
+  void CleanupForShutdownOnCompositorThread();
   void TearDownOnCompositorThread();
 
   // Start variables to be accessed only on |task_runner_|.
@@ -48,7 +90,7 @@ class VIZ_SERVICE_EXPORT VizCompositorThreadRunner {
   std::unique_ptr<FrameSinkManagerImpl> frame_sink_manager_;
   // End variables to be accessed only on |task_runner_|.
 
-  std::unique_ptr<base::Thread> thread_;
+  std::unique_ptr<VizCompositorThreadType> thread_;
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(VizCompositorThreadRunner);

components/viz/service/main/viz_main_impl.cc

@@ -11,21 +11,13 @@
 #include "base/message_loop/message_loop.h"
 #include "base/power_monitor/power_monitor_device_source.h"
 #include "base/single_thread_task_runner.h"
-#include "base/threading/sequenced_task_runner_handle.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "build/build_config.h"
-#include "components/viz/common/switches.h"
-#include "components/viz/service/display_embedder/gpu_display_provider.h"
-#include "components/viz/service/display_embedder/in_process_gpu_memory_buffer_manager.h"
-#include "components/viz/service/display_embedder/server_shared_bitmap_manager.h"
-#include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
 #include "components/viz/service/gl/gpu_service_impl.h"
 #include "gpu/command_buffer/common/activity_flags.h"
 #include "gpu/config/gpu_preferences.h"
 #include "gpu/config/gpu_switches.h"
-#include "gpu/ipc/common/gpu_memory_buffer_support.h"
 #include "gpu/ipc/gpu_in_process_thread_service.h"
-#include "gpu/ipc/service/gpu_memory_buffer_factory.h"
 #include "gpu/ipc/service/gpu_watchdog_thread.h"
 #include "media/gpu/buildflags.h"
 #include "services/metrics/public/cpp/delegating_ukm_recorder.h"
@@ -33,7 +25,6 @@
 #include "services/metrics/public/mojom/constants.mojom.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "third_party/skia/include/core/SkFontLCDConfig.h"
-#include "ui/gfx/switches.h"
 
 #if defined(OS_CHROMEOS) && BUILDFLAG(USE_VAAPI)
 #include "media/gpu/vaapi/vaapi_wrapper.h"
@@ -45,31 +36,6 @@
 
 namespace {
 
-std::unique_ptr<viz::CompositorThreadType> CreateAndStartCompositorThread() {
-  const char* thread_name = "VizCompositorThread";
-
-#if defined(OS_ANDROID)
-  auto thread = std::make_unique<base::android::JavaHandlerThread>(
-      thread_name, base::ThreadPriority::DISPLAY);
-  thread->Start();
-  return thread;
-#else
-  auto thread = std::make_unique<base::Thread>(thread_name);
-  base::Thread::Options thread_options;
-#if defined(OS_WIN)
-  // Windows needs a UI message loop for child HWND. Other platforms can use the
-  // default message loop type.
-  thread_options.message_loop_type = base::MessageLoop::TYPE_UI;
-#endif
-
-#if defined(OS_CHROMEOS)
-  thread_options.priority = base::ThreadPriority::DISPLAY;
-#endif
-  CHECK(thread->StartWithOptions(thread_options));
-  return thread;
-#endif
-}
-
 std::unique_ptr<base::Thread> CreateAndStartIOThread() {
   // TODO(sad): We do not need the IO thread once gpu has a separate process.
   // It should be possible to use |main_task_runner_| for doing IO tasks.
@@ -145,29 +111,23 @@ VizMainImpl::VizMainImpl(Delegate* delegate,
   if (!dependencies_.io_thread_task_runner)
     io_thread_ = CreateAndStartIOThread();
   if (dependencies_.create_display_compositor) {
-    compositor_thread_ = CreateAndStartCompositorThread();
-    compositor_thread_task_runner_ = compositor_thread_->task_runner();
-    if (delegate_)
+    viz_compositor_thread_runner_ =
+        std::make_unique<VizCompositorThreadRunner>();
+    if (delegate_) {
       delegate_->PostCompositorThreadCreated(
-          compositor_thread_task_runner_.get());
+          viz_compositor_thread_runner_->task_runner());
+    }
   }
 
   CreateUkmRecorderIfNeeded(dependencies.connector);
 
-  // We need to provide GpuServiceImpl a callback to exit the GPU process. With
-  // OOP-D this requires destroying RootCompositorFrameSinkImpls on the
-  // compositor thread while the GPU thread is still running to avoid deadlock.
-  // For non OOP-D we can simply quit the GPU thread RunLoop.
-  base::OnceClosure exit_callback =
-      compositor_thread_
-          ? base::BindOnce(&VizMainImpl::ExitProcess, base::Unretained(this))
-          : base::BindOnce(&base::RunLoop::QuitCurrentDeprecated);
   gpu_service_ = std::make_unique<GpuServiceImpl>(
       gpu_init_->gpu_info(), gpu_init_->TakeWatchdogThread(), io_task_runner(),
       gpu_init_->gpu_feature_info(), gpu_init_->gpu_preferences(),
       gpu_init_->gpu_info_for_hardware_gpu(),
       gpu_init_->gpu_feature_info_for_hardware_gpu(),
-      gpu_init_->vulkan_implementation(), std::move(exit_callback));
+      gpu_init_->vulkan_implementation(),
+      base::BindOnce(&VizMainImpl::ExitProcess, base::Unretained(this)));
   if (dependencies_.create_display_compositor)
     gpu_service_->set_oopd_enabled();
 }
@@ -182,18 +142,11 @@ VizMainImpl::~VizMainImpl() {
   binding_.Close();
   associated_binding_.Close();
 
-  if (compositor_thread_) {
-    // Destroy all objects owned on the compositor thread before shutting down
-    // the thread. All RootCompositorFrameSinks must be destroyed before now,
-    // otherwise the compositor thread will deadlock waiting for a response from
-    // the blocked GPU thread.
-    compositor_thread_task_runner_->PostTask(
-        FROM_HERE, base::BindOnce(&VizMainImpl::TearDownOnCompositorThread,
-                                  base::Unretained(this)));
-    compositor_thread_->Stop();
-    compositor_thread_.reset();
-    compositor_thread_task_runner_.reset();
-  }
+  // If the VizCompositorThread was started then this will block until the
+  // thread has been shutdown. All RootCompositorFrameSinks must be destroyed
+  // before now, otherwise the compositor thread will deadlock waiting for a
+  // response from the blocked GPU thread.
+  viz_compositor_thread_runner_.reset();
 
   if (ukm_recorder_)
     ukm::DelegatingUkmRecorder::Get()->RemoveDelegate(ukm_recorder_.get());
@@ -283,7 +236,7 @@ void VizMainImpl::CreateUkmRecorderIfNeeded(
 
 void VizMainImpl::CreateFrameSinkManager(
     mojom::FrameSinkManagerParamsPtr params) {
-  DCHECK(compositor_thread_task_runner_);
+  DCHECK(viz_compositor_thread_runner_);
   DCHECK(gpu_thread_task_runner_->BelongsToCurrentThread());
   if (!gpu_service_ || !gpu_service_->is_initialized()) {
     DCHECK(pending_frame_sink_manager_params_.is_null());
@@ -295,7 +248,6 @@ void VizMainImpl::CreateFrameSinkManager(
 
 void VizMainImpl::CreateFrameSinkManagerInternal(
     mojom::FrameSinkManagerParamsPtr params) {
-  DCHECK(!task_executor_);
   DCHECK(gpu_service_);
   DCHECK(gpu_thread_task_runner_->BelongsToCurrentThread());
 
@@ -315,84 +267,26 @@ void VizMainImpl::CreateFrameSinkManagerInternal(
       gpu_service_->gpu_feature_info(),
       gpu_service_->gpu_channel_manager()->gpu_preferences());
 
-  compositor_thread_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&VizMainImpl::CreateFrameSinkManagerOnCompositorThread,
-                     base::Unretained(this), std::move(params)));
-}
-
-void VizMainImpl::CreateFrameSinkManagerOnCompositorThread(
-    mojom::FrameSinkManagerParamsPtr params) {
-  DCHECK(compositor_thread_task_runner_->BelongsToCurrentThread());
-  DCHECK(!frame_sink_manager_);
-
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-
-  server_shared_bitmap_manager_ = std::make_unique<ServerSharedBitmapManager>();
-  base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
-      server_shared_bitmap_manager_.get(), "viz::ServerSharedBitmapManager",
-      base::ThreadTaskRunnerHandle::Get());
-
-  auto* gpu_channel_manager = gpu_service_->gpu_channel_manager();
-  gpu::ImageFactory* image_factory = nullptr;
-  if (gpu_channel_manager->gpu_memory_buffer_factory()) {
-    image_factory =
-        gpu_channel_manager->gpu_memory_buffer_factory()->AsImageFactory();
-  }
-  display_provider_ = std::make_unique<GpuDisplayProvider>(
-      params->restart_id, gpu_service_.get(), task_executor_,
-      gpu_channel_manager->delegate(),
-      std::make_unique<InProcessGpuMemoryBufferManager>(gpu_channel_manager),
-      image_factory, server_shared_bitmap_manager_.get(),
-      command_line->HasSwitch(switches::kHeadless),
-      command_line->HasSwitch(switches::kRunAllCompositorStagesBeforeDraw));
-
-  mojom::FrameSinkManagerClientPtr client(
-      std::move(params->frame_sink_manager_client));
-  base::Optional<uint32_t> activation_deadline_in_frames;
-  if (params->use_activation_deadline)
-    activation_deadline_in_frames = params->activation_deadline_in_frames;
-  frame_sink_manager_ = std::make_unique<FrameSinkManagerImpl>(
-      server_shared_bitmap_manager_.get(), activation_deadline_in_frames,
-      display_provider_.get());
-  frame_sink_manager_->BindAndSetClient(std::move(params->frame_sink_manager),
-                                        nullptr, std::move(client));
-}
-
-void VizMainImpl::TearDownOnCompositorThread() {
-  DCHECK(compositor_thread_task_runner_->BelongsToCurrentThread());
-
-  base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
-      server_shared_bitmap_manager_.get());
-
-  frame_sink_manager_.reset();
-  display_provider_.reset();
-  server_shared_bitmap_manager_.reset();
-}
-
-void VizMainImpl::CleanupForShutdownOnCompositorThread() {
-  DCHECK(compositor_thread_task_runner_->BelongsToCurrentThread());
-
-  if (frame_sink_manager_)
-    frame_sink_manager_->ForceShutdown();
+  viz_compositor_thread_runner_->CreateFrameSinkManager(
+      std::move(params), task_executor_, gpu_service_.get());
 }
 
 void VizMainImpl::ExitProcess() {
   DCHECK(gpu_thread_task_runner_->BelongsToCurrentThread());
-  DCHECK(compositor_thread_task_runner_);
 
   // Close mojom::VizMain bindings first so the browser can't try to reconnect.
   binding_.Close();
   associated_binding_.Close();
 
-  // PostTask to the compositor thread to cleanup and then exit the GPU thread
-  // RunLoop once that is finished. Unretained is safe here because |this| owns
-  // |compositor_thread_|.
-  compositor_thread_task_runner_->PostTaskAndReply(
-      FROM_HERE,
-      base::BindOnce(&VizMainImpl::CleanupForShutdownOnCompositorThread,
-                     base::Unretained(this)),
+  if (viz_compositor_thread_runner_) {
+    // OOP-D requires destroying RootCompositorFrameSinkImpls on the compositor
+    // thread while the GPU thread is still running to avoid deadlock. Quit GPU
+    // thread TaskRunner after cleanup on compositor thread is finished.
+    viz_compositor_thread_runner_->CleanupForShutdown(
         base::BindOnce([]() { base::RunLoop::QuitCurrentDeprecated(); }));
+  } else {
+    base::RunLoop::QuitCurrentDeprecated();
+  }
 }
 
 void VizMainImpl::PreSandboxStartup() {

components/viz/service/main/viz_main_impl.h

@@ -10,6 +10,7 @@
 #include "base/threading/thread.h"
 #include "build/build_config.h"
 #include "components/discardable_memory/client/client_discardable_shared_memory_manager.h"
+#include "components/viz/service/main/viz_compositor_thread_runner.h"
 #include "gpu/ipc/in_process_command_buffer.h"
 #include "gpu/ipc/service/gpu_init.h"
 #include "mojo/public/cpp/bindings/associated_binding_set.h"
@@ -18,10 +19,6 @@
 #include "services/viz/privileged/interfaces/viz_main.mojom.h"
 #include "ui/gfx/font_render_params.h"
 
-#if defined(OS_ANDROID)
-#include "base/android/java_handler_thread.h"
-#endif
-
 namespace gpu {
 class SyncPointManager;
 }  // namespace gpu
@@ -35,10 +32,7 @@ class MojoUkmRecorder;
 }
 
 namespace viz {
-class DisplayProvider;
-class FrameSinkManagerImpl;
 class GpuServiceImpl;
-class ServerSharedBitmapManager;
 
 #if defined(OS_ANDROID)
 using CompositorThreadType = base::android::JavaHandlerThread;
@@ -121,17 +115,8 @@ class VizMainImpl : public gpu::GpuSandboxHelper, public mojom::VizMain {
   void CreateUkmRecorderIfNeeded(service_manager::Connector* connector);
 
   void CreateFrameSinkManagerInternal(mojom::FrameSinkManagerParamsPtr params);
-  void CreateFrameSinkManagerOnCompositorThread(
-      mojom::FrameSinkManagerParamsPtr params);
-
-  void TearDownOnCompositorThread();
 
-  // Performs necessary cleanup on the compositor thread to allow for a clean
-  // process exit.
-  void CleanupForShutdownOnCompositorThread();
-
-  // Cleanly exits the process. This is only used with OOP-D when there is a
-  // compositor thread.
+  // Cleanly exits the process.
   void ExitProcess();
 
   // gpu::GpuSandboxHelper:
@@ -162,24 +147,24 @@ class VizMainImpl : public gpu::GpuSandboxHelper, public mojom::VizMain {
   // This is created for OOP-D only. It allows the display compositor to use
   // InProcessCommandBuffer to send GPU commands to the GPU thread from the
   // compositor thread.
+  // TODO(kylechar): The only reason this member variable exists is so the last
+  // reference is released and the object is destroyed on the GPU thread. This
+  // works because |task_executor_| is destroyed after the VizCompositorThread
+  // has been shutdown. All usage of CommandBufferTaskExecutor has the same
+  // pattern, where the last scoped_refptr is released on the GPU thread after
+  // all InProcessCommandBuffers are destroyed, so the class doesn't need to be
+  // RefCountedThreadSafe.
   scoped_refptr<gpu::CommandBufferTaskExecutor> task_executor_;
 
   // If the gpu service is not yet ready then we stash pending
   // FrameSinkManagerParams.
   mojom::FrameSinkManagerParamsPtr pending_frame_sink_manager_params_;
 
-  // Provides mojo interfaces for creating and managing FrameSinks. These live
-  // on the compositor thread.
-  std::unique_ptr<ServerSharedBitmapManager> server_shared_bitmap_manager_;
-  std::unique_ptr<DisplayProvider> display_provider_;
-  std::unique_ptr<FrameSinkManagerImpl> frame_sink_manager_;
+  // Runs the VizCompositorThread for the display compositor with OOP-D.
+  std::unique_ptr<VizCompositorThreadRunner> viz_compositor_thread_runner_;
 
   const scoped_refptr<base::SingleThreadTaskRunner> gpu_thread_task_runner_;
 
-  // The main thread for the display compositor.
-  std::unique_ptr<CompositorThreadType> compositor_thread_;
-  scoped_refptr<base::SingleThreadTaskRunner> compositor_thread_task_runner_;
-
   std::unique_ptr<ukm::MojoUkmRecorder> ukm_recorder_;
   std::unique_ptr<base::PowerMonitor> power_monitor_;
   mojo::Binding<mojom::VizMain> binding_;