X11: Use CopyFromParent colormap when possible (Reland)

ui/base/x/x11_util.cc

 // Copyright (c) 2012 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.
 
 // This file defines utility functions for X11 (Linux only). This code has been
 // ported from XCB since we can't use XCB on Ubuntu while its 32-bit support
 // remains woefully incomplete.
 
 #include "ui/base/x/x11_util.h"
 
@@ skipping 1402 matching lines @@
 
 #if !defined(OS_CHROMEOS)
 
 // static
 XVisualManager* XVisualManager::GetInstance() {
   return base::Singleton<XVisualManager>::get();
 }
 
 XVisualManager::XVisualManager()
     : display_(gfx::GetXDisplay()),
       system_visual_id_(0),

[Daniel Erat, 2016/11/02 15:14:46]

nit: also initialize default_visual_id_ to 0 here for consistency

[Tom (Use chromium acct), 2016/11/02 17:38:58]

Done.

       transparent_visual_id_(0),
       using_software_rendering_(false),
       have_gpu_argb_visual_(false) {
   int visuals_len;
   XVisualInfo visual_template;
   visual_template.screen = DefaultScreen(display_);
   gfx::XScopedPtr<XVisualInfo[]> visual_list(XGetVisualInfo(
       display_, VisualScreenMask, &visual_template, &visuals_len));
   for (int i = 0; i < visuals_len; ++i)
     visuals_[visual_list[i].visualid].reset(new XVisualData(visual_list[i]));
 
   XAtom NET_WM_CM_S0 = XInternAtom(display_, "_NET_WM_CM_S0", False);
   using_compositing_wm_ = XGetSelectionOwner(display_, NET_WM_CM_S0) != None;
 
   // Choose the opaque visual.
-  XWindowAttributes attribs;
-  Window root = XDefaultRootWindow(display_);
-  Status status = XGetWindowAttributes(display_, root, &attribs);
-  DCHECK_NE(0, status);
-  system_visual_id_ = attribs.visual->visualid;
+  default_visual_id_ =
+      XVisualIDFromVisual(DefaultVisual(display_, DefaultScreen(display_)));
+  system_visual_id_ = default_visual_id_;

[Daniel Erat, 2016/11/02 15:14:46]

i was initially confused about why there are two members with the same value,
but then i saw that system_visual_id_ can be updated by
XVisualManager::OnGPUInfoChanged.

[Tom (Use chromium acct), 2016/11/02 17:38:58]

Added a small comment in x11_util_internal.h to clarify this

   DCHECK(system_visual_id_);
   DCHECK(visuals_.find(system_visual_id_) != visuals_.end());
 
   // Choose the transparent visual.
   for (const auto& pair : visuals_) {
     // 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.
     const XVisualInfo& info = pair.second->visual_info;
     if (info.depth == 32 && info.visual->red_mask == 0xff0000 &&
         info.visual->green_mask == 0x00ff00 &&
         info.visual->blue_mask == 0x0000ff) {
       transparent_visual_id_ = info.visualid;
       break;
     }
   }
   if (transparent_visual_id_)
     DCHECK(visuals_.find(transparent_visual_id_) != visuals_.end());
 }
 
 XVisualManager::~XVisualManager() {}
 
 void XVisualManager::ChooseVisualForWindow(bool want_argb_visual,
                                            Visual** visual,
                                            int* depth,
                                            Colormap* colormap,
                                            bool* using_argb_visual) {
   bool use_argb = want_argb_visual && using_compositing_wm_ &&
                   (using_software_rendering_ || have_gpu_argb_visual_);
-  XVisualData& visual_data =
-      *visuals_[use_argb && transparent_visual_id_ ? transparent_visual_id_
-                                                   : system_visual_id_];
+  VisualID visual_id = use_argb && transparent_visual_id_
+                           ? transparent_visual_id_
+                           : system_visual_id_;
+  XVisualData& visual_data = *visuals_[visual_id];
+  const XVisualInfo& visual_info = visual_data.visual_info;
+
+  bool is_default_visual = visual_id == default_visual_id_;
+
   if (visual)
-    *visual = visual_data.visual_info.visual;
+    *visual = is_default_visual ? CopyFromParent : visual_info.visual;
   if (depth)
-    *depth = visual_data.visual_info.depth;
+    *depth = is_default_visual ? CopyFromParent : visual_info.depth;
   if (colormap)
-    *colormap = visual_data.GetColormap();
+    *colormap = is_default_visual ? CopyFromParent : visual_data.GetColormap();
   if (using_argb_visual)
     *using_argb_visual = use_argb;
 }
 
 bool XVisualManager::OnGPUInfoChanged(bool software_rendering,
                                       VisualID system_visual_id,
                                       VisualID transparent_visual_id) {
   // TODO(thomasanderson): Cache these visual IDs as a property of the root
   // window so that newly created browser processes can get them immediately.
   if ((system_visual_id && !visuals_.count(system_visual_id)) ||
@@ skipping 25 matching lines @@
   return colormap_;
 }
 
 #endif
 
 // ----------------------------------------------------------------------------
 // End of x11_util_internal.h
 
 
 }  // namespace ui