mus: Always use the GPU compositor for mus.

content/browser/compositor/gpu_process_transport_factory.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/compositor/gpu_process_transport_factory.h"
 
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 89 matching lines @@
 using gpu::gles2::GLES2Interface;
 
 namespace {
 
 const int kNumRetriesBeforeSoftwareFallback = 4;
 
 bool IsUsingMus() {
   return shell::ShellIsRemote();
 }
 
+static bool ShouldCreateGpuOutputSurface(ui::Compositor* compositor) {
+#if defined(OS_CHROMEOS)
+  // Software fallback does not happen on Chrome OS.
+  return true;
+#endif
+
+  if (IsUsingMus()) {
+    // There will always be a GPU process.
+    return true;
+  }
+
+#if defined(OS_WIN)
+  if (compositor && ::GetProp(compositor->widget(), kForceSoftwareCompositor) &&
+      ::RemoveProp(compositor->widget(), kForceSoftwareCompositor))
+    return false;
+#endif
+
+  return content::GpuDataManagerImpl::GetInstance()
+      ->CanUseGpuBrowserCompositor();
+}
+
 scoped_refptr<content::ContextProviderCommandBuffer> CreateContextCommon(
     scoped_refptr<gpu::GpuChannelHost> gpu_channel_host,
     gpu::SurfaceHandle surface_handle,
     bool need_alpha_channel,
     bool support_locking,
     content::ContextProviderCommandBuffer* shared_context_provider,
     content::command_buffer_metrics::ContextType type) {
-  DCHECK(
-      content::GpuDataManagerImpl::GetInstance()->CanUseGpuBrowserCompositor());
+  DCHECK(ShouldCreateGpuOutputSurface(nullptr));

[danakj, 2016/08/12 17:48:29]

This is tied to the change for shared main thread context so explanation there
should explain this.

   DCHECK(gpu_channel_host);
 
   // This is called from a few places to create different contexts:
   // - The shared main thread context (offscreen).
   // - The compositor context, which is used by the browser compositor
   //   (offscreen) for synchronization mostly, and by the display compositor
   //   (onscreen, except for with mus) for actual GL drawing.
   // - The compositor worker context (offscreen) used for GPU raster.
   // So ask for capabilities needed by any of these cases (we can optimize by
   // branching on |surface_handle| being null if these needs diverge).
@@ skipping 128 matching lines @@
             ca_layers_disabled));
   }
 #elif defined(OS_ANDROID)
   validator.reset(
       new display_compositor::CompositorOverlayCandidateValidatorAndroid());
 #endif
 
   return validator;
 }
 
-static bool ShouldCreateGpuOutputSurface(ui::Compositor* compositor) {
-#if defined(OS_CHROMEOS)
-  // Software fallback does not happen on Chrome OS.
-  return true;
-#endif
-
-#if defined(OS_WIN)
-  if (::GetProp(compositor->widget(), kForceSoftwareCompositor) &&
-      ::RemoveProp(compositor->widget(), kForceSoftwareCompositor))
-    return false;
-#endif
-
-  return GpuDataManagerImpl::GetInstance()->CanUseGpuBrowserCompositor();
-}
-
 void GpuProcessTransportFactory::CreateOutputSurface(
     base::WeakPtr<ui::Compositor> compositor) {
   DCHECK(!!compositor);
   PerCompositorData* data = per_compositor_data_[compositor.get()];
   if (!data) {
     data = CreatePerCompositorData(compositor.get());
   } else {
     // TODO(danakj): We can destroy the |data->display| here when the compositor
     // destroys its OutputSurface before calling back here.
     data->display_output_surface = nullptr;
@@ skipping 35 matching lines @@
     return;
 
   PerCompositorData* data = it->second;
   DCHECK(data);
 
   if (num_attempts > kNumRetriesBeforeSoftwareFallback) {
 #if defined(OS_CHROMEOS)
     LOG(FATAL) << "Unable to create a UI graphics context, and cannot use "
                << "software compositing on ChromeOS.";
 #endif
     create_gpu_output_surface = false;

[danakj, 2016/08/12 17:48:29]

You probably want to not do this on mus too no?

   }
 
 #if defined(OS_WIN)
   gfx::RenderingWindowManager::GetInstance()->RegisterParent(
       compositor->widget());
 #endif
 
   scoped_refptr<cc::VulkanInProcessContextProvider> vulkan_context_provider =
       SharedVulkanContextProvider();
   scoped_refptr<ContextProviderCommandBuffer> context_provider;
   if (create_gpu_output_surface && !vulkan_context_provider) {
     // Try to reuse existing worker context provider.
     if (shared_worker_context_provider_) {
       bool lost;
       {
         // Note: If context is lost, we delete reference after releasing the
         // lock.
         base::AutoLock lock(*shared_worker_context_provider_->GetLock());
         lost = shared_worker_context_provider_->ContextGL()
                    ->GetGraphicsResetStatusKHR() != GL_NO_ERROR;
       }
       if (lost)
         shared_worker_context_provider_ = nullptr;
     }
 
     scoped_refptr<gpu::GpuChannelHost> gpu_channel_host;
-    if (GpuDataManagerImpl::GetInstance()->CanUseGpuBrowserCompositor())
+    if (ShouldCreateGpuOutputSurface(nullptr))

[danakj, 2016/08/12 17:48:29]

I don't think this should change. The fact that we're here means that
ShouldCreateGpuOutputSurface was previously true already.

I think that CanUseGpuBrowserCompositor can change tho, due to gpu crashes and
stuff. But the other things in ShouldCreateGpuOutputSurface can't. So this is
checking a bunch of redundant things which obfuscates things.

Is CanUseGpuBrowserCompositor not returning true for mus? Cuz that sounds
problematic if this code does not agree with the GpuDataManager.

       gpu_channel_host = std::move(established_channel_host);
 
     if (!gpu_channel_host) {
       shared_worker_context_provider_ = nullptr;
     } else {
       if (!shared_worker_context_provider_) {
         bool need_alpha_channel = false;
         const bool support_locking = true;
         shared_worker_context_provider_ =
             CreateContextCommon(gpu_channel_host, gpu::kNullSurfaceHandle,
@@ skipping 385 matching lines @@
   if (data->display_output_surface)
     data->display_output_surface->SetSurfaceSuspendedForRecycle(suspended);
 }
 #endif
 
 scoped_refptr<cc::ContextProvider>
 GpuProcessTransportFactory::SharedMainThreadContextProvider() {
   if (shared_main_thread_contexts_)
     return shared_main_thread_contexts_;
 
-  if (!GpuDataManagerImpl::GetInstance()->CanUseGpuBrowserCompositor())
+  if (!ShouldCreateGpuOutputSurface(nullptr))

[danakj, 2016/08/12 17:48:29]

This might be right but it has nothing to do with the patch description, which
is using gpu on mus. I'd like to know an explanation for this being correct
either way..

     return nullptr;
 
   DCHECK(gpu_channel_factory_);
   scoped_refptr<gpu::GpuChannelHost> gpu_channel_host =
       gpu_channel_factory_->EstablishGpuChannelSync();
   if (!gpu_channel_host)
     return nullptr;
 
   // We need a separate context from the compositor's so that skia and gl_helper
   // don't step on each other.
@@ skipping 78 matching lines @@
       shared_vulkan_context_provider_ =
           cc::VulkanInProcessContextProvider::Create();
     }
 
     shared_vulkan_context_provider_initialized_ = true;
   }
   return shared_vulkan_context_provider_;
 }
 
 }  // namespace content