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/+/1953708Reviewed-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

@@ -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",

ui/ozone/platform/wayland/host/shell_object_factory.cc

+// 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

ui/ozone/platform/wayland/host/shell_object_factory.h

+// 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

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,

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;
 

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() {

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_;
 

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;
 }
 

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);

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

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_;