Bento: AltTabMode: Save mode in userPrefs and support multi-profile

- Support independent alt-tab mode selection for multi-profile.
- Save alt-tab mode to active user prefs and restore after a re-login
and a crash.

Bug: 1157105
Test: Manual test, `ash_unittests --gtest_filter=MultiUserWindowCycleControllerTest.*`
Change-Id: I59208f3f9726e776a10f0761c8aa431bf11b2080
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2616320Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Cattalyya Nuengsigkapian <cattalyya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843922}

ash/ash_prefs.cc

@@ -38,6 +38,7 @@
 #include "ash/touch/touch_devices_controller.h"
 #include "ash/wallpaper/wallpaper_controller_impl.h"
 #include "ash/wm/desks/desks_restore_util.h"
+#include "ash/wm/window_cycle/window_cycle_controller.h"
 #include "chromeos/components/quick_answers/public/cpp/quick_answers_prefs.h"
 #include "chromeos/constants/chromeos_pref_names.h"
 #include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
@@ -77,6 +78,7 @@ void RegisterProfilePrefs(PrefRegistrySimple* registry, bool for_test) {
   TouchDevicesController::RegisterProfilePrefs(registry, for_test);
   tray::VPNListView::RegisterProfilePrefs(registry);
   MediaTray::RegisterProfilePrefs(registry);
+  WindowCycleController::RegisterProfilePrefs(registry);
 
   // Provide prefs registered in the browser for ash_unittests.
   if (for_test) {

ash/public/cpp/ash_pref_names.cc

@@ -145,6 +145,10 @@ const char kLiveCaptionEnabled[] =
 // regardless of the state of a11y features.
 const char kShouldAlwaysShowAccessibilityMenu[] = "settings.a11y.enable_menu";
 
+// A boolean pref which determines whether alt-tab should show only windows in
+// the current desk or all windows.
+const char kAltTabPerDesk[] = "ash.alttab.per_desk";
+
 // A dictionary storing the number of times and most recent time all contextual
 // tooltips have been shown.
 const char kContextualTooltips[] = "settings.contextual_tooltip.shown_info";

ash/public/cpp/ash_pref_names.h

@@ -57,6 +57,8 @@ ASH_PUBLIC_EXPORT extern const char kAccessibilityDictationEnabled[];
 ASH_PUBLIC_EXPORT extern const char kLiveCaptionEnabled[];
 ASH_PUBLIC_EXPORT extern const char kShouldAlwaysShowAccessibilityMenu[];
 
+ASH_PUBLIC_EXPORT extern const char kAltTabPerDesk[];
+
 ASH_PUBLIC_EXPORT extern const char kContextualTooltips[];
 
 ASH_PUBLIC_EXPORT extern const char kDesksNamesList[];

ash/shell.cc

@@ -557,7 +557,6 @@ Shell::Shell(std::unique_ptr<ShellDelegate> shell_delegate)
       shell_delegate_(std::move(shell_delegate)),
       shutdown_controller_(std::make_unique<ShutdownControllerImpl>()),
       system_tray_notifier_(std::make_unique<SystemTrayNotifier>()),
-      window_cycle_controller_(std::make_unique<WindowCycleController>()),
       native_cursor_manager_(nullptr) {
   // Ash doesn't properly remove pre-target-handlers.
   ui::EventHandler::DisableCheckTargets();
@@ -944,6 +943,7 @@ void Shell::Init(
   peripheral_battery_notifier_ = std::make_unique<PeripheralBatteryNotifier>(
       peripheral_battery_listener_.get());
   power_event_observer_.reset(new PowerEventObserver());
+  window_cycle_controller_ = std::make_unique<WindowCycleController>();
 
   if (features::IsCaptureModeEnabled()) {
     capture_mode_controller_ = std::make_unique<CaptureModeController>(

ash/wm/window_cycle/window_cycle_controller.cc

@@ -9,6 +9,7 @@
 #include "ash/metrics/task_switch_source.h"
 #include "ash/metrics/user_metrics_recorder.h"
 #include "ash/public/cpp/ash_features.h"
+#include "ash/public/cpp/ash_pref_names.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
@@ -24,6 +25,8 @@
 #include "ash/wm/window_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
+#include "components/prefs/pref_change_registrar.h"
+#include "components/prefs/pref_service.h"
 
 namespace ash {
 
@@ -69,9 +72,15 @@ void ReportPossibleDesksSwitchStats(int active_desk_container_id_before_cycle) {
 //////////////////////////////////////////////////////////////////////////////
 // WindowCycleController, public:
 
-WindowCycleController::WindowCycleController() = default;
+WindowCycleController::WindowCycleController() {
+  if (features::IsBentoEnabled())
+    Shell::Get()->session_controller()->AddObserver(this);
+}
 
-WindowCycleController::~WindowCycleController() = default;
+WindowCycleController::~WindowCycleController() {
+  if (features::IsBentoEnabled())
+    Shell::Get()->session_controller()->RemoveObserver(this);
+}
 
 // static
 bool WindowCycleController::CanCycle() {
@@ -82,6 +91,14 @@ bool WindowCycleController::CanCycle() {
          !Shell::Get()->desks_controller()->AreDesksBeingModified();
 }
 
+// static
+void WindowCycleController::RegisterProfilePrefs(PrefRegistrySimple* registry) {
+  if (features::IsBentoEnabled()) {
+    registry->RegisterBooleanPref(prefs::kAltTabPerDesk,
+                                  DesksMruType::kAllDesks);
+  }
+}
+
 void WindowCycleController::HandleCycleWindow(Direction direction) {
   if (!CanCycle())
     return;
@@ -157,6 +174,29 @@ bool WindowCycleController::IsWindowListVisible() {
   return window_cycle_list_ && window_cycle_list_->ShouldShowUi();
 }
 
+bool WindowCycleController::IsInteractiveAltTabModeAllowed() {
+  return features::IsBentoEnabled() &&
+         Shell::Get()->desks_controller()->GetNumberOfDesks() > 1;
+}
+
+bool WindowCycleController::IsAltTabPerActiveDesk() {
+  return IsInteractiveAltTabModeAllowed() && active_user_pref_service_
+             ? active_user_pref_service_->GetBoolean(prefs::kAltTabPerDesk)
+             : features::IsAltTabLimitedToActiveDesk();
+}
+
+bool WindowCycleController::IsSwitchingMode() {
+  return features::IsBentoEnabled() && is_switching_mode_;
+}
+
+void WindowCycleController::OnActiveUserPrefServiceChanged(
+    PrefService* pref_service) {
+  if (!features::IsBentoEnabled())
+    return;
+  active_user_pref_service_ = pref_service;
+  InitFromUserPrefs();
+}
+
 //////////////////////////////////////////////////////////////////////////////
 // WindowCycleController, private:
 
@@ -172,36 +212,6 @@ WindowCycleController::WindowList WindowCycleController::CreateWindowList() {
   return window_list;
 }
 
-void WindowCycleController::SetAltTabMode(DesksMruType alt_tab_mode) {
-  if (!IsInteractiveAltTabModeAllowed() || alt_tab_mode_ == alt_tab_mode)
-    return;
-  alt_tab_mode_ = alt_tab_mode;
-  MaybeResetCycleList();
-
-  is_switching_mode_ = true;
-  // When user first press alt + tab, `HandleCycleForwardMRU` triggers
-  // `HandleCycleWindow(WindowCycleController::FORWARD)` since it considers
-  // the initial tab as forward cycling. Therefore, switching the mode
-  // should imitate the same forward cycling behavior after the cycle is reset.
-  HandleCycleWindow(WindowCycleController::FORWARD);
-  is_switching_mode_ = false;
-}
-
-bool WindowCycleController::IsSwitchingMode() {
-  return features::IsBentoEnabled() && is_switching_mode_;
-}
-
-bool WindowCycleController::IsInteractiveAltTabModeAllowed() {
-  return features::IsBentoEnabled() &&
-         Shell::Get()->desks_controller()->GetNumberOfDesks() > 1;
-}
-
-bool WindowCycleController::IsAltTabPerActiveDesk() {
-  return IsInteractiveAltTabModeAllowed()
-             ? alt_tab_mode_ == kActiveDesk
-             : features::IsAltTabLimitedToActiveDesk();
-}
-
 void WindowCycleController::SaveCurrentActiveDeskAndWindow(
     const WindowCycleController::WindowList& window_list) {
   active_desk_container_id_before_cycle_ =
@@ -241,4 +251,40 @@ void WindowCycleController::StopCycling() {
       true);
 }
 
+void WindowCycleController::InitFromUserPrefs() {
+  DCHECK(active_user_pref_service_);
+  DCHECK(features::IsBentoEnabled());
+
+  pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
+  pref_change_registrar_->Init(active_user_pref_service_);
+  pref_change_registrar_->Add(
+      prefs::kAltTabPerDesk,
+      base::BindRepeating(&WindowCycleController::OnAltTabModePrefChanged,
+                          base::Unretained(this)));
+
+  OnAltTabModePrefChanged();
+}
+
+void WindowCycleController::OnAltTabModePrefChanged() {
+  if (!IsInteractiveAltTabModeAllowed())
+    return;
+
+  is_switching_mode_ = true;
+
+  // Update the window cycle list.
+  MaybeResetCycleList();
+  // Update the highlighted window in the window cycle list.
+  // When user first press alt + tab, `HandleCycleForwardMRU` triggers
+  // `HandleCycleWindow(WindowCycleController::FORWARD)` since it considers
+  // the initial tab as forward cycling. Therefore, switching the mode
+  // should imitate the same forward cycling behavior after the cycle is reset.
+  HandleCycleWindow(WindowCycleController::FORWARD);
+
+  // Update tab slider button UI.
+  if (window_cycle_list_)
+    window_cycle_list_->OnModePrefsChanged();
+
+  is_switching_mode_ = false;
+}
+
 }  // namespace ash

ash/wm/window_cycle/window_cycle_controller.h

@@ -8,10 +8,13 @@
 #include <memory>
 
 #include "ash/ash_export.h"
+#include "ash/public/cpp/session/session_observer.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/wm/mru_window_tracker.h"
 #include "base/macros.h"
 #include "base/time/time.h"
+#include "components/prefs/pref_change_registrar.h"
+#include "components/prefs/pref_registry_simple.h"
 
 namespace aura {
 class Window;
@@ -33,19 +36,27 @@ class WindowCycleList;
 // until the cycling ends.  Thus we maintain the state of the windows
 // at the beginning of the gesture so you can cycle through in a consistent
 // order.
-class ASH_EXPORT WindowCycleController {
+class ASH_EXPORT WindowCycleController : public SessionObserver {
  public:
   using WindowList = std::vector<aura::Window*>;
 
   enum Direction { FORWARD, BACKWARD };
 
   WindowCycleController();
-  virtual ~WindowCycleController();
+  ~WindowCycleController() override;
 
   // Returns true if cycling through windows is enabled. This is false at
   // certain times, such as when the lock screen is visible.
   static bool CanCycle();
 
+  // Registers alt-tab related profile prefs.
+  static void RegisterProfilePrefs(PrefRegistrySimple* registry);
+
+  // Returns the WindowCycleList.
+  const WindowCycleList* window_cycle_list() const {
+    return window_cycle_list_.get();
+  }
+
   // Cycles between windows in the given |direction|. This moves the focus ring
   // to the window in the given |direction| and also scrolls the list.
   void HandleCycleWindow(Direction direction);
@@ -82,22 +93,14 @@ class ASH_EXPORT WindowCycleController {
   // Returns whether or not the window cycle view is visible.
   bool IsWindowListVisible();
 
-  // Returns the WindowCycleList.
-  const WindowCycleList* window_cycle_list() const {
-    return window_cycle_list_.get();
-  }
-
-  // Sets alt-tab mode to all desks (default) or active desk.
-  void SetAltTabMode(DesksMruType alt_tab_mode_);
-
   // Checks if switching between alt-tab mode via the tab slider is allowed.
   // Returns true if Bento flag is enabled and users have multiple desks.
   bool IsInteractiveAltTabModeAllowed();
 
   // Checks if alt-tab should be per active desk. If
   // `IsInteractiveAltTabModeAllowed()`, alt-tab mode depends on users'
-  // |alt_tab_mode_| selection. Otherwise, it'll default to all desk unless
-  // LimitAltTabToActiveDesk flag is explicitly enabled.
+  // |prefs::kAltTabPerDesk| selection. Otherwise, it'll default to all desk
+  // unless LimitAltTabToActiveDesk flag is explicitly enabled.
   bool IsAltTabPerActiveDesk();
 
   // Returns true while switching the alt-tab mode and Bento flag is enabled.
@@ -106,6 +109,9 @@ class ASH_EXPORT WindowCycleController {
   // window correctly.
   bool IsSwitchingMode();
 
+  // SessionObserver:
+  void OnActiveUserPrefServiceChanged(PrefService* pref_service) override;
+
  private:
   // Gets a list of windows from the currently open windows, removing windows
   // with transient roots already in the list. The returned list of windows
@@ -122,6 +128,12 @@ class ASH_EXPORT WindowCycleController {
 
   void StopCycling();
 
+  void InitFromUserPrefs();
+
+  // Triggers alt-tab UI updates when the alt-tab mode is updated in the active
+  // user prefs.
+  void OnAltTabModePrefChanged();
+
   std::unique_ptr<WindowCycleList> window_cycle_list_;
 
   // Tracks the ID of the active desk container before window cycling starts. It
@@ -135,15 +147,16 @@ class ASH_EXPORT WindowCycleController {
   // Non-null while actively cycling.
   std::unique_ptr<WindowCycleEventFilter> event_filter_;
 
-  // Tracks alt-tab mode to display all windows from all desks by default.
-  // An alternative mode is active desk where only windows from the current desk
-  // are shown in alt-tab.
-  // TODO(crbug.com/1157105): Store per-desk mode in primary user pref.
-  DesksMruType alt_tab_mode_ = DesksMruType::kAllDesks;
-
   // Tracks whether alt-tab mode is currently switching or not.
   bool is_switching_mode_ = false;
 
+  // The pref service of the currently active user. Can be null in
+  // ash_unittests.
+  PrefService* active_user_pref_service_ = nullptr;
+
+  // The pref change registrar to observe changes in prefs value.
+  std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
+
   DISALLOW_COPY_AND_ASSIGN(WindowCycleController);
 };
 

ash/wm/window_cycle/window_cycle_list.cc

@@ -580,6 +580,11 @@ class WindowCycleView : public views::WidgetDelegateView,
     return target_window_;
   }
 
+  void OnModePrefsChanged() {
+    if (tab_slider_container_)
+      tab_slider_container_->OnModePrefsChanged();
+  }
+
   // ui::ImplicitAnimationObserver:
   void OnImplicitAnimationsCompleted() override {
     occlusion_tracker_pauser_.reset();
@@ -590,7 +595,7 @@ class WindowCycleView : public views::WidgetDelegateView,
   views::View* mirror_container_ = nullptr;
 
   // Tab slider and no recent items are only used when Bento is enabled.
-  views::View* tab_slider_container_ = nullptr;
+  WindowCycleTabSlider* tab_slider_container_ = nullptr;
   views::Label* no_recent_items_label_ = nullptr;
 
   // The |target_window_| is the window that has the focus ring. When the user
@@ -741,6 +746,11 @@ bool WindowCycleList::ShouldShowUi() {
   return windows_.size() > 1u;
 }
 
+void WindowCycleList::OnModePrefsChanged() {
+  if (cycle_view_)
+    cycle_view_->OnModePrefsChanged();
+}
+
 // static
 void WindowCycleList::DisableInitialDelayForTesting() {
   g_disable_initial_delay = true;

ash/wm/window_cycle/window_cycle_list.h

@@ -65,12 +65,16 @@ class ASH_EXPORT WindowCycleList : public aura::WindowObserver,
   // Returns true if the window list overlay should be shown.
   bool ShouldShowUi();
 
+  // Updates window cycle tab slider when the mode prefs is updated.
+  void OnModePrefsChanged();
+
   void set_user_did_accept(bool user_did_accept) {
     user_did_accept_ = user_did_accept;
   }
 
  private:
   friend class WindowCycleControllerTest;
+  friend class MultiUserWindowCycleControllerTest;
   friend class InteractiveWindowCycleListGestureHandlerTest;
 
   static void DisableInitialDelayForTesting();

ash/wm/window_cycle/window_cycle_tab_slider.cc

@@ -4,12 +4,15 @@
 
 #include "ash/wm/window_cycle/window_cycle_tab_slider.h"
 
+#include "ash/public/cpp/ash_pref_names.h"
+#include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/style/ash_color_provider.h"
 #include "ash/wm/mru_window_tracker.h"
 #include "ash/wm/window_cycle/window_cycle_controller.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/prefs/pref_service.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/canvas.h"
 #include "ui/views/background.h"
@@ -48,21 +51,30 @@ WindowCycleTabSlider::WindowCycleTabSlider()
           AshColorProvider::ControlsLayerType::kControlBackgroundColorInactive),
       tab_slider_round_radius));
 
-  per_desk_mode_ =
-      Shell::Get()->window_cycle_controller()->IsAltTabPerActiveDesk();
-
-  all_desks_tab_slider_button_->SetToggled(!per_desk_mode_);
-  current_desk_tab_slider_button_->SetToggled(per_desk_mode_);
+  OnModePrefsChanged();
 }
 
 void WindowCycleTabSlider::OnModeChanged(bool per_desk) {
-  if (per_desk_mode_ == per_desk)
+  // Save to the active user prefs.
+  auto* prefs = Shell::Get()->session_controller()->GetActivePrefService();
+  if (!prefs) {
+    // Can be null in tests.
+    return;
+  }
+  // Avoid an unnecessary update if any.
+  if (per_desk == prefs->GetBoolean(prefs::kAltTabPerDesk))
     return;
-  per_desk_mode_ = per_desk;
-  all_desks_tab_slider_button_->SetToggled(!per_desk_mode_);
-  current_desk_tab_slider_button_->SetToggled(per_desk_mode_);
-  Shell::Get()->window_cycle_controller()->SetAltTabMode(
-      per_desk_mode_ ? DesksMruType::kActiveDesk : DesksMruType::kAllDesks);
+  prefs->SetBoolean(prefs::kAltTabPerDesk, per_desk);
+  OnModePrefsChanged();
+}
+
+void WindowCycleTabSlider::OnModePrefsChanged() {
+  // Read alt-tab mode from user prefs via |IsAltTabPerActiveDesk|, which
+  // handle multiple cases of different flags enabled and the number of desk.
+  bool per_desk =
+      Shell::Get()->window_cycle_controller()->IsAltTabPerActiveDesk();
+  all_desks_tab_slider_button_->SetToggled(!per_desk);
+  current_desk_tab_slider_button_->SetToggled(per_desk);
 }
 
 BEGIN_METADATA(WindowCycleTabSlider, views::View)

ash/wm/window_cycle/window_cycle_tab_slider.h

@@ -22,12 +22,15 @@ class ASH_EXPORT WindowCycleTabSlider : public views::View {
   WindowCycleTabSlider& operator=(const WindowCycleTabSlider&) = delete;
   ~WindowCycleTabSlider() override = default;
 
+  // Updates user prefs when users switch the button.
   void OnModeChanged(bool per_desk);
 
+  // Updates UI when user prefs change.
+  void OnModePrefsChanged();
+
   // TODO(crbug.com/1157087): Add tab slider animation.
 
  private:
-  bool per_desk_mode_ = false;
 
   WindowCycleTabSliderButton* all_desks_tab_slider_button_;
   WindowCycleTabSliderButton* current_desk_tab_slider_button_;