ozone/wayland: fix grabbing of menus with touch events.

According to the xdg_popup protocol, popups can be grabbed
upon touch down, pointer press or keyboard down events. It's
also fine to store a serial of a pointer press event, wait
until pointer released event comes and grab a popup
using the serial before the last event (basically, the one
that came with pointer press).

However, the same technique doesn't work with touch events and
some compositors such as Mutter just dismiss the popup. Some
versions of Weston also do the same.

Previously, we just disabled grabs for gnome environment, but
it's not that correct. There is a better way to handle that.

Instead, store serials from both touch down and up events and
make it possible to check what was the last event. If it was
touch down, do not grab the popup. If the last event was
up, avoid grabbing the popup as long as we can't use the
serial from the event before touch up event came.

Bug: 1123521
Change-Id: If9869271328372a80f55d81a469960f3965f7b6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2385280
Commit-Queue: Maksim Sisov (GMT+3) <msisov@igalia.com>
Reviewed-by: Nick Yamane <nickdiego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#804691}

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

@@ -133,7 +133,7 @@ void WaylandConnection::SetCursorBitmap(const std::vector<SkBitmap>& bitmaps,
                                         const gfx::Point& location) {
   if (!cursor_)
     return;
-  cursor_->UpdateBitmap(bitmaps, location, serial_);
+  cursor_->UpdateBitmap(bitmaps, location, serial());
 }
 
 bool WaylandConnection::IsDragInProgress() const {

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

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/events/event.h"
 #include "ui/ozone/platform/wayland/common/wayland_object.h"
 #include "ui/ozone/platform/wayland/host/wayland_clipboard.h"
 #include "ui/ozone/platform/wayland/host/wayland_data_drag_controller.h"
@@ -38,6 +39,12 @@ class GtkPrimarySelectionDeviceManager;
 
 class WaylandConnection {
  public:
+  // Stores the last serial and the event type it is associated with.
+  struct EventSerial {
+    uint32_t serial = 0;
+    EventType event_type = EventType::ET_UNKNOWN;
+  };
+
   WaylandConnection();
   WaylandConnection(const WaylandConnection&) = delete;
   WaylandConnection& operator=(const WaylandConnection&) = delete;
@@ -65,8 +72,11 @@ class WaylandConnection {
     return linux_explicit_synchronization_.get();
   }
 
-  void set_serial(uint32_t serial) { serial_ = serial; }
-  uint32_t serial() const { return serial_; }
+  void set_serial(uint32_t serial, EventType event_type) {
+    serial_ = {serial, event_type};
+  }
+  uint32_t serial() const { return serial_.serial; }
+  EventSerial event_serial() const { return serial_; }
 
   void SetCursorBitmap(const std::vector<SkBitmap>& bitmaps,
                        const gfx::Point& location);
@@ -204,7 +214,7 @@ class WaylandConnection {
 
   bool scheduled_flush_ = false;
 
-  uint32_t serial_ = 0;
+  EventSerial serial_;
 };
 
 }  // namespace ui

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

@@ -130,9 +130,9 @@ void WaylandKeyboard::Key(void* data,
   WaylandKeyboard* keyboard = static_cast<WaylandKeyboard*>(data);
   DCHECK(keyboard);
 
-  keyboard->connection_->set_serial(serial);
-
   bool down = state == WL_KEYBOARD_KEY_STATE_PRESSED;
+  if (down)
+    keyboard->connection_->set_serial(serial, ET_KEY_PRESSED);
   int device_id = keyboard->device_id();
 
   keyboard->auto_repeat_handler_.UpdateKeyRepeat(

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

@@ -103,14 +103,10 @@ void WaylandPointer::Button(void* data,
       return;
   }
 
-  // Set serial only on button presses. Popup windows can be created on
-  // button/touch presses, and, thus, require the serial of the last serial when
-  // the button was pressed. Otherwise, Wayland server dismisses the popup
-  // requests (see the protocol definition).
-  if (state == WL_POINTER_BUTTON_STATE_PRESSED)
-    pointer->connection_->set_serial(serial);
   EventType type = state == WL_POINTER_BUTTON_STATE_PRESSED ? ET_MOUSE_PRESSED
                                                             : ET_MOUSE_RELEASED;
+  if (type == ET_MOUSE_PRESSED)
+    pointer->connection_->set_serial(serial, type);
   pointer->delegate_->OnPointerButtonEvent(type, changed_button);
 }
 

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

@@ -5,6 +5,7 @@
 #include "ui/ozone/platform/wayland/host/wayland_touch.h"
 
 #include "base/time/time.h"
+#include "ui/events/types/event_type.h"
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/ozone/platform/wayland/common/wayland_util.h"
 #include "ui/ozone/platform/wayland/host/wayland_connection.h"
@@ -44,7 +45,7 @@ void WaylandTouch::Down(void* data,
 
   WaylandTouch* touch = static_cast<WaylandTouch*>(data);
   DCHECK(touch);
-  touch->connection_->set_serial(serial);
+  touch->connection_->set_serial(serial, ET_TOUCH_PRESSED);
 
   WaylandWindow* window = wl::RootWindowFromWlSurface(surface);
   gfx::PointF location(wl_fixed_to_double(x), wl_fixed_to_double(y));
@@ -61,6 +62,8 @@ void WaylandTouch::Up(void* data,
   WaylandTouch* touch = static_cast<WaylandTouch*>(data);
   DCHECK(touch);
 
+  touch->connection_->set_serial(serial, ET_TOUCH_RELEASED);
+
   base::TimeTicks timestamp =
       base::TimeTicks() + base::TimeDelta::FromMilliseconds(time);
   touch->delegate_->OnTouchReleaseEvent(timestamp, id);

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

@@ -2029,7 +2029,28 @@ TEST_P(WaylandWindowTest, CreatesPopupOnTouchDownSerial) {
 
   auto* test_popup = GetPopupByWindow(popup.get());
   ASSERT_TRUE(test_popup);
-  EXPECT_NE(test_popup->grab_serial(), touch_up_serial);
+
+  // Touch events are the exception. We can't use the serial that was sent
+  // before the "up" event. Otherwise, some compositors may dismiss popups.
+  // Thus, no serial must be used.
+  EXPECT_EQ(test_popup->grab_serial(), 0U);
+
+  popup->Hide();
+
+  // Send a single down event now.
+  wl_touch_send_down(server_.seat()->touch()->resource(), touch_down_serial, 0,
+                     surface_->resource(), 0 /* id */, wl_fixed_from_int(50),
+                     wl_fixed_from_int(100));
+
+  Sync();
+
+  popup->Show(false);
+
+  Sync();
+
+  test_popup = GetPopupByWindow(popup.get());
+  ASSERT_TRUE(test_popup);
+
   EXPECT_EQ(test_popup->grab_serial(), touch_down_serial);
 }
 

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

@@ -11,7 +11,9 @@
 
 #include "base/environment.h"
 #include "base/nix/xdg_util.h"
+#include "ui/events/event.h"
 #include "ui/events/event_constants.h"
+#include "ui/events/types/event_type.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"
@@ -456,34 +458,13 @@ bool XDGPopupWrapperImpl::CanGrabPopup(WaylandConnection* connection) const {
   if (connection->IsDragInProgress() || !connection->seat())
     return false;
 
-  // According to the spec, the grab call can only be done on a key press, mouse
-  // press or touch down. However, there is a problem with popup windows and
-  // touch events so long as Chromium creates them only on consequent touch up
-  // events. That results in Wayland compositors dismissing popups. To fix the
-  // issue, do not use grab with touch events. Please note that explicit grab
-  // means that a Wayland compositor dismisses a popup whenever the user clicks
-  // outside the created surfaces. If the explicit grab is not used, the popups
-  // are not dismissed in such cases. What is more, current non-ozone X11
-  // implementation does the same. This means there is no functionality changes
-  // and we do things right.
-  //
-  // We cannot know what was the last event. Instead, we can check if the window
-  // has pointer or keyboard focus. If so, the popup will be explicitly grabbed.
-  //
-  // There is a bug in the gnome/mutter - if explicit grab is not used,
-  // unmapping of a wl_surface (aka destroying xdg_popup and surface) to hide a
-  // window results in weird behaviour. That is, a popup continues to be visible
-  // on a display and it results in a crash of the entire session. Thus, just
-  // continue to use grab here and avoid showing popups for touch events on
-  // gnome/mutter. That is better than crashing the entire system. Otherwise,
-  // Chromium has to change the way how it reacts on touch events - instead of
-  // creating a menu on touch up, it must do it on touch down events.
-  // https://gitlab.gnome.org/GNOME/mutter/issues/698#note_562601.
-  std::unique_ptr<base::Environment> env(base::Environment::Create());
-  return (base::nix::GetDesktopEnvironment(env.get()) ==
-          base::nix::DESKTOP_ENVIRONMENT_GNOME) ||
-         (wayland_window_->parent_window()->has_pointer_focus() ||
-          wayland_window_->parent_window()->has_keyboard_focus());
+  // According to the definition of the xdg protocol, the grab request must be
+  // used in response to some sort of user action like a button press, key
+  // press, or touch down event.
+  EventType last_event_type = connection->event_serial().event_type;
+  return last_event_type == ET_TOUCH_PRESSED ||
+         last_event_type == ET_KEY_PRESSED ||
+         last_event_type == ET_MOUSE_PRESSED;
 }
 
 void XDGPopupWrapperImpl::Configure(void* data,