Adding forwards / backwards cycling hotkey through logged in users

This does not add the keys to the keyboard overlay page. I am investigating that in parallel (note that these keys are only in an experiment behind (several) flags and as such this might not even be required for now).

BUG=321629
TEST=unittest (additionally tested visually)

Review URL: https://codereview.chromium.org/104403003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@238774 0039d316-1c4b-4281-b951-d872f2087c98

14 files changed, 156 insertions, 22 deletions

diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc
index ac0096f..3c3e831 100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -624,9 +624,7 @@ bool HandleLock(ui::KeyboardCode key_code) {
   return true;
 }
 
-bool HandleSwitchToNextUser() {
-  content::RecordAction(UserMetricsAction("Accel_Switch_To_Next_User"));
-
+bool HandleCycleUser(SessionStateDelegate::CycleUser cycle_user) {
   if (!Shell::GetInstance()->delegate()->IsMultiProfilesEnabled())
     return false;
   ash::SessionStateDelegate* delegate =
@@ -635,7 +633,15 @@ bool HandleSwitchToNextUser() {
     return false;
   MultiProfileUMA::RecordSwitchActiveUser(
       MultiProfileUMA::SWITCH_ACTIVE_USER_BY_ACCELERATOR);
-  delegate->SwitchActiveUserToNext();
+  switch (cycle_user) {
+    case SessionStateDelegate::CYCLE_TO_NEXT_USER:
+      content::RecordAction(UserMetricsAction("Accel_Switch_To_Next_User"));
+      break;
+    case SessionStateDelegate::CYCLE_TO_PREVIOUS_USER:
+      content::RecordAction(UserMetricsAction("Accel_Switch_To_Previous_User"));
+      break;
+  }
+  delegate->CycleActiveUser(cycle_user);
   return true;
 }
 
@@ -941,7 +947,9 @@ bool AcceleratorController::PerformAction(int action,
     case SWAP_PRIMARY_DISPLAY:
       return HandleSwapPrimaryDisplay();
     case SWITCH_TO_NEXT_USER:
-      return HandleSwitchToNextUser();
+      return HandleCycleUser(SessionStateDelegate::CYCLE_TO_NEXT_USER);
+    case SWITCH_TO_PREVIOUS_USER:
+      return HandleCycleUser(SessionStateDelegate::CYCLE_TO_PREVIOUS_USER);
     case TOGGLE_SPOKEN_FEEDBACK:
       return HandleToggleSpokenFeedback();
     case TOGGLE_WIFI:
diff --git a/ash/accelerators/accelerator_table.cc b/ash/accelerators/accelerator_table.cc
index 4ffc1ab..cd3a580 100644
--- a/ash/accelerators/accelerator_table.cc
+++ b/ash/accelerators/accelerator_table.cc
@@ -81,7 +81,9 @@ const AcceleratorData kAcceleratorData[] = {
   { true, ui::VKEY_Z, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
     TOGGLE_SPOKEN_FEEDBACK },
   { true, ui::VKEY_CONTROL, ui::EF_CONTROL_DOWN, SILENCE_SPOKEN_FEEDBACK},
-  { true, ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+  { true, ui::VKEY_OEM_COMMA, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+    SWITCH_TO_PREVIOUS_USER },
+  { true, ui::VKEY_OEM_PERIOD, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
     SWITCH_TO_NEXT_USER },
 #endif  // defined(OS_CHROMEOS)
   { true, ui::VKEY_I, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, OPEN_FEEDBACK_PAGE },
diff --git a/ash/accelerators/accelerator_table.h b/ash/accelerators/accelerator_table.h
index 21bbf6a..d120960 100644
--- a/ash/accelerators/accelerator_table.h
+++ b/ash/accelerators/accelerator_table.h
@@ -135,6 +135,7 @@ enum AcceleratorAction {
   OPEN_CROSH,
   OPEN_FILE_MANAGER,
   SWITCH_TO_NEXT_USER,
+  SWITCH_TO_PREVIOUS_USER,
 #endif
 };
 
diff --git a/ash/session_state_delegate.h b/ash/session_state_delegate.h
index 47570ff..91c6229 100644
--- a/ash/session_state_delegate.h
+++ b/ash/session_state_delegate.h
@@ -33,6 +33,12 @@ typedef std::vector<std::string> UserIdList;
 // Delegate for checking and modifying the session state.
 class ASH_EXPORT SessionStateDelegate {
  public:
+  // Defines the cycle direction for |CycleActiveUser|.
+  enum CycleUser {
+    CYCLE_TO_NEXT_USER = 0,  // Cycle to the next user.
+    CYCLE_TO_PREVIOUS_USER,  // Cycle to the previous user.
+  };
+
   virtual ~SessionStateDelegate() {};
 
   // Returns the maximum possible number of logged in users.
@@ -96,9 +102,9 @@ class ASH_EXPORT SessionStateDelegate {
   // (if that user has already signed in).
   virtual void SwitchActiveUser(const std::string& user_id) = 0;
 
-  // Switches the active user to the next user, with the same ordering as
-  // GetLoggedInUsers.
-  virtual void SwitchActiveUserToNext() = 0;
+  // Switches the active user to the next or previous user, with the same
+  // ordering as GetLoggedInUsers.
+  virtual void CycleActiveUser(CycleUser cycle_user) = 0;
 
   // Adds or removes sessions state observer.
   virtual void AddSessionStateObserver(SessionStateObserver* observer) = 0;
diff --git a/ash/session_state_delegate_stub.cc b/ash/session_state_delegate_stub.cc
index 6daa593..cea4132 100644
--- a/ash/session_state_delegate_stub.cc
+++ b/ash/session_state_delegate_stub.cc
@@ -82,7 +82,7 @@ void SessionStateDelegateStub::GetLoggedInUsers(UserIdList* users) {
 void SessionStateDelegateStub::SwitchActiveUser(const std::string& user_id) {
 }
 
-void SessionStateDelegateStub::SwitchActiveUserToNext() {
+void SessionStateDelegateStub::CycleActiveUser(CycleUser cycle_user) {
 }
 
 void SessionStateDelegateStub::AddSessionStateObserver(
diff --git a/ash/session_state_delegate_stub.h b/ash/session_state_delegate_stub.h
index 6fe1eb9..f496e6b 100644
--- a/ash/session_state_delegate_stub.h
+++ b/ash/session_state_delegate_stub.h
@@ -38,7 +38,7 @@ class SessionStateDelegateStub : public SessionStateDelegate {
       ash::MultiProfileIndex index) const OVERRIDE;
   virtual void GetLoggedInUsers(UserIdList* users) OVERRIDE;
   virtual void SwitchActiveUser(const std::string& user_id) OVERRIDE;
-  virtual void SwitchActiveUserToNext() OVERRIDE;
+  virtual void CycleActiveUser(CycleUser cycle_user) OVERRIDE;
   virtual void AddSessionStateObserver(
       ash::SessionStateObserver* observer) OVERRIDE;
   virtual void RemoveSessionStateObserver(
diff --git a/ash/test/test_session_state_delegate.cc b/ash/test/test_session_state_delegate.cc
index 8032f4f..5c16807 100644
--- a/ash/test/test_session_state_delegate.cc
+++ b/ash/test/test_session_state_delegate.cc
@@ -147,7 +147,7 @@ void TestSessionStateDelegate::SwitchActiveUser(const std::string& user_id) {
   activated_user_ = user_id;
 }
 
-void TestSessionStateDelegate::SwitchActiveUserToNext() {
+void TestSessionStateDelegate::CycleActiveUser(CycleUser cycle_user) {
   activated_user_ = "someone@tray";
 }
 
diff --git a/ash/test/test_session_state_delegate.h b/ash/test/test_session_state_delegate.h
index fcd375d..8c26c88 100644
--- a/ash/test/test_session_state_delegate.h
+++ b/ash/test/test_session_state_delegate.h
@@ -41,7 +41,7 @@ class TestSessionStateDelegate : public SessionStateDelegate {
       ash::MultiProfileIndex index) const OVERRIDE;
   virtual void GetLoggedInUsers(UserIdList* users) OVERRIDE;
   virtual void SwitchActiveUser(const std::string& user_id) OVERRIDE;
-  virtual void SwitchActiveUserToNext() OVERRIDE;
+  virtual void CycleActiveUser(CycleUser cycle_user) OVERRIDE;
   virtual void AddSessionStateObserver(
       ash::SessionStateObserver* observer) OVERRIDE;
   virtual void RemoveSessionStateObserver(
diff --git a/chrome/browser/ui/ash/session_state_delegate_chromeos.cc b/chrome/browser/ui/ash/session_state_delegate_chromeos.cc
index d018c11..b361755 100644
--- a/chrome/browser/ui/ash/session_state_delegate_chromeos.cc
+++ b/chrome/browser/ui/ash/session_state_delegate_chromeos.cc
@@ -142,7 +142,7 @@ void SessionStateDelegateChromeos::SwitchActiveUser(
   chromeos::UserManager::Get()->SwitchActiveUser(user_id);
 }
 
-void SessionStateDelegateChromeos::SwitchActiveUserToNext() {
+void SessionStateDelegateChromeos::CycleActiveUser(CycleUser cycle_user) {
   // Make sure there is a user to switch to.
   if (NumberOfLoggedInUsers() <= 1)
     return;
@@ -164,11 +164,21 @@ void SessionStateDelegateChromeos::SwitchActiveUserToNext() {
   if (it == logged_in_users.end())
     return;
 
-  // Get the next user's email, wrapping to the start of the list if necessary.
-  if (++it == logged_in_users.end())
-    user_id = (*logged_in_users.begin())->email();
-  else
-    user_id = (*it)->email();
+  // Get the user's email to select, wrapping to the start/end of the list if
+  // necessary.
+  switch (cycle_user) {
+    case CYCLE_TO_NEXT_USER:
+      if (++it == logged_in_users.end())
+        user_id = (*logged_in_users.begin())->email();
+      else
+        user_id = (*it)->email();
+      break;
+    case CYCLE_TO_PREVIOUS_USER:
+      if (it == logged_in_users.begin())
+        it = logged_in_users.end();
+      user_id = (*(--it))->email();
+      break;
+  }
 
   // Switch using the transformed |user_id|.
   chromeos::UserManager::Get()->SwitchActiveUser(user_id);
diff --git a/chrome/browser/ui/ash/session_state_delegate_chromeos.h b/chrome/browser/ui/ash/session_state_delegate_chromeos.h
index 45a6fe6..4bd3a56 100644
--- a/chrome/browser/ui/ash/session_state_delegate_chromeos.h
+++ b/chrome/browser/ui/ash/session_state_delegate_chromeos.h
@@ -42,7 +42,7 @@ class SessionStateDelegateChromeos
       ash::MultiProfileIndex index) const OVERRIDE;
   virtual void GetLoggedInUsers(ash::UserIdList* users) OVERRIDE;
   virtual void SwitchActiveUser(const std::string& user_id) OVERRIDE;
-  virtual void SwitchActiveUserToNext() OVERRIDE;
+  virtual void CycleActiveUser(CycleUser cycle_user) OVERRIDE;
   virtual void AddSessionStateObserver(
       ash::SessionStateObserver* observer) OVERRIDE;
   virtual void RemoveSessionStateObserver(
diff --git a/chrome/browser/ui/ash/session_state_delegate_chromeos_unittest.cc b/chrome/browser/ui/ash/session_state_delegate_chromeos_unittest.cc
new file mode 100644
index 0000000..58faba0
--- /dev/null
+++ b/chrome/browser/ui/ash/session_state_delegate_chromeos_unittest.cc
@@ -0,0 +1,106 @@
+// Copyright 2013 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/session_state_delegate_chromeos.h"
+
+#include <string>
+#include <vector>
+
+#include "chrome/browser/chromeos/login/fake_user_manager.h"
+#include "chrome/test/base/testing_profile_manager.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+
+class SessionStateDelegateChromeOSTest : public testing::Test {
+ protected:
+  SessionStateDelegateChromeOSTest() : user_manager_(NULL) {
+  }
+
+  virtual ~SessionStateDelegateChromeOSTest() {
+  }
+
+  virtual void SetUp() OVERRIDE {
+    // Initialize the UserManager singleton to a fresh FakeUserManager instance.
+    user_manager_ = new chromeos::FakeUserManager;
+    user_manager_enabler_.reset(
+        new chromeos::ScopedUserManagerEnabler(user_manager_));
+
+    // Create our SessionStateDelegate to experiment with.
+    session_state_delegate_.reset(new SessionStateDelegateChromeos());
+    testing::Test::SetUp();
+  }
+
+  virtual void TearDown() OVERRIDE {
+    testing::Test::TearDown();
+    session_state_delegate_.reset();
+    user_manager_enabler_.reset();
+    user_manager_ = NULL;
+  }
+
+  // Add and log in a user to the session.
+  void UserAddedToSession(std::string user) {
+    user_manager()->AddUser(user);
+    user_manager()->LoginUser(user);
+  }
+
+  // Get the active user.
+  const std::string& GetActiveUser() {
+    return chromeos::UserManager::Get()->GetActiveUser()->email();
+  }
+
+  chromeos::FakeUserManager* user_manager() { return user_manager_; }
+  SessionStateDelegateChromeos* session_state_delegate() {
+    return session_state_delegate_.get();
+  }
+
+ private:
+  scoped_ptr<chromeos::ScopedUserManagerEnabler> user_manager_enabler_;
+  scoped_ptr<SessionStateDelegateChromeos> session_state_delegate_;
+
+  // Not owned.
+  chromeos::FakeUserManager* user_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(SessionStateDelegateChromeOSTest);
+};
+
+// Make sure that cycling one user does not cause any harm.
+TEST_F(SessionStateDelegateChromeOSTest, CyclingOneUser) {
+  UserAddedToSession("firstuser@test.com");
+
+  EXPECT_EQ("firstuser@test.com", GetActiveUser());
+  session_state_delegate()->CycleActiveUser(
+      ash::SessionStateDelegate::CYCLE_TO_NEXT_USER);
+  EXPECT_EQ("firstuser@test.com", GetActiveUser());
+  session_state_delegate()->CycleActiveUser(
+      ash::SessionStateDelegate::CYCLE_TO_PREVIOUS_USER);
+  EXPECT_EQ("firstuser@test.com", GetActiveUser());
+}
+
+// Cycle three users forwards and backwards to see that it works.
+TEST_F(SessionStateDelegateChromeOSTest, CyclingThreeUsers) {
+  UserAddedToSession("firstuser@test.com");
+  UserAddedToSession("seconduser@test.com");
+  UserAddedToSession("thirduser@test.com");
+  const ash::SessionStateDelegate::CycleUser forward =
+      ash::SessionStateDelegate::CYCLE_TO_NEXT_USER;
+
+  // Cycle forward.
+  EXPECT_EQ("firstuser@test.com", GetActiveUser());
+  session_state_delegate()->CycleActiveUser(forward);
+  EXPECT_EQ("seconduser@test.com", GetActiveUser());
+  session_state_delegate()->CycleActiveUser(forward);
+  EXPECT_EQ("thirduser@test.com", GetActiveUser());
+  session_state_delegate()->CycleActiveUser(forward);
+  EXPECT_EQ("firstuser@test.com", GetActiveUser());
+
+  // Cycle backwards.
+  const ash::SessionStateDelegate::CycleUser backward =
+      ash::SessionStateDelegate::CYCLE_TO_PREVIOUS_USER;
+  session_state_delegate()->CycleActiveUser(backward);
+  EXPECT_EQ("thirduser@test.com", GetActiveUser());
+  session_state_delegate()->CycleActiveUser(backward);
+  EXPECT_EQ("seconduser@test.com", GetActiveUser());
+  session_state_delegate()->CycleActiveUser(backward);
+  EXPECT_EQ("firstuser@test.com", GetActiveUser());
+}
diff --git a/chrome/browser/ui/ash/session_state_delegate_views.cc b/chrome/browser/ui/ash/session_state_delegate_views.cc
index 4c86889..f6704f0 100644
--- a/chrome/browser/ui/ash/session_state_delegate_views.cc
+++ b/chrome/browser/ui/ash/session_state_delegate_views.cc
@@ -87,7 +87,7 @@ void SessionStateDelegate::SwitchActiveUser(const std::string& user_id) {
   NOTIMPLEMENTED();
 }
 
-void SessionStateDelegate::SwitchActiveUserToNext() {
+void SessionStateDelegate::CycleActiveUser(CycleUser cycle_user) {
   NOTIMPLEMENTED();
 }
 
diff --git a/chrome/browser/ui/ash/session_state_delegate_views.h b/chrome/browser/ui/ash/session_state_delegate_views.h
index aa055ea..fc805ba 100644
--- a/chrome/browser/ui/ash/session_state_delegate_views.h
+++ b/chrome/browser/ui/ash/session_state_delegate_views.h
@@ -39,7 +39,7 @@ class SessionStateDelegate : public ash::SessionStateDelegate {
       ash::MultiProfileIndex index) const OVERRIDE;
   virtual void GetLoggedInUsers(ash::UserIdList* users) OVERRIDE;
   virtual void SwitchActiveUser(const std::string& user_id) OVERRIDE;
-  virtual void SwitchActiveUserToNext() OVERRIDE;
+  virtual void CycleActiveUser(CycleUser cycle_user) OVERRIDE;
   virtual void AddSessionStateObserver(
       ash::SessionStateObserver* observer) OVERRIDE;
   virtual void RemoveSessionStateObserver(
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index a653c8d..402551c 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -1443,6 +1443,7 @@
         'browser/ui/ash/multi_user/multi_user_context_menu_chromeos_unittest.cc',
         'browser/ui/ash/multi_user/multi_user_window_manager_chromeos_unittest.cc',
         'browser/ui/ash/screenshot_taker_unittest.cc',
+        'browser/ui/ash/session_state_delegate_chromeos_unittest.cc',
         'browser/ui/ash/window_positioner_unittest.cc',
         'browser/ui/autofill/account_chooser_model_unittest.cc',
         'browser/ui/autofill/autofill_dialog_controller_unittest.cc',