Use DragEventGenerator for SplitView Resize test

This also adds extra scenarios.
* Resizing with overview on the other side.
* touch event

I also found the scenario that splitview controller can be
called recursively during shutdown, which is fixed by the change
in SplitViewController

Bug: None
Change-Id: I293ed17bba4961d338ac0f9a389f42f8f233d4ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1637244
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666091}

ash/wm/splitview/split_view_controller.cc

@@ -261,7 +261,9 @@ class SplitViewController::DividerSnapAnimation
         CurrentValueBetween(starting_position_, ending_position_);
     split_view_controller_->NotifyDividerPositionChanged();
     split_view_controller_->UpdateSnappedWindowsAndDividerBounds();
-    split_view_controller_->SetWindowsTransformDuringResizing();
+    // Updating the window may stop animation.
+    if (is_animating())
+      split_view_controller_->SetWindowsTransformDuringResizing();
   }
 
   SplitViewController* split_view_controller_;
@@ -672,8 +674,11 @@ void SplitViewController::EndSplitView(EndReason end_reason) {
   const bool is_divider_animating = IsDividerAnimating();
   if (is_resizing_ || is_divider_animating) {
     is_resizing_ = false;
-    if (is_divider_animating)
-      StopAndShoveAnimatedDivider();
+    if (is_divider_animating) {
+      // Don't call StopAndShoveAnimatedDivider as it will call observers.
+      divider_snap_animation_->Stop();
+      divider_position_ = divider_snap_animation_->ending_position();
+    }
     EndResizeImpl();
   }
 

chrome/browser/ui/BUILD.gn

@@ -3900,6 +3900,8 @@ static_library("test_support") {
 
   if (is_chromeos) {
     sources += [
+      "ash/ash_test_util.cc",
+      "ash/ash_test_util.h",
       "ash/fake_tablet_mode_controller.cc",
       "ash/fake_tablet_mode_controller.h",
       "ash/test_login_screen.cc",

chrome/browser/ui/ash/ash_test_util.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 "chrome/browser/ui/ash/ash_test_util.h"
+
+#include "ash/public/cpp/window_properties.h"
+#include "ash/public/cpp/window_state_type.h"
+#include "base/run_loop.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/aura/window.h"
+#include "ui/aura/window_observer.h"
+#include "ui/base/test/ui_controls.h"
+#include "ui/wm/core/window_util.h"
+
+namespace test {
+namespace {
+// Wait until the window's state changes to given the snapped state.
+// The window should stay alive, so no need to observer destroying.
+class SnapWaiter : public aura::WindowObserver {
+ public:
+  SnapWaiter(aura::Window* window, ash::WindowStateType type)
+      : window_(window), type_(type) {
+    window->AddObserver(this);
+  }
+  ~SnapWaiter() override { window_->RemoveObserver(this); }
+
+  // aura::WindowObserver:
+  void OnWindowPropertyChanged(aura::Window* window,
+                               const void* key,
+                               intptr_t old) override {
+    if (key == ash::kWindowStateTypeKey && IsSnapped())
+      run_loop_.Quit();
+  }
+
+  void Wait() { run_loop_.Run(); }
+
+  bool IsSnapped() const {
+    return window_->GetProperty(ash::kWindowStateTypeKey) == type_;
+  }
+
+ private:
+  aura::Window* window_;
+  ash::WindowStateType type_;
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(SnapWaiter);
+};
+
+}  // namespace
+
+void ActivateAndSnapWindow(aura::Window* window, ash::WindowStateType type) {
+  DCHECK(window);
+  if (!wm::IsActiveWindow(window))
+    wm::ActivateWindow(window);
+
+  ASSERT_TRUE(wm::IsActiveWindow(window));
+
+  SnapWaiter snap_waiter(window, type);
+  ASSERT_TRUE(type == ash::WindowStateType::kRightSnapped ||
+              type == ash::WindowStateType::kLeftSnapped);
+
+  // Early return if it's already snapped.
+  if (snap_waiter.IsSnapped())
+    return;
+
+  ui_controls::SendKeyPress(window,
+                            type == ash::WindowStateType::kLeftSnapped
+                                ? ui::VKEY_OEM_4
+                                : ui::VKEY_OEM_6,
+                            /*control=*/false,
+                            /*shift=*/false,
+                            /*alt=*/true,
+                            /*command=*/false);
+  snap_waiter.Wait();
+}
+
+}  // namespace test

chrome/browser/ui/ash/ash_test_util.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 CHROME_BROWSER_UI_ASH_ASH_TEST_UTIL_H_
+#define CHROME_BROWSER_UI_ASH_ASH_TEST_UTIL_H_
+
+namespace aura {
+class Window;
+}
+
+namespace ash {
+enum class WindowStateType;
+}
+
+namespace test {
+
+// The snap window. This will activate the |window|.
+void ActivateAndSnapWindow(aura::Window* window, ash::WindowStateType type);
+
+}  // namespace test
+
+#endif  // CHROME_BROWSER_UI_ASH_ASH_TEST_UTIL_H_

chrome/browser/ui/ash/split_view_interactive_uitest.cc

@@ -2,157 +2,164 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ash/public/cpp/ash_switches.h"
 #include "ash/public/cpp/test/shell_test_api.h"
-#include "ash/shell.h"
-#include "ash/wm/splitview/split_view_controller.h"
+#include "ash/public/cpp/window_state_type.h"
+#include "base/command_line.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/system/sys_info.h"
 #include "base/task/post_task.h"
 #include "base/test/bind_test_util.h"
+#include "chrome/browser/ui/ash/ash_test_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/interactive_test_utils.h"
+#include "chrome/test/base/perf/drag_event_generator.h"
 #include "chrome/test/base/perf/performance_test.h"
 #include "ui/aura/window.h"
-#include "ui/aura/window_tree_host.h"
 #include "ui/compositor/compositor.h"
-#include "ui/compositor/compositor_switches.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
-#include "ui/gl/gl_switches.h"
-#include "ui/views/widget/widget.h"
-#include "ui/views/widget/widget_observer.h"
+#include "ui/wm/core/wm_core_switches.h"
 
 namespace {
 
-class SplitViewTest : public UIPerformanceTest {
+class SplitViewTest
+    : public UIPerformanceTest,
+      public testing::WithParamInterface<::testing::tuple<bool, bool>> {
  public:
   SplitViewTest() = default;
   ~SplitViewTest() override = default;
 
   // UIPerformanceTest:
   void SetUpCommandLine(base::CommandLine* command_line) override {
-    command_line->AppendSwitch(ash::switches::kAshEnableTabletMode);
+    // We're not interested in window transition animation in this test,
+    // so just disable it.
+    command_line->AppendSwitch(wm::switches::kWindowAnimationsDisabled);
   }
+  void SetUpOnMainThread() override {
+    UIPerformanceTest::SetUpOnMainThread();
 
- private:
-  std::vector<std::string> GetUMAHistogramNames() const override {
-    return {"Ash.SplitViewResize.PresentationTime.TabletMode.MultiWindow"};
+    use_ntp_ = std::get<0>(GetParam());
+    use_touch_ = std::get<1>(GetParam());
+
+    if (use_ntp_)
+      ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL));
   }
 
-  DISALLOW_COPY_AND_ASSIGN(SplitViewTest);
-};
+  bool use_touch() const { return use_touch_; }
 
-// Used to wait for a window resize to show up on screen.
-class WidgetResizeWaiter : public views::WidgetObserver {
- public:
-  explicit WidgetResizeWaiter(views::Widget* widget) : widget_(widget) {
-    widget_->AddObserver(this);
-  }
+  void SetUMAToObserve(std::string name) { uma_name_ = std::move(name); }
 
-  ~WidgetResizeWaiter() override {
-    widget_->RemoveObserver(this);
-    EXPECT_FALSE(waiting_for_frame_);
+  Browser* CreateBrowserMaybeWithNtp() {
+    Browser* new_browser = CreateBrowser(browser()->profile());
+    if (use_ntp_)
+      ui_test_utils::NavigateToURL(new_browser,
+                                   GURL(chrome::kChromeUINewTabURL));
+    return new_browser;
   }
 
-  void WaitForDisplay() {
-    do {
-      if (waiting_for_frame_) {
-        run_loop_ = std::make_unique<base::RunLoop>();
-        run_loop_->Run();
-        EXPECT_FALSE(waiting_for_frame_);
-      }
-      ash::ShellTestApi().WaitForNoPointerHoldLock();
-    } while (waiting_for_frame_);
+  void WaitForWarmup() {
+    // If running on device, wait a bit so that the display is actually powered
+    // on.
+    // TODO: this should be better factored.
+    base::TimeDelta warmup = base::TimeDelta::FromSeconds(
+        base::SysInfo::IsRunningOnChromeOS() ? 5 : 0);
+    // Give ntp at least 1 seconds to be fully resized.
+    if (use_ntp_ && warmup.is_zero())
+      warmup += base::TimeDelta::FromSeconds(1);
+
+    if (!warmup.is_zero()) {
+      base::RunLoop run_loop;
+      base::PostDelayedTask(FROM_HERE, run_loop.QuitClosure(), warmup);
+      run_loop.Run();
+    }
   }
 
  private:
-  void OnFramePresented(const gfx::PresentationFeedback& feedback) {
-    waiting_for_frame_ = false;
-    if (run_loop_)
-      run_loop_->Quit();
-  }
-
-  // views::WidgetObserver:
-  void OnWidgetBoundsChanged(views::Widget* widget,
-                             const gfx::Rect& new_bounds) override {
-    ui::Compositor* compositor =
-        widget->GetNativeWindow()->GetHost()->compositor();
-    ASSERT_TRUE(compositor);
-    waiting_for_frame_ = true;
-    compositor->RequestPresentationTimeForNextFrame(base::BindOnce(
-        &WidgetResizeWaiter::OnFramePresented, base::Unretained(this)));
+  std::vector<std::string> GetUMAHistogramNames() const override {
+    return {uma_name_};
   }
 
-  views::Widget* widget_;
-  bool waiting_for_frame_ = false;
-  std::unique_ptr<base::RunLoop> run_loop_;
+  bool use_ntp_;
+  bool use_touch_;
+  std::string uma_name_;
 
-  DISALLOW_COPY_AND_ASSIGN(WidgetResizeWaiter);
+  DISALLOW_COPY_AND_ASSIGN(SplitViewTest);
 };
 
-IN_PROC_BROWSER_TEST_F(SplitViewTest, SplitViewResize) {
+IN_PROC_BROWSER_TEST_P(SplitViewTest, ResizeTwoWindows) {
+  SetUMAToObserve(
+      "Ash.SplitViewResize.PresentationTime.TabletMode.MultiWindow");
+
   // This test is intended to gauge performance of resizing windows while in
   // tablet mode split view. It does the following:
   // . creates two browser windows.
-  // . enters tablet mode.
   // . snaps one to the left, one to the right.
+  // . enters tablet mode.
   // . drags the resizer, which triggers resizing both browsers.
-  browser()->window()->Maximize();
-
-  Browser* browser2 = CreateBrowser(browser()->profile());
-  browser2->window()->Show();
-  browser2->window()->Maximize();
-
-  // If running on device, wait a bit so that the display is actually powered
-  // on.
-  // TODO: this should be better factored.
-  if (base::SysInfo::IsRunningOnChromeOS()) {
-    base::RunLoop run_loop;
-    base::PostDelayedTask(FROM_HERE, run_loop.QuitClosure(),
-                          base::TimeDelta::FromSeconds(5));
-    run_loop.Run();
-  }
 
-  ash::ShellTestApi().EnableTabletModeWindowManager(true);
+  aura::Window* browser_window = browser()->window()->GetNativeWindow();
+  test::ActivateAndSnapWindow(browser_window,
+                              ash::WindowStateType::kLeftSnapped);
 
-  views::Widget* browser_widget =
-      BrowserView::GetBrowserViewForBrowser(browser())->GetWidget();
-  views::Widget* browser2_widget =
-      BrowserView::GetBrowserViewForBrowser(browser2)->GetWidget();
-  ash::Shell* shell = ash::Shell::Get();
-  shell->split_view_controller()->SnapWindow(browser2_widget->GetNativeWindow(),
-                                             ash::SplitViewController::LEFT);
-  shell->split_view_controller()->SnapWindow(browser_widget->GetNativeWindow(),
-                                             ash::SplitViewController::RIGHT);
+  Browser* browser2 = CreateBrowserMaybeWithNtp();
+  aura::Window* browser2_window = browser2->window()->GetNativeWindow();
+  test::ActivateAndSnapWindow(browser2_window,
+                              ash::WindowStateType::kRightSnapped);
+  ash::ShellTestApi().EnableTabletModeWindowManager(true);
 
-  ash::ShellTestApi().WaitForNoPointerHoldLock();
+  WaitForWarmup();
 
   const gfx::Size display_size =
       display::Screen::GetScreen()->GetPrimaryDisplay().bounds().size();
-  const gfx::Point start_position(display_size.width() / 2,
-                                  display_size.height() / 2);
+  const gfx::Point start_position(gfx::Rect(display_size).CenterPoint());
   TRACE_EVENT_ASYNC_BEGIN0("ui", "Interaction.ui_WindowResize", this);
-  ASSERT_TRUE(
-      ui_test_utils::SendMouseMoveSync(
-          gfx::Point(start_position.x(), start_position.y())) &&
-      ui_test_utils::SendMouseEventsSync(ui_controls::LEFT, ui_controls::DOWN));
-
-  constexpr int kNumIncrements = 20;
-  int width = browser_widget->GetWindowBoundsInScreen().width();
-  for (int i = 0; i < kNumIncrements; ++i) {
-    const gfx::Point resize_point(start_position.x() - i - 1,
-                                  start_position.y());
-    WidgetResizeWaiter observer(browser_widget);
-    ASSERT_TRUE(ui_test_utils::SendMouseMoveSync(resize_point));
-    observer.WaitForDisplay();
-    ++width;
-    EXPECT_EQ(width, browser_widget->GetWindowBoundsInScreen().width());
-  }
+  gfx::Point end_position(start_position);
+  end_position.set_x(end_position.x() - 60);
+  ui_test_utils::DragEventGenerator generator(
+      std::make_unique<ui_test_utils::InterpolatedProducer>(
+          start_position, end_position,
+          base::TimeDelta::FromMilliseconds(1000)),
+      use_touch());
+  generator.Wait();
+  TRACE_EVENT_ASYNC_END0("ui", "Interaction.ui_WindowResize", this);
+}
+
+IN_PROC_BROWSER_TEST_P(SplitViewTest, ResizeWithOverview) {
+  SetUMAToObserve(
+      "Ash.SplitViewResize.PresentationTime.TabletMode.WithOverview");
+
+  Browser* browser2 = CreateBrowserMaybeWithNtp();
+
+  aura::Window* browser2_window = browser2->window()->GetNativeWindow();
+  test::ActivateAndSnapWindow(browser2_window,
+                              ash::WindowStateType::kRightSnapped);
+
+  ash::ShellTestApi().EnableTabletModeWindowManager(true);
+  ash::ShellTestApi().WaitForOverviewAnimationState(
+      ash::OverviewAnimationState::kEnterAnimationComplete);
+
+  WaitForWarmup();
+
+  const gfx::Point start_position =
+      display::Screen::GetScreen()->GetPrimaryDisplay().bounds().CenterPoint();
+  TRACE_EVENT_ASYNC_BEGIN0("ui", "Interaction.ui_WindowResize", this);
+  gfx::Point end_position(start_position);
+  end_position.set_x(end_position.x() - 60);
+  ui_test_utils::DragEventGenerator generator(
+      std::make_unique<ui_test_utils::InterpolatedProducer>(
+          start_position, end_position,
+          base::TimeDelta::FromMilliseconds(1000)),
+      use_touch());
+  generator.Wait();
   TRACE_EVENT_ASYNC_END0("ui", "Interaction.ui_WindowResize", this);
 }
 
+INSTANTIATE_TEST_SUITE_P(,
+                         SplitViewTest,
+                         ::testing::Combine(/*ntp=*/testing::Bool(),
+                                            /*touch=*/testing::Bool()));
+
 }  // namespace

chrome/browser/ui/ash/window_resize_interactive_uitest.cc

@@ -2,12 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ash/public/cpp/window_properties.h"
 #include "ash/public/cpp/window_state_type.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/task/post_task.h"
+#include "chrome/browser/ui/ash/ash_test_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/test/base/interactive_test_utils.h"
@@ -107,41 +107,6 @@ class MultiPointProducer
   DISALLOW_COPY_AND_ASSIGN(MultiPointProducer);
 };
 
-// Wait until the window's state changed to given snapped state.
-// The window should stay alive, so no need to observer destroying.
-class SnapWaiter : public aura::WindowObserver {
- public:
-  SnapWaiter(aura::Window* window, ash::WindowStateType type)
-      : window_(window), type_(type) {
-    window->AddObserver(this);
-  }
-  ~SnapWaiter() override { window_->RemoveObserver(this); }
-
-  // aura::WindowObserver:
-  void OnWindowPropertyChanged(aura::Window* window,
-                               const void* key,
-                               intptr_t old) override {
-    if (key == ash::kWindowStateTypeKey && IsSnapped())
-      run_loop_.Quit();
-  }
-
-  void Wait() {
-    if (!IsSnapped())
-      run_loop_.Run();
-  }
-
- private:
-  bool IsSnapped() const {
-    return window_->GetProperty(ash::kWindowStateTypeKey) == type_;
-  }
-
-  aura::Window* window_;
-  ash::WindowStateType type_;
-  base::RunLoop run_loop_;
-
-  DISALLOW_COPY_AND_ASSIGN(SnapWaiter);
-};
-
 }  // namespace
 
 // Test window resize performance in clamshell mode.
@@ -191,13 +156,8 @@ class WindowResizeTest
 
 IN_PROC_BROWSER_TEST_P(WindowResizeTest, Single) {
   aura::Window* browser_window = browser()->window()->GetNativeWindow();
-  SnapWaiter snap_waiter(browser_window, ash::WindowStateType::kLeftSnapped);
-  ui_controls::SendKeyPress(browser_window, ui::VKEY_OEM_4,
-                            /*control=*/false,
-                            /*shift=*/false,
-                            /*alt=*/true,
-                            /*command=*/false);
-  snap_waiter.Wait();
+  test::ActivateAndSnapWindow(browser_window,
+                              ash::WindowStateType::kLeftSnapped);
 
   gfx::Rect bounds = browser_window->GetBoundsInScreen();
   gfx::Point start_point = gfx::Point(bounds.right_center());
@@ -221,15 +181,8 @@ IN_PROC_BROWSER_TEST_P(WindowResizeTest, Single) {
 
 IN_PROC_BROWSER_TEST_P(WindowResizeTest, Multi) {
   aura::Window* browser_window = browser()->window()->GetNativeWindow();
-  {
-    SnapWaiter snap_waiter(browser_window, ash::WindowStateType::kLeftSnapped);
-    ui_controls::SendKeyPress(browser_window, ui::VKEY_OEM_4,
-                              /*control=*/false,
-                              /*shift=*/false,
-                              /*alt=*/true,
-                              /*command=*/false);
-    snap_waiter.Wait();
-  }
+  test::ActivateAndSnapWindow(browser_window,
+                              ash::WindowStateType::kLeftSnapped);
 
   Browser* browser2 = CreateBrowser(browser()->profile());
   if (use_ntp()) {
@@ -237,18 +190,10 @@ IN_PROC_BROWSER_TEST_P(WindowResizeTest, Multi) {
     ui_test_utils::NavigateToURL(browser2, ntp_url);
   }
 
+  aura::Window* browser_window2 = browser2->window()->GetNativeWindow();
   // Snap Right
-  {
-    aura::Window* browser_window2 = browser2->window()->GetNativeWindow();
-    SnapWaiter snap_waiter(browser_window2,
-                           ash::WindowStateType::kRightSnapped);
-    ui_controls::SendKeyPress(browser_window2, ui::VKEY_OEM_6,
-                              /*control=*/false,
-                              /*shift=*/false,
-                              /*alt=*/true,
-                              /*command=*/false);
-    snap_waiter.Wait();
-  }
+  test::ActivateAndSnapWindow(browser_window2,
+                              ash::WindowStateType::kRightSnapped);
 
   gfx::Rect bounds = browser_window->GetBoundsInScreen();
   gfx::Point start_point = gfx::Point(bounds.right_center());