StatusIconLinuxX11: use wm_role_name to identify status icon window

So, I removed the x_pending_visual_id from the DWTHLinux and started
to set the wm_role_name in order to identify status-icon window.

That helps to remove X11 specific calls done in outside X11Window class
and brings us closer to Ozone/X11.

This is a prerequisite CL to split current implementation into two parts:
client and platform.

Bug: 1043089
Change-Id: I4de1a34cd38ca0fb760717dc1e9a3d3c8348a728
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2007709
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#734375}

chrome/browser/ui/BUILD.gn

@@ -2577,6 +2577,7 @@ jumbo_static_library("ui") {
       configs += [ "//build/config/linux:x11" ]
       deps += [
         "//components/dbus/menu",
+        "//ui/base:wm_role_names",
         "//ui/events/devices",
         "//ui/events/devices/x11",
         "//ui/events/platform/x11",

chrome/browser/ui/views/status_icons/status_icon_linux_x11.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/shell_integration_linux.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_tree_host.h"
+#include "ui/base/wm_role_names_linux.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/events/platform/x11/x11_event_source.h"
 #include "ui/gfx/canvas.h"
@@ -88,10 +89,6 @@ void StatusIconLinuxX11::OnSetDelegate() {
   // We outlive the host, so no need to remove ourselves as an observer.
   host->AddObserver(this);
 
-  int visual_id;
-  if (ui::GetIntProperty(manager, "_NET_SYSTEM_TRAY_VISUAL", &visual_id))
-    host->SetPendingXVisualId(visual_id);
-
   const int width = std::max(1, delegate_->GetImage().width());
   const int height = std::max(1, delegate_->GetImage().height());
 
@@ -105,6 +102,7 @@ void StatusIconLinuxX11::OnSetDelegate() {
   params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
   params.native_widget = native_widget.release();
   params.desktop_window_tree_host = host.release();
+  params.wm_role_name = ui::kStatusIconWmRoleName;
   params.wm_class_name = shell_integration_linux::GetProgramClassName();
   params.wm_class_class = shell_integration_linux::GetProgramClassClass();
   // The status icon is a tiny window that doesn't update very often, so

ui/base/BUILD.gn

@@ -1195,6 +1195,17 @@ if (is_win) {
   }
 }
 
+if (is_linux) {
+  # This source set defines linux wm role names and must not have any external
+  # dependencies and be kept as minimal as possible.
+  source_set("wm_role_names") {
+    sources = [
+      "wm_role_names_linux.cc",
+      "wm_role_names_linux.h",
+    ]
+  }
+}
+
 # This target is added as a dependency of browser interactive_ui_tests. It must
 # be source_set, otherwise the linker will drop the tests as dead code.
 source_set("base_interactive_ui_tests") {

ui/base/wm_role_names_linux.cc

@@ -0,0 +1,11 @@
+// Copyright 2020 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 "ui/base/wm_role_names_linux.h"
+
+namespace ui {
+
+const char kStatusIconWmRoleName[] = "status-icon";
+
+}  // namespace ui

ui/base/wm_role_names_linux.h

@@ -0,0 +1,16 @@
+// Copyright 2020 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.
+
+#ifndef UI_BASE_WM_ROLE_NAMES_LINUX_H_
+#define UI_BASE_WM_ROLE_NAMES_LINUX_H_
+
+namespace ui {
+
+// Return the status-icon wm_role_name. Only used to construct status-icon
+// windows for x11.
+extern const char kStatusIconWmRoleName[];
+
+}  // namespace ui
+
+#endif  // UI_BASE_WM_ROLE_NAMES_LINUX_H_

ui/base/x/BUILD.gn

@@ -69,6 +69,7 @@ jumbo_component("x") {
     "//net",
     "//skia",
     "//ui/base:hit_test",
+    "//ui/base:wm_role_names",
     "//ui/base/clipboard:clipboard_types",
     "//ui/display/util",
     "//ui/events",

ui/base/x/x11_window.cc

@@ -13,6 +13,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "third_party/skia/include/core/SkRegion.h"
 #include "ui/base/hit_test_x11.h"
+#include "ui/base/wm_role_names_linux.h"
 #include "ui/base/x/x11_pointer_grab.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/base/x/x11_util_internal.h"
@@ -205,8 +206,17 @@ void XWindow::Init(const Configuration& config) {
       enable_transparent_visuals = config.type == WindowType::kDrag;
   }
 
+  int visual_id;
+  if (config.wm_role_name == kStatusIconWmRoleName) {
+    std::string atom_name =
+        "_NET_SYSTEM_TRAY_S" + base::NumberToString(DefaultScreen(xdisplay_));
+    XID manager =
+        XGetSelectionOwner(xdisplay_, gfx::GetAtom(atom_name.c_str()));
+    if (ui::GetIntProperty(manager, "_NET_SYSTEM_TRAY_VISUAL", &visual_id))
+      visual_id_ = visual_id;
+  }
+
   Visual* visual = CopyFromParent;
-  SetVisualId(config.visual_id);
   int depth = CopyFromParent;
   Colormap colormap = CopyFromParent;
   ui::XVisualManager* visual_manager = ui::XVisualManager::GetInstance();
@@ -1603,14 +1613,6 @@ void XWindow::UnconfineCursor() {
   has_pointer_barriers_ = false;
 }
 
-void XWindow::SetVisualId(base::Optional<int> visual_id) {
-  if (!visual_id.has_value())
-    return;
-
-  DCHECK_GE(visual_id.value(), 0);
-  visual_id_ = visual_id.value();
-}
-
 void XWindow::UpdateWindowRegion(XRegion* xregion) {
   // If a custom window shape was supplied then apply it.
   if (use_custom_shape()) {

ui/base/x/x11_window.h

@@ -88,7 +88,6 @@ class COMPONENT_EXPORT(UI_BASE_X) XWindow {
     std::string wm_class_name;
     std::string wm_class_class;
     std::string wm_role_name;
-    base::Optional<int> visual_id;
   };
 
   XWindow();
@@ -221,8 +220,6 @@ class COMPONENT_EXPORT(UI_BASE_X) XWindow {
 
   void UnconfineCursor();
 
-  void SetVisualId(base::Optional<int> visual_id);
-
   void UpdateWindowRegion(XRegion* xregion);
 
   void NotifyBoundsChanged(const gfx::Rect& new_bounds_in_px);

ui/platform_window/platform_window_init_properties.h

@@ -94,9 +94,6 @@ struct COMPONENT_EXPORT(PLATFORM_WINDOW) PlatformWindowInitProperties {
   std::string wm_class_name;
   std::string wm_class_class;
 
-  // Stores visual id for the system tray in X11.
-  base::Optional<int> x_visual_id;
-
   X11ExtensionDelegate* x11_extension_delegate = nullptr;
 #endif
 };

ui/platform_window/x11/x11_window.cc

@@ -33,45 +33,46 @@ namespace ui {
 
 namespace {
 
-ui::XWindow::Configuration ConvertInitPropertiesToXWindowConfig(
-    const PlatformWindowInitProperties& properties) {
-  using WindowType = ui::XWindow::WindowType;
-  using WindowOpacity = ui::XWindow::WindowOpacity;
-  ui::XWindow::Configuration config;
+XWindow::WindowOpacity GetXWindowOpacity(PlatformWindowOpacity opacity) {
+  using WindowOpacity = XWindow::WindowOpacity;
+  switch (opacity) {
+    case PlatformWindowOpacity::kInferOpacity:
+      return WindowOpacity::kInferOpacity;
+    case PlatformWindowOpacity::kOpaqueWindow:
+      return WindowOpacity::kOpaqueWindow;
+    case PlatformWindowOpacity::kTranslucentWindow:
+      return WindowOpacity::kTranslucentWindow;
+  }
+  NOTREACHED() << "Uknown window opacity.";
+  return WindowOpacity::kInferOpacity;
+}
 
-  switch (properties.type) {
+XWindow::WindowType GetXWindowType(PlatformWindowType window_type) {
+  using WindowType = XWindow::WindowType;
+  switch (window_type) {
     case PlatformWindowType::kWindow:
-      config.type = WindowType::kWindow;
-      break;
+      return WindowType::kWindow;
     case PlatformWindowType::kMenu:
-      config.type = WindowType::kMenu;
-      break;
+      return WindowType::kMenu;
     case PlatformWindowType::kTooltip:
-      config.type = WindowType::kTooltip;
-      break;
+      return WindowType::kTooltip;
     case PlatformWindowType::kPopup:
-      config.type = WindowType::kPopup;
-      break;
+      return WindowType::kPopup;
     case PlatformWindowType::kDrag:
-      config.type = WindowType::kDrag;
+      return WindowType::kDrag;
       break;
     case PlatformWindowType::kBubble:
-      config.type = WindowType::kBubble;
-      break;
-  }
-
-  switch (properties.opacity) {
-    case PlatformWindowOpacity::kInferOpacity:
-      config.opacity = WindowOpacity::kInferOpacity;
-      break;
-    case PlatformWindowOpacity::kOpaqueWindow:
-      config.opacity = WindowOpacity::kOpaqueWindow;
-      break;
-    case PlatformWindowOpacity::kTranslucentWindow:
-      config.opacity = WindowOpacity::kTranslucentWindow;
-      break;
+      return WindowType::kBubble;
   }
+  NOTREACHED() << "Uknown window type.";
+  return WindowType::kWindow;
+}
 
+ui::XWindow::Configuration ConvertInitPropertiesToXWindowConfig(
+    const PlatformWindowInitProperties& properties) {
+  ui::XWindow::Configuration config;
+  config.type = GetXWindowType(properties.type);
+  config.opacity = GetXWindowOpacity(properties.opacity);
   config.bounds = properties.bounds;
   config.icon = properties.icon;
   config.force_show_in_taskbar = properties.force_show_in_taskbar;
@@ -83,7 +84,6 @@ ui::XWindow::Configuration ConvertInitPropertiesToXWindowConfig(
   config.wm_class_class = properties.wm_class_class;
   config.wm_role_name = properties.wm_role_name;
   config.activatable = properties.activatable;
-  config.visual_id = properties.x_visual_id;
   config.prefer_dark_theme = properties.prefer_dark_theme;
   config.background_color = properties.background_color;
   return config;

ui/views/widget/desktop_aura/desktop_window_tree_host_linux.cc

@@ -119,10 +119,6 @@ void DesktopWindowTreeHostLinux::CleanUpWindowList(
   open_windows_ = nullptr;
 }
 
-void DesktopWindowTreeHostLinux::SetPendingXVisualId(int x_visual_id) {
-  pending_x_visual_id_ = x_visual_id;
-}
-
 gfx::Rect DesktopWindowTreeHostLinux::GetXRootWindowOuterBounds() const {
   // TODO(msisov): must be removed as soon as all X11 low-level bits are moved
   // to Ozone.
@@ -333,8 +329,6 @@ void DesktopWindowTreeHostLinux::AddAdditionalInitProperties(
   properties->wm_class_class = params.wm_class_class;
   properties->wm_role_name = params.wm_role_name;
 
-  properties->x_visual_id = pending_x_visual_id_;
-
   DCHECK(!properties->x11_extension_delegate);
   properties->x11_extension_delegate = this;
 }

ui/views/widget/desktop_aura/desktop_window_tree_host_linux.h

@@ -55,10 +55,6 @@ class VIEWS_EXPORT DesktopWindowTreeHostLinux
   // internal list of open windows.
   static void CleanUpWindowList(void (*func)(aura::Window* window));
 
-  // This must be called before the window is created, because the visual cannot
-  // be changed after. Useful for X11. Not in use for Wayland.
-  void SetPendingXVisualId(int x_visual_id);
-
   // Returns the current bounds in terms of the X11 Root Window including the
   // borders provided by the window manager (if any). Not in use for Wayland.
   gfx::Rect GetXRootWindowOuterBounds() const;
@@ -129,11 +125,6 @@ class VIEWS_EXPORT DesktopWindowTreeHostLinux
   // other consumer handled them.
   std::unique_ptr<WindowEventFilterLinux> non_client_window_event_filter_;
 
-  // X11 may set set a visual id for the system tray icon before the host is
-  // initialized. This value will be passed down to PlatformWindow during
-  // initialization of the host.
-  base::Optional<int> pending_x_visual_id_;
-
   std::unique_ptr<CompositorObserver> compositor_observer_;
 
   std::unique_ptr<aura::ScopedWindowTargeter> targeter_for_modal_;

ui/views/widget/desktop_aura/desktop_window_tree_host_platform.cc

@@ -42,36 +42,43 @@ bool DetermineInactivity(ui::WindowShowState show_state) {
   return show_state == ui::SHOW_STATE_INACTIVE;
 }
 
-ui::PlatformWindowInitProperties ConvertWidgetInitParamsToInitProperties(
-    const Widget::InitParams& params) {
-  ui::PlatformWindowInitProperties properties;
+ui::PlatformWindowOpacity GetPlatformWindowOpacity(
+    Widget::InitParams::WindowOpacity opacity) {
+  switch (opacity) {
+    case Widget::InitParams::WindowOpacity::kInferred:
+      return ui::PlatformWindowOpacity::kInferOpacity;
+    case Widget::InitParams::WindowOpacity::kOpaque:
+      return ui::PlatformWindowOpacity::kOpaqueWindow;
+    case Widget::InitParams::WindowOpacity::kTranslucent:
+      return ui::PlatformWindowOpacity::kTranslucentWindow;
+  }
+  return ui::PlatformWindowOpacity::kOpaqueWindow;
+}
 
-  switch (params.type) {
+ui::PlatformWindowType GetPlatformWindowType(
+    Widget::InitParams::Type window_type) {
+  switch (window_type) {
     case Widget::InitParams::TYPE_WINDOW:
-      properties.type = ui::PlatformWindowType::kWindow;
-      break;
-
+      return ui::PlatformWindowType::kWindow;
     case Widget::InitParams::TYPE_MENU:
-      properties.type = ui::PlatformWindowType::kMenu;
-      break;
-
+      return ui::PlatformWindowType::kMenu;
     case Widget::InitParams::TYPE_TOOLTIP:
-      properties.type = ui::PlatformWindowType::kTooltip;
-      break;
-
+      return ui::PlatformWindowType::kTooltip;
     case Widget::InitParams::TYPE_DRAG:
-      properties.type = ui::PlatformWindowType::kDrag;
-      break;
-
+      return ui::PlatformWindowType::kDrag;
     case Widget::InitParams::TYPE_BUBBLE:
-      properties.type = ui::PlatformWindowType::kBubble;
-      break;
-
+      return ui::PlatformWindowType::kBubble;
     default:
-      properties.type = ui::PlatformWindowType::kPopup;
-      break;
+      return ui::PlatformWindowType::kPopup;
   }
+  NOTREACHED();
+  return ui::PlatformWindowType::kPopup;
+}
 
+ui::PlatformWindowInitProperties ConvertWidgetInitParamsToInitProperties(
+    const Widget::InitParams& params) {
+  ui::PlatformWindowInitProperties properties;
+  properties.type = GetPlatformWindowType(params.type);
   properties.activatable =
       params.activatable == Widget::InitParams::ACTIVATABLE_YES;
   properties.force_show_in_taskbar = params.force_show_in_taskbar;
@@ -80,22 +87,11 @@ ui::PlatformWindowInitProperties ConvertWidgetInitParamsToInitProperties(
   properties.visible_on_all_workspaces = params.visible_on_all_workspaces;
   properties.remove_standard_frame = params.remove_standard_frame;
   properties.workspace = params.workspace;
+  properties.opacity = GetPlatformWindowOpacity(params.opacity);
 
   if (params.parent && params.parent->GetHost())
     properties.parent_widget = params.parent->GetHost()->GetAcceleratedWidget();
 
-  switch (params.opacity) {
-    case Widget::InitParams::WindowOpacity::kInferred:
-      properties.opacity = ui::PlatformWindowOpacity::kInferOpacity;
-      break;
-    case Widget::InitParams::WindowOpacity::kOpaque:
-      properties.opacity = ui::PlatformWindowOpacity::kOpaqueWindow;
-      break;
-    case Widget::InitParams::WindowOpacity::kTranslucent:
-      properties.opacity = ui::PlatformWindowOpacity::kTranslucentWindow;
-      break;
-  }
-
   return properties;
 }