lacros: Add PopupBrowserTest.LongPressOnTabOpensNonEmptyMenu

Regression test for https://crbug.com/1157664. Verifies that opening
a menu via long-press on a tab does not result in a popup window with
empty bounds. In production this bug caused a Wayland protocol error
and lacros crash.

Adds a new test crosapi to inject touch events from the ash side.
See comments in test for why we do it this way.

I verified that this test fails appropriately when the fix for the
original bug is reverted. The test does not reproduce the Wayland
protocol error, because that error is in code that only runs when
graphics overlay forwarding is enabled, which only happens on
device. However, it's sufficient for the test to verify that the
popup in lacros has non-empty bounds.

Bug: 1157664
Test: lacros_chrome_browsertests

Change-Id: Ia8aabaec1d6ec4bc72d1e77eda9a7f0efbe5f0c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2643026
Commit-Queue: James Cook <jamescook@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Auto-Submit: James Cook <jamescook@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846264}

chrome/browser/chromeos/crosapi/test_controller_ash.cc

@@ -9,6 +9,7 @@
 #include "ash/wm/overview/overview_controller.h"
 #include "ash/wm/overview/overview_observer.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
+#include "base/optional.h"
 #include "base/task/post_task.h"
 #include "build/chromeos_buildflags.h"
 #include "chrome/browser/chromeos/crosapi/window_util.h"
@@ -18,21 +19,27 @@
 #include "ui/events/base_event_utils.h"
 #include "ui/events/event.h"
 #include "ui/events/event_source.h"
+#include "ui/events/types/event_type.h"
 #include "ui/gfx/geometry/point.h"
 
 namespace crosapi {
 
 namespace {
 
+// Returns whether the dispatcher or target was destroyed.
+bool Dispatch(aura::WindowTreeHost* host, ui::Event* event) {
+  ui::EventDispatchDetails dispatch_details =
+      host->GetEventSource()->SendEventToSink(event);
+  return dispatch_details.dispatcher_destroyed ||
+         dispatch_details.target_destroyed;
+}
+
 // Returns whether the dispatcher or target was destroyed.
 bool DispatchMouseEvent(aura::Window* window, ui::EventType type) {
   const gfx::Point center = window->bounds().CenterPoint();
   ui::MouseEvent press(type, center, center, ui::EventTimeForNow(),
                        ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON);
-  ui::EventDispatchDetails dispatch_details =
-      window->GetHost()->GetEventSource()->SendEventToSink(&press);
-  return dispatch_details.dispatcher_destroyed ||
-         dispatch_details.target_destroyed;
+  return Dispatch(window->GetHost(), &press);
 }
 
 // Enables or disables tablet mode and waits for the transition to finish.
@@ -98,6 +105,64 @@ void TestControllerAsh::ExitTabletMode(ExitTabletModeCallback callback) {
   std::move(callback).Run();
 }
 
+void TestControllerAsh::SendTouchEvent(const std::string& window_id,
+                                       mojom::TouchEventType type,
+                                       uint8_t pointer_id,
+                                       const gfx::PointF& location_in_window,
+                                       SendTouchEventCallback cb) {
+  aura::Window* window = GetShellSurfaceWindow(window_id);
+  if (!window) {
+    std::move(cb).Run();
+    return;
+  }
+  // Newer lacros might send an enum we don't know about.
+  if (!mojom::IsKnownEnumValue(type)) {
+    LOG(WARNING) << "Unknown event type: " << type;
+    std::move(cb).Run();
+    return;
+  }
+  ui::EventType event_type;
+  switch (type) {
+    case mojom::TouchEventType::kUnknown:
+      // |type| is not optional, so kUnknown is never expected.
+      NOTREACHED();
+      return;
+    case mojom::TouchEventType::kPressed:
+      event_type = ui::ET_TOUCH_PRESSED;
+      break;
+    case mojom::TouchEventType::kMoved:
+      event_type = ui::ET_TOUCH_MOVED;
+      break;
+    case mojom::TouchEventType::kReleased:
+      event_type = ui::ET_TOUCH_RELEASED;
+      break;
+    case mojom::TouchEventType::kCancelled:
+      event_type = ui::ET_TOUCH_CANCELLED;
+      break;
+  }
+  // Compute location relative to display root window.
+  gfx::PointF location_in_root(location_in_window);
+  aura::Window::ConvertPointToTarget(window, window->GetRootWindow(),
+                                     &location_in_root);
+  ui::PointerDetails details(ui::EventPointerType::kTouch, pointer_id, 1.0f,
+                             1.0f, 0.0f);
+  ui::TouchEvent touch_event(event_type, location_in_window, location_in_root,
+                             ui::EventTimeForNow(), details);
+  Dispatch(window->GetHost(), &touch_event);
+  std::move(cb).Run();
+}
+
+void TestControllerAsh::GetWindowPositionInScreen(
+    const std::string& window_id,
+    GetWindowPositionInScreenCallback cb) {
+  aura::Window* window = GetShellSurfaceWindow(window_id);
+  if (!window) {
+    std::move(cb).Run(base::nullopt);
+    return;
+  }
+  std::move(cb).Run(window->GetBoundsInScreen().origin());
+}
+
 void TestControllerAsh::WaiterFinished(OverviewWaiter* waiter) {
   for (size_t i = 0; i < overview_waiters_.size(); ++i) {
     if (waiter == overview_waiters_[i].get()) {

chrome/browser/chromeos/crosapi/test_controller_ash.h

@@ -30,6 +30,13 @@ class TestControllerAsh : public mojom::TestController {
   void ExitOverviewMode(ExitOverviewModeCallback callback) override;
   void EnterTabletMode(EnterTabletModeCallback callback) override;
   void ExitTabletMode(ExitTabletModeCallback callback) override;
+  void SendTouchEvent(const std::string& window_id,
+                      mojom::TouchEventType type,
+                      uint8_t pointer_id,
+                      const gfx::PointF& location_in_window,
+                      SendTouchEventCallback cb) override;
+  void GetWindowPositionInScreen(const std::string& window_id,
+                                 GetWindowPositionInScreenCallback cb) override;
 
  private:
   class OverviewWaiter;

chrome/browser/lacros/popup_lacros_browsertest.cc

+// Copyright 2021 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 "base/optional.h"
+#include "base/test/bind.h"
+#include "base/timer/timer.h"
+#include "chrome/browser/lacros/browser_test_util.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/tabs/tab.h"
+#include "chrome/browser/ui/views/tabs/tab_strip.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chromeos/crosapi/mojom/test_controller.mojom-test-utils.h"
+#include "chromeos/crosapi/mojom/test_controller.mojom.h"
+#include "chromeos/lacros/lacros_chrome_service_impl.h"
+#include "content/public/test/browser_test.h"
+#include "ui/aura/window.h"
+#include "ui/gfx/geometry/point.h"
+#include "ui/gfx/geometry/point_f.h"
+#include "ui/views/test/widget_test.h"
+
+namespace {
+
+// Waits for the window identified by |window_id| to have its ash-side window
+// position in DIP screen coordinates set to |target_position|.
+void WaitForWindowPositionInScreen(const std::string& window_id,
+                                   const gfx::Point& target_position) {
+  auto* lacros_chrome_service = chromeos::LacrosChromeServiceImpl::Get();
+  base::RunLoop outer_loop;
+  auto wait_for_position = base::BindLambdaForTesting([&]() {
+    base::RunLoop inner_loop(base::RunLoop::Type::kNestableTasksAllowed);
+    base::Optional<gfx::Point> position;
+    lacros_chrome_service->test_controller_remote()->GetWindowPositionInScreen(
+        window_id,
+        base::BindLambdaForTesting([&](const base::Optional<gfx::Point>& p) {
+          position = p;
+          inner_loop.Quit();
+        }));
+    inner_loop.Run();
+    if (position && *position == target_position)
+      outer_loop.Quit();
+  });
+  base::RepeatingTimer timer;
+  timer.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(1),
+              std::move(wait_for_position));
+  outer_loop.Run();
+}
+
+using PopupBrowserTest = InProcessBrowserTest;
+
+// Regression test for https://crbug.com/1157664. Verifying that opening a
+// menu via long-press on a tab does not result in a popup window with empty
+// bounds. In bug caused a Wayland protocol error and lacros crash.
+IN_PROC_BROWSER_TEST_F(PopupBrowserTest, LongPressOnTabOpensNonEmptyMenu) {
+  auto* lacros_chrome_service = chromeos::LacrosChromeServiceImpl::Get();
+  ASSERT_TRUE(lacros_chrome_service->IsTestControllerAvailable());
+  // This test requires the tablet mode API.
+  if (lacros_chrome_service->GetInterfaceVersion(
+          crosapi::mojom::TestController::Uuid_) < 3) {
+    LOG(WARNING) << "Unsupported ash version.";
+    return;
+  }
+
+  // Ensure the browser is maximized. The bug only occurs when the tab strip is
+  // near the top of the screen.
+  browser()->window()->Maximize();
+
+  // Wait for the window to be created.
+  aura::Window* window = browser()->window()->GetNativeWindow();
+  std::string window_id =
+      browser_test_util::GetWindowId(window->GetRootWindow());
+  browser_test_util::WaitForWindowCreation(window_id);
+
+  // Wait for the window to be globally positioned at 0,0. It will eventually
+  // have this position because it is maximized. We cannot assert the position
+  // lacros-side because Wayland clients do not know the position of their
+  // windows on the display.
+  WaitForWindowPositionInScreen(window_id, gfx::Point(0, 0));
+
+  // Precondition: The browser is the only open widget.
+  std::set<views::Widget*> initial_widgets =
+      views::test::WidgetTest::GetAllWidgets();
+  ASSERT_EQ(1u, initial_widgets.size());
+  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
+  views::Widget* browser_widget = browser_view->GetWidget();
+  ASSERT_EQ(browser_widget, *initial_widgets.begin());
+
+  // Find the center position of the first tab. We cannot use "screen"
+  // coordinates because the Wayland client does not know where its window is
+  // located on the screen. Use widget-relative position instead.
+  Tab* tab = browser_view->tabstrip()->tab_at(0);
+  gfx::Point tab_center_in_widget = tab->GetLocalBounds().CenterPoint();
+  views::View::ConvertPointToWidget(tab, &tab_center_in_widget);
+
+  // Generate a touch press in ash, because the bug requires lacros to receive
+  // events over the Wayland connection from ash.
+  crosapi::mojom::TestControllerAsyncWaiter waiter(
+      lacros_chrome_service->test_controller_remote().get());
+  waiter.SendTouchEvent(window_id, crosapi::mojom::TouchEventType::kPressed,
+                        /*pointer_id=*/0u, gfx::PointF(tab_center_in_widget));
+
+  // Wait long enough that the gesture recognizer will decide this is a long
+  // press gesture. We cannot directly inject a long press from ash because
+  // Wayland only transports press/move/release events, not gestures.
+  base::RunLoop loop2;
+  base::OneShotTimer timer2;
+  timer2.Start(FROM_HERE, base::TimeDelta::FromSeconds(1), loop2.QuitClosure());
+  loop2.Run();
+
+  // Release the touch in ash.
+  waiter.SendTouchEvent(window_id, crosapi::mojom::TouchEventType::kReleased,
+                        /*pointer_id=*/0u, gfx::PointF(tab_center_in_widget));
+
+  // Wait for the popup menu to be created and positioned on screen.
+  base::RunLoop loop3;
+  auto wait_for_popup_on_screen = base::BindLambdaForTesting([&]() {
+    std::set<views::Widget*> widgets = views::test::WidgetTest::GetAllWidgets();
+    widgets.erase(browser_widget);
+    if (widgets.size() == 0u)
+      return;
+    // The popup was created.
+    views::Widget* popup = *widgets.begin();
+    // The popup's top edge may be off the top of the screen. Wait for it to be
+    // positioned on screen. The bug involved the repositioning code. We know
+    // that 0 means the top of the screen because the window is maximized.
+    if (popup->GetRestoredBounds().y() < 0)
+      return;
+    loop3.Quit();
+  });
+  base::RepeatingTimer timer3;
+  timer3.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(1),
+               std::move(wait_for_popup_on_screen));
+  loop3.Run();
+
+  // Find the popup.
+  std::set<views::Widget*> widgets = views::test::WidgetTest::GetAllWidgets();
+  widgets.erase(browser_widget);
+  ASSERT_EQ(1u, widgets.size());
+  views::Widget* popup = *widgets.begin();
+
+  // The popup has valid bounds.
+  gfx::Rect popup_bounds = popup->GetRestoredBounds();
+  EXPECT_FALSE(popup_bounds.IsEmpty()) << popup_bounds.ToString();
+}
+
+}  // namespace

chrome/test/BUILD.gn

@@ -3224,6 +3224,7 @@ if (is_chromeos_lacros) {
       "../browser/lacros/message_center_lacros_browsertest.cc",
       "../browser/lacros/metrics_reporting_lacros_browsertest.cc",
       "../browser/lacros/overview_lacros_browsertest.cc",
+      "../browser/lacros/popup_lacros_browsertest.cc",
       "../browser/lacros/screen_manager_lacros_browsertest.cc",
       "../browser/lacros/tablet_mode_lacros_browsertest.cc",
     ]

chromeos/crosapi/mojom/test_controller.mojom

@@ -4,11 +4,26 @@
 
 module crosapi.mojom;
 
+import "ui/gfx/geometry/mojom/geometry.mojom";
+
+[Stable, Extensible]
+enum TouchEventType {
+  kUnknown = 0,
+  kPressed = 1,
+  kMoved = 2,
+  kReleased = 3,
+  // Touch events can be cancelled if mouse capture or touch capture changes in
+  // the middle of a gesture. For example, a long-press might change window or
+  // focus activation state in a way that cancels the gesture, even though the
+  // user has not released their finger. See ui::GestureRecognizer.
+  kCancelled = 4
+};
+
 // This interface is implemented by Ash-Chrome.
 // This interface provides tests a mechanism to mutate or query ash.
 // In the future, this interface may merge with an automation or a11y interface.
-// Next version: 3
-// Next method id: 6
+// Next version: 4
+// Next method id: 8
 [Stable, Uuid="1f93f9d7-e466-466c-a675-c21b48cf30d3"]
 interface TestController {
   // Queries whether a window with the given |window_id| exists. |window_id|
@@ -39,4 +54,25 @@ interface TestController {
   // are finished.
   [MinVersion=2]
   ExitTabletMode@5() => ();
+
+  // Creates a touch event and dispatches it to the window with |window_id|.
+  // Returns immediately if the window does not exist. The |pointer_id|
+  // specifies which touch-point is involved in a multi-touch gesture.
+  // |pointer_id| is typically 0, meaning the first touch-point, usually the
+  // index finger. |location_in_window| is in DIPs, in coordinates relative to
+  // the window origin with 0,0 representing top-left. Note that the location
+  // is a float, as some input devices report sub-pixel positions for touch
+  // events.
+  [MinVersion=3]
+  SendTouchEvent@7(string window_id, TouchEventType type, uint8 pointer_id,
+                   gfx.mojom.PointF location_in_window) => ();
+
+  // Returns the position of a window's top-left corner in global "screen"
+  // coordinates in DIPs. By design, Wayland clients do not know the global
+  // position of their windows on the display. However, for window manager
+  // integration testing, some tests may need to assert a window is in a certain
+  // position (e.g. at the top of the display). Returns null if the window does
+  // not exist.
+  [MinVersion=3]
+  GetWindowPositionInScreen@8(string window_id) => (gfx.mojom.Point? position);
 };