On Linux desktop, when "use_virtualized_gl_context" is true

and when --use-gl=egl then the browser is showing garbage.

Indeed in GpuCommandBufferStub::OnInitialize the call to
context->Initialize(surface_.get(), gpu_preference_)" fails.
The error actually comes from eglMakeContext which returns
EGL_BAD_MATCH. Because surface config is not compatible with
context config.

Problem is that EGL_BUFFER_SIZE is always 32 for off screen
surfaces (see gl_surface_egl.cc::InitializeOneOff). Whereas
for on screen surfaces EGL_BUFFER_SIZE is the window depth.
This depth is by default 24 unless --enable-transparent-visuals
is passed to the command line.

When using mesa the error is raised here:
/* If the context has a config then it must match that of the two
* surfaces */
if (ctx->Config) {
if ((draw && draw->Config != ctx->Config) ||
  (read && read->Config != ctx->Config))
return _eglError(EGL_BAD_MATCH, "eglMakeCurrent");
from:
cgit.freedesktop.org/mesa/mesa/tree/src/egl/main/eglcontext.c#n630

This patch pass the main window depth from browser to gpu process
though a new switch kWindowDepth.
This allow GLSurfaceEGL::InitializeOneOff to select an EGLConfig
that matches with future ON screen surfaces's EGLConfig.

Also note that long term plan is to always enable transparent visual,
see http://crbug.com/369209.

This patch also move the kEnableTransparentVisuals switch from view
to x11 switches as it is defined to x11 only.

BUG=557389

R=jbauman@chromium.org, hendrikw@chromium.org, piman@chromium.org, sadrul@chromium.org, cwallez1@chromium.org, kbr@chromium.org

Review URL: https://codereview.chromium.org/1429083002

TEST= chrome --use-gl=egl --use_virtualized_gl_contexts

Review URL: https://codereview.chromium.org/1429083002

Cr-Commit-Position: refs/heads/master@{#361114}

12 files changed, 146 insertions, 70 deletions

diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc
index 57a74fc..8f93e67 100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -162,7 +162,9 @@
 #endif
 
 #if defined(USE_X11)
+#include "ui/base/x/x11_util_internal.h"
 #include "ui/gfx/x/x11_connection.h"
+#include "ui/gfx/x/x11_switches.h"
 #include "ui/gfx/x/x11_types.h"
 #endif
 
@@ -1374,6 +1376,17 @@ bool BrowserMainLoop::InitializeToolkit() {
 #if defined(USE_X11)
   if (!gfx::GetXDisplay())
     return false;
+
+#if !defined(OS_CHROMEOS)
+  // InitializeToolkit is called before CreateStartupTasks which one starts the
+  // gpu process.
+  int depth = 0;
+  ui::ChooseVisualForWindow(NULL, &depth);
+  DCHECK(depth > 0);
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kWindowDepth, base::IntToString(depth));
+#endif
+
 #endif
 
   // Env creates the compositor. Aura widgets need the compositor to be created
diff --git a/content/browser/gpu/gpu_process_host.cc b/content/browser/gpu/gpu_process_host.cc
index ba1f793..ced0ab6 100644
--- a/content/browser/gpu/gpu_process_host.cc
+++ b/content/browser/gpu/gpu_process_host.cc
@@ -137,6 +137,7 @@ static const char* const kSwitchNames[] = {
   switches::kOzonePlatform,
 #endif
 #if defined(USE_X11) && !defined(OS_CHROMEOS)
+  switches::kWindowDepth,
   switches::kX11Display,
 #endif
 };
diff --git a/content/gpu/gpu_main.cc b/content/gpu/gpu_main.cc
index d21c88d..6c7e8af 100644
--- a/content/gpu/gpu_main.cc
+++ b/content/gpu/gpu_main.cc
@@ -61,6 +61,7 @@
 
 #if defined(USE_X11)
 #include "ui/base/x/x11_util.h"
+#include "ui/gfx/x/x11_switches.h"
 #endif
 
 #if defined(OS_LINUX)
@@ -147,6 +148,12 @@ int GpuMain(const MainFunctionParams& parameters) {
       SEM_NOOPENFILEERRORBOX);
 #elif defined(USE_X11)
   ui::SetDefaultX11ErrorHandlers();
+
+#if !defined(OS_CHROMEOS)
+  DCHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kWindowDepth));
+#endif
+
 #endif
 
   logging::SetLogMessageHandler(GpuProcessLogMessageHandler);
diff --git a/ui/base/x/x11_util.cc b/ui/base/x/x11_util.cc
index f36aa01..b253a77 100644
--- a/ui/base/x/x11_util.cc
+++ b/ui/base/x/x11_util.cc
@@ -53,6 +53,11 @@
 #include "ui/gfx/skia_util.h"
 #include "ui/gfx/x/x11_error_tracker.h"
 
+#if !defined(OS_CHROMEOS)
+#include "base/command_line.h"
+#include "ui/gfx/x/x11_switches.h"
+#endif
+
 #if defined(OS_FREEBSD)
 #include <sys/sysctl.h>
 #include <sys/types.h>
@@ -1409,6 +1414,64 @@ void LogErrorEventDescription(XDisplay* dpy,
       << " (" << request_str << ")";
 }
 
+#if !defined(OS_CHROMEOS)
+void ChooseVisualForWindow(Visual** visual, int* depth) {
+  static Visual* s_visual = NULL;
+  static int s_depth = 0;
+
+  if (!s_visual) {
+    XDisplay* display = gfx::GetXDisplay();
+    XAtom NET_WM_CM_S0 = XInternAtom(display, "_NET_WM_CM_S0", False);
+
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kEnableTransparentVisuals) &&
+        XGetSelectionOwner(display, NET_WM_CM_S0) != None) {
+      // Choose the first ARGB8888 visual
+      XVisualInfo visual_template;
+      visual_template.screen = 0;
+
+      int visuals_len;
+      gfx::XScopedPtr<XVisualInfo[]> visual_list(XGetVisualInfo(
+          display, VisualScreenMask, &visual_template, &visuals_len));
+      for (int i = 0; i < visuals_len; ++i) {
+        // Why support only 8888 ARGB? Because it's all that GTK+ supports. In
+        // gdkvisual-x11.cc, they look for this specific visual and use it for
+        // all their alpha channel using needs.
+        //
+        // TODO(erg): While the following does find a valid visual, some GL
+        // drivers
+        // don't believe that this has an alpha channel. According to marcheu@,
+        // this should work on open source driver though. (It doesn't work with
+        // NVidia's binaries currently.) http://crbug.com/369209
+        const XVisualInfo& info = visual_list[i];
+        if (info.depth == 32 && info.visual->red_mask == 0xff0000 &&
+            info.visual->green_mask == 0x00ff00 &&
+            info.visual->blue_mask == 0x0000ff) {
+          s_visual = info.visual;
+          s_depth = info.depth;
+          break;
+        }
+      }
+    } else {
+      XWindowAttributes windowAttribs;
+      Window root = XDefaultRootWindow(display);
+      Status status = XGetWindowAttributes(display, root, &windowAttribs);
+      DCHECK(status != 0);
+      s_visual = windowAttribs.visual;
+      s_depth = windowAttribs.depth;
+    }
+  }  // !s_visual
+
+  DCHECK(s_visual);
+  DCHECK(s_depth > 0);
+
+  if (visual)
+    *visual = s_visual;
+  if (depth)
+    *depth = s_depth;
+}
+#endif
+
 // ----------------------------------------------------------------------------
 // End of x11_util_internal.h
 
diff --git a/ui/base/x/x11_util_internal.h b/ui/base/x/x11_util_internal.h
index f663fc5..d5c598b 100644
--- a/ui/base/x/x11_util_internal.h
+++ b/ui/base/x/x11_util_internal.h
@@ -44,6 +44,15 @@ UI_BASE_EXPORT void SetX11ErrorHandlers(XErrorHandler error_handler,
 UI_BASE_EXPORT void LogErrorEventDescription(Display* dpy,
                                              const XErrorEvent& error_event);
 
+// --------------------------------------------------------------------------
+// Selects a visual with a preference for alpha support on compositing window
+// managers. The caller must compare depth to 32 to know if the returned visual
+// supports transparency. NULL parameters are allowed to install or query the
+// cached visual and depth.
+#if !defined(OS_CHROMEOS)
+UI_BASE_EXPORT void ChooseVisualForWindow(Visual** visual, int* depth);
+#endif
+
 }  // namespace ui
 
 #endif  // UI_BASE_X_X11_UTIL_INTERNAL_H_
diff --git a/ui/gfx/x/x11_switches.cc b/ui/gfx/x/x11_switches.cc
index bda0ad3..fc2c8c1 100644
--- a/ui/gfx/x/x11_switches.cc
+++ b/ui/gfx/x/x11_switches.cc
@@ -7,6 +7,17 @@
 namespace switches {
 
 #if !defined(OS_CHROMEOS)
+// When enabled, tries to get a transparent X11 visual so that we can have
+// per-pixel alpha in windows.
+//
+// TODO(erg): Remove this switch once we've stabilized the code
+// path. http://crbug.com/369209
+const char kEnableTransparentVisuals[] = "enable-transparent-visuals";
+
+// Color bit depth of the main window created in the browser process and matches
+// XWindowAttributes.depth.
+const char kWindowDepth[] = "window-depth";
+
 // Which X11 display to connect to. Emulates the GTK+ "--display=" command line
 // argument.
 const char kX11Display[] = "display";
diff --git a/ui/gfx/x/x11_switches.h b/ui/gfx/x/x11_switches.h
index 4044c51..28029d0 100644
--- a/ui/gfx/x/x11_switches.h
+++ b/ui/gfx/x/x11_switches.h
@@ -10,6 +10,8 @@
 namespace switches {
 
 #if !defined(OS_CHROMEOS)
+GFX_EXPORT extern const char kEnableTransparentVisuals[];
+GFX_EXPORT extern const char kWindowDepth[];
 GFX_EXPORT extern const char kX11Display[];
 #endif
 
diff --git a/ui/gl/gl_surface_egl.cc b/ui/gl/gl_surface_egl.cc
index 7c369848..677c356 100644
--- a/ui/gl/gl_surface_egl.cc
+++ b/ui/gl/gl_surface_egl.cc
@@ -13,6 +13,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "ui/gfx/geometry/rect.h"
@@ -35,6 +36,10 @@ extern "C" {
 #include "ui/ozone/public/surface_factory_ozone.h"
 #endif
 
+#if defined(USE_X11) && !defined(OS_CHROMEOS)
+#include "ui/gfx/x/x11_switches.h"
+#endif
+
 #if !defined(EGL_FIXED_SIZE_ANGLE)
 #define EGL_FIXED_SIZE_ANGLE 0x3201
 #endif
@@ -273,9 +278,27 @@ bool GLSurfaceEGL::InitializeOneOff() {
           switches::kEnableUnsafeES3APIs)) {
     renderable_type = EGL_OPENGL_ES3_BIT;
   }
+
+  EGLint buffer_size = 32;
+  EGLint alpha_size = 8;
+
+#if defined(USE_X11) && !defined(OS_CHROMEOS)
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kWindowDepth)) {
+    std::string depth =
+        base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+            switches::kWindowDepth);
+
+    bool succeed = base::StringToInt(depth, &buffer_size);
+    DCHECK(succeed);
+
+    alpha_size = buffer_size == 32 ? 8 : 0;
+  }
+#endif
+
   const EGLint kConfigAttribs[] = {
-    EGL_BUFFER_SIZE, 32,
-    EGL_ALPHA_SIZE, 8,
+    EGL_BUFFER_SIZE, buffer_size,
+    EGL_ALPHA_SIZE, alpha_size,
     EGL_BLUE_SIZE, 8,
     EGL_GREEN_SIZE, 8,
     EGL_RED_SIZE, 8,
diff --git a/ui/views/views_switches.cc b/ui/views/views_switches.cc
index 41d2ce5..8690ec0 100644
--- a/ui/views/views_switches.cc
+++ b/ui/views/views_switches.cc
@@ -16,15 +16,6 @@ namespace switches {
 const char kDisableViewsRectBasedTargeting[] =
     "disable-views-rect-based-targeting";
 
-#if defined(USE_X11) && !defined(OS_CHROMEOS)
-// When enabled, tries to get a transparent X11 visual so that we can have
-// per-pixel alpha in windows.
-//
-// TODO(erg): Remove this switch once we've stabilized the code
-// path. http://crbug.com/369209
-const char kEnableTransparentVisuals[] = "enable-transparent-visuals";
-#endif
-
 bool IsRectBasedTargetingEnabled() {
 #if defined(OS_CHROMEOS) || defined(OS_WIN) || defined(OS_LINUX)
   return !base::CommandLine::ForCurrentProcess()->HasSwitch(
diff --git a/ui/views/views_switches.h b/ui/views/views_switches.h
index 0fb09db..9e50294 100644
--- a/ui/views/views_switches.h
+++ b/ui/views/views_switches.h
@@ -15,10 +15,6 @@ namespace switches {
 // Please keep alphabetized.
 VIEWS_EXPORT extern const char kDisableViewsRectBasedTargeting[];
 
-#if defined(USE_X11) && !defined(OS_CHROMEOS)
-VIEWS_EXPORT extern const char kEnableTransparentVisuals[];
-#endif
-
 // Returns true if rect-based targeting in views should be used.
 VIEWS_EXPORT bool IsRectBasedTargetingEnabled();
 
diff --git a/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc b/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
index b717445..f84153d 100644
--- a/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
+++ b/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
@@ -25,6 +25,7 @@
 #include "ui/base/hit_test.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/base/x/x11_util.h"
+#include "ui/base/x/x11_util_internal.h"
 #include "ui/events/devices/x11/device_data_manager_x11.h"
 #include "ui/events/devices/x11/device_list_cache_x11.h"
 #include "ui/events/devices/x11/touch_factory_x11.h"
@@ -1117,30 +1118,20 @@ void DesktopWindowTreeHostX11::InitX11Window(
   if (swa.override_redirect)
     attribute_mask |= CWOverrideRedirect;
 
-  // Detect whether we're running inside a compositing manager. If so, try to
-  // use the ARGB visual. Otherwise, just use our parent's visual.
-  Visual* visual = CopyFromParent;
-  int depth = CopyFromParent;
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableTransparentVisuals) &&
-      XGetSelectionOwner(xdisplay_, atom_cache_.GetAtom("_NET_WM_CM_S0")) !=
-          None) {
-    Visual* rgba_visual = GetARGBVisual();
-    if (rgba_visual) {
-      visual = rgba_visual;
-      depth = 32;
-
-      attribute_mask |= CWColormap;
-      swa.colormap = XCreateColormap(xdisplay_, x_root_window_, visual,
-                                     AllocNone);
-
-      // x.org will BadMatch if we don't set a border when the depth isn't the
-      // same as the parent depth.
-      attribute_mask |= CWBorderPixel;
-      swa.border_pixel = 0;
-
-      use_argb_visual_ = true;
-    }
+  Visual* visual;
+  int depth;
+  ui::ChooseVisualForWindow(&visual, &depth);
+  if (depth == 32) {
+    attribute_mask |= CWColormap;
+    swa.colormap =
+        XCreateColormap(xdisplay_, x_root_window_, visual, AllocNone);
+
+    // x.org will BadMatch if we don't set a border when the depth isn't the
+    // same as the parent depth.
+    attribute_mask |= CWBorderPixel;
+    swa.border_pixel = 0;
+
+    use_argb_visual_ = true;
   }
 
   bounds_in_pixels_ = ToPixelRect(params.bounds);
@@ -1619,33 +1610,6 @@ void DesktopWindowTreeHostX11::SerializeImageRepresentation(
       data->push_back(bitmap.getColor(x, y));
 }
 
-Visual* DesktopWindowTreeHostX11::GetARGBVisual() {
-  XVisualInfo visual_template;
-  visual_template.screen = 0;
-
-  int visuals_len;
-  gfx::XScopedPtr<XVisualInfo[]> visual_list(XGetVisualInfo(
-      xdisplay_, VisualScreenMask, &visual_template, &visuals_len));
-  for (int i = 0; i < visuals_len; ++i) {
-    // Why support only 8888 ARGB? Because it's all that GTK+ supports. In
-    // gdkvisual-x11.cc, they look for this specific visual and use it for all
-    // their alpha channel using needs.
-    //
-    // TODO(erg): While the following does find a valid visual, some GL drivers
-    // don't believe that this has an alpha channel. According to marcheu@,
-    // this should work on open source driver though. (It doesn't work with
-    // NVidia's binaries currently.) http://crbug.com/369209
-    const XVisualInfo& info = visual_list[i];
-    if (info.depth == 32 && info.visual->red_mask == 0xff0000 &&
-        info.visual->green_mask == 0x00ff00 &&
-        info.visual->blue_mask == 0x0000ff) {
-      return info.visual;
-    }
-  }
-
-  return nullptr;
-}
-
 std::list<XID>& DesktopWindowTreeHostX11::open_windows() {
   if (!open_windows_)
     open_windows_ = new std::list<XID>();
diff --git a/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h b/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h
index 20399df..c6d9257 100644
--- a/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h
+++ b/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h
@@ -235,10 +235,6 @@ class VIEWS_EXPORT DesktopWindowTreeHostX11
   void SerializeImageRepresentation(const gfx::ImageSkiaRep& rep,
                                     std::vector<unsigned long>* data);
 
-  // Returns an 8888 ARGB visual. Can return NULL if there is no matching
-  // visual on this display.
-  Visual* GetARGBVisual();
-
   // See comment for variable open_windows_.
   static std::list<XID>& open_windows();