Update home launcher state waiter for tast tests

The waiter was not checking whether the home launcher visibility matches
the expected state before starting the wait, and was relying on
HomeLauncherAnimationComplete getting called (which only works for
transitions initiated by user actions - e.g. drag gesture or pressing
the home button). This changes the API implementation to work for wider
set of cases - for example to wait for launcher transition to end after
transition to tablet mode. The implementation now:
1.  Verifies that app list state is as expected before checking home
    launcher visibility(e.g. during tablet mode transition the app
    list may animate from closed to fullscreen state)
2.  Home launcher state waiter now returns early if the launcher already
    has the target visibility.
3.  Waits for the visibility state to change - the waiter remains the
    same in the test API, but the state change callback now runs
    whenever the home launcher visibility changes to a final value.

BUG=1147906, 1143980

Change-Id: I3728f12d81241ad84e0db95ea50779bd2598e1f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2530594
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826665}

ash/app_list/app_list_controller_impl.cc

@@ -944,9 +944,6 @@ void AppListControllerImpl::OnHomeLauncherAnimationComplete(
   // Animations can be reversed (e.g. in a drag). Let's ensure the target
   // visibility is correct first.
   OnVisibilityChanged(shown, display_id);
-
-  if (!home_launcher_animation_callback_.is_null())
-    home_launcher_animation_callback_.Run(shown);
 }
 
 void AppListControllerImpl::OnHomeLauncherPositionChanged(int percent_shown,
@@ -1492,6 +1489,9 @@ void AppListControllerImpl::OnVisibilityChanged(bool visible,
     GetAssistantViewDelegate()->OnHostViewVisibilityChanged(real_visibility);
     for (auto& observer : observers_)
       observer.OnAppListVisibilityChanged(real_visibility, display_id);
+
+    if (!home_launcher_animation_callback_.is_null())
+      home_launcher_animation_callback_.Run(real_visibility);
   }
 }
 

ash/autotest_private_api_utils.cc

@@ -11,6 +11,7 @@
 #include "ash/wm/mru_window_tracker.h"
 #include "ash/wm/tablet_mode/scoped_skip_user_session_blocked_check.h"
 #include "base/callback_helpers.h"
+#include "base/optional.h"
 
 namespace ash {
 namespace {
@@ -78,6 +79,17 @@ class LauncherStateWaiter {
   DISALLOW_COPY_AND_ASSIGN(LauncherStateWaiter);
 };
 
+bool WaitForHomeLauncherState(bool target_visible, base::OnceClosure closure) {
+  if (Shell::Get()->app_list_controller()->IsVisible(
+          /*display_id=*/base::nullopt) == target_visible) {
+    std::move(closure).Run();
+    return true;
+  }
+
+  new HomeLauncherStateWaiter(target_visible, std::move(closure));
+  return false;
+}
+
 }  // namespace
 
 std::vector<aura::Window*> GetAppWindowList() {
@@ -87,32 +99,32 @@ std::vector<aura::Window*> GetAppWindowList() {
 
 bool WaitForLauncherState(AppListViewState target_state,
                           base::OnceClosure closure) {
-  // In the tablet mode, some of the app-list state switching is handled
-  // differently. For open and close, HomeLauncherGestureHandler handles the
-  // gestures and animation. HomeLauncherStateWaiter can wait for such
-  // animation. For switching between the search and apps-grid,
-  // LauncherStateWaiter can wait for the animation.
-  bool should_wait_for_home_launcher = false;
-  if (Shell::Get()->tablet_mode_controller()->InTabletMode() &&
-      target_state != AppListViewState::kFullscreenSearch) {
-    // App-list can't enter into kPeeking or kHalf state. Thus |target_state|
-    // should be either kClosed or kFullscreenAllApps.
+  const bool in_tablet_mode =
+      Shell::Get()->tablet_mode_controller()->InTabletMode();
+  if (in_tablet_mode) {
+    // App-list can't enter kPeeking or kHalf state in tablet mode. Thus
+    // |target_state| should be either kClosed, kFullscreenAllApps or
+    // kFullscreenSearch.
     DCHECK(target_state == AppListViewState::kClosed ||
-           target_state == AppListViewState::kFullscreenAllApps);
-    const AppListViewState current_state =
-        Shell::Get()->app_list_controller()->GetAppListViewState();
-    should_wait_for_home_launcher =
-        (target_state == AppListViewState::kClosed) ||
-        (current_state != AppListViewState::kFullscreenSearch);
+           target_state == AppListViewState::kFullscreenAllApps ||
+           target_state == AppListViewState::kFullscreenSearch);
   }
-  if (should_wait_for_home_launcher) {
-    // We don't check if the home launcher is animating to the target visibility
-    // because a) home launcher behavior is deterministic, b) correctly
-    // deteching if the home launcher is animating to visibile/invisible require
-    // some refactoring.
-    bool target_visible = target_state != AppListViewState::kClosed;
-    new HomeLauncherStateWaiter(target_visible, std::move(closure));
-  } else {
+
+  // In the tablet mode, home launcher visibility state needs special handling,
+  // as app list view visibility does not match home launcher visibility. The
+  // app list view is always visible, but the home launcher may be obscured by
+  // app windows. The waiter interprets waits for kClosed state as waits
+  // "home launcher not visible" state - note that the app list view
+  // is actually expected to be in a visible state.
+  AppListViewState effective_target_state =
+      in_tablet_mode && target_state == AppListViewState::kClosed
+          ? AppListViewState::kFullscreenAllApps
+          : target_state;
+
+  base::Optional<bool> target_home_launcher_visibility;
+  if (in_tablet_mode)
+    target_home_launcher_visibility = target_state != AppListViewState::kClosed;
+
   // Don't wait if the launcher is already in the target state and not
   // animating.
   auto* app_list_view =
@@ -121,14 +133,26 @@ bool WaitForLauncherState(AppListViewState target_state,
       app_list_view &&
       app_list_view->GetWidget()->GetLayer()->GetAnimator()->is_animating();
   bool at_target_state =
-        (!app_list_view && target_state == AppListViewState::kClosed) ||
-        (app_list_view && app_list_view->app_list_state() == target_state);
+      (!app_list_view && effective_target_state == AppListViewState::kClosed) ||
+      (app_list_view &&
+       app_list_view->app_list_state() == effective_target_state);
   if (at_target_state && !animating) {
+    // In tablet mode, ensure that the home launcher is in the expected state.
+    if (target_home_launcher_visibility.has_value()) {
+      return WaitForHomeLauncherState(*target_home_launcher_visibility,
+                                      std::move(closure));
+    }
     std::move(closure).Run();
     return true;
   }
-    new LauncherStateWaiter(target_state, std::move(closure));
-  }
+
+  // In tablet mode, ensure that the home launcher is in the expected state.
+  base::OnceClosure callback =
+      target_home_launcher_visibility.has_value()
+          ? base::BindOnce(base::IgnoreResult(&WaitForHomeLauncherState),
+                           *target_home_launcher_visibility, std::move(closure))
+          : std::move(closure);
+  new LauncherStateWaiter(target_state, std::move(callback));
   return false;
 }