Cros:Make sure the hotseat only changes state once between transitions

A few changes:
 - Catches some hotseat corner cases causing multiple state changes
 - adds tests to catch these multi-state changes
 - Adds the ability to determine that Overview is in the process of
   being toggled. Before this cl, overview would be in the process
   of being shown but it was not clear if it was being shown or hidden
   until it was too late.

Bug: 1018346
Change-Id: I6e8dfd0a9b0256134770e207345f4926bfc512ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1885173
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710555}

ash/app_list/app_list_controller_impl.cc

@@ -39,7 +39,6 @@
 #include "ash/root_window_controller.h"
 #include "ash/screen_util.h"
 #include "ash/session/session_controller_impl.h"
-#include "ash/shelf/shelf_layout_manager.h"
 #include "ash/shell.h"
 #include "ash/wallpaper/wallpaper_controller_impl.h"
 #include "ash/wm/mru_window_tracker.h"
@@ -591,6 +590,10 @@ ash::AppListViewState AppListControllerImpl::GetAppListViewState() {
   return model_->state_fullscreen();
 }
 
+bool AppListControllerImpl::ShouldHomeLauncherBeVisible() const {
+  return IsTabletMode() && !HasVisibleWindows();
+}
+
 void AppListControllerImpl::OnShelfAlignmentChanged(aura::Window* root_window) {
   if (!IsTabletMode())
     DismissAppList();
@@ -604,6 +607,10 @@ void AppListControllerImpl::OnShellDestroying() {
 
 void AppListControllerImpl::OnOverviewModeWillStart() {
   if (IsTabletMode()) {
+    scoped_suspend_visibility_update_ =
+        std::make_unique<ShelfLayoutManager::ScopedSuspendVisibilityUpdate>(
+            RootWindowController::ForWindow(presenter_.GetWindow())
+                ->GetShelfLayoutManager());
     const int64_t display_id = last_visible_display_id_;
     OnHomeLauncherTargetPositionChanged(false /* showing */, display_id);
     OnVisibilityWillChange(false /* visible */, display_id);
@@ -616,6 +623,8 @@ void AppListControllerImpl::OnOverviewModeStartingAnimationComplete(
     bool canceled) {
   if (!IsTabletMode())
     return;
+  if (scoped_suspend_visibility_update_)
+    scoped_suspend_visibility_update_.reset();
   OnHomeLauncherAnimationComplete(false /* shown */, last_visible_display_id_);
 }
 

ash/app_list/app_list_controller_impl.h

@@ -28,6 +28,7 @@
 #include "ash/public/cpp/tablet_mode_observer.h"
 #include "ash/public/cpp/wallpaper_controller_observer.h"
 #include "ash/session/session_observer.h"
+#include "ash/shelf/shelf_layout_manager.h"
 #include "ash/shell_observer.h"
 #include "ash/wm/mru_window_tracker.h"
 #include "ash/wm/overview/overview_observer.h"
@@ -156,6 +157,8 @@ class ASH_EXPORT AppListControllerImpl
                                  AppListShowSource show_source,
                                  base::TimeTicks event_time_stamp);
   ash::AppListViewState GetAppListViewState();
+  // Returns whether the home launcher should be visible.
+  bool ShouldHomeLauncherBeVisible() const;
 
   // AppListViewDelegate:
   AppListModel* GetModel() override;
@@ -422,6 +425,11 @@ class ASH_EXPORT AppListControllerImpl
 
   StateTransitionAnimationCallback state_transition_animation_callback_;
 
+  // Used to prevent ShelfLayoutManager updating visibility state when overview
+  // is showing over the AppList.
+  std::unique_ptr<ShelfLayoutManager::ScopedSuspendVisibilityUpdate>
+      scoped_suspend_visibility_update_;
+
   base::ObserverList<AppListControllerObserver> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(AppListControllerImpl);

ash/shelf/shelf_layout_manager.cc

@@ -1058,7 +1058,14 @@ HotseatState ShelfLayoutManager::CalculateHotseatState(
         case AppListControllerImpl::HomeLauncherAnimationState::kHiding:
           return in_overview ? HotseatState::kExtended : HotseatState::kHidden;
         case AppListControllerImpl::HomeLauncherAnimationState::kFinished:
-          if (app_list_controller->IsVisible())
+          // Consider the AppList visible if it is beginning to show. Also
+          // detect the case where the last window is being minimized.
+          const bool app_list_visible =
+              app_list_controller->IsVisible() ||
+              app_list_controller->GetTargetVisibility() ||
+              (!in_overview &&
+               app_list_controller->ShouldHomeLauncherBeVisible());
+          if (app_list_visible)
             return HotseatState::kShown;
           if (in_split_view)
             return HotseatState::kHidden;
@@ -1066,8 +1073,9 @@ HotseatState ShelfLayoutManager::CalculateHotseatState(
             return HotseatState::kExtended;
           if (visibility_state == SHELF_AUTO_HIDE) {
             if (auto_hide_state == SHELF_AUTO_HIDE_HIDDEN ||
-                should_hide_hotseat_)
+                should_hide_hotseat_) {
               return HotseatState::kHidden;
+            }
             return HotseatState::kExtended;
           }
           if (shelf_widget_->hotseat_widget()->is_manually_extended() &&

ash/shelf/shelf_layout_manager_unittest.cc

@@ -3768,6 +3768,197 @@ TEST_F(HotseatShelfLayoutManagerTest, HotseatHidesWhenSwipedToBezel) {
   EXPECT_EQ(HotseatState::kHidden, GetShelfLayoutManager()->hotseat_state());
 }
 
+namespace {
+
+class OverviewAnimationWaiter : public OverviewObserver {
+ public:
+  OverviewAnimationWaiter() {
+    Shell::Get()->overview_controller()->AddObserver(this);
+  }
+
+  ~OverviewAnimationWaiter() override {
+    Shell::Get()->overview_controller()->RemoveObserver(this);
+  }
+
+  // Note this could only be called once because RunLoop would not run after
+  // Quit is called. Create a new instance if there's need to wait again.
+  void Wait() { run_loop_.Run(); }
+
+  // OverviewObserver:
+  void OnOverviewModeStartingAnimationComplete(bool cancel) override {
+    run_loop_.Quit();
+  }
+  void OnOverviewModeEndingAnimationComplete(bool cancel) override {
+    run_loop_.Quit();
+  }
+
+ private:
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(OverviewAnimationWaiter);
+};
+
+}  // namespace
+
+// Tests that home -> overview results in only one hotseat state change.
+TEST_P(HotseatShelfLayoutManagerTest, HomeToOverviewChangesStateOnce) {
+  GetPrimaryShelf()->SetAutoHideBehavior(GetParam());
+  TabletModeControllerTestApi().EnterTabletMode();
+
+  // First, try with no windows open.
+  const gfx::Point overview_button_center = GetPrimaryShelf()
+                                                ->shelf_widget()
+                                                ->status_area_widget()
+                                                ->overview_button_tray()
+                                                ->GetBoundsInScreen()
+                                                .CenterPoint();
+
+  {
+    HotseatStateWatcher watcher(GetShelfLayoutManager());
+    OverviewAnimationWaiter waiter;
+    GetEventGenerator()->GestureTapAt(overview_button_center);
+    waiter.Wait();
+    watcher.CheckEqual({HotseatState::kExtended});
+  }
+
+  // Open a window, then open the home launcher.
+  std::unique_ptr<aura::Window> window =
+      AshTestBase::CreateTestWindow(gfx::Rect(0, 0, 400, 400));
+  wm::ActivateWindow(window.get());
+  if (GetParam() == SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS)
+    SwipeUpOnShelf();
+  views::View* home_button = GetPrimaryShelf()->shelf_widget()->GetHomeButton();
+  GetEventGenerator()->GestureTapAt(
+      home_button->GetBoundsInScreen().CenterPoint());
+  GetAppListTestHelper()->CheckVisibility(true);
+  // Activate overview and expect the hotseat only changes state to extended.
+  {
+    HotseatStateWatcher watcher(GetShelfLayoutManager());
+    OverviewAnimationWaiter waiter;
+    GetEventGenerator()->GestureTapAt(overview_button_center);
+    waiter.Wait();
+
+    watcher.CheckEqual({HotseatState::kExtended});
+  }
+}
+
+// Tests that home -> in-app results in only one state change.
+TEST_P(HotseatShelfLayoutManagerTest, HomeToInAppChangesStateOnce) {
+  GetPrimaryShelf()->SetAutoHideBehavior(GetParam());
+  TabletModeControllerTestApi().EnterTabletMode();
+
+  // Go to in-app, the hotseat should hide.
+  HotseatStateWatcher watcher(GetShelfLayoutManager());
+  std::unique_ptr<aura::Window> window =
+      AshTestBase::CreateTestWindow(gfx::Rect(0, 0, 400, 400));
+  wm::ActivateWindow(window.get());
+
+  watcher.CheckEqual({HotseatState::kHidden});
+}
+
+// Tests that in-app -> home via closing the only window, swiping from the
+// bottom of the shelf, and tapping the home launcher button results in only one
+// state change.
+TEST_P(HotseatShelfLayoutManagerTest, InAppToHomeChangesStateOnce) {
+  GetPrimaryShelf()->SetAutoHideBehavior(GetParam());
+  TabletModeControllerTestApi().EnterTabletMode();
+
+  // Go to in-app with an extended hotseat.
+  std::unique_ptr<aura::Window> window =
+      AshTestBase::CreateTestWindow(gfx::Rect(0, 0, 400, 400));
+  wm::ActivateWindow(window.get());
+  SwipeUpOnShelf();
+
+  // Press the HomeLauncher button, the hotseat should transition directly to
+  // kShown.
+  {
+    HotseatStateWatcher watcher(GetShelfLayoutManager());
+    views::View* home_button =
+        GetPrimaryShelf()->shelf_widget()->GetHomeButton();
+    GetEventGenerator()->GestureTapAt(
+        home_button->GetBoundsInScreen().CenterPoint());
+
+    watcher.CheckEqual({HotseatState::kShown});
+  }
+  // Go to in-app.
+  window->Show();
+  wm::ActivateWindow(window.get());
+
+  // Extend the hotseat, then Swipe up to go home. the hotseat should transition
+  // directly to kShown.
+  SwipeUpOnShelf();
+  {
+    HotseatStateWatcher watcher(GetShelfLayoutManager());
+    SwipeUpOnShelf();
+    watcher.CheckEqual({HotseatState::kShown});
+  }
+
+  // Nothing left to test for autohidden shelf.
+  if (GetParam() == SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS)
+    return;
+
+  // Go to in-app and do not extend the hotseat.
+  window->Show();
+  wm::ActivateWindow(window.get());
+
+  // Press the HomeLauncher button, the hotseat should transition directly to
+  // kShown.
+  {
+    HotseatStateWatcher watcher(GetShelfLayoutManager());
+    views::View* home_button =
+        GetPrimaryShelf()->shelf_widget()->GetHomeButton();
+    GetEventGenerator()->GestureTapAt(
+        home_button->GetBoundsInScreen().CenterPoint());
+
+    watcher.CheckEqual({HotseatState::kShown});
+  }
+}
+
+// Tests that in-app -> overview results in only one state change with an
+// autohidden shelf.
+TEST_F(HotseatShelfLayoutManagerTest,
+       InAppToOverviewChangesStateOnceAutohiddenShelf) {
+  GetPrimaryShelf()->SetAutoHideBehavior(SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS);
+  TabletModeControllerTestApi().EnterTabletMode();
+
+  // Test going to overview mode using the controller from an autohide hidden
+  // shelf. Go to in-app.
+  std::unique_ptr<aura::Window> window =
+      AshTestBase::CreateTestWindow(gfx::Rect(0, 0, 400, 400));
+  wm::ActivateWindow(window.get());
+  {
+    HotseatStateWatcher watcher(GetShelfLayoutManager());
+    // Enter overview by using the controller.
+    OverviewAnimationWaiter waiter;
+    Shell::Get()->overview_controller()->StartOverview();
+    waiter.Wait();
+
+    watcher.CheckEqual({HotseatState::kExtended});
+  }
+  {
+    OverviewAnimationWaiter waiter;
+    Shell::Get()->overview_controller()->EndOverview();
+    waiter.Wait();
+  }
+
+  // Test in-app -> overview again with the autohide shown shelf.
+  EXPECT_TRUE(ShelfConfig::Get()->is_in_app());
+  EXPECT_EQ(ShelfAutoHideState::SHELF_AUTO_HIDE_HIDDEN,
+            GetShelfLayoutManager()->auto_hide_state());
+  SwipeUpOnShelf();
+  {
+    HotseatStateWatcher watcher(GetShelfLayoutManager());
+    // Enter overview by using the controller.
+    OverviewAnimationWaiter waiter;
+    Shell::Get()->overview_controller()->StartOverview();
+    waiter.Wait();
+
+    watcher.CheckEqual({});
+    EXPECT_EQ(HotseatState::kExtended,
+              GetShelfLayoutManager()->hotseat_state());
+  }
+}
+
 class ShelfLayoutManagerWindowDraggingTest : public ShelfLayoutManagerTestBase {
  public:
   ShelfLayoutManagerWindowDraggingTest() = default;
@@ -4057,38 +4248,6 @@ TEST_F(ShelfLayoutManagerKeyboardTest, ShelfShouldChangeWorkAreaInStickyMode) {
             display::Screen::GetScreen()->GetPrimaryDisplay().work_area());
 }
 
-namespace {
-
-class OverviewAnimationWaiter : public OverviewObserver {
- public:
-  OverviewAnimationWaiter() {
-    Shell::Get()->overview_controller()->AddObserver(this);
-  }
-
-  ~OverviewAnimationWaiter() override {
-    Shell::Get()->overview_controller()->RemoveObserver(this);
-  }
-
-  // Note this could only be called once because RunLoop would not run after
-  // Quit is called. Create a new instance if there's need to wait again.
-  void Wait() { run_loop_.Run(); }
-
-  // OverviewObserver:
-  void OnOverviewModeStartingAnimationComplete(bool cancel) override {
-    run_loop_.Quit();
-  }
-  void OnOverviewModeEndingAnimationComplete(bool cancel) override {
-    run_loop_.Quit();
-  }
-
- private:
-  base::RunLoop run_loop_;
-
-  DISALLOW_COPY_AND_ASSIGN(OverviewAnimationWaiter);
-};
-
-}  // namespace
-
 // Make sure we don't update the work area during overview animation
 // (crbug.com/947343).
 TEST_P(ShelfLayoutManagerTest, NoShelfUpdateDuringOverviewAnimation) {