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}

content/browser/compositor/gpu_process_transport_factory.cc

@@ -653,6 +653,8 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
 
 void GpuProcessTransportFactory::DisableGpuCompositing(
     ui::Compositor* guilty_compositor) {
+  DLOG(ERROR) << "Switching to software compositing.";
+
   // Change the result of IsGpuCompositingDisabled() before notifying anything.
   is_gpu_compositing_disabled_ = true;
 

content/browser/compositor/viz_process_transport_factory.cc

@@ -31,7 +31,6 @@
 #include "content/public/common/content_switches.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "gpu/command_buffer/client/raster_interface.h"
-#include "gpu/command_buffer/common/context_result.h"
 #include "gpu/ipc/client/gpu_channel_host.h"
 #include "services/ui/public/cpp/gpu/context_provider_command_buffer.h"
 #include "ui/compositor/reflector.h"
@@ -129,7 +128,7 @@ VizProcessTransportFactory::VizProcessTransportFactory(
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kDisableGpu) ||
       command_line->HasSwitch(switches::kDisableGpuCompositing)) {
-    CompositingModeFallbackToSoftware();
+    DisableGpuCompositing(nullptr);
   }
 }
 
@@ -196,8 +195,15 @@ VizProcessTransportFactory::SharedMainThreadContextProvider() {
     return nullptr;
 
   if (!main_context_provider_) {
-    CreateContextProviders(
-        gpu_channel_establish_factory_->EstablishGpuChannelSync());
+    auto context_result = gpu::ContextResult::kTransientFailure;
+    while (context_result == gpu::ContextResult::kTransientFailure) {
+      context_result = TryCreateContextsForGpuCompositing(
+          gpu_channel_establish_factory_->EstablishGpuChannelSync());
+
+      if (context_result == gpu::ContextResult::kFatalFailure)
+        DisableGpuCompositing(nullptr);
+    }
+    // On kFatalFailure |main_context_provider_| will be null.
   }
 
   return main_context_provider_;
@@ -360,11 +366,16 @@ viz::GLHelper* VizProcessTransportFactory::GetGLHelper() {
   return nullptr;
 }
 
-void VizProcessTransportFactory::CompositingModeFallbackToSoftware() {
-  // This may happen multiple times, since when the viz process (re)starts, it
-  // will send this notification if gpu is disabled.
-  if (is_gpu_compositing_disabled_)
-    return;
+void VizProcessTransportFactory::OnContextLost() {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&VizProcessTransportFactory::OnLostMainThreadSharedContext,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
+void VizProcessTransportFactory::DisableGpuCompositing(
+    ui::Compositor* guilty_compositor) {
+  DLOG(ERROR) << "Switching to software compositing.";
 
   // Change the result of IsGpuCompositingDisabled() before notifying anything.
   is_gpu_compositing_disabled_ = true;
@@ -394,8 +405,15 @@ void VizProcessTransportFactory::CompositingModeFallbackToSoftware() {
   to_release.reserve(compositor_data_map_.size());
   for (auto& pair : compositor_data_map_) {
     ui::Compositor* compositor = pair.first;
-    if (!compositor->force_software_compositor())
+    // The |guilty_compositor| is in the process of setting up its FrameSink
+    // so removing it from |compositor_data_map_| would be both pointless and
+    // the cause of a crash.
+    // Compositors with force_software_compositor() do not follow the global
+    // compositing mode, so they do not need to changed.
+    if (compositor != guilty_compositor &&
+        !compositor->force_software_compositor()) {
       to_release.push_back(compositor);
+    }
   }
   for (ui::Compositor* compositor : to_release) {
     // Compositor expects to be not visible when releasing its FrameSink.
@@ -410,13 +428,6 @@ void VizProcessTransportFactory::CompositingModeFallbackToSoftware() {
   GpuDataManagerImpl::GetInstance()->NotifyGpuInfoUpdate();
 }
 
-void VizProcessTransportFactory::OnContextLost() {
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::BindOnce(&VizProcessTransportFactory::OnLostMainThreadSharedContext,
-                     weak_ptr_factory_.GetWeakPtr()));
-}
-
 void VizProcessTransportFactory::OnGpuProcessLost() {
   // Reconnect HostFrameSinkManager to new GPU process.
   ConnectHostFrameSinkManager();
@@ -432,18 +443,18 @@ void VizProcessTransportFactory::OnEstablishedGpuChannel(
   bool gpu_compositing =
       !is_gpu_compositing_disabled_ && !compositor->force_software_compositor();
 
-  // Only try to make contexts for gpu compositing.
   if (gpu_compositing) {
-    // TODO(kylechar): Check GPU compositing status from GpuFeatureInfo.
-
-    if (!gpu_channel_host ||
-        !CreateContextProviders(std::move(gpu_channel_host))) {
-      // Retry on failure. If this isn't possible we should hear that we're
-      // falling back to software compositing from the viz process eventually.
+    auto context_result =
+        TryCreateContextsForGpuCompositing(std::move(gpu_channel_host));
+    if (context_result == gpu::ContextResult::kTransientFailure) {
+      // Get a new GpuChannelHost and retry context creation.
       gpu_channel_establish_factory_->EstablishGpuChannel(
           base::BindOnce(&VizProcessTransportFactory::OnEstablishedGpuChannel,
                          weak_ptr_factory_.GetWeakPtr(), compositor_weak_ptr));
       return;
+    } else if (context_result == gpu::ContextResult::kFatalFailure) {
+      DisableGpuCompositing(compositor);
+      gpu_compositing = false;
     }
   }
 
@@ -530,27 +541,32 @@ void VizProcessTransportFactory::OnEstablishedGpuChannel(
 #endif
 }
 
-bool VizProcessTransportFactory::CreateContextProviders(
+gpu::ContextResult
+VizProcessTransportFactory::TryCreateContextsForGpuCompositing(
     scoped_refptr<gpu::GpuChannelHost> gpu_channel_host) {
-  constexpr bool kSharedWorkerContextSupportsLocking = true;
-  constexpr bool kSharedWorkerContextSupportsGLES2 = false;
-  constexpr bool kSharedWorkerContextSupportsRaster = true;
-  constexpr bool kSharedWorkerContextSupportsGrContext = false;
-  constexpr bool kCompositorContextSupportsLocking = false;
-  constexpr bool kCompositorContextSupportsGLES2 = true;
-  constexpr bool kCompositorContextSupportsRaster = false;
-  constexpr bool kCompositorContextSupportsGrContext = true;
+  DCHECK(!is_gpu_compositing_disabled_);
 
-  if (main_context_provider_ && IsContextLost(main_context_provider_.get())) {
-    main_context_provider_->RemoveObserver(this);
-    main_context_provider_ = nullptr;
-  }
+  // Fallback to software compositing if there is no IPC channel.
+  if (!gpu_channel_host)
+    return gpu::ContextResult::kFatalFailure;
+
+  // Fallback to software compositing if GPU compositing is blacklisted.
+  auto gpu_compositing_status =
+      gpu_channel_host->gpu_feature_info()
+          .status_values[gpu::GPU_FEATURE_TYPE_GPU_COMPOSITING];
+  if (gpu_compositing_status != gpu::kGpuFeatureStatusEnabled)
+    return gpu::ContextResult::kFatalFailure;
 
   if (worker_context_provider_ &&
       IsWorkerContextLost(worker_context_provider_.get()))
     worker_context_provider_ = nullptr;
 
   if (!worker_context_provider_) {
+    constexpr bool kSharedWorkerContextSupportsLocking = true;
+    constexpr bool kSharedWorkerContextSupportsGLES2 = false;
+    constexpr bool kSharedWorkerContextSupportsRaster = true;
+    constexpr bool kSharedWorkerContextSupportsGrContext = false;
+
     worker_context_provider_ = CreateContextProviderImpl(
         gpu_channel_host, GetGpuMemoryBufferManager(),
         kSharedWorkerContextSupportsLocking, kSharedWorkerContextSupportsGLES2,
@@ -561,32 +577,42 @@ bool VizProcessTransportFactory::CreateContextProviders(
     // Don't observer context loss on |worker_context_provider_| here, that is
     // already observered by LayerTreeFrameSink. The lost context will be caught
     // when recreating LayerTreeFrameSink(s).
-    auto result = worker_context_provider_->BindToCurrentThread();
-    if (result != gpu::ContextResult::kSuccess) {
+    auto context_result = worker_context_provider_->BindToCurrentThread();
+    if (context_result != gpu::ContextResult::kSuccess) {
       worker_context_provider_ = nullptr;
-      return false;
+      return context_result;
     }
   }
 
+  if (main_context_provider_ && IsContextLost(main_context_provider_.get())) {
+    main_context_provider_->RemoveObserver(this);
+    main_context_provider_ = nullptr;
+  }
+
   if (!main_context_provider_) {
+    constexpr bool kCompositorContextSupportsLocking = false;
+    constexpr bool kCompositorContextSupportsGLES2 = true;
+    constexpr bool kCompositorContextSupportsRaster = false;
+    constexpr bool kCompositorContextSupportsGrContext = true;
+
     main_context_provider_ = CreateContextProviderImpl(
         std::move(gpu_channel_host), GetGpuMemoryBufferManager(),
         kCompositorContextSupportsLocking, kCompositorContextSupportsGLES2,
         kCompositorContextSupportsRaster, kCompositorContextSupportsGrContext,
         ui::command_buffer_metrics::UI_COMPOSITOR_CONTEXT);
     main_context_provider_->SetDefaultTaskRunner(resize_task_runner_);
-    main_context_provider_->AddObserver(this);
 
-    auto result = main_context_provider_->BindToCurrentThread();
-    if (result != gpu::ContextResult::kSuccess) {
-      main_context_provider_->RemoveObserver(this);
-      main_context_provider_ = nullptr;
+    auto context_result = main_context_provider_->BindToCurrentThread();
+    if (context_result != gpu::ContextResult::kSuccess) {
       worker_context_provider_ = nullptr;
-      return false;
+      main_context_provider_ = nullptr;
+      return context_result;
     }
+
+    main_context_provider_->AddObserver(this);
   }
 
-  return true;
+  return gpu::ContextResult::kSuccess;
 }
 
 void VizProcessTransportFactory::OnLostMainThreadSharedContext() {

content/browser/compositor/viz_process_transport_factory.h

@@ -15,9 +15,9 @@
 #include "components/viz/common/surfaces/frame_sink_id_allocator.h"
 #include "content/browser/compositor/image_transport_factory.h"
 #include "content/browser/compositor/in_process_display_client.h"
+#include "gpu/command_buffer/common/context_result.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "services/viz/privileged/interfaces/compositing/frame_sink_manager.mojom.h"
-#include "services/viz/public/interfaces/compositing/compositing_mode_watcher.mojom.h"
 #include "services/viz/public/interfaces/compositing/compositor_frame_sink.mojom.h"
 #include "ui/compositor/compositor.h"
 
@@ -53,7 +53,6 @@ class ExternalBeginFrameControllerClientImpl;
 class VizProcessTransportFactory : public ui::ContextFactory,
                                    public ui::ContextFactoryPrivate,
                                    public ImageTransportFactory,
-                                   public viz::mojom::CompositingModeWatcher,
                                    public viz::ContextLostObserver {
  public:
   VizProcessTransportFactory(
@@ -107,9 +106,6 @@ class VizProcessTransportFactory : public ui::ContextFactory,
   ui::ContextFactoryPrivate* GetContextFactoryPrivate() override;
   viz::GLHelper* GetGLHelper() override;
 
-  // viz::mojom::CompositingModeWatcher implementation.
-  void CompositingModeFallbackToSoftware() override;
-
   // viz::ContextLostObserver implementation.
   void OnContextLost() override;
 
@@ -134,6 +130,12 @@ class VizProcessTransportFactory : public ui::ContextFactory,
     DISALLOW_COPY_AND_ASSIGN(CompositorData);
   };
 
+  // Disables GPU compositing. This notifies UI and renderer compositors to drop
+  // LayerTreeFrameSinks and request new ones. If fallback happens while
+  // creating a new LayerTreeFrameSink for UI compositor it should be passed in
+  // as |guilty_compositor| to avoid extra work and reentrancy problems.
+  void DisableGpuCompositing(ui::Compositor* guilty_compositor);
+
   // Provided as a callback when the GPU process has crashed.
   void OnGpuProcessLost();
 
@@ -143,10 +145,15 @@ class VizProcessTransportFactory : public ui::ContextFactory,
       base::WeakPtr<ui::Compositor> compositor_weak_ptr,
       scoped_refptr<gpu::GpuChannelHost> gpu_channel);
 
-  // Creates the necessary shared worker and compositor ContextProviders. If the
-  // ContextProviders already exist and haven't been lost then it will do
-  // nothing. Returns true if ContextProviders exist.
-  bool CreateContextProviders(
+  // Tries to create the raster and main thread ContextProviders. If the
+  // ContextProviders already exist and haven't been lost then this will do
+  // nothing. Also verifies |gpu_channel_host| and checks if GPU compositing is
+  // blacklisted.
+  //
+  // Returns kSuccess if caller can use GPU compositing, kTransientFailure if
+  // caller should try again or kFatalFailure if caller should fallback to
+  // software compositing.
+  gpu::ContextResult TryCreateContextsForGpuCompositing(
       scoped_refptr<gpu::GpuChannelHost> gpu_channel_host);
 
   void OnLostMainThreadSharedContext();