path: root/0006-ozone-wayland-extract-popup-methods-to-WaylandPopup.patch

From 0cff96c5189fee16e0b2f79685c86e73216652aa Mon Sep 17 00:00:00 2001
From: Maksim Sisov <msisov@igalia.com>
Date: Wed, 11 Dec 2019 09:50:09 +0000
Subject: [PATCH 6/9] ozone/wayland: extract popup methods to WaylandPopup

This change does not bring any functional changes and just
extracts menus/popups methods to a separate class named
WaylandPopup.

Also, to avoid code duplicate, the XdgPopupWrapperImpl::Initialize
gets a parent xdg surface and passes it to ::InitializeStable and
::InitializeV6 methods.

Test: ozone_unittests
Bug: 1028919
Change-Id: I216e1374b7e6e97080e40043634db91a20a7b4cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1958989
Reviewed-by: Michael Spang <spang@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#723755}
---
 ui/ozone/platform/wayland/BUILD.gn            |   2 +
 .../platform/wayland/common/wayland_util.cc   |   5 +
 .../platform/wayland/common/wayland_util.h    |   4 +
 .../platform/wayland/host/wayland_popup.cc    | 210 +++++++++++++++
 .../platform/wayland/host/wayland_popup.h     |  49 ++++
 .../platform/wayland/host/wayland_window.cc   | 243 +++---------------
 .../platform/wayland/host/wayland_window.h    |  41 +--
 .../wayland/host/wayland_window_factory.cc    |   3 +-
 .../wayland/host/xdg_popup_wrapper_impl.cc    |  83 +++---
 .../wayland/host/xdg_popup_wrapper_impl.h     |   8 +-
 10 files changed, 371 insertions(+), 277 deletions(-)
 create mode 100644 ui/ozone/platform/wayland/host/wayland_popup.cc
 create mode 100644 ui/ozone/platform/wayland/host/wayland_popup.h

diff --git a/ui/ozone/platform/wayland/BUILD.gn b/ui/ozone/platform/wayland/BUILD.gn
index b6059c7d9962..4bfe93352f34 100644
--- a/ui/ozone/platform/wayland/BUILD.gn
+++ b/ui/ozone/platform/wayland/BUILD.gn
@@ -85,6 +85,8 @@ source_set("wayland") {
     "host/wayland_output_manager.h",
     "host/wayland_pointer.cc",
     "host/wayland_pointer.h",
+    "host/wayland_popup.cc",
+    "host/wayland_popup.h",
     "host/wayland_screen.cc",
     "host/wayland_screen.h",
     "host/wayland_shm.cc",
diff --git a/ui/ozone/platform/wayland/common/wayland_util.cc b/ui/ozone/platform/wayland/common/wayland_util.cc
index e0430b860adf..578bab8930c8 100644
--- a/ui/ozone/platform/wayland/common/wayland_util.cc
+++ b/ui/ozone/platform/wayland/common/wayland_util.cc
@@ -155,4 +155,9 @@ gfx::Rect TranslateBoundsToTopLevelCoordinates(const gfx::Rect& child_bounds,
       child_bounds.size());
 }
 
+bool IsMenuType(ui::PlatformWindowType type) {
+  return type == ui::PlatformWindowType::kMenu ||
+         type == ui::PlatformWindowType::kPopup;
+}
+
 }  // namespace wl
diff --git a/ui/ozone/platform/wayland/common/wayland_util.h b/ui/ozone/platform/wayland/common/wayland_util.h
index 48332e059da0..3db369654025 100644
--- a/ui/ozone/platform/wayland/common/wayland_util.h
+++ b/ui/ozone/platform/wayland/common/wayland_util.h
@@ -15,6 +15,7 @@
 #include "base/macros.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/ozone/platform/wayland/common/wayland_object.h"
+#include "ui/platform_window/platform_window_init_properties.h"
 
 class SkBitmap;
 
@@ -58,6 +59,9 @@ gfx::Rect TranslateBoundsToParentCoordinates(const gfx::Rect& child_bounds,
 gfx::Rect TranslateBoundsToTopLevelCoordinates(const gfx::Rect& child_bounds,
                                                const gfx::Rect& parent_bounds);
 
+// Says if the type is kPopup or kMenu.
+bool IsMenuType(ui::PlatformWindowType type);
+
 }  // namespace wl
 
 #endif  // UI_OZONE_PLATFORM_WAYLAND_COMMON_WAYLAND_UTIL_H_
diff --git a/ui/ozone/platform/wayland/host/wayland_popup.cc b/ui/ozone/platform/wayland/host/wayland_popup.cc
new file mode 100644
index 000000000000..45aac6e51bfc
--- /dev/null
+++ b/ui/ozone/platform/wayland/host/wayland_popup.cc
@@ -0,0 +1,210 @@
+// 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/wayland_popup.h"
+
+#include "ui/ozone/platform/wayland/common/wayland_util.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/wayland_buffer_manager_host.h"
+#include "ui/ozone/platform/wayland/host/wayland_connection.h"
+
+namespace ui {
+
+WaylandPopup::WaylandPopup(PlatformWindowDelegate* delegate,
+                           WaylandConnection* connection)
+    : WaylandWindow(delegate, connection) {}
+
+WaylandPopup::~WaylandPopup() = default;
+
+void WaylandPopup::CreateShellPopup() {
+  if (GetBounds().IsEmpty())
+    return;
+
+  // TODO(jkim): Consider how to support DropArrow window on tabstrip.
+  // When it starts dragging, as described the protocol, https://goo.gl/1Mskq3,
+  // the client must have an active implicit grab. If we try to create a popup
+  // window while dragging is executed, it gets 'popup_done' directly from
+  // Wayland compositor and it's destroyed through 'popup_done'. It causes
+  // a crash when aura::Window is destroyed.
+  // https://crbug.com/875164
+  if (connection()->IsDragInProgress()) {
+    LOG(ERROR) << "Wayland can't create a popup window during dragging.";
+    return;
+  }
+
+  DCHECK(parent_window() && !shell_popup_);
+
+  auto bounds_px = AdjustPopupWindowPosition();
+
+  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 WaylandPopup::Show(bool inactive) {
+  set_keyboard_focus(true);
+
+  if (!shell_popup_) {
+    // When showing a sub-menu after it has been previously shown and hidden,
+    // Wayland sends SetBounds prior to Show, and |bounds_px| takes the pixel
+    // bounds.  This makes a difference against the normal flow when the
+    // window is created (see |Initialize|).  To equalize things, rescale
+    // |bounds_px_| to DIP.  It will be adjusted while creating the popup.
+    SetBounds(gfx::ScaleToRoundedRect(GetBounds(), 1.0 / ui_scale()));
+    CreateShellPopup();
+    connection()->ScheduleFlush();
+  }
+
+  UpdateBufferScale(false);
+}
+
+void WaylandPopup::Hide() {
+  if (child_window())
+    child_window()->Hide();
+
+  if (shell_popup_) {
+    parent_window()->set_child_window(nullptr);
+    shell_popup_.reset();
+  }
+
+  // Detach buffer from surface in order to completely shutdown popups and
+  // tooltips, and release resources.
+  connection()->buffer_manager_host()->ResetSurfaceContents(GetWidget());
+}
+
+bool WaylandPopup::IsVisible() const {
+  return !!shell_popup_;
+}
+
+bool WaylandPopup::HasCapture() const {
+  // WaylandPopups always have captures.
+  return shell_popup();
+}
+
+void WaylandPopup::HandlePopupConfigure(const gfx::Rect& bounds_dip) {
+  DCHECK(shell_popup());
+  DCHECK(parent_window());
+
+  SetBufferScale(parent_window()->buffer_scale(), true);
+
+  gfx::Rect new_bounds_dip = bounds_dip;
+
+  // It's not enough to just set new bounds. If it is a menu window, whose
+  // parent is a top level window a.k.a browser window, it can be flipped
+  // vertically along y-axis and have negative values set. Chromium cannot
+  // understand that and starts to position nested menu windows incorrectly. To
+  // fix that, we have to bear in mind that Wayland compositor does not share
+  // global coordinates for any surfaces, and Chromium assumes the top level
+  // window is always located at 0,0 origin. What is more, child windows must
+  // always be positioned relative to parent window local surface coordinates.
+  // Thus, if the menu window is flipped along y-axis by Wayland and its origin
+  // is above the top level parent window, the origin of the top level window
+  // has to be shifted by that value on y-axis so that the origin of the menu
+  // becomes x,0, and events can be handled normally.
+  if (!wl::IsMenuType(parent_window()->type())) {
+    gfx::Rect parent_bounds = parent_window()->GetBounds();
+    // The menu window is flipped along y-axis and have x,-y origin. Shift the
+    // parent top level window instead.
+    if (new_bounds_dip.y() < 0) {
+      // Move parent bounds along y-axis.
+      parent_bounds.set_y(-(new_bounds_dip.y() * buffer_scale()));
+      new_bounds_dip.set_y(0);
+    } else {
+      // If the menu window is located at correct origin from the browser point
+      // of view, return the top level window back to 0,0.
+      parent_bounds.set_y(0);
+    }
+    parent_window()->SetBounds(parent_bounds);
+  } else {
+    // The nested menu windows are located relative to the parent menu windows.
+    // Thus, the location must be translated to be relative to the top level
+    // window, which automatically becomes the same as relative to an origin of
+    // a display.
+    new_bounds_dip = gfx::ScaleToRoundedRect(
+        wl::TranslateBoundsToTopLevelCoordinates(
+            gfx::ScaleToRoundedRect(new_bounds_dip, buffer_scale()),
+            parent_window()->GetBounds()),
+        1.0 / buffer_scale());
+    DCHECK(new_bounds_dip.y() >= 0);
+  }
+
+  SetBoundsDip(new_bounds_dip);
+}
+
+void WaylandPopup::OnCloseRequest() {
+  // Before calling OnCloseRequest, the |shell_popup_| must become hidden and
+  // only then call OnCloseRequest().
+  DCHECK(!shell_popup_);
+  WaylandWindow::OnCloseRequest();
+}
+
+bool WaylandPopup::OnInitialize(PlatformWindowInitProperties properties) {
+  if (!wl::IsMenuType(type()))
+    return false;
+
+  set_parent_window(GetParentWindow(properties.parent_widget));
+  if (!parent_window()) {
+    LOG(ERROR) << "Failed to get a parent window for this popup";
+    return false;
+  }
+  // If parent window is known in advanced, we may set the scale early.
+  SetBufferScale(parent_window()->buffer_scale(), false);
+  set_ui_scale(parent_window()->ui_scale());
+  CreateShellPopup();
+  return true;
+}
+
+gfx::Rect WaylandPopup::AdjustPopupWindowPosition() {
+  auto* top_level_parent = wl::IsMenuType(parent_window()->type())
+                               ? parent_window()->parent_window()
+                               : parent_window();
+  DCHECK(top_level_parent);
+  DCHECK(buffer_scale() == top_level_parent->buffer_scale());
+  DCHECK(ui_scale() == top_level_parent->ui_scale());
+
+  // Chromium positions windows in screen coordinates, but Wayland requires them
+  // to be in local surface coordinates a.k.a relative to parent window.
+  const gfx::Rect parent_bounds_dip =
+      gfx::ScaleToRoundedRect(parent_window()->GetBounds(), 1.0 / ui_scale());
+  gfx::Rect new_bounds_dip =
+      wl::TranslateBoundsToParentCoordinates(GetBounds(), parent_bounds_dip);
+
+  // Chromium may decide to position nested menu windows on the left side
+  // instead of the right side of parent menu windows when the size of the
+  // window becomes larger than the display it is shown on. It's correct when
+  // the window is located on one display and occupies the whole work area, but
+  // as soon as it's moved and there is space on the right side, Chromium
+  // continues positioning the nested menus on the left side relative to the
+  // parent menu (Wayland does not provide clients with global coordinates).
+  // Instead, reposition that window to be on the right side of the parent menu
+  // window and let the compositor decide how to position it if it does not fit
+  // a single display. However, there is one exception - if the window is
+  // maximized, let Chromium position it on the left side as long as the Wayland
+  // compositor may decide to position the nested window on the right side of
+  // the parent menu window, which results in showing it on a second display if
+  // more than one display is used.
+  if (wl::IsMenuType(parent_window()->type()) &&
+      parent_window()->parent_window() &&
+      (parent_window()->parent_window()->GetPlatformWindowState() !=
+       PlatformWindowState::kMaximized)) {
+    auto* top_level_window = parent_window()->parent_window();
+    DCHECK(top_level_window && !wl::IsMenuType(top_level_window->type()));
+    if (new_bounds_dip.x() <= 0 && top_level_window->GetPlatformWindowState() !=
+                                       PlatformWindowState::kMaximized) {
+      // Position the child menu window on the right side of the parent window
+      // and let the Wayland compositor decide how to do constraint
+      // adjustments.
+      int new_x = parent_bounds_dip.width() -
+                  (new_bounds_dip.width() + new_bounds_dip.x());
+      new_bounds_dip.set_x(new_x);
+    }
+  }
+  return gfx::ScaleToRoundedRect(new_bounds_dip, ui_scale() / buffer_scale());
+}
+
+}  // namespace ui
diff --git a/ui/ozone/platform/wayland/host/wayland_popup.h b/ui/ozone/platform/wayland/host/wayland_popup.h
new file mode 100644
index 000000000000..2d45986ba36a
--- /dev/null
+++ b/ui/ozone/platform/wayland/host/wayland_popup.h
@@ -0,0 +1,49 @@
+// 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_WAYLAND_POPUP_H_
+#define UI_OZONE_PLATFORM_WAYLAND_HOST_WAYLAND_POPUP_H_
+
+#include "ui/ozone/platform/wayland/host/wayland_window.h"
+
+namespace ui {
+
+class WaylandConnection;
+class ShellPopupWrapper;
+
+class WaylandPopup : public WaylandWindow {
+ public:
+  WaylandPopup(PlatformWindowDelegate* delegate, WaylandConnection* connection);
+  ~WaylandPopup() override;
+
+  ShellPopupWrapper* shell_popup() const { return shell_popup_.get(); }
+
+  // PlatformWindow
+  void Show(bool inactive) override;
+  void Hide() override;
+  bool IsVisible() const override;
+  bool HasCapture() const override;
+
+ private:
+  // WaylandWindow overrides:
+  void HandlePopupConfigure(const gfx::Rect& bounds) override;
+  void OnCloseRequest() override;
+  bool OnInitialize(PlatformWindowInitProperties properties) override;
+
+  // Creates a popup window, which is visible as a menu window.
+  void CreateShellPopup();
+
+  // Returns bounds with origin relative to parent window's origin.
+  gfx::Rect AdjustPopupWindowPosition();
+
+  // Wrappers around xdg v5 and xdg v6 objects. WaylandPopup doesn't
+  // know anything about the version.
+  std::unique_ptr<ShellPopupWrapper> shell_popup_;
+
+  DISALLOW_COPY_AND_ASSIGN(WaylandPopup);
+};
+
+}  // namespace ui
+
+#endif  // UI_OZONE_PLATFORM_WAYLAND_HOST_WAYLAND_POPUP_H_
diff --git a/ui/ozone/platform/wayland/host/wayland_window.cc b/ui/ozone/platform/wayland/host/wayland_window.cc
index cdc95f904347..045f922a759e 100644
--- a/ui/ozone/platform/wayland/host/wayland_window.cc
+++ b/ui/ozone/platform/wayland/host/wayland_window.cc
@@ -14,8 +14,6 @@
 #include "ui/events/ozone/events_ozone.h"
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/ozone/platform/wayland/common/wayland_util.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/wayland_buffer_manager_host.h"
 #include "ui/ozone/platform/wayland/host/wayland_connection.h"
 #include "ui/ozone/platform/wayland/host/wayland_cursor_position.h"
@@ -78,35 +76,6 @@ gfx::AcceleratedWidget WaylandWindow::GetWidget() const {
   return surface_.id();
 }
 
-void WaylandWindow::CreateShellPopup() {
-  if (bounds_px_.IsEmpty())
-    return;
-
-  // TODO(jkim): Consider how to support DropArrow window on tabstrip.
-  // When it starts dragging, as described the protocol, https://goo.gl/1Mskq3,
-  // the client must have an active implicit grab. If we try to create a popup
-  // window while dragging is executed, it gets 'popup_done' directly from
-  // Wayland compositor and it's destroyed through 'popup_done'. It causes
-  // a crash when aura::Window is destroyed.
-  // https://crbug.com/875164
-  if (connection_->IsDragInProgress()) {
-    surface_.reset();
-    LOG(ERROR) << "Wayland can't create a popup window during dragging.";
-    return;
-  }
-
-  DCHECK(parent_window_ && !shell_popup_);
-
-  auto bounds_px = AdjustPopupWindowPosition();
-
-  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::CreateAndShowTooltipSubSurface() {
   // Since Aura does not not provide a reference parent window, needed by
   // Wayland, we get the current focused window to place and show the tooltips.
@@ -156,41 +125,17 @@ void WaylandWindow::SetPointerFocus(bool focus) {
 }
 
 void WaylandWindow::Show(bool inactive) {
-  if (!is_tooltip_)  // Tooltip windows should not get keyboard focus
-    set_keyboard_focus(true);
-
-  if (is_tooltip_) {
-    CreateAndShowTooltipSubSurface();
-    return;
-  }
-
-  if (!shell_popup_) {
-    // When showing a sub-menu after it has been previously shown and hidden,
-    // Wayland sends SetBounds prior to Show, and |bounds_px| takes the pixel
-    // bounds.  This makes a difference against the normal flow when the
-    // window is created (see |Initialize|).  To equalize things, rescale
-    // |bounds_px_| to DIP.  It will be adjusted while creating the popup.
-    bounds_px_ = gfx::ScaleToRoundedRect(bounds_px_, 1.0 / ui_scale_);
-    CreateShellPopup();
-    connection_->ScheduleFlush();
-  }
+  DCHECK(is_tooltip_);
+  CreateAndShowTooltipSubSurface();
 
   UpdateBufferScale(false);
 }
 
 void WaylandWindow::Hide() {
-  if (is_tooltip_) {
-    tooltip_subsurface_.reset();
-  } else {
-    if (child_window_)
-      child_window_->Hide();
-    if (shell_popup_) {
-      parent_window_->set_child_window(nullptr);
-      shell_popup_.reset();
-    }
-  }
+  DCHECK(is_tooltip_);
+  tooltip_subsurface_.reset();
 
-  // Detach buffer from surface in order to completely shutdown popups and
+  // Detach buffer from surface in order to completely shutdown menus and
   // tooltips, and release resources.
   connection_->buffer_manager_host()->ResetSurfaceContents(GetWidget());
 }
@@ -200,7 +145,7 @@ void WaylandWindow::Close() {
 }
 
 bool WaylandWindow::IsVisible() const {
-  return !!shell_popup_;
+  return !!tooltip_subsurface_;
 }
 
 void WaylandWindow::PrepareForShutdown() {}
@@ -225,8 +170,8 @@ void WaylandWindow::SetTitle(const base::string16& title) {}
 
 void WaylandWindow::SetCapture() {
   // Wayland does implicit grabs, and doesn't allow for explicit grabs. The
-  // exception to that are popups, but we explicitly send events to a
-  // parent popup if such exists.
+  // exception to that are menus, but we explicitly send events to a
+  // parent menu if such exists.
 }
 
 void WaylandWindow::ReleaseCapture() {
@@ -234,8 +179,7 @@ void WaylandWindow::ReleaseCapture() {
 }
 
 bool WaylandWindow::HasCapture() const {
-  // If WaylandWindow is a popup window, assume it has the capture.
-  return shell_popup() ? true : has_implicit_grab_;
+  return has_implicit_grab_;
 }
 
 void WaylandWindow::ToggleFullscreen() {}
@@ -319,14 +263,14 @@ void WaylandWindow::SetWindowIcons(const gfx::ImageSkia& window_icon,
 void WaylandWindow::SizeConstraintsChanged() {}
 
 bool WaylandWindow::CanDispatchEvent(const PlatformEvent& event) {
-  // This window is a nested popup window, all the events must be forwarded
-  // to the main popup window.
-  if (child_window_ && child_window_->shell_popup())
-    return !!shell_popup_.get();
+  // This window is a nested menu window, all the events must be forwarded
+  // to the main menu window.
+  if (child_window_ && wl::IsMenuType(child_window_->type()))
+    return wl::IsMenuType(type());
 
   // If this is a nested menu window with a parent, it mustn't recieve any
   // events.
-  if (parent_window_ && parent_window_->shell_popup())
+  if (parent_window_ && wl::IsMenuType(parent_window_->type()))
     return false;
 
   // If another window has capture, return early before checking focus.
@@ -344,7 +288,6 @@ bool WaylandWindow::CanDispatchEvent(const PlatformEvent& event) {
 
 uint32_t WaylandWindow::DispatchEvent(const PlatformEvent& native_event) {
   Event* event = static_cast<Event*>(native_event);
-
   if (event->IsLocatedEvent()) {
     // Wayland sends locations in DIP so they need to be translated to
     // physical pixels.
@@ -355,13 +298,14 @@ uint32_t WaylandWindow::DispatchEvent(const PlatformEvent& native_event) {
   }
 
   // If the window does not have a pointer focus, but received this event, it
-  // means the window is a popup window with a child popup window. In this case,
-  // the location of the event must be converted from the nested popup to the
-  // main popup, which the menu controller needs to properly handle events.
-  if (event->IsLocatedEvent() && shell_popup()) {
-    // Parent window of the main menu window is not a popup, but rather an
+  // means the window is a menu window with a child menu window. In this case,
+  // the location of the event must be converted from the nested menu to the
+  // main menu, which the menu controller needs to properly handle events.
+  if (event->IsLocatedEvent() && wl::IsMenuType(type())) {
+    // Parent window of the main menu window is not a menu, but rather an
     // xdg surface.
-    DCHECK(!parent_window_->shell_popup() || !parent_window_->is_tooltip_);
+    DCHECK(!wl::IsMenuType(parent_window_->type()) ||
+           !parent_window_->is_tooltip_);
     auto* window =
         connection_->wayland_window_manager()->GetCurrentFocusedWindow();
     if (window) {
@@ -387,59 +331,10 @@ void WaylandWindow::HandleSurfaceConfigure(int32_t widht,
 }
 
 void WaylandWindow::HandlePopupConfigure(const gfx::Rect& bounds_dip) {
-  DCHECK(shell_popup());
-  DCHECK(parent_window_);
-
-  SetBufferScale(parent_window_->buffer_scale_, true);
-
-  gfx::Rect new_bounds_dip = bounds_dip;
-
-  // It's not enough to just set new bounds. If it is a menu window, whose
-  // parent is a top level window aka browser window, it can be flipped
-  // vertically along y-axis and have negative values set. Chromium cannot
-  // understand that and starts to position nested menu windows incorrectly. To
-  // fix that, we have to bear in mind that Wayland compositor does not share
-  // global coordinates for any surfaces, and Chromium assumes the top level
-  // window is always located at 0,0 origin. What is more, child windows must
-  // always be positioned relative to parent window local surface coordinates.
-  // Thus, if the menu window is flipped along y-axis by Wayland and its origin
-  // is above the top level parent window, the origin of the top level window
-  // has to be shifted by that value on y-axis so that the origin of the menu
-  // becomes x,0, and events can be handled normally.
-  if (!parent_window_->shell_popup()) {
-    gfx::Rect parent_bounds = parent_window_->GetBounds();
-    // The menu window is flipped along y-axis and have x,-y origin. Shift the
-    // parent top level window instead.
-    if (new_bounds_dip.y() < 0) {
-      // Move parent bounds along y-axis.
-      parent_bounds.set_y(-(new_bounds_dip.y() * buffer_scale_));
-      new_bounds_dip.set_y(0);
-    } else {
-      // If the menu window is located at correct origin from the browser point
-      // of view, return the top level window back to 0,0.
-      parent_bounds.set_y(0);
-    }
-    parent_window_->SetBounds(parent_bounds);
-  } else {
-    // The nested menu windows are located relative to the parent menu windows.
-    // Thus, the location must be translated to be relative to the top level
-    // window, which automatically becomes the same as relative to an origin of
-    // a display.
-    new_bounds_dip = gfx::ScaleToRoundedRect(
-        wl::TranslateBoundsToTopLevelCoordinates(
-            gfx::ScaleToRoundedRect(new_bounds_dip, buffer_scale_),
-            parent_window_->GetBounds()),
-        1.0 / buffer_scale_);
-    DCHECK(new_bounds_dip.y() >= 0);
-  }
-
-  SetBoundsDip(new_bounds_dip);
+  NOTREACHED() << "Only shell popups must receive HandlePopupConfigure calls.";
 }
 
 void WaylandWindow::OnCloseRequest() {
-  // Before calling OnCloseRequest, the |shell_popup_| must become hidden and
-  // only then call OnCloseRequest().
-  DCHECK(!shell_popup_);
   delegate_->OnCloseRequest();
 }
 
@@ -470,6 +365,7 @@ bool WaylandWindow::Initialize(PlatformWindowInitProperties properties) {
   DCHECK_EQ(buffer_scale_, 1);
   bounds_px_ = properties.bounds;
   opacity_ = properties.opacity;
+  type_ = properties.type;
 
   surface_.reset(wl_compositor_create_surface(connection_->compositor()));
   if (!surface_) {
@@ -481,44 +377,18 @@ bool WaylandWindow::Initialize(PlatformWindowInitProperties properties) {
 
   connection_->wayland_window_manager()->AddWindow(GetWidget(), this);
 
-  ui::PlatformWindowType ui_window_type = properties.type;
-  switch (ui_window_type) {
-    case ui::PlatformWindowType::kMenu:
-    case ui::PlatformWindowType::kPopup:
-      parent_window_ = GetParentWindow(properties.parent_widget);
-
-      // Popups need to know their scale earlier to position themselves.
-      if (!parent_window_) {
-        LOG(ERROR) << "Failed to get a parent window for this popup";
-        return false;
-      }
-
-      SetBufferScale(parent_window_->buffer_scale_, false);
-      ui_scale_ = parent_window_->ui_scale_;
-
-      // TODO(msisov, jkim): Handle notification windows, which are marked
-      // as popup windows as well. Those are the windows that do not have
-      // parents and pop up when the browser receives a notification.
-      CreateShellPopup();
-      break;
-    case ui::PlatformWindowType::kTooltip:
-      // Tooltips subsurfaces are created on demand, upon ::Show calls.
-      is_tooltip_ = true;
-      break;
-    case ui::PlatformWindowType::kWindow:
-    case ui::PlatformWindowType::kBubble:
-    case ui::PlatformWindowType::kDrag:
-      if (!OnInitialize(std::move(properties)))
-        return false;
-      break;
-  }
+  if (type_ == ui::PlatformWindowType::kTooltip)
+    is_tooltip_ = true;
+
+  if (!OnInitialize(std::move(properties)))
+    return false;
 
   connection_->ScheduleFlush();
 
   PlatformEventSource::GetInstance()->AddPlatformEventDispatcher(this);
   delegate_->OnAcceleratedWidgetAvailable(GetWidget());
 
-  // Will do nothing for popups because they have got their scale above.
+  // Will do nothing for menus because they have got their scale above.
   UpdateBufferScale(false);
 
   MaybeUpdateOpaqueRegion();
@@ -575,10 +445,10 @@ void WaylandWindow::AddSurfaceListener() {
 }
 
 void WaylandWindow::AddEnteredOutputId(struct wl_output* output) {
-  // Wayland does weird things for popups so instead of tracking outputs that
+  // Wayland does weird things for menus so instead of tracking outputs that
   // we entered or left, we take that from the parent window and ignore this
   // event.
-  if (shell_popup())
+  if (wl::IsMenuType(type()))
     return;
 
   const uint32_t entered_output_id =
@@ -591,10 +461,10 @@ void WaylandWindow::AddEnteredOutputId(struct wl_output* output) {
 }
 
 void WaylandWindow::RemoveEnteredOutputId(struct wl_output* output) {
-  // Wayland does weird things for popups so instead of tracking outputs that
+  // Wayland does weird things for menus so instead of tracking outputs that
   // we entered or left, we take that from the parent window and ignore this
   // event.
-  if (shell_popup())
+  if (wl::IsMenuType(type()))
     return;
 
   const uint32_t left_output_id =
@@ -654,53 +524,6 @@ void WaylandWindow::UpdateCursorPositionFromEvent(
   }
 }
 
-gfx::Rect WaylandWindow::AdjustPopupWindowPosition() const {
-  auto* parent_window = parent_window_->shell_popup()
-                            ? parent_window_->parent_window_
-                            : parent_window_;
-  DCHECK(parent_window);
-  DCHECK(buffer_scale_ == parent_window->buffer_scale_);
-  DCHECK(ui_scale_ == parent_window->ui_scale_);
-
-  // Chromium positions windows in screen coordinates, but Wayland requires them
-  // to be in local surface coordinates aka relative to parent window.
-  const gfx::Rect parent_bounds_dip =
-      gfx::ScaleToRoundedRect(parent_window_->GetBounds(), 1.0 / ui_scale_);
-  gfx::Rect new_bounds_dip =
-      wl::TranslateBoundsToParentCoordinates(bounds_px_, parent_bounds_dip);
-
-  // Chromium may decide to position nested menu windows on the left side
-  // instead of the right side of parent menu windows when the size of the
-  // window becomes larger than the display it is shown on. It's correct when
-  // the window is located on one display and occupies the whole work area, but
-  // as soon as it's moved and there is space on the right side, Chromium
-  // continues positioning the nested menus on the left side relative to the
-  // parent menu (Wayland does not provide clients with global coordinates).
-  // Instead, reposition that window to be on the right side of the parent menu
-  // window and let the compositor decide how to position it if it does not fit
-  // a single display. However, there is one exception - if the window is
-  // maximized, let Chromium position it on the left side as long as the Wayland
-  // compositor may decide to position the nested window on the right side of
-  // the parent menu window, which results in showing it on a second display if
-  // more than one display is used.
-  if (parent_window_->shell_popup() && parent_window_->parent_window_ &&
-      (parent_window_->parent_window()->GetPlatformWindowState() !=
-       PlatformWindowState::kMaximized)) {
-    auto* top_level_window = parent_window_->parent_window_;
-    DCHECK(top_level_window && !top_level_window->shell_popup());
-    if (new_bounds_dip.x() <= 0 && top_level_window->GetPlatformWindowState() !=
-                                       PlatformWindowState::kMaximized) {
-      // Position the child menu window on the right side of the parent window
-      // and let the Wayland compositor decide how to do constraint
-      // adjustements.
-      int new_x = parent_bounds_dip.width() -
-                  (new_bounds_dip.width() + new_bounds_dip.x());
-      new_bounds_dip.set_x(new_x);
-    }
-  }
-  return gfx::ScaleToRoundedRect(new_bounds_dip, ui_scale_ / buffer_scale_);
-}
-
 WaylandWindow* WaylandWindow::GetTopLevelWindow() {
   return parent_window_ ? parent_window_->GetTopLevelWindow() : this;
 }
diff --git a/ui/ozone/platform/wayland/host/wayland_window.h b/ui/ozone/platform/wayland/host/wayland_window.h
index b9ee70d0ecd2..3eca745b38c4 100644
--- a/ui/ozone/platform/wayland/host/wayland_window.h
+++ b/ui/ozone/platform/wayland/host/wayland_window.h
@@ -30,7 +30,6 @@ namespace ui {
 class BitmapCursorOzone;
 class OSExchangeData;
 class WaylandConnection;
-class ShellPopupWrapper;
 
 class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
  public:
@@ -53,8 +52,10 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
   void UpdateBufferScale(bool update_bounds);
 
   wl_surface* surface() const { return surface_.get(); }
-  ShellPopupWrapper* shell_popup() const { return shell_popup_.get(); }
 
+  void set_parent_window(WaylandWindow* parent_window) {
+    parent_window_ = parent_window;
+  }
   WaylandWindow* parent_window() const { return parent_window_; }
 
   gfx::AcceleratedWidget GetWidget() const;
@@ -75,6 +76,7 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
   // Set a child of this window. It is very important in case of nested
   // shell_popups as long as they must be destroyed in the back order.
   void set_child_window(WaylandWindow* window) { child_window_ = window; }
+  WaylandWindow* child_window() const { return child_window_; }
 
   // Set whether this window has an implicit grab (often referred to as capture
   // in Chrome code). Implicit grabs happen while a pointer is down.
@@ -82,11 +84,15 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
   bool has_implicit_grab() const { return has_implicit_grab_; }
 
   int32_t buffer_scale() const { return buffer_scale_; }
+  int32_t ui_scale() const { return ui_scale_; }
 
   const base::flat_set<uint32_t>& entered_outputs_ids() const {
     return entered_outputs_ids_;
   }
 
+  // Returns current type of the window.
+  PlatformWindowType type() const { return type_; }
+
   // PlatformWindow
   void Show(bool inactive) override;
   void Hide() override;
@@ -131,9 +137,10 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
                                       bool is_maximized,
                                       bool is_fullscreen,
                                       bool is_activated);
-  void HandlePopupConfigure(const gfx::Rect& bounds);
+  virtual void HandlePopupConfigure(const gfx::Rect& bounds);
 
-  void OnCloseRequest();
+  // Handles close requests.
+  virtual void OnCloseRequest();
 
   // Notifies about drag/drop session events.
   virtual void OnDragEnter(const gfx::PointF& point,
@@ -156,23 +163,25 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
   // Sets bounds in dip.
   void SetBoundsDip(const gfx::Rect& bounds_dip);
 
+  // Gets a parent window for this window.
+  WaylandWindow* GetParentWindow(gfx::AcceleratedWidget parent_widget);
+
+  // Sets the buffer scale.
+  void SetBufferScale(int32_t scale, bool update_bounds);
+
+  // Sets the ui scale.
+  void set_ui_scale(int32_t ui_scale) { ui_scale_ = ui_scale; }
+
  private:
   FRIEND_TEST_ALL_PREFIXES(WaylandScreenTest, SetBufferScale);
 
   // Initializes the WaylandWindow with supplied properties.
   bool Initialize(PlatformWindowInitProperties properties);
 
-  void SetBufferScale(int32_t scale, bool update_bounds);
-
-  // Creates a popup window, which is visible as a menu window.
-  void CreateShellPopup();
   // Creates (if necessary) and show subsurface window, to host
   // tooltip's content.
   void CreateAndShowTooltipSubSurface();
 
-  // Gets a parent window for this window.
-  WaylandWindow* GetParentWindow(gfx::AcceleratedWidget parent_widget);
-
   // Returns a root parent window.
   WaylandWindow* GetRootParentWindow();
 
@@ -184,9 +193,6 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
 
   void UpdateCursorPositionFromEvent(std::unique_ptr<Event> event);
 
-  // Returns bounds with origin relative to parent window's origin.
-  gfx::Rect AdjustPopupWindowPosition() const;
-
   WaylandWindow* GetTopLevelWindow();
 
   // It's important to set opaque region for opaque windows (provides
@@ -214,10 +220,6 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
   wl::Object<wl_surface> surface_;
   wl::Object<wl_subsurface> tooltip_subsurface_;
 
-  // Wrappers around xdg v5 and xdg v6 objects. WaylandWindow doesn't
-  // know anything about the version.
-  std::unique_ptr<ShellPopupWrapper> shell_popup_;
-
   // The current cursor bitmap (immutable).
   scoped_refptr<BitmapCursorOzone> bitmap_;
 
@@ -253,6 +255,9 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
   // we ask its parent.
   base::flat_set<uint32_t> entered_outputs_ids_;
 
+  // The type of the current WaylandWindow object.
+  ui::PlatformWindowType type_ = ui::PlatformWindowType::kWindow;
+
   DISALLOW_COPY_AND_ASSIGN(WaylandWindow);
 };
 
diff --git a/ui/ozone/platform/wayland/host/wayland_window_factory.cc b/ui/ozone/platform/wayland/host/wayland_window_factory.cc
index 892902e7f845..4c887fa3c1c8 100644
--- a/ui/ozone/platform/wayland/host/wayland_window_factory.cc
+++ b/ui/ozone/platform/wayland/host/wayland_window_factory.cc
@@ -6,6 +6,7 @@
 
 #include <memory>
 
+#include "ui/ozone/platform/wayland/host/wayland_popup.h"
 #include "ui/ozone/platform/wayland/host/wayland_surface.h"
 #include "ui/ozone/platform/wayland/host/wayland_window.h"
 
@@ -21,7 +22,7 @@ std::unique_ptr<WaylandWindow> WaylandWindow::Create(
     case PlatformWindowType::kMenu:
     case PlatformWindowType::kPopup:
       // TODO(msisov): Add WaylandPopup.
-      window.reset(new WaylandWindow(delegate, connection));
+      window.reset(new WaylandPopup(delegate, connection));
       break;
     case PlatformWindowType::kTooltip:
       // TODO(msisov): Add WaylandSubsurface.
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 e85236ce47d7..62b54680e9e7 100644
--- a/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.cc
+++ b/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.cc
@@ -12,10 +12,11 @@
 #include "base/nix/xdg_util.h"
 #include "ui/events/event_constants.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/ozone/platform/wayland/common/wayland_util.h"
 #include "ui/ozone/platform/wayland/host/wayland_connection.h"
 #include "ui/ozone/platform/wayland/host/wayland_pointer.h"
+#include "ui/ozone/platform/wayland/host/wayland_popup.h"
 #include "ui/ozone/platform/wayland/host/wayland_surface.h"
-#include "ui/ozone/platform/wayland/host/wayland_window.h"
 #include "ui/ozone/platform/wayland/host/xdg_surface_wrapper_impl.h"
 
 namespace ui {
@@ -268,38 +269,46 @@ 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,
-                                           const gfx::Rect& bounds) {
-  static const struct xdg_popup_listener xdg_popup_listener = {
-      &XDGPopupWrapperImpl::ConfigureStable,
-      &XDGPopupWrapperImpl::PopupDoneStable,
-  };
+  if (!connection->shell() && !connection->shell_v6()) {
+    NOTREACHED() << "Wrong shell protocol";
+    return false;
+  }
 
-  XDGSurfaceWrapperImpl* parent_xdg_surface;
+  XDGSurfaceWrapperImpl* parent_xdg_surface = nullptr;
   // If the parent window is a popup, the surface of that popup must be used as
   // a parent.
-  if (wayland_window_->parent_window()->shell_popup()) {
-    XDGPopupWrapperImpl* popup = reinterpret_cast<XDGPopupWrapperImpl*>(
-        wayland_window_->parent_window()->shell_popup());
+  if (wl::IsMenuType(wayland_window_->parent_window()->type())) {
+    auto* wayland_popup =
+        static_cast<WaylandPopup*>(wayland_window_->parent_window());
+    XDGPopupWrapperImpl* popup =
+        static_cast<XDGPopupWrapperImpl*>(wayland_popup->shell_popup());
     parent_xdg_surface = popup->xdg_surface();
   } else {
     WaylandSurface* wayland_surface =
         static_cast<WaylandSurface*>(wayland_window_->parent_window());
-    parent_xdg_surface = reinterpret_cast<XDGSurfaceWrapperImpl*>(
-        wayland_surface->shell_surface());
+    parent_xdg_surface =
+        static_cast<XDGSurfaceWrapperImpl*>(wayland_surface->shell_surface());
   }
 
-  if (!parent_xdg_surface)
+  if (!xdg_surface_ || !parent_xdg_surface)
     return false;
 
+  if (connection->shell())
+    return InitializeStable(connection, bounds, parent_xdg_surface);
+  else if (connection->shell_v6())
+    return InitializeV6(connection, bounds, parent_xdg_surface);
+  return false;
+}
+
+bool XDGPopupWrapperImpl::InitializeStable(
+    WaylandConnection* connection,
+    const gfx::Rect& bounds,
+    XDGSurfaceWrapperImpl* parent_xdg_surface) {
+  static const struct xdg_popup_listener xdg_popup_listener = {
+      &XDGPopupWrapperImpl::ConfigureStable,
+      &XDGPopupWrapperImpl::PopupDoneStable,
+  };
+
   struct xdg_positioner* positioner = CreatePositionerStable(
       connection, wayland_window_->parent_window(), bounds);
   if (!positioner)
@@ -369,7 +378,7 @@ struct xdg_positioner* XDGPopupWrapperImpl::CreatePositionerStable(
   MenuType menu_type = MenuType::TYPE_UNKNOWN;
   if (is_right_click_menu)
     menu_type = MenuType::TYPE_RIGHT_CLICK;
-  else if (parent_window->shell_popup())
+  else if (wl::IsMenuType(parent_window->type()))
     menu_type = MenuType::TYPE_3DOT_CHILD_MENU;
   else
     menu_type = MenuType::TYPE_3DOT_PARENT_MENU;
@@ -390,33 +399,15 @@ struct xdg_positioner* XDGPopupWrapperImpl::CreatePositionerStable(
   return positioner;
 }
 
-bool XDGPopupWrapperImpl::InitializeV6(WaylandConnection* connection,
-                                       const gfx::Rect& bounds) {
+bool XDGPopupWrapperImpl::InitializeV6(
+    WaylandConnection* connection,
+    const gfx::Rect& bounds,
+    XDGSurfaceWrapperImpl* parent_xdg_surface) {
   static const struct zxdg_popup_v6_listener zxdg_popup_v6_listener = {
       &XDGPopupWrapperImpl::ConfigureV6,
       &XDGPopupWrapperImpl::PopupDoneV6,
   };
 
-  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 (wayland_window_->parent_window()->shell_popup()) {
-    XDGPopupWrapperImpl* popup = reinterpret_cast<XDGPopupWrapperImpl*>(
-        wayland_window_->parent_window()->shell_popup());
-    parent_xdg_surface = popup->xdg_surface();
-  } else {
-    WaylandSurface* wayland_surface =
-        static_cast<WaylandSurface*>(wayland_window_->parent_window());
-    parent_xdg_surface = reinterpret_cast<XDGSurfaceWrapperImpl*>(
-        wayland_surface->shell_surface());
-  }
-
-  if (!parent_xdg_surface)
-    return false;
-
   zxdg_positioner_v6* positioner =
       CreatePositionerV6(connection, wayland_window_->parent_window(), bounds);
   if (!positioner)
@@ -488,7 +479,7 @@ zxdg_positioner_v6* XDGPopupWrapperImpl::CreatePositionerV6(
   MenuType menu_type = MenuType::TYPE_UNKNOWN;
   if (is_right_click_menu)
     menu_type = MenuType::TYPE_RIGHT_CLICK;
-  else if (parent_window->shell_popup())
+  else if (wl::IsMenuType(parent_window->type()))
     menu_type = MenuType::TYPE_3DOT_CHILD_MENU;
   else
     menu_type = MenuType::TYPE_3DOT_PARENT_MENU;
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 2a900818ff6a..c0479cfff7fb 100644
--- a/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.h
+++ b/ui/ozone/platform/wayland/host/xdg_popup_wrapper_impl.h
@@ -27,12 +27,16 @@ class XDGPopupWrapperImpl : public ShellPopupWrapper {
                   const gfx::Rect& bounds) override;
 
  private:
-  bool InitializeStable(WaylandConnection* connection, const gfx::Rect& bounds);
+  bool InitializeStable(WaylandConnection* connection,
+                        const gfx::Rect& bounds,
+                        XDGSurfaceWrapperImpl* parent_xdg_surface);
   struct xdg_positioner* CreatePositionerStable(WaylandConnection* connection,
                                                 WaylandWindow* parent_window,
                                                 const gfx::Rect& bounds);
 
-  bool InitializeV6(WaylandConnection* connection, const gfx::Rect& bounds);
+  bool InitializeV6(WaylandConnection* connection,
+                    const gfx::Rect& bounds,
+                    XDGSurfaceWrapperImpl* parent_xdg_surface);
   struct zxdg_positioner_v6* CreatePositionerV6(WaylandConnection* connection,
                                                 WaylandWindow* parent_window,
                                                 const gfx::Rect& bounds);
-- 
2.24.1