X11 and Ozone: Pass visual id in the init parameters.

Instead of calling XWindow::SetVisualId from DWTHX11:SetVisualId,
the latter was moved to DWTHLinux and now stores the pending
visual id value as base::Optional<int> and passes it to the
PlatformWindowInitProperties on DWTHLinux::AddAdditionalInitProperties
call.

Having SetVisualId calling XWindow is dangerous and leads to segfaults
and XWindow, which is X11Window, which is PlatformWindow, might not
exist by that time.

Bug: 1003347, 990756
Change-Id: Ic62a2974078f27f7f813e44919b51b6539783f2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1803274Reviewed-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@{#698735}

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

@@ -82,7 +82,7 @@ void StatusIconLinuxX11::OnSetDelegate() {
 
   int visual_id;
   if (ui::GetIntProperty(manager, "_NET_SYSTEM_TRAY_VISUAL", &visual_id))
-    host->SetVisualId(visual_id);
+    host->SetPendingXVisualId(visual_id);
 
   const int width = std::max(1, delegate_->GetImage().width());
   const int height = std::max(1, delegate_->GetImage().height());

ui/base/x/x11_window.cc

@@ -203,6 +203,7 @@ void XWindow::Init(const Configuration& config) {
   }
 
   Visual* visual = CopyFromParent;
+  SetVisualId(config.visual_id);
   int depth = CopyFromParent;
   Colormap colormap = CopyFromParent;
   ui::XVisualManager* visual_manager = ui::XVisualManager::GetInstance();
@@ -1591,4 +1592,12 @@ 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();
+}
+
 }  // namespace ui

ui/base/x/x11_window.h

@@ -83,6 +83,7 @@ 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();
@@ -146,7 +147,6 @@ class COMPONENT_EXPORT(UI_BASE_X) XWindow {
   bool use_custom_shape() const { return custom_window_shape_; }
   bool was_minimized() const { return was_minimized_; }
   bool has_alpha() const { return visual_has_alpha_; }
-  void set_visual_id(VisualID visual_id) { visual_id_ = visual_id; }
   base::Optional<int> workspace() const { return workspace_; }
 
   XDisplay* display() const { return xdisplay_; }
@@ -207,6 +207,8 @@ class COMPONENT_EXPORT(UI_BASE_X) XWindow {
 
   void UnconfineCursor();
 
+  void SetVisualId(base::Optional<int> visual_id);
+
   // Interface that must be used by a class that inherits the XWindow to receive
   // different messages from X Server.
   virtual void OnXWindowCreated() = 0;

ui/platform_window/platform_window_init_properties.h

@@ -84,6 +84,9 @@ struct PlatformWindowInitProperties {
   std::string wm_role_name;
   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;
 #endif
 };
 

ui/platform_window/x11/x11_window.cc

@@ -66,6 +66,7 @@ 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;
   return config;
 }
 

ui/views/widget/desktop_aura/desktop_window_tree_host_linux.cc

@@ -23,6 +23,10 @@ DesktopWindowTreeHostLinux::DesktopWindowTreeHostLinux(
 
 DesktopWindowTreeHostLinux::~DesktopWindowTreeHostLinux() = default;
 
+void DesktopWindowTreeHostLinux::SetPendingXVisualId(int x_visual_id) {
+  pending_x_visual_id_ = x_visual_id;
+}
+
 void DesktopWindowTreeHostLinux::OnNativeWidgetCreated(
     const Widget::InitParams& params) {
   AddNonClientEventFilter();
@@ -108,6 +112,8 @@ void DesktopWindowTreeHostLinux::AddAdditionalInitProperties(
   properties->wm_class_name = params.wm_class_name;
   properties->wm_class_class = params.wm_class_class;
   properties->wm_role_name = params.wm_role_name;
+
+  properties->x_visual_id = pending_x_visual_id_;
 }
 
 void DesktopWindowTreeHostLinux::AddNonClientEventFilter() {

ui/views/widget/desktop_aura/desktop_window_tree_host_linux.h

@@ -22,6 +22,10 @@ class VIEWS_EXPORT DesktopWindowTreeHostLinux
       DesktopNativeWidgetAura* desktop_native_widget_aura);
   ~DesktopWindowTreeHostLinux() override;
 
+  // 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);
+
  protected:
   // Overridden from DesktopWindowTreeHost:
   void OnNativeWidgetCreated(const Widget::InitParams& params) override;
@@ -56,6 +60,11 @@ class VIEWS_EXPORT DesktopWindowTreeHostLinux
   // A handler for events intended for non client area.
   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_;
+
   DISALLOW_COPY_AND_ASSIGN(DesktopWindowTreeHostLinux);
 };
 

ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc

@@ -834,11 +834,6 @@ DesktopWindowTreeHostX11::GetKeyboardLayoutMap() {
   return {};
 }
 
-void DesktopWindowTreeHostX11::SetVisualId(VisualID visual_id) {
-  DCHECK_EQ(GetXWindow()->window(), x11::None);
-  GetXWindow()->set_visual_id(visual_id);
-}
-
 void DesktopWindowTreeHostX11::OnBoundsChanged(const gfx::Rect& new_bounds) {
   ResetWindowRegion();
   WindowTreeHostPlatform::OnBoundsChanged(new_bounds);

ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h

@@ -88,10 +88,6 @@ class VIEWS_EXPORT DesktopWindowTreeHostX11 : public DesktopWindowTreeHostLinux,
   // Returns a map of KeyboardEvent code to KeyboardEvent key values.
   base::flat_map<std::string, std::string> GetKeyboardLayoutMap() override;
 
-  // This must be called before the window is created, because the visual cannot
-  // be changed after.
-  void SetVisualId(VisualID visual_id);
-
  protected:
   // Overridden from DesktopWindowTreeHost:
   void Init(const Widget::InitParams& params) override;