mus+ash: Enable Chrome HW rendering in mus+ash

content/browser/compositor/gpu_process_transport_factory.cc

Index: content/browser/compositor/gpu_process_transport_factory.cc
diff --git a/content/browser/compositor/gpu_process_transport_factory.cc b/content/browser/compositor/gpu_process_transport_factory.cc
index 124b04b2473a014843413fb91be5b649e32c8a5a..8cc1da59bb90a44d4e03f9a880c3638858bf1c81 100644
--- a/content/browser/compositor/gpu_process_transport_factory.cc
+++ b/content/browser/compositor/gpu_process_transport_factory.cc
@@ -60,7 +60,10 @@
 #include "ui/gfx/geometry/size.h"
 
 #if defined(MOJO_RUNNER_CLIENT)
+#include "content/browser/compositor/mus_browser_compositor_output_surface.h"
+#include "content/public/common/mojo_shell_connection.h"
 #include "services/shell/runner/common/client_util.h"
+#include "services/ui/common/gpu_service.h"
 #endif
 
 #if defined(OS_WIN)
@@ -101,6 +104,14 @@ namespace {
 
 const int kNumRetriesBeforeSoftwareFallback = 4;
 
+bool IsUsingMus() {
+#if defined(MOJO_RUNNER_CLIENT)
+  return shell::ShellIsRemote();
+#else
+  return false;
+#endif
+}
+
 scoped_refptr<content::ContextProviderCommandBuffer> CreateContextCommon(
     scoped_refptr<gpu::GpuChannelHost> gpu_channel_host,
     gpu::SurfaceHandle surface_handle,
@@ -124,7 +135,8 @@ scoped_refptr<content::ContextProviderCommandBuffer> CreateContextCommon(
   // not need alpha, stencil, depth, antialiasing. The display compositor does
   // not use these things either, so we can request nothing here.
   gpu::gles2::ContextCreationAttribHelper attributes;
-  attributes.alpha_size = -1;
+  // For mus, we need the alpha channel for "onscreen" context.

[danakj, 2016/07/07 20:16:41]

For mus the context is not onscreen anymore, so the comments above wrong. Can
you fix them instead?

Here's some suggestions

".. and by the display compositor (onscreen, except when using mus) for actual
GL drawing"

"The display compositor does not use these things either (except for alpha
channel when using mus for non-opaque ui that overlaps the system's window
borders), so we can request only that when needed."

Is that right what I wrote?

[Peng, 2016/07/07 21:33:37]

Done.

+  attributes.alpha_size = (IsUsingMus() && surface_handle) ? 8 : -1;
   attributes.depth_size = 0;
   attributes.stencil_size = 0;
   attributes.samples = 0;
@@ -134,6 +146,9 @@ scoped_refptr<content::ContextProviderCommandBuffer> CreateContextCommon(
 
   constexpr bool automatic_flushes = false;
 
+  // For mus, we create offscreen context.

[danakj, 2016/07/07 20:16:41]

"an offscreen context"

[Peng, 2016/07/07 21:33:36]

Done.

+  surface_handle = IsUsingMus() ? gpu::kNullSurfaceHandle : surface_handle;

[danakj, 2016/07/07 20:16:41]

As the comment above says, this code is used for creating more than one context.
I think this code should be at a callsite for this method, not inside the common
code.

Same for the above IsUsingMus(), this method should take parameters for things
it wants to be optional like "needs_alpha_channel".

[Peng, 2016/07/07 21:33:37]

Done.

+
   GURL url("chrome://gpu/GpuProcessTransportFactory::CreateContextCommon");
   return make_scoped_refptr(new content::ContextProviderCommandBuffer(
       std::move(gpu_channel_host), gpu::GPU_STREAM_DEFAULT,
@@ -256,9 +271,8 @@ static bool ShouldCreateGpuOutputSurface(ui::Compositor* compositor) {
 #if defined(MOJO_RUNNER_CLIENT)
   // Chrome running as a mojo app currently can only use software compositing.

[danakj, 2016/07/07 20:16:41]

This comment is wrong now?

[Peng, 2016/07/07 21:33:36]

Done.

   // TODO(rjkroege): http://crbug.com/548451
-  if (shell::ShellIsRemote()) {
+  if (shell::ShellIsRemote() && !ui::GpuService::UseChromeGpuCommandBuffer())
     return false;
-  }
 #endif
 
 #if defined(OS_CHROMEOS)
@@ -294,15 +308,22 @@ void GpuProcessTransportFactory::CreateOutputSurface(
 #endif
 
   const bool use_vulkan = static_cast<bool>(SharedVulkanContextProvider());
-
+  const bool use_mus = IsUsingMus();
   const bool create_gpu_output_surface =
       ShouldCreateGpuOutputSurface(compositor.get());
   if (create_gpu_output_surface && !use_vulkan) {
-    BrowserGpuChannelHostFactory::instance()->EstablishGpuChannel(
-        CAUSE_FOR_GPU_LAUNCH_SHARED_WORKER_THREAD_CONTEXT,
+    base::Closure callback(
         base::Bind(&GpuProcessTransportFactory::EstablishedGpuChannel,
                    callback_factory_.GetWeakPtr(), compositor,
                    create_gpu_output_surface, 0));
+    if (!use_mus) {
+      BrowserGpuChannelHostFactory::instance()->EstablishGpuChannel(
+          CAUSE_FOR_GPU_LAUNCH_SHARED_WORKER_THREAD_CONTEXT, callback);
+#if defined(MOJO_RUNNER_CLIENT)
+    } else {

[piman, 2016/07/07 21:02:20]

nit: I would prefer to avoid mixing compile-time #ifdefs and run-time brackets -
it makes the code really hard to maintain.

How about:

    if (!use_mus) {
      BrowserGpuChannelHostFactory::instance()->EstablishGpuChannel(
          CAUSE_FOR_GPU_LAUNCH_SHARED_WORKER_THREAD_CONTEXT, callback);
    } else {
#if defined(MOJO_RUNNER_CLIENT)
      ui::GpuService::GetInstance()->EstablishGpuChannel(callback);
#else
      NOTREACHED();
#endif
    }

[Peng, 2016/07/07 21:33:37]

Done.

+      ui::GpuService::GetInstance()->EstablishGpuChannel(callback);
+#endif
+    }
   } else {
     EstablishedGpuChannel(compositor, create_gpu_output_surface, 0);
   }
@@ -339,7 +360,7 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
 
   scoped_refptr<cc::VulkanInProcessContextProvider> vulkan_context_provider =
       SharedVulkanContextProvider();
-
+  const bool use_mus = IsUsingMus();
   scoped_refptr<ContextProviderCommandBuffer> context_provider;
   if (create_gpu_output_surface && !vulkan_context_provider) {
     // Try to reuse existing worker context provider.
@@ -360,8 +381,14 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
     if (GpuDataManagerImpl::GetInstance()->CanUseGpuBrowserCompositor()) {
       // We attempted to do EstablishGpuChannel already, so we just use
       // GetGpuChannel() instead of EstablishGpuChannelSync().
-      gpu_channel_host =
-          BrowserGpuChannelHostFactory::instance()->GetGpuChannel();
+      if (!use_mus) {
+        gpu_channel_host =
+            BrowserGpuChannelHostFactory::instance()->GetGpuChannel();
+#if defined(MOJO_RUNNER_CLIENT)
+      } else {

[piman, 2016/07/07 21:02:20]

nit: ditto

[Peng, 2016/07/07 21:33:37]

Done.

+        gpu_channel_host = ui::GpuService::GetInstance()->GetGpuChannel();
+#endif
+      }
     }
 
     if (!gpu_channel_host) {
@@ -418,11 +445,18 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
 
     if (!created_gpu_browser_compositor) {
       // Try again.
-      BrowserGpuChannelHostFactory::instance()->EstablishGpuChannel(
-          CAUSE_FOR_GPU_LAUNCH_SHARED_WORKER_THREAD_CONTEXT,
+      base::Closure callback(
           base::Bind(&GpuProcessTransportFactory::EstablishedGpuChannel,
                      callback_factory_.GetWeakPtr(), compositor,
                      create_gpu_output_surface, num_attempts + 1));
+      if (!use_mus) {
+        BrowserGpuChannelHostFactory::instance()->EstablishGpuChannel(
+            CAUSE_FOR_GPU_LAUNCH_SHARED_WORKER_THREAD_CONTEXT, callback);
+#if defined(MOJO_RUNNER_CLIENT)
+      } else {

[piman, 2016/07/07 21:02:20]

nit: ditto

[Peng, 2016/07/07 21:33:37]

Done.

+        ui::GpuService::GetInstance()->EstablishGpuChannel(callback);
+#endif
+      }
       return;
     }
   }
@@ -491,12 +525,23 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
             validator;
 #if !defined(OS_MACOSX)
         // Overlays are only supported on surfaceless output surfaces on Mac.
-        validator = CreateOverlayCandidateValidator(compositor->widget());
+        if (!use_mus)
+          validator = CreateOverlayCandidateValidator(compositor->widget());
 #endif
-        display_output_surface =
-            base::WrapUnique(new GpuBrowserCompositorOutputSurface(
-                context_provider, compositor->vsync_manager(),
-                begin_frame_source.get(), std::move(validator)));
+        if (!use_mus) {
+          display_output_surface =
+              base::WrapUnique(new GpuBrowserCompositorOutputSurface(
+                  context_provider, compositor->vsync_manager(),
+                  begin_frame_source.get(), std::move(validator)));
+#if defined(MOJO_RUNNER_CLIENT)

[danakj, 2016/07/07 20:16:41]

Why are things inside #if defined, but the source files are always present in
the build?

[Peng, 2016/07/07 21:33:37]

I think the MOJO_RUNNER_CLIENT is only defined with GN build. For gyp build, the
macro is not defined, and mus_browser_compositor_output_surface.{cc,h} are not
in gyp files too.

+        } else {
+          display_output_surface =
+              base::WrapUnique(new MusBrowserCompositorOutputSurface(
+                  data->surface_handle, context_provider,
+                  compositor->vsync_manager(), begin_frame_source.get(),
+                  std::move(validator)));
+#endif
+        }
       }
     }
   }
@@ -732,9 +777,19 @@ GpuProcessTransportFactory::SharedMainThreadContextProvider() {
 
   if (!GpuDataManagerImpl::GetInstance()->CanUseGpuBrowserCompositor())
     return nullptr;
-  scoped_refptr<gpu::GpuChannelHost> gpu_channel_host(
-      BrowserGpuChannelHostFactory::instance()->EstablishGpuChannelSync(
-          CAUSE_FOR_GPU_LAUNCH_BROWSER_SHARED_MAIN_THREAD_CONTEXT));
+
+  const bool use_mus = IsUsingMus();
+  scoped_refptr<gpu::GpuChannelHost> gpu_channel_host;
+  if (!use_mus) {
+    gpu_channel_host =
+        BrowserGpuChannelHostFactory::instance()->EstablishGpuChannelSync(
+            CAUSE_FOR_GPU_LAUNCH_BROWSER_SHARED_MAIN_THREAD_CONTEXT);
+#if defined(MOJO_RUNNER_CLIENT)
+  } else {

[piman, 2016/07/07 21:02:20]

nit: ditto

[Peng, 2016/07/07 21:33:37]

Done.

+    gpu_channel_host = ui::GpuService::GetInstance()->EstablishGpuChannelSync();
+#endif
+  }
+
   if (!gpu_channel_host)
     return nullptr;