path: root/0001-ozone-wayland-Extract-ShellObjectFactory-to-a-separa.patch

From 40bbda2b7265437d6dea1ce9d3361f85147fedf1 Mon Sep 17 00:00:00 2001
From: Maksim Sisov <msisov@igalia.com>
Date: Fri, 6 Dec 2019 19:35:16 +0000
Subject: [PATCH 1/9] ozone/wayland: Extract ShellObjectFactory to a separate
 file.

This CL
1) extracts ShellObjectFactory into a separate file and keeps
the same creation and initialization logic of wrappers of shell objects,
2) extends ShellPopupWrapper and ShellSurfaceWrapper interfaces by adding
Initialize method so that it is more convenient to initialize
XdgPopupWrapperImpl XdgSurfaceWrapperImpl and let them handle initialization
based on the available xdg-shell version,
3) makes WaylandWindow use the extracted ShellObjectFactory,
4) makes InitializeStable and InitializeV6 methods of
XDGWrapper objects be private members. The decision on which
one to call is made in the main Initialize method now.
5) reconsiders the logging whether we need to keep pointers to
some objects or not.

Bug: 1028919
Change-Id: I87fd5fc0d0b35222f26c460c9174da6053dc320e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1953708
Reviewed-by: Michael Spang <spang@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#722572}
---
 ui/ozone/platform/wayland/BUILD.gn            |   2 +
 .../wayland/host/shell_object_factory.cc      |  47 +++++
 .../wayland/host/shell_object_factory.h       |  44 +++++
 .../wayland/host/shell_popup_wrapper.h        |   6 +
 .../wayland/host/shell_surface_wrapper.h      |   4 +
 .../platform/wayland/host/wayland_window.cc   |  72 +------
 .../platform/wayland/host/wayland_window.h    |   7 -
 .../wayland/host/xdg_popup_wrapper_impl.cc    |  48 ++---
 .../wayland/host/xdg_popup_wrapper_impl.h     |  19 +-
 .../wayland/host/xdg_surface_wrapper_impl.cc  | 181 ++++++++++--------
 .../wayland/host/xdg_surface_wrapper_impl.h   |  27 +--
 11 files changed, 261 insertions(+), 196 deletions(-)
 create mode 100644 ui/ozone/platform/wayland/host/shell_object_factory.cc
 create mode 100644 ui/ozone/platform/wayland/host/shell_object_factory.h

diff --git a/ui/ozone/platform/wayland/BUILD.gn b/ui/ozone/platform/wayland/BUILD.gn
index 37e7ffb5d83c..9c2cbc52500b 100644
--- a/ui/ozone/platform/wayland/BUILD.gn
+++ b/ui/ozone/platform/wayland/BUILD.gn
@@ -45,6 +45,8 @@ source_set("wayland") {
     "host/internal/wayland_data_offer_base.h",
     "host/internal/wayland_data_source_base.cc",
     "host/internal/wayland_data_source_base.h",
+    "host/shell_object_factory.cc",
+    "host/shell_object_factory.h",
     "host/shell_popup_wrapper.cc",
     "host/shell_popup_wrapper.h",
     "host/shell_surface_wrapper.cc",
diff --git a/ui/ozone/platform/wayland/host/shell_object_factory.cc b/ui/ozone/platform/wayland/host/shell_object_factory.cc
new file mode 100644
index 000000000000..57383be20da5
--- /dev/null
+++ b/ui/ozone/platform/wayland/host/shell_object_factory.cc
@@ -0,0 +1,47 @@
+// Copyright 2019 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/ozone/platform/wayland/host/shell_object_factory.h"
+
+#include "ui/ozone/platform/wayland/host/wayland_connection.h"
+#include "ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.h"
+#include "ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.h"
+
+namespace ui {
+
+ShellObjectFactory::ShellObjectFactory() = default;
+ShellObjectFactory::~ShellObjectFactory() = default;
+
+std::unique_ptr<ShellSurfaceWrapper>
+ShellObjectFactory::CreateShellSurfaceWrapper(WaylandConnection* connection,
+                                              WaylandWindow* wayland_window) {
+  if (connection->shell() || connection->shell_v6()) {
+    auto surface =
+        std::make_unique<XDGSurfaceWrapperImpl>(wayland_window, connection);
+    return surface->Initialize(true /* with_top_level */) ? std::move(surface)
+                                                          : nullptr;
+  }
+  LOG(WARNING) << "Shell protocol is not available.";
+  return nullptr;
+}
+
+std::unique_ptr<ShellPopupWrapper> ShellObjectFactory::CreateShellPopupWrapper(
+    WaylandConnection* connection,
+    WaylandWindow* wayland_window,
+    const gfx::Rect& bounds) {
+  if (connection->shell() || connection->shell_v6()) {
+    auto surface =
+        std::make_unique<XDGSurfaceWrapperImpl>(wayland_window, connection);
+    if (!surface->Initialize(false /* with_top_level */))
+      return nullptr;
+
+    auto popup = std::make_unique<XDGPopupWrapperImpl>(std::move(surface),
+                                                       wayland_window);
+    return popup->Initialize(connection, bounds) ? std::move(popup) : nullptr;
+  }
+  LOG(WARNING) << "Shell protocol is not available.";
+  return nullptr;
+}
+
+}  // namespace ui
\ No newline at end of file
diff --git a/ui/ozone/platform/wayland/host/shell_object_factory.h b/ui/ozone/platform/wayland/host/shell_object_factory.h
new file mode 100644
index 000000000000..a915326c23fb
--- /dev/null
+++ b/ui/ozone/platform/wayland/host/shell_object_factory.h
@@ -0,0 +1,44 @@
+// Copyright 2019 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_OZONE_PLATFORM_WAYLAND_HOST_SHELL_OBJECT_FACTORY_H_
+#define UI_OZONE_PLATFORM_WAYLAND_HOST_SHELL_OBJECT_FACTORY_H_
+
+#include <memory>
+
+namespace gfx {
+class Rect;
+}
+
+namespace ui {
+
+class ShellSurfaceWrapper;
+class ShellPopupWrapper;
+class WaylandConnection;
+class WaylandWindow;
+
+// Shell factory that creates shell objects for different protocols. Preferred
+// protocols are defined in the following order:
+// 1) xdg-shell-stable-protocol.
+// 2) xdg-shell-v6-unstable-protocol.
+class ShellObjectFactory {
+ public:
+  ShellObjectFactory();
+  ~ShellObjectFactory();
+
+  // Creates and initializes a ShellSurfaceWrapper.
+  std::unique_ptr<ShellSurfaceWrapper> CreateShellSurfaceWrapper(
+      WaylandConnection* connection,
+      WaylandWindow* wayland_window);
+
+  // Creates and intitializes a ShellPopupSurface.
+  std::unique_ptr<ShellPopupWrapper> CreateShellPopupWrapper(
+      WaylandConnection* connection,
+      WaylandWindow* wayland_window,
+      const gfx::Rect& bounds);
+};
+
+}  // namespace ui
+
+#endif  // UI_OZONE_PLATFORM_WAYLAND_HOST_SHELL_OBJECT_FACTORY_H_
\ No newline at end of file
diff --git a/ui/ozone/platform/wayland/host/shell_popup_wrapper.h b/ui/ozone/platform/wayland/host/shell_popup_wrapper.h
index 952c0a2a278a..2970549dd09b 100644
--- a/ui/ozone/platform/wayland/host/shell_popup_wrapper.h
+++ b/ui/ozone/platform/wayland/host/shell_popup_wrapper.h
@@ -10,6 +10,8 @@
 
 namespace ui {
 
+class WaylandConnection;
+
 enum class MenuType {
   TYPE_RIGHT_CLICK,
   TYPE_3DOT_PARENT_MENU,
@@ -67,6 +69,10 @@ inline WlConstraintAdjustment operator&(WlConstraintAdjustment a,
 class ShellPopupWrapper {
  public:
   virtual ~ShellPopupWrapper() {}
+
+  // Initializes the popup surface.
+  virtual bool Initialize(WaylandConnection* connection,
+                          const gfx::Rect& bounds) = 0;
 };
 
 gfx::Rect GetAnchorRect(MenuType menu_type,
diff --git a/ui/ozone/platform/wayland/host/shell_surface_wrapper.h b/ui/ozone/platform/wayland/host/shell_surface_wrapper.h
index 49b27e4c7214..c75e87a627b4 100644
--- a/ui/ozone/platform/wayland/host/shell_surface_wrapper.h
+++ b/ui/ozone/platform/wayland/host/shell_surface_wrapper.h
@@ -21,6 +21,10 @@ class ShellSurfaceWrapper {
  public:
   virtual ~ShellSurfaceWrapper() {}
 
+  // Initializes the ShellSurface. Some protocols may require to create shell
+  // surface without toplevel role and assign a popup role to it later.
+  virtual bool Initialize(bool with_toplevel) = 0;
+
   // Sets a native window to maximized state.
   virtual void SetMaximized() = 0;
 
diff --git a/ui/ozone/platform/wayland/host/wayland_window.cc b/ui/ozone/platform/wayland/host/wayland_window.cc
index 4787a1f3b396..95c6e5a46b3e 100644
--- a/ui/ozone/platform/wayland/host/wayland_window.cc
+++ b/ui/ozone/platform/wayland/host/wayland_window.cc
@@ -16,6 +16,7 @@
 #include "ui/events/event_utils.h"
 #include "ui/events/ozone/events_ozone.h"
 #include "ui/gfx/geometry/point_f.h"
+#include "ui/ozone/platform/wayland/host/shell_object_factory.h"
 #include "ui/ozone/platform/wayland/host/shell_popup_wrapper.h"
 #include "ui/ozone/platform/wayland/host/shell_surface_wrapper.h"
 #include "ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h"
@@ -23,50 +24,12 @@
 #include "ui/ozone/platform/wayland/host/wayland_cursor_position.h"
 #include "ui/ozone/platform/wayland/host/wayland_output_manager.h"
 #include "ui/ozone/platform/wayland/host/wayland_pointer.h"
-#include "ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.h"
-#include "ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.h"
 #include "ui/platform_window/platform_window_handler/wm_drop_handler.h"
 
 namespace ui {
 
 namespace {
 
-// Factory, which decides which version type of xdg object to build.
-class XDGShellObjectFactory {
- public:
-  XDGShellObjectFactory() = default;
-  ~XDGShellObjectFactory() = default;
-
-  std::unique_ptr<ShellPopupWrapper> CreateXDGPopup(
-      WaylandConnection* connection,
-      WaylandWindow* wayland_window,
-      const gfx::Rect& bounds) {
-    std::unique_ptr<XDGSurfaceWrapperImpl> surface =
-        std::make_unique<XDGSurfaceWrapperImpl>(wayland_window);
-    if (connection->shell()) {
-      surface->InitializeStable(connection, wayland_window->surface(), false);
-      std::unique_ptr<XDGPopupWrapperImpl> popup =
-          std::make_unique<XDGPopupWrapperImpl>(std::move(surface),
-                                                wayland_window);
-      popup->InitializeStable(connection, wayland_window->surface(),
-                              wayland_window->parent_window(), bounds);
-      return popup;
-    }
-    DCHECK(connection->shell_v6());
-
-    surface->InitializeV6(connection, wayland_window->surface(), false);
-    std::unique_ptr<XDGPopupWrapperImpl> popup =
-        std::make_unique<XDGPopupWrapperImpl>(std::move(surface),
-                                              wayland_window);
-    popup->InitializeV6(connection, wayland_window->surface(),
-                        wayland_window->parent_window(), bounds);
-    return popup;
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(XDGShellObjectFactory);
-};
-
 // Translates bounds relative to top level window to specified parent.
 gfx::Rect TranslateBoundsToParentCoordinates(const gfx::Rect& child_bounds,
                                              const gfx::Rect& parent_bounds) {
@@ -89,7 +52,6 @@ WaylandWindow::WaylandWindow(PlatformWindowDelegate* delegate,
                              WaylandConnection* connection)
     : delegate_(delegate),
       connection_(connection),
-      xdg_shell_objects_factory_(new XDGShellObjectFactory()),
       state_(PlatformWindowState::kNormal),
       pending_state_(PlatformWindowState::kUnknown) {
   // Set a class property key, which allows |this| to be used for interactive
@@ -124,8 +86,6 @@ WaylandWindow* WaylandWindow::FromSurface(wl_surface* surface) {
 }
 
 bool WaylandWindow::Initialize(PlatformWindowInitProperties properties) {
-  DCHECK(xdg_shell_objects_factory_);
-
   // Properties contain DIP bounds but the buffer scale is initially 1 so it's
   // OK to assign.  The bounds will be recalculated when the buffer scale
   // changes.
@@ -244,35 +204,19 @@ void WaylandWindow::CreateShellPopup() {
 
   auto bounds_px = AdjustPopupWindowPosition();
 
-  shell_popup_ =
-      xdg_shell_objects_factory_->CreateXDGPopup(connection_, this, bounds_px);
-
-  if (!shell_popup_) {
+  ShellObjectFactory factory;
+  shell_popup_ = factory.CreateShellPopupWrapper(connection_, this, bounds_px);
+  if (!shell_popup_)
     CHECK(false) << "Failed to create Wayland shell popup";
-  }
 
   parent_window_->set_child_window(this);
 }
 
 void WaylandWindow::CreateShellSurface() {
-  std::unique_ptr<XDGSurfaceWrapperImpl> xdg_surface =
-      std::make_unique<XDGSurfaceWrapperImpl>(this);
-  if (!xdg_surface) {
-    CHECK(false) << "Failed to create Wayland shell surface";
-    return;
-  }
-
-  if (connection_->shell()) {
-    if (!xdg_surface->InitializeStable(connection_, surface_.get())) {
-      CHECK(false) << "Failed to initialize Wayland shell surface";
-    }
-  } else {
-    DCHECK(connection_->shell_v6());
-    if (!xdg_surface->InitializeV6(connection_, surface_.get())) {
-      CHECK(false) << "Failed to initialize Wayland shell surface";
-    }
-  }
-  shell_surface_ = std::move(xdg_surface);
+  ShellObjectFactory factory;
+  shell_surface_ = factory.CreateShellSurfaceWrapper(connection_, this);
+  if (!shell_surface_)
+    CHECK(false) << "Failed to initialize Wayland shell surface";
 }
 
 void WaylandWindow::CreateAndShowTooltipSubSurface() {
diff --git a/ui/ozone/platform/wayland/host/wayland_window.h b/ui/ozone/platform/wayland/host/wayland_window.h
index 2745dde85c32..4233216d7a6f 100644
--- a/ui/ozone/platform/wayland/host/wayland_window.h
+++ b/ui/ozone/platform/wayland/host/wayland_window.h
@@ -35,10 +35,6 @@ class WaylandConnection;
 class ShellPopupWrapper;
 class ShellSurfaceWrapper;
 
-namespace {
-class XDGShellObjectFactory;
-}  // namespace
-
 class WaylandWindow : public PlatformWindow,
                       public PlatformEventDispatcher,
                       public WmMoveResizeHandler,
@@ -229,9 +225,6 @@ class WaylandWindow : public PlatformWindow,
   WaylandWindow* parent_window_ = nullptr;
   WaylandWindow* child_window_ = nullptr;
 
-  // Creates xdg objects based on xdg shell version.
-  std::unique_ptr<XDGShellObjectFactory> xdg_shell_objects_factory_;
-
   wl::Object<wl_surface> surface_;
   wl::Object<wl_subsurface> tooltip_subsurface_;
 
diff --git a/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.cc b/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.cc
index 0c6683828e74..abf6e7fa6461 100644
--- a/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.cc
+++ b/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.cc
@@ -260,40 +260,45 @@ XDGPopupWrapperImpl::XDGPopupWrapperImpl(
     WaylandWindow* wayland_window)
     : wayland_window_(wayland_window), xdg_surface_(std::move(surface)) {
   DCHECK(xdg_surface_);
+  DCHECK(wayland_window_ && wayland_window_->parent_window());
 }
 
-XDGPopupWrapperImpl::~XDGPopupWrapperImpl() {}
+XDGPopupWrapperImpl::~XDGPopupWrapperImpl() = default;
+
+bool XDGPopupWrapperImpl::Initialize(WaylandConnection* connection,
+                                     const gfx::Rect& bounds) {
+  if (connection->shell())
+    return InitializeStable(connection, bounds);
+  else if (connection->shell_v6())
+    return InitializeV6(connection, bounds);
+  NOTREACHED() << "Wrong shell protocol";
+  return false;
+}
 
 bool XDGPopupWrapperImpl::InitializeStable(WaylandConnection* connection,
-                                           wl_surface* surface,
-                                           WaylandWindow* parent_window,
                                            const gfx::Rect& bounds) {
-  DCHECK(connection && surface && parent_window);
   static const struct xdg_popup_listener xdg_popup_listener = {
       &XDGPopupWrapperImpl::ConfigureStable,
       &XDGPopupWrapperImpl::PopupDoneStable,
   };
 
-  if (!xdg_surface_)
-    return false;
-
   XDGSurfaceWrapperImpl* parent_xdg_surface;
   // If the parent window is a popup, the surface of that popup must be used as
   // a parent.
-  if (parent_window->shell_popup()) {
-    XDGPopupWrapperImpl* popup =
-        reinterpret_cast<XDGPopupWrapperImpl*>(parent_window->shell_popup());
+  if (wayland_window_->parent_window()->shell_popup()) {
+    XDGPopupWrapperImpl* popup = reinterpret_cast<XDGPopupWrapperImpl*>(
+        wayland_window_->parent_window()->shell_popup());
     parent_xdg_surface = popup->xdg_surface();
   } else {
     parent_xdg_surface = reinterpret_cast<XDGSurfaceWrapperImpl*>(
-        parent_window->shell_surface());
+        wayland_window_->parent_window()->shell_surface());
   }
 
   if (!parent_xdg_surface)
     return false;
 
-  struct xdg_positioner* positioner =
-      CreatePositionerStable(connection, parent_window, bounds);
+  struct xdg_positioner* positioner = CreatePositionerStable(
+      connection, wayland_window_->parent_window(), bounds);
   if (!positioner)
     return false;
 
@@ -337,7 +342,7 @@ bool XDGPopupWrapperImpl::InitializeStable(WaylandConnection* connection,
   }
   xdg_popup_add_listener(xdg_popup_.get(), &xdg_popup_listener, this);
 
-  wl_surface_commit(surface);
+  wl_surface_commit(wayland_window_->surface());
   return true;
 }
 
@@ -383,10 +388,7 @@ struct xdg_positioner* XDGPopupWrapperImpl::CreatePositionerStable(
 }
 
 bool XDGPopupWrapperImpl::InitializeV6(WaylandConnection* connection,
-                                       wl_surface* surface,
-                                       WaylandWindow* parent_window,
                                        const gfx::Rect& bounds) {
-  DCHECK(connection && surface && parent_window);
   static const struct zxdg_popup_v6_listener zxdg_popup_v6_listener = {
       &XDGPopupWrapperImpl::ConfigureV6,
       &XDGPopupWrapperImpl::PopupDoneV6,
@@ -398,20 +400,20 @@ bool XDGPopupWrapperImpl::InitializeV6(WaylandConnection* connection,
   XDGSurfaceWrapperImpl* parent_xdg_surface;
   // If the parent window is a popup, the surface of that popup must be used as
   // a parent.
-  if (parent_window->shell_popup()) {
-    XDGPopupWrapperImpl* popup =
-        reinterpret_cast<XDGPopupWrapperImpl*>(parent_window->shell_popup());
+  if (wayland_window_->parent_window()->shell_popup()) {
+    XDGPopupWrapperImpl* popup = reinterpret_cast<XDGPopupWrapperImpl*>(
+        wayland_window_->parent_window()->shell_popup());
     parent_xdg_surface = popup->xdg_surface();
   } else {
     parent_xdg_surface = reinterpret_cast<XDGSurfaceWrapperImpl*>(
-        parent_window->shell_surface());
+        wayland_window_->parent_window()->shell_surface());
   }
 
   if (!parent_xdg_surface)
     return false;
 
   zxdg_positioner_v6* positioner =
-      CreatePositionerV6(connection, parent_window, bounds);
+      CreatePositionerV6(connection, wayland_window_->parent_window(), bounds);
   if (!positioner)
     return false;
 
@@ -457,7 +459,7 @@ bool XDGPopupWrapperImpl::InitializeV6(WaylandConnection* connection,
   zxdg_popup_v6_add_listener(zxdg_popup_v6_.get(), &zxdg_popup_v6_listener,
                              this);
 
-  wl_surface_commit(surface);
+  wl_surface_commit(wayland_window_->surface());
   return true;
 }
 
diff --git a/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.h b/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.h
index 371f18157d73..2a900818ff6a 100644
--- a/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.h
+++ b/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.h
@@ -23,19 +23,16 @@ class XDGPopupWrapperImpl : public ShellPopupWrapper {
   ~XDGPopupWrapperImpl() override;
 
   // XDGPopupWrapper:
-  bool InitializeStable(WaylandConnection* connection,
-                        wl_surface* surface,
-                        WaylandWindow* parent_window,
-                        const gfx::Rect& bounds);
-  bool InitializeV6(WaylandConnection* connection,
-                    wl_surface* surface,
-                    WaylandWindow* parent_window,
-                    const gfx::Rect& bounds);
+  bool Initialize(WaylandConnection* connection,
+                  const gfx::Rect& bounds) override;
 
  private:
+  bool InitializeStable(WaylandConnection* connection, const gfx::Rect& bounds);
   struct xdg_positioner* CreatePositionerStable(WaylandConnection* connection,
                                                 WaylandWindow* parent_window,
                                                 const gfx::Rect& bounds);
+
+  bool InitializeV6(WaylandConnection* connection, const gfx::Rect& bounds);
   struct zxdg_positioner_v6* CreatePositionerV6(WaylandConnection* connection,
                                                 WaylandWindow* parent_window,
                                                 const gfx::Rect& bounds);
@@ -64,9 +61,15 @@ class XDGPopupWrapperImpl : public ShellPopupWrapper {
 
   XDGSurfaceWrapperImpl* xdg_surface();
 
+  // Non-owned WaylandWindow that uses this popup.
   WaylandWindow* const wayland_window_;
+
+  // Ground surface for this popup.
   std::unique_ptr<XDGSurfaceWrapperImpl> xdg_surface_;
+
+  // XDG Shell Stable object.
   wl::Object<xdg_popup> xdg_popup_;
+  // XDG Shell V6 object.
   wl::Object<zxdg_popup_v6> zxdg_popup_v6_;
 
   DISALLOW_COPY_AND_ASSIGN(XDGPopupWrapperImpl);
diff --git a/ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.cc b/ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.cc
index d4078e1bc139..e7df772ae472 100644
--- a/ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.cc
+++ b/ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.cc
@@ -15,84 +15,19 @@
 
 namespace ui {
 
-XDGSurfaceWrapperImpl::XDGSurfaceWrapperImpl(WaylandWindow* wayland_window)
-    : wayland_window_(wayland_window) {}
+XDGSurfaceWrapperImpl::XDGSurfaceWrapperImpl(WaylandWindow* wayland_window,
+                                             WaylandConnection* connection)
+    : wayland_window_(wayland_window), connection_(connection) {}
 
 XDGSurfaceWrapperImpl::~XDGSurfaceWrapperImpl() {}
 
-bool XDGSurfaceWrapperImpl::InitializeStable(WaylandConnection* connection,
-                                             wl_surface* surface,
-                                             bool with_toplevel) {
-  static const xdg_surface_listener xdg_surface_listener = {
-      &XDGSurfaceWrapperImpl::ConfigureStable,
-  };
-  static const xdg_toplevel_listener xdg_toplevel_listener = {
-      &XDGSurfaceWrapperImpl::ConfigureTopLevelStable,
-      &XDGSurfaceWrapperImpl::CloseTopLevelStable,
-  };
-
-  // if this surface is created for the popup role, mark that it requires
-  // configuration acknowledgement on each configure event.
-  surface_for_popup_ = !with_toplevel;
-
-  xdg_surface_.reset(xdg_wm_base_get_xdg_surface(connection->shell(), surface));
-  if (!xdg_surface_) {
-    LOG(ERROR) << "Failed to create xdg_surface";
-    return false;
-  }
-  xdg_surface_add_listener(xdg_surface_.get(), &xdg_surface_listener, this);
-  // XDGPopup requires a separate surface to be created, so this is just a
-  // request to get an xdg_surface for it.
-  if (surface_for_popup_)
-    return true;
-
-  xdg_toplevel_.reset(xdg_surface_get_toplevel(xdg_surface_.get()));
-  if (!xdg_toplevel_) {
-    LOG(ERROR) << "Failed to create xdg_toplevel";
-    return false;
-  }
-  xdg_toplevel_add_listener(xdg_toplevel_.get(), &xdg_toplevel_listener, this);
-  wl_surface_commit(surface);
-  return true;
-}
-
-bool XDGSurfaceWrapperImpl::InitializeV6(WaylandConnection* connection,
-                                         wl_surface* surface,
-                                         bool with_toplevel) {
-  static const zxdg_surface_v6_listener zxdg_surface_v6_listener = {
-      &XDGSurfaceWrapperImpl::ConfigureV6,
-  };
-  static const zxdg_toplevel_v6_listener zxdg_toplevel_v6_listener = {
-      &XDGSurfaceWrapperImpl::ConfigureTopLevelV6,
-      &XDGSurfaceWrapperImpl::CloseTopLevelV6,
-  };
-
-  // if this surface is created for the popup role, mark that it requires
-  // configuration acknowledgement on each configure event.
-  surface_for_popup_ = !with_toplevel;
-
-  zxdg_surface_v6_.reset(
-      zxdg_shell_v6_get_xdg_surface(connection->shell_v6(), surface));
-  if (!zxdg_surface_v6_) {
-    LOG(ERROR) << "Failed to create zxdg_surface";
-    return false;
-  }
-  zxdg_surface_v6_add_listener(zxdg_surface_v6_.get(),
-                               &zxdg_surface_v6_listener, this);
-  // XDGPopupV6 requires a separate surface to be created, so this is just a
-  // request to get an xdg_surface for it.
-  if (surface_for_popup_)
-    return true;
-
-  zxdg_toplevel_v6_.reset(zxdg_surface_v6_get_toplevel(zxdg_surface_v6_.get()));
-  if (!zxdg_toplevel_v6_) {
-    LOG(ERROR) << "Failed to create zxdg_toplevel";
-    return false;
-  }
-  zxdg_toplevel_v6_add_listener(zxdg_toplevel_v6_.get(),
-                                &zxdg_toplevel_v6_listener, this);
-  wl_surface_commit(surface);
-  return true;
+bool XDGSurfaceWrapperImpl::Initialize(bool with_toplevel) {
+  if (connection_->shell())
+    return InitializeStable(with_toplevel);
+  else if (connection_->shell_v6())
+    return InitializeV6(with_toplevel);
+  NOTREACHED() << "Wrong shell protocol";
+  return false;
 }
 
 void XDGSurfaceWrapperImpl::SetMaximized() {
@@ -142,25 +77,25 @@ void XDGSurfaceWrapperImpl::SetMinimized() {
 
 void XDGSurfaceWrapperImpl::SurfaceMove(WaylandConnection* connection) {
   if (xdg_toplevel_) {
-    xdg_toplevel_move(xdg_toplevel_.get(), connection->seat(),
-                      connection->serial());
+    xdg_toplevel_move(xdg_toplevel_.get(), connection_->seat(),
+                      connection_->serial());
   } else {
     DCHECK(zxdg_toplevel_v6_);
-    zxdg_toplevel_v6_move(zxdg_toplevel_v6_.get(), connection->seat(),
-                          connection->serial());
+    zxdg_toplevel_v6_move(zxdg_toplevel_v6_.get(), connection_->seat(),
+                          connection_->serial());
   }
 }
 
 void XDGSurfaceWrapperImpl::SurfaceResize(WaylandConnection* connection,
                                           uint32_t hittest) {
   if (xdg_toplevel_) {
-    xdg_toplevel_resize(xdg_toplevel_.get(), connection->seat(),
-                        connection->serial(),
+    xdg_toplevel_resize(xdg_toplevel_.get(), connection_->seat(),
+                        connection_->serial(),
                         wl::IdentifyDirection(*connection, hittest));
   } else {
     DCHECK(zxdg_toplevel_v6_);
-    zxdg_toplevel_v6_resize(zxdg_toplevel_v6_.get(), connection->seat(),
-                            connection->serial(),
+    zxdg_toplevel_v6_resize(zxdg_toplevel_v6_.get(), connection_->seat(),
+                            connection_->serial(),
                             wl::IdentifyDirection(*connection, hittest));
   }
 }
@@ -317,4 +252,84 @@ xdg_surface* XDGSurfaceWrapperImpl::xdg_surface() const {
   return xdg_surface_.get();
 }
 
+bool XDGSurfaceWrapperImpl::InitializeStable(bool with_toplevel) {
+  static const xdg_surface_listener xdg_surface_listener = {
+      &XDGSurfaceWrapperImpl::ConfigureStable,
+  };
+  static const xdg_toplevel_listener xdg_toplevel_listener = {
+      &XDGSurfaceWrapperImpl::ConfigureTopLevelStable,
+      &XDGSurfaceWrapperImpl::CloseTopLevelStable,
+  };
+
+  // if this surface is created for the popup role, mark that it requires
+  // configuration acknowledgement on each configure event.
+  surface_for_popup_ = !with_toplevel;
+
+  xdg_surface_.reset(xdg_wm_base_get_xdg_surface(connection_->shell(),
+                                                 wayland_window_->surface()));
+  if (!xdg_surface_) {
+    LOG(ERROR) << "Failed to create xdg_surface";
+    return false;
+  }
+  xdg_surface_add_listener(xdg_surface_.get(), &xdg_surface_listener, this);
+  // XDGPopup requires a separate surface to be created, so this is just a
+  // request to get an xdg_surface for it.
+  if (surface_for_popup_) {
+    connection_->ScheduleFlush();
+    return true;
+  }
+
+  xdg_toplevel_.reset(xdg_surface_get_toplevel(xdg_surface_.get()));
+  if (!xdg_toplevel_) {
+    LOG(ERROR) << "Failed to create xdg_toplevel";
+    return false;
+  }
+  xdg_toplevel_add_listener(xdg_toplevel_.get(), &xdg_toplevel_listener, this);
+  wl_surface_commit(wayland_window_->surface());
+
+  connection_->ScheduleFlush();
+  return true;
+}
+
+bool XDGSurfaceWrapperImpl::InitializeV6(bool with_toplevel) {
+  static const zxdg_surface_v6_listener zxdg_surface_v6_listener = {
+      &XDGSurfaceWrapperImpl::ConfigureV6,
+  };
+  static const zxdg_toplevel_v6_listener zxdg_toplevel_v6_listener = {
+      &XDGSurfaceWrapperImpl::ConfigureTopLevelV6,
+      &XDGSurfaceWrapperImpl::CloseTopLevelV6,
+  };
+
+  // if this surface is created for the popup role, mark that it requires
+  // configuration acknowledgement on each configure event.
+  surface_for_popup_ = !with_toplevel;
+
+  zxdg_surface_v6_.reset(zxdg_shell_v6_get_xdg_surface(
+      connection_->shell_v6(), wayland_window_->surface()));
+  if (!zxdg_surface_v6_) {
+    LOG(ERROR) << "Failed to create zxdg_surface";
+    return false;
+  }
+  zxdg_surface_v6_add_listener(zxdg_surface_v6_.get(),
+                               &zxdg_surface_v6_listener, this);
+  // XDGPopupV6 requires a separate surface to be created, so this is just a
+  // request to get an xdg_surface for it.
+  if (surface_for_popup_) {
+    connection_->ScheduleFlush();
+    return true;
+  }
+
+  zxdg_toplevel_v6_.reset(zxdg_surface_v6_get_toplevel(zxdg_surface_v6_.get()));
+  if (!zxdg_toplevel_v6_) {
+    LOG(ERROR) << "Failed to create zxdg_toplevel";
+    return false;
+  }
+  zxdg_toplevel_v6_add_listener(zxdg_toplevel_v6_.get(),
+                                &zxdg_toplevel_v6_listener, this);
+  wl_surface_commit(wayland_window_->surface());
+
+  connection_->ScheduleFlush();
+  return true;
+}
+
 }  // namespace ui
diff --git a/ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.h b/ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.h
index e14956ebe3e7..b34f4b2281ec 100644
--- a/ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.h
+++ b/ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.h
@@ -7,11 +7,10 @@
 
 #include "ui/ozone/platform/wayland/host/shell_surface_wrapper.h"
 
+#include "base/macros.h"
 #include "base/strings/string16.h"
 #include "ui/ozone/platform/wayland/common/wayland_object.h"
 
-#include "base/macros.h"
-
 namespace gfx {
 class Rect;
 }
@@ -24,15 +23,12 @@ class WaylandWindow;
 // Surface wrapper for xdg-shell stable and xdg-shell-unstable-v6
 class XDGSurfaceWrapperImpl : public ShellSurfaceWrapper {
  public:
-  XDGSurfaceWrapperImpl(WaylandWindow* wayland_window);
+  XDGSurfaceWrapperImpl(WaylandWindow* wayland_window,
+                        WaylandConnection* connection);
   ~XDGSurfaceWrapperImpl() override;
 
-  bool InitializeV6(WaylandConnection* connection,
-                    wl_surface* surface,
-                    bool with_toplevel = true);
-  bool InitializeStable(WaylandConnection* connection,
-                        wl_surface* surface,
-                        bool with_toplevel = true);
+  // ShellSurfaceWrapper overrides:
+  bool Initialize(bool with_toplevel) override;
   void SetMaximized() override;
   void UnSetMaximized() override;
   void SetFullscreen() override;
@@ -76,8 +72,17 @@ class XDGSurfaceWrapperImpl : public ShellSurfaceWrapper {
   zxdg_surface_v6* zxdg_surface() const;
 
  private:
-  WaylandWindow* wayland_window_;
-  uint32_t pending_configure_serial_;
+  // Initializes using XDG Shell Stable protocol.
+  bool InitializeStable(bool with_toplevel);
+  // Initializes using XDG Shell V6 protocol.
+  bool InitializeV6(bool with_toplevel);
+
+  // Non-owing WaylandWindow that uses this surface wrapper.
+  WaylandWindow* const wayland_window_;
+  WaylandConnection* const connection_;
+
+  uint32_t pending_configure_serial_ = 0;
+
   wl::Object<zxdg_surface_v6> zxdg_surface_v6_;
   wl::Object<zxdg_toplevel_v6> zxdg_toplevel_v6_;
   wl::Object<struct xdg_surface> xdg_surface_;
-- 
2.24.1