StartOverview() and EndOverview() should return the correct value.

- StartOverview() should return true if enter overview successfully,
false otherwise
- EndOverview() should return true if end overview successfully, false
otherwise
- Other small refactorings

No functionality change in this CL.

Bug: None
Change-Id: I47fcf479bda3d51da98ee693e0a5acbe51f80c1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1785625
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694964}

ash/system/overview/overview_button_tray.cc

@@ -135,18 +135,20 @@ bool OverviewButtonTray::PerformAction(const ui::Event& event) {
           : base::make_optional(event.time_stamp());
 
   OverviewController* controller = Shell::Get()->overview_controller();
-  // Note: Toggling overview mode will fail if there is no window to show, the
-  // screen is locked, a modal dialog is open or is running in kiosk app
-  // session.
-  bool performed;
   if (controller->InOverviewSession())
-    performed = controller->EndOverview();
+    controller->EndOverview();
   else
-    performed = controller->StartOverview();
+    controller->StartOverview();
   Shell::Get()->metrics()->RecordUserMetricsAction(UMA_TRAY_OVERVIEW);
-  return performed;
+
+  // The return value doesn't matter here. OnOverviewModeStarting() and
+  // OnOverviewModeEnded() will do the right thing to set the button state.
+  return true;
 }
 
+void OverviewButtonTray::HandlePerformActionResult(bool action_performed,
+                                                   const ui::Event& event) {}
+
 void OverviewButtonTray::OnSessionStateChanged(
     session_manager::SessionState state) {
   UpdateIconVisibility();

ash/system/overview/overview_button_tray.h

@@ -53,6 +53,8 @@ class ASH_EXPORT OverviewButtonTray : public TrayBackgroundView,
 
   // ActionableView:
   bool PerformAction(const ui::Event& event) override;
+  void HandlePerformActionResult(bool action_performed,
+                                 const ui::Event& event) override;
 
   // SessionObserver:
   void OnSessionStateChanged(session_manager::SessionState state) override;

ash/wm/overview/overview_controller.cc

@@ -285,18 +285,24 @@ bool OverviewController::StartOverview(
   if (InOverviewSession())
     return true;
 
-  return ToggleOverview(type);
+  if (!CanEnterOverview())
+    return false;
+
+  ToggleOverview(type);
+  return true;
 }
 
-// TODO(flackr): Make OverviewController observe the activation of
-// windows, so we can remove OverviewDelegate.
 bool OverviewController::EndOverview(
     OverviewSession::EnterExitOverviewType type) {
   // No need to end overview if overview is already ended.
   if (!InOverviewSession())
     return true;
 
-  return ToggleOverview(type);
+  if (!CanEndOverview(type))
+    return false;
+
+  ToggleOverview(type);
+  return true;
 }
 
 bool OverviewController::InOverviewSession() const {
@@ -530,17 +536,13 @@ OverviewController::GetItemWindowListInOverviewGridsForTest() {
   return windows;
 }
 
-bool OverviewController::ToggleOverview(
+void OverviewController::ToggleOverview(
     OverviewSession::EnterExitOverviewType type) {
   // Hide the virtual keyboard as it obstructs the overview mode.
   // Don't need to hide if it's the a11y keyboard, as overview mode
   // can accept text input and it resizes correctly with the a11y keyboard.
   keyboard::KeyboardUIController::Get()->HideKeyboardImplicitlyByUser();
 
-  // Prevent toggling overview during the split view divider snap animation.
-  if (Shell::Get()->split_view_controller()->IsDividerAnimating())
-    return true;
-
   auto windows =
       Shell::Get()->mru_window_tracker()->BuildMruWindowList(kActiveDesk);
 
@@ -558,18 +560,7 @@ bool OverviewController::ToggleOverview(
   window_util::RemoveTransientDescendants(&windows);
 
   if (InOverviewSession()) {
-    // Do not allow ending overview if we're in single split mode unless swiping
-    // up from the shelf in tablet mode, or ending overview immediately without
-    // animations.
-    if (windows.empty() &&
-        Shell::Get()->split_view_controller()->InTabletSplitViewMode() &&
-        Shell::Get()->split_view_controller()->state() !=
-            SplitViewState::kBothSnapped &&
-        type != OverviewSession::EnterExitOverviewType::kSwipeFromShelf &&
-        type != OverviewSession::EnterExitOverviewType::kImmediateExit) {
-      return true;
-    }
-
+    DCHECK(CanEndOverview(type));
     TRACE_EVENT_ASYNC_BEGIN0("ui", "OverviewController::ExitOverview", this);
 
     // Suspend occlusion tracker until the exit animation is complete.
@@ -630,10 +621,7 @@ bool OverviewController::ToggleOverview(
     if (delayed_animations_.empty())
       OnEndingAnimationComplete(/*canceled=*/false);
   } else {
-    // Don't start overview if it is not allowed.
-    if (!CanEnterOverview())
-      return false;
-
+    DCHECK(CanEnterOverview());
     TRACE_EVENT_ASYNC_BEGIN0("ui", "OverviewController::EnterOverview", this);
 
     // Clear any animations that may be running from last overview end.
@@ -694,7 +682,6 @@ bool OverviewController::ToggleOverview(
                                base::Time::Now() - last_overview_session_time_);
     }
   }
-  return true;
 }
 
 // static
@@ -703,6 +690,10 @@ void OverviewController::SetDoNotChangeWallpaperForTests() {
 }
 
 bool OverviewController::CanEnterOverview() {
+  // Prevent toggling overview during the split view divider snap animation.
+  if (Shell::Get()->split_view_controller()->IsDividerAnimating())
+    return false;
+
   // Don't allow a window overview if the user session is not active (e.g.
   // locked or in user-adding screen) or a modal dialog is open or running in
   // kiosk app session.
@@ -715,6 +706,28 @@ bool OverviewController::CanEnterOverview() {
          !session_controller->IsRunningInAppMode();
 }
 
+bool OverviewController::CanEndOverview(
+    OverviewSession::EnterExitOverviewType type) {
+  SplitViewController* split_view_controller =
+      Shell::Get()->split_view_controller();
+  // Prevent toggling overview during the split view divider snap animation.
+  if (split_view_controller->IsDividerAnimating())
+    return false;
+
+  // Do not allow ending overview if we're in single split mode unless swiping
+  // up from the shelf in tablet mode, or ending overview immediately without
+  // animations.
+  if (split_view_controller->InTabletSplitViewMode() &&
+      split_view_controller->state() != SplitViewState::kBothSnapped &&
+      InOverviewSession() && overview_session_->IsEmpty() &&
+      type != OverviewSession::EnterExitOverviewType::kSwipeFromShelf &&
+      type != OverviewSession::EnterExitOverviewType::kImmediateExit) {
+    return false;
+  }
+
+  return true;
+}
+
 void OverviewController::OnStartingAnimationComplete(bool canceled) {
   if (IsWallpaperChangeAllowed() && !canceled)
     overview_wallpaper_controller_->Blur(/*animate_only=*/true);

ash/wm/overview/overview_controller.h

@@ -29,9 +29,9 @@ class ASH_EXPORT OverviewController : public OverviewDelegate,
   OverviewController();
   ~OverviewController() override;
 
-  // Starts/Ends overview with |type|. Returns true if successful (showing
-  // overview would be unsuccessful if there are no windows to show). Depending
-  // on |type| the enter/exit animation will look different.
+  // Starts/Ends overview with |type|. Returns true if enter or exit overview
+  // successful. Depending on |type| the enter/exit animation will look
+  // different.
   bool StartOverview(OverviewSession::EnterExitOverviewType type =
                          OverviewSession::EnterExitOverviewType::kNormal);
   bool EndOverview(OverviewSession::EnterExitOverviewType type =
@@ -115,18 +115,19 @@ class ASH_EXPORT OverviewController : public OverviewDelegate,
   FRIEND_TEST_ALL_PREFIXES(TabletModeControllerTest,
                            DisplayDisconnectionDuringOverview);
 
-  // Attempts to toggle overview mode and returns true if successful (showing
-  // overview would be unsuccessful if there are no windows to show). Depending
-  // on |type| the enter/exit animation will look different.
-  bool ToggleOverview(OverviewSession::EnterExitOverviewType type =
+  // Toggle overview mode. Depending on |type| the enter/exit animation will
+  // look different.
+  void ToggleOverview(OverviewSession::EnterExitOverviewType type =
                           OverviewSession::EnterExitOverviewType::kNormal);
 
   // There is no need to blur or dim the wallpaper for tests.
   static void SetDoNotChangeWallpaperForTests();
 
-  // Returns true if selecting windows in an overview is enabled. This is false
-  // at certain times, such as when the lock screen is visible.
+  // Returns true if it's possible to enter or exit overview mode in the current
+  // configuration. This can be false at certain times, such as when the lock
+  // screen is visible we can't overview mode.
   bool CanEnterOverview();
+  bool CanEndOverview(OverviewSession::EnterExitOverviewType type);
 
   void OnStartingAnimationComplete(bool canceled);
   void OnEndingAnimationComplete(bool canceled);