Commit 7138b785 authored by Maksim Sisov's avatar Maksim Sisov Committed by Commit Bot

ozone/wayland: Fix drop down menus for touch events.

In case of touch events, we may not find currently focused windows
as menu windows might be created on touch up events. Thus,
GetCurrentFocusedWindow returns nullptr.

To fix that, use a concept of a currently active window. That is,
Wayland sets a window to "active" state if it has had a focus.
There can be only one focused window at a time.

Thus, if there is no a focused window, use currently active window
as a parent. That's the best effort one could make to find a parent
window for a newly created non-toplevel window. At some point,
it might be worth reconsidering the design and make aura always pass
parent widgets for new windows if applicable.

Bug: 1123521
Change-Id: Ib9479632e049df082ed17e3089680006a32f19fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2385279
Commit-Queue: Maksim Sisov (GMT+3) <msisov@igalia.com>
Reviewed-by: default avatarNick Yamane <nickdiego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#804674}
parent 96f752e8
...@@ -102,13 +102,16 @@ void WaylandAuxiliaryWindow::CreateSubsurface() { ...@@ -102,13 +102,16 @@ void WaylandAuxiliaryWindow::CreateSubsurface() {
bool WaylandAuxiliaryWindow::OnInitialize( bool WaylandAuxiliaryWindow::OnInitialize(
PlatformWindowInitProperties properties) { PlatformWindowInitProperties properties) {
DCHECK(!parent_window());
// If we do not have parent window provided, we must always use a focused // If we do not have parent window provided, we must always use a focused
// window or a window that entered drag whenever the subsurface is created. // window or a window that entered drag whenever the subsurface is created.
if (properties.parent_widget == gfx::kNullAcceleratedWidget) { if (properties.parent_widget == gfx::kNullAcceleratedWidget)
DCHECK(!parent_window());
return true; return true;
}
set_parent_window(GetParentWindow(properties.parent_widget)); set_parent_window(
connection()->wayland_window_manager()->FindParentForNewWindow(
properties.parent_widget));
return true; return true;
} }
......
...@@ -131,15 +131,8 @@ void WaylandPopup::OnCloseRequest() { ...@@ -131,15 +131,8 @@ void WaylandPopup::OnCloseRequest() {
} }
bool WaylandPopup::OnInitialize(PlatformWindowInitProperties properties) { bool WaylandPopup::OnInitialize(PlatformWindowInitProperties properties) {
if (!wl::IsMenuType(type())) DCHECK(wl::IsMenuType(type()));
return false; DCHECK(parent_window());
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.
root_surface()->SetBufferScale(parent_window()->buffer_scale(), false); root_surface()->SetBufferScale(parent_window()->buffer_scale(), false);
set_ui_scale(parent_window()->ui_scale()); set_ui_scale(parent_window()->ui_scale());
return true; return true;
......
...@@ -345,6 +345,10 @@ bool WaylandToplevelWindow::OnInitialize( ...@@ -345,6 +345,10 @@ bool WaylandToplevelWindow::OnInitialize(
return true; return true;
} }
bool WaylandToplevelWindow::IsActive() const {
return is_active_;
}
bool WaylandToplevelWindow::RunMoveLoop(const gfx::Vector2d& drag_offset) { bool WaylandToplevelWindow::RunMoveLoop(const gfx::Vector2d& drag_offset) {
DCHECK(connection()->window_drag_controller()); DCHECK(connection()->window_drag_controller());
return connection()->window_drag_controller()->Drag(this, drag_offset); return connection()->window_drag_controller()->Drag(this, drag_offset);
......
...@@ -76,6 +76,7 @@ class WaylandToplevelWindow : public WaylandWindow, ...@@ -76,6 +76,7 @@ class WaylandToplevelWindow : public WaylandWindow,
void OnDragLeave() override; void OnDragLeave() override;
void OnDragSessionClose(uint32_t dnd_action) override; void OnDragSessionClose(uint32_t dnd_action) override;
bool OnInitialize(PlatformWindowInitProperties properties) override; bool OnInitialize(PlatformWindowInitProperties properties) override;
bool IsActive() const override;
// WmMoveLoopHandler: // WmMoveLoopHandler:
bool RunMoveLoop(const gfx::Vector2d& drag_offset) override; bool RunMoveLoop(const gfx::Vector2d& drag_offset) override;
......
...@@ -361,28 +361,6 @@ bool WaylandWindow::Initialize(PlatformWindowInitProperties properties) { ...@@ -361,28 +361,6 @@ bool WaylandWindow::Initialize(PlatformWindowInitProperties properties) {
return true; return true;
} }
WaylandWindow* WaylandWindow::GetParentWindow(
gfx::AcceleratedWidget parent_widget) {
auto* parent_window =
connection_->wayland_window_manager()->GetWindow(parent_widget);
// If propagated parent has already had a child, it means that |this| is a
// submenu of a 3-dot menu. In aura, the parent of a 3-dot menu and its
// submenu is the main native widget, which is the main window. In contrast,
// Wayland requires a menu window to be a parent of a submenu window. Thus,
// check if the suggested parent has a child. If yes, take its child as a
// parent of |this|.
// Another case is a notifcation window or a drop down window, which do not
// have a parent in aura. In this case, take the current focused window as a
// parent.
if (!parent_window)
parent_window =
connection_->wayland_window_manager()->GetCurrentFocusedWindow();
return parent_window ? parent_window->GetTopMostChildWindow() : nullptr;
}
WaylandWindow* WaylandWindow::GetRootParentWindow() { WaylandWindow* WaylandWindow::GetRootParentWindow() {
return parent_window_ ? parent_window_->GetRootParentWindow() : this; return parent_window_ ? parent_window_->GetRootParentWindow() : this;
} }
...@@ -467,6 +445,11 @@ bool WaylandWindow::IsOpaqueWindow() const { ...@@ -467,6 +445,11 @@ bool WaylandWindow::IsOpaqueWindow() const {
return opacity_ == ui::PlatformWindowOpacity::kOpaqueWindow; return opacity_ == ui::PlatformWindowOpacity::kOpaqueWindow;
} }
bool WaylandWindow::IsActive() const {
// Please read the comment where the IsActive method is declared.
return false;
}
uint32_t WaylandWindow::DispatchEventToDelegate( uint32_t WaylandWindow::DispatchEventToDelegate(
const PlatformEvent& native_event) { const PlatformEvent& native_event) {
auto* event = static_cast<Event*>(native_event); auto* event = static_cast<Event*>(native_event);
......
...@@ -183,6 +183,11 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher { ...@@ -183,6 +183,11 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
// Returns true iff this window is opaque. // Returns true iff this window is opaque.
bool IsOpaqueWindow() const; bool IsOpaqueWindow() const;
// Says if the current window is set as active by the Wayland server. This
// only applies to toplevel surfaces (surfaces such as popups, subsurfaces do
// not support that).
virtual bool IsActive() const;
protected: protected:
WaylandWindow(PlatformWindowDelegate* delegate, WaylandWindow(PlatformWindowDelegate* delegate,
WaylandConnection* connection); WaylandConnection* connection);
...@@ -193,9 +198,6 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher { ...@@ -193,9 +198,6 @@ class WaylandWindow : public PlatformWindow, public PlatformEventDispatcher {
// Sets bounds in dip. // Sets bounds in dip.
void SetBoundsDip(const gfx::Rect& bounds_dip); void SetBoundsDip(const gfx::Rect& bounds_dip);
// Gets a parent window for this window.
WaylandWindow* GetParentWindow(gfx::AcceleratedWidget parent_widget);
void set_ui_scale(int32_t ui_scale) { ui_scale_ = ui_scale; } void set_ui_scale(int32_t ui_scale) { ui_scale_ = ui_scale; }
private: private:
......
...@@ -22,28 +22,41 @@ std::unique_ptr<WaylandWindow> WaylandWindow::Create( ...@@ -22,28 +22,41 @@ std::unique_ptr<WaylandWindow> WaylandWindow::Create(
switch (properties.type) { switch (properties.type) {
case PlatformWindowType::kMenu: case PlatformWindowType::kMenu:
case PlatformWindowType::kPopup: case PlatformWindowType::kPopup:
// We are unable to create a popup or menu window, because they require a if (connection->IsDragInProgress()) {
// parent window to be set. Thus, create a normal window instead then.
if (properties.parent_widget == gfx::kNullAcceleratedWidget &&
!connection->wayland_window_manager()->GetCurrentFocusedWindow()) {
window.reset(new WaylandToplevelWindow(delegate, connection));
} else if (connection->IsDragInProgress()) {
// We are in the process of drag and requested a popup. Most probably, // We are in the process of drag and requested a popup. Most probably,
// it is an arrow window. // it is an arrow window.
window.reset(new WaylandAuxiliaryWindow(delegate, connection)); window = std::make_unique<WaylandAuxiliaryWindow>(delegate, connection);
} else { } else {
window.reset(new WaylandPopup(delegate, connection)); auto* parent_window =
connection->wayland_window_manager()->FindParentForNewWindow(
properties.parent_widget);
if (parent_window) {
// Set the parent window in advance otherwise it is not possible to
// know if the WaylandPopup is able to find one and if
// WaylandWindow::Initialize() fails or not. Otherwise,
// WaylandWindow::Create() returns nullptr and makes the browser to
// fail. To fix this problem, search for the parent window and if
// one is not found, create WaylandToplevelWindow instead. It's
// also worth noting that searching twice (one time here and another
// by WaylandPopup) is a bad practice, and the parent window is set
// here instead.
window = std::make_unique<WaylandPopup>(delegate, connection);
window->set_parent_window(parent_window);
} else {
window =
std::make_unique<WaylandToplevelWindow>(delegate, connection);
}
} }
break; break;
case PlatformWindowType::kTooltip: case PlatformWindowType::kTooltip:
window.reset(new WaylandAuxiliaryWindow(delegate, connection)); window = std::make_unique<WaylandAuxiliaryWindow>(delegate, connection);
break; break;
case PlatformWindowType::kWindow: case PlatformWindowType::kWindow:
case PlatformWindowType::kBubble: case PlatformWindowType::kBubble:
case PlatformWindowType::kDrag: case PlatformWindowType::kDrag:
// TODO(msisov): Figure out what kind of surface we need to create for // TODO(msisov): Figure out what kind of surface we need to create for
// bubble and drag windows. // bubble and drag windows.
window.reset(new WaylandToplevelWindow(delegate, connection)); window = std::make_unique<WaylandToplevelWindow>(delegate, connection);
break; break;
default: default:
NOTREACHED(); NOTREACHED();
......
...@@ -82,6 +82,37 @@ WaylandWindow* WaylandWindowManager::GetCurrentKeyboardFocusedWindow() const { ...@@ -82,6 +82,37 @@ WaylandWindow* WaylandWindowManager::GetCurrentKeyboardFocusedWindow() const {
return nullptr; return nullptr;
} }
WaylandWindow* WaylandWindowManager::FindParentForNewWindow(
gfx::AcceleratedWidget parent_widget) const {
auto* parent_window = GetWindow(parent_widget);
// If propagated parent has already had a child, it means that |this| is a
// submenu of a 3-dot menu. In aura, the parent of a 3-dot menu and its
// submenu is the main native widget, which is the main window. In contrast,
// Wayland requires a menu window to be a parent of a submenu window. Thus,
// check if the suggested parent has a child. If yes, take its child as a
// parent of |this|.
// Another case is a notification window or a drop down window, which does not
// have a parent in aura. In this case, take the current focused window as a
// parent.
if (!parent_window)
parent_window = GetCurrentFocusedWindow();
// If there is no current focused window, figure out the current active window
// set by the Wayland server. Only one window at a time can be set as active.
if (!parent_window) {
auto windows = GetAllWindows();
for (auto* window : windows) {
if (window->IsActive()) {
parent_window = window;
break;
}
}
}
return parent_window ? parent_window->GetTopMostChildWindow() : nullptr;
}
std::vector<WaylandWindow*> WaylandWindowManager::GetWindowsOnOutput( std::vector<WaylandWindow*> WaylandWindowManager::GetWindowsOnOutput(
uint32_t output_id) { uint32_t output_id) {
std::vector<WaylandWindow*> result; std::vector<WaylandWindow*> result;
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/containers/flat_map.h" #include "base/containers/flat_map.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/observer_list.h" #include "base/observer_list.h"
#include "ui/gfx/geometry/size_f.h"
#include "ui/gfx/native_widget_types.h" #include "ui/gfx/native_widget_types.h"
#include "ui/ozone/platform/wayland/host/wayland_window_observer.h" #include "ui/ozone/platform/wayland/host/wayland_window_observer.h"
...@@ -55,6 +56,13 @@ class WaylandWindowManager { ...@@ -55,6 +56,13 @@ class WaylandWindowManager {
// Returns a current focused window by keyboard. // Returns a current focused window by keyboard.
WaylandWindow* GetCurrentKeyboardFocusedWindow() const; WaylandWindow* GetCurrentKeyboardFocusedWindow() const;
// Returns a parent window suitable for newly created non-toplevel windows. If
// the |parent_widget| is gfx::kNullAcceleratedWidget, either the currently
// focused or the active window is used. If the found parent has children
// windows, the one on top the of the stack is used as a parent.
WaylandWindow* FindParentForNewWindow(
gfx::AcceleratedWidget parent_widget) const;
// TODO(crbug.com/971525): remove this in favor of targeted subscription of // TODO(crbug.com/971525): remove this in favor of targeted subscription of
// windows to their outputs. // windows to their outputs.
std::vector<WaylandWindow*> GetWindowsOnOutput(uint32_t output_id); std::vector<WaylandWindow*> GetWindowsOnOutput(uint32_t output_id);
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "ui/base/hit_test.h" #include "ui/base/hit_test.h"
#include "ui/events/base_event_utils.h" #include "ui/events/base_event_utils.h"
#include "ui/events/event.h" #include "ui/events/event.h"
#include "ui/gfx/native_widget_types.h"
#include "ui/gfx/overlay_transform.h" #include "ui/gfx/overlay_transform.h"
#include "ui/ozone/platform/wayland/common/wayland_util.h" #include "ui/ozone/platform/wayland/common/wayland_util.h"
#include "ui/ozone/platform/wayland/host/wayland_subsurface.h" #include "ui/ozone/platform/wayland/host/wayland_subsurface.h"
...@@ -2122,6 +2123,117 @@ TEST_P(WaylandWindowTest, OneWaylandSubsurface) { ...@@ -2122,6 +2123,117 @@ TEST_P(WaylandWindowTest, OneWaylandSubsurface) {
EXPECT_TRUE(test_subsurface->sync()); EXPECT_TRUE(test_subsurface->sync());
} }
TEST_P(WaylandWindowTest, UsesCorrectParentForChildrenWindows) {
uint32_t serial = 0;
MockPlatformWindowDelegate window_delegate;
std::unique_ptr<WaylandWindow> window = CreateWaylandWindowWithParams(
PlatformWindowType::kWindow, gfx::kNullAcceleratedWidget,
gfx::Rect(10, 10, 100, 100), &window_delegate);
EXPECT_TRUE(window);
window->Show(false);
std::unique_ptr<WaylandWindow> another_window = CreateWaylandWindowWithParams(
PlatformWindowType::kWindow, gfx::kNullAcceleratedWidget,
gfx::Rect(10, 10, 300, 400), &window_delegate);
EXPECT_TRUE(another_window);
another_window->Show(false);
Sync();
auto* window1 = window.get();
auto* window2 = window_.get();
auto* window3 = another_window.get();
// Make sure windows are not "active".
auto empty_state = MakeStateArray({});
SendConfigureEvent(xdg_surface_, 0, 0, ++serial, empty_state.get());
auto* xdg_surface_window =
server_
.GetObject<wl::MockSurface>(window->root_surface()->GetSurfaceId())
->xdg_surface();
SendConfigureEvent(xdg_surface_window, 0, 0, ++serial, empty_state.get());
auto* xdg_surface_another_window =
server_
.GetObject<wl::MockSurface>(
another_window->root_surface()->GetSurfaceId())
->xdg_surface();
SendConfigureEvent(xdg_surface_another_window, 0, 0, ++serial,
empty_state.get());
Sync();
// Case 1: provided parent window's widget..
MockPlatformWindowDelegate menu_window_delegate;
std::unique_ptr<WaylandWindow> menu_window = CreateWaylandWindowWithParams(
PlatformWindowType::kMenu, window1->GetWidget(),
gfx::Rect(10, 10, 10, 10), &menu_window_delegate);
EXPECT_TRUE(menu_window->parent_window() == window1);
// Case 2: didn't provide parent window's widget - must use current focused.
//
// Subcase 1: pointer focus.
window2->SetPointerFocus(true);
menu_window = CreateWaylandWindowWithParams(
PlatformWindowType::kMenu, gfx::kNullAcceleratedWidget,
gfx::Rect(10, 10, 10, 10), &menu_window_delegate);
EXPECT_TRUE(menu_window->parent_window() == window2);
EXPECT_TRUE(wl::IsMenuType(menu_window->type()));
// Subcase 2: keyboard focus.
window2->SetPointerFocus(false);
window2->set_keyboard_focus(true);
menu_window = CreateWaylandWindowWithParams(
PlatformWindowType::kMenu, gfx::kNullAcceleratedWidget,
gfx::Rect(10, 10, 10, 10), &menu_window_delegate);
// Mustn't be able to create a menu window, but rather creates a toplevel
// window as we must provide at least something.
EXPECT_TRUE(menu_window);
// Make it create xdg objects.
menu_window->Show(false);
Sync();
auto* menu_window_xdg =
server_
.GetObject<wl::MockSurface>(
another_window->root_surface()->GetSurfaceId())
->xdg_surface();
EXPECT_TRUE(menu_window_xdg);
EXPECT_TRUE(menu_window_xdg->xdg_toplevel());
EXPECT_FALSE(menu_window_xdg->xdg_popup());
// Subcase 3: touch focus.
window2->set_keyboard_focus(false);
window2->set_touch_focus(true);
menu_window = CreateWaylandWindowWithParams(
PlatformWindowType::kMenu, gfx::kNullAcceleratedWidget,
gfx::Rect(10, 10, 10, 10), &menu_window_delegate);
EXPECT_TRUE(menu_window->parent_window() == window2);
EXPECT_TRUE(wl::IsMenuType(menu_window->type()));
// Case 3: neither of the windows are focused. However, there is one that is
// active. Must use that then.
window2->set_touch_focus(false);
auto active = InitializeWlArrayWithActivatedState();
SendConfigureEvent(xdg_surface_another_window, 0, 0, ++serial, active.get());
Sync();
menu_window = CreateWaylandWindowWithParams(
PlatformWindowType::kMenu, gfx::kNullAcceleratedWidget,
gfx::Rect(10, 10, 10, 10), &menu_window_delegate);
EXPECT_TRUE(menu_window->parent_window() == window3);
EXPECT_TRUE(wl::IsMenuType(menu_window->type()));
}
INSTANTIATE_TEST_SUITE_P(XdgVersionStableTest, INSTANTIATE_TEST_SUITE_P(XdgVersionStableTest,
WaylandWindowTest, WaylandWindowTest,
::testing::Values(kXdgShellStable)); ::testing::Values(kXdgShellStable));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment