Refactor: Separate the tray user from the tray user separator

BUG=306227
TEST=unittests

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

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

7 files changed, 142 insertions, 52 deletions

diff --git a/ash/ash.gyp b/ash/ash.gyp
index ed323ed..b1022ae 100644
--- a/ash/ash.gyp
+++ b/ash/ash.gyp
@@ -393,6 +393,8 @@
         'system/user/login_status.h',
         'system/user/tray_user.cc',
         'system/user/tray_user.h',
+        'system/user/tray_user_separator.cc',
+        'system/user/tray_user_separator.h',
         'system/user/update_observer.h',
         'system/user/user_observer.h',
         'system/web_notification/web_notification_tray.cc',
diff --git a/ash/system/tray/system_tray.cc b/ash/system/tray/system_tray.cc
index 65d9ffe..387c2da 100644
--- a/ash/system/tray/system_tray.cc
+++ b/ash/system/tray/system_tray.cc
@@ -26,6 +26,7 @@
 #include "ash/system/tray_update.h"
 #include "ash/system/user/login_status.h"
 #include "ash/system/user/tray_user.h"
+#include "ash/system/user/tray_user_separator.h"
 #include "ash/system/web_notification/web_notification_tray.h"
 #include "base/command_line.h"
 #include "base/logging.h"
@@ -164,14 +165,17 @@ void SystemTray::CreateItems(SystemTrayDelegate* delegate) {
   int maximum_user_profiles =
       shell->delegate()->IsMultiProfilesEnabled() ?
           shell->session_state_delegate()->GetMaximumNumberOfLoggedInUsers() :
-          0;
-  // Note: We purposely use one more item then logged in users to account for
-  // the additional separator.
-  for (int i = 0; i <= maximum_user_profiles; i++) {
+          1;
+  for (int i = 0; i < maximum_user_profiles; i++) {
     internal::TrayUser* tray_user = new internal::TrayUser(this, i);
     AddTrayItem(tray_user);
     user_items_.push_back(tray_user);
   }
+  if (maximum_user_profiles > 1) {
+    // Add a special double line separator between users and the rest of the
+    // menu if more then one user is logged in.
+    AddTrayItem(new internal::TrayUserSeparator(this));
+  }
 #endif
 
   tray_accessibility_ = new internal::TrayAccessibility(this);
diff --git a/ash/system/user/tray_user.cc b/ash/system/user/tray_user.cc
index 65ac529..fd9d59b 100644
--- a/ash/system/user/tray_user.cc
+++ b/ash/system/user/tray_user.cc
@@ -1128,8 +1128,7 @@ TrayUser::TrayUser(SystemTray* system_tray, MultiProfileIndex index)
       user_(NULL),
       layout_view_(NULL),
       avatar_(NULL),
-      label_(NULL),
-      separator_shown_(false) {
+      label_(NULL) {
   Shell::GetInstance()->system_tray_notifier()->AddUserObserver(this);
 }
 
@@ -1138,8 +1137,6 @@ TrayUser::~TrayUser() {
 }
 
 TrayUser::TestState TrayUser::GetStateForTest() const {
-  if (separator_shown_)
-    return SEPARATOR;
   if (!user_)
     return HIDDEN;
   return user_->GetStateForTest();
@@ -1196,15 +1193,6 @@ views::View* TrayUser::CreateDefaultView(user::LoginStatus status) {
 
   int logged_in_users = session_state_delegate->NumberOfLoggedInUsers();
 
-  // If there are multiple users logged in, the users will be separated from the
-  // rest of the menu by a separator.
-  if (multiprofile_index_ ==
-          session_state_delegate->GetMaximumNumberOfLoggedInUsers() &&
-      logged_in_users > 1) {
-    separator_shown_ = true;
-    return new views::View();
-  }
-
   // Do not show more UserView's then there are logged in users.
   if (multiprofile_index_ >= logged_in_users)
     return NULL;
@@ -1221,7 +1209,6 @@ void TrayUser::DestroyTrayView() {
   layout_view_ = NULL;
   avatar_ = NULL;
   label_ = NULL;
-  separator_shown_ = false;
 }
 
 void TrayUser::DestroyDefaultView() {
@@ -1407,7 +1394,7 @@ MultiProfileIndex TrayUser::GetTrayIndex() {
   // In case of multi profile we need to mirror the indices since the system
   // tray items are in the reverse order then the menu items.
   return shell->session_state_delegate()->GetMaximumNumberOfLoggedInUsers() -
-             multiprofile_index_;
+             1 - multiprofile_index_;
 }
 
 int TrayUser::GetTrayItemRadius() {
diff --git a/ash/system/user/tray_user.h b/ash/system/user/tray_user.h
index 0794e98..079bf75 100644
--- a/ash/system/user/tray_user.h
+++ b/ash/system/user/tray_user.h
@@ -42,7 +42,6 @@ class ASH_EXPORT TrayUser : public SystemTrayItem,
   // Allows unit tests to see if the item was created.
   enum TestState {
     HIDDEN,               // The item is hidden.
-    SEPARATOR,            // the item gets shown as a separator.
     SHOWN,                // The item gets presented to the user.
     HOVERED,              // The item is hovered and presented to the user.
     ACTIVE,               // The item was clicked and can add a user.
@@ -99,9 +98,6 @@ class ASH_EXPORT TrayUser : public SystemTrayItem,
   tray::RoundedImageView* avatar_;
   views::Label* label_;
 
-  // True if this element is the separator and it is shown.
-  bool separator_shown_;
-
   DISALLOW_COPY_AND_ASSIGN(TrayUser);
 };
 
diff --git a/ash/system/user/tray_user_separator.cc b/ash/system/user/tray_user_separator.cc
new file mode 100644
index 0000000..dc63bff
--- /dev/null
+++ b/ash/system/user/tray_user_separator.cc
@@ -0,0 +1,48 @@
+// 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 "ash/system/user/tray_user_separator.h"
+
+#include "ash/session_state_delegate.h"
+#include "ash/shell.h"
+#include "ui/views/view.h"
+
+namespace ash {
+namespace internal {
+
+TrayUserSeparator::TrayUserSeparator(SystemTray* system_tray)
+    : SystemTrayItem(system_tray),
+      separator_shown_(false) {
+}
+
+views::View* TrayUserSeparator::CreateTrayView(user::LoginStatus status) {
+  return NULL;
+}
+
+views::View* TrayUserSeparator::CreateDefaultView(user::LoginStatus status) {
+  if (status == user::LOGGED_IN_NONE)
+    return NULL;
+
+  const SessionStateDelegate* session_state_delegate =
+      Shell::GetInstance()->session_state_delegate();
+
+  // If the screen is locked, or only a single user is shown, show nothing.
+  if (session_state_delegate->IsUserSessionBlocked() ||
+      session_state_delegate->NumberOfLoggedInUsers() < 2)
+    return NULL;
+
+  separator_shown_ = true;
+  return new views::View();
+}
+
+views::View* TrayUserSeparator::CreateDetailedView(user::LoginStatus status) {
+  return NULL;
+}
+
+void TrayUserSeparator::DestroyDefaultView() {
+  separator_shown_ = false;
+}
+
+}  // namespace internal
+}  // namespace ash
diff --git a/ash/system/user/tray_user_separator.h b/ash/system/user/tray_user_separator.h
new file mode 100644
index 0000000..62e5209
--- /dev/null
+++ b/ash/system/user/tray_user_separator.h
@@ -0,0 +1,47 @@
+// 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.
+
+#ifndef ASH_SYSTEM_USER_TRAY_USER_SEPARATOR_H_
+#define ASH_SYSTEM_USER_TRAY_USER_SEPARATOR_H_
+
+#include "ash/ash_export.h"
+#include "ash/system/tray/system_tray_item.h"
+
+namespace ash {
+namespace internal {
+
+// This tray item is showing an additional separator line between the logged in
+// users and the rest of the system tray menu. The separator will only be shown
+// when there are at least two users logged in.
+class ASH_EXPORT TrayUserSeparator : public SystemTrayItem {
+ public:
+  explicit TrayUserSeparator(SystemTray* system_tray);
+  virtual ~TrayUserSeparator() {}
+
+  // Returns true if the separator gets shown.
+  bool separator_shown() { return separator_shown_; }
+
+ private:
+  // Overridden from SystemTrayItem.
+  virtual views::View* CreateTrayView(user::LoginStatus status) OVERRIDE;
+  virtual views::View* CreateDefaultView(user::LoginStatus status) OVERRIDE;
+  virtual views::View* CreateDetailedView(user::LoginStatus status) OVERRIDE;
+  virtual void DestroyTrayView() OVERRIDE {}
+  virtual void DestroyDefaultView() OVERRIDE;
+  virtual void DestroyDetailedView() OVERRIDE {}
+  virtual void UpdateAfterLoginStatusChange(
+      user::LoginStatus status) OVERRIDE {}
+  virtual void UpdateAfterShelfAlignmentChange(
+      ShelfAlignment alignment) OVERRIDE {}
+
+  // True if the separator gets shown.
+  bool separator_shown_;
+
+  DISALLOW_COPY_AND_ASSIGN(TrayUserSeparator);
+};
+
+}  // namespace internal
+}  // namespace ash
+
+#endif  // ASH_SYSTEM_USER_TRAY_USER_SEPARATOR_H_
diff --git a/ash/system/user/tray_user_unittest.cc b/ash/system/user/tray_user_unittest.cc
index f969344..fbcbb784 100644
--- a/ash/system/user/tray_user_unittest.cc
+++ b/ash/system/user/tray_user_unittest.cc
@@ -11,6 +11,7 @@
 #include "ash/shell_delegate.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/user/tray_user.h"
+#include "ash/system/user/tray_user_separator.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/test_session_state_delegate.h"
 #include "ash/test/test_shell_delegate.h"
@@ -51,21 +52,30 @@ class TrayUserTest : public ash::test::AshTestBase {
   SystemTray* tray() { return tray_; }
   ash::test::TestSessionStateDelegate* delegate() { return delegate_; }
   ash::internal::TrayUser* tray_user(int index) { return tray_user_[index]; }
+  ash::internal::TrayUserSeparator* tray_user_separator() {
+    return tray_user_separator_;
+  }
 
  private:
   ShelfLayoutManager* shelf_;
   SystemTray* tray_;
   ash::test::TestSessionStateDelegate* delegate_;
+
   // Note that the ownership of these items is on the shelf.
   std::vector<ash::internal::TrayUser*> tray_user_;
 
+  // The separator between the tray users and the rest of the menu.
+  // Note: The item will get owned by the shelf.
+  TrayUserSeparator* tray_user_separator_;
+
   DISALLOW_COPY_AND_ASSIGN(TrayUserTest);
 };
 
 TrayUserTest::TrayUserTest()
     : shelf_(NULL),
       tray_(NULL),
-      delegate_(NULL) {
+      delegate_(NULL),
+      tray_user_separator_(NULL) {
 }
 
 void TrayUserTest::SetUp() {
@@ -92,12 +102,13 @@ void TrayUserTest::InitializeParameters(int users_logged_in,
 
   // Instead of using the existing tray panels we create new ones which makes
   // the access easier.
-  // Note that we create one more element then there can be users to show a
-  // separator.
-  for (int i = 0; i <= delegate_->GetMaximumNumberOfLoggedInUsers(); i++) {
+  for (int i = 0; i < delegate_->GetMaximumNumberOfLoggedInUsers(); i++) {
     tray_user_.push_back(new ash::internal::TrayUser(tray_, i));
     tray_->AddTrayItem(tray_user_[i]);
   }
+  // We then add also the separator.
+  tray_user_separator_ = new ash::internal::TrayUserSeparator(tray_);
+  tray_->AddTrayItem(tray_user_separator_);
 }
 
 void TrayUserTest::ShowTrayMenu(aura::test::EventGenerator* generator) {
@@ -132,26 +143,27 @@ TEST_F(TrayUserTest, SingleUserModeDoesNotAllowAddingUser) {
 
   EXPECT_FALSE(tray()->IsAnyBubbleVisible());
 
-  for (int i = 0; i <= delegate()->GetMaximumNumberOfLoggedInUsers(); i++)
+  for (int i = 0; i < delegate()->GetMaximumNumberOfLoggedInUsers(); i++)
     EXPECT_EQ(ash::internal::TrayUser::HIDDEN,
               tray_user(i)->GetStateForTest());
+  EXPECT_FALSE(tray_user_separator()->separator_shown());
 
   ShowTrayMenu(&generator);
 
   EXPECT_TRUE(tray()->HasSystemBubble());
   EXPECT_TRUE(tray()->IsAnyBubbleVisible());
 
-  for (int i = 0; i <= delegate()->GetMaximumNumberOfLoggedInUsers(); i++)
+  for (int i = 0; i < delegate()->GetMaximumNumberOfLoggedInUsers(); i++)
     EXPECT_EQ(i == 0 ? ash::internal::TrayUser::SHOWN :
                        ash::internal::TrayUser::HIDDEN,
               tray_user(i)->GetStateForTest());
-
+  EXPECT_FALSE(tray_user_separator()->separator_shown());
   tray()->CloseSystemBubble();
 }
 
 #if defined(OS_CHROMEOS)
 // Make sure that in multi user mode the user panel can be activated and there
-// will be one panel for each user plus a separator.
+// will be one panel for each user plus one additional separator at the end.
 // Note: the mouse watcher (for automatic closing upon leave) cannot be tested
 // here since it does not work with the event system in unit tests.
 TEST_F(TrayUserTest, MutiUserModeDoesNotAllowToAddUser) {
@@ -169,9 +181,9 @@ TEST_F(TrayUserTest, MutiUserModeDoesNotAllowToAddUser) {
 
     // Verify that nothing is shown.
     EXPECT_FALSE(tray()->IsAnyBubbleVisible());
-    for (int i = 0; i <= max_users; i++)
+    for (int i = 0; i < max_users; i++)
       EXPECT_FALSE(tray_user(i)->GetStateForTest());
-
+    EXPECT_FALSE(tray_user_separator()->separator_shown());
     // After clicking on the tray the menu should get shown and for each logged
     // in user we should get a visible item. In addition, the separator should
     // show up when we reach more then one user.
@@ -186,9 +198,7 @@ TEST_F(TrayUserTest, MutiUserModeDoesNotAllowToAddUser) {
     }
 
     // Check the visibility of the separator.
-    EXPECT_EQ(j > 1 ? ash::internal::TrayUser::SEPARATOR :
-                      ash::internal::TrayUser::HIDDEN,
-              tray_user(max_users)->GetStateForTest());
+    EXPECT_EQ(j > 1 ? true : false, tray_user_separator()->separator_shown());
 
     // Move the mouse over the user item and it should hover.
     MoveOverUserItem(&generator, 0);
@@ -245,30 +255,26 @@ TEST_F(TrayUserTest, CheckTrayUserItems) {
   int max_users = delegate()->GetMaximumNumberOfLoggedInUsers();
   // Checking now for each amount of users that the proper items are visible in
   // the tray. The proper item is hereby:
-  // 3 -> User #1
-  // 2 -> User #2
-  // 1 -> User #3
-  // 0 -> Separator (never shown)
+  // 2 -> User #1
+  // 1 -> User #2
+  // 0 -> User #3
   // Note: Tray items are required to populate system tray items as well as the
   // system tray menu. The system tray menu changes it's appearance with the
   // addition of more users, but the system tray does not create new items after
-  // it got created. The logic to know if an item is present, not present or
-  // even "only" a "separator" is inside the tray user items. This test tries
-  // to test as close as possible the "real thing" and as such the break is
-  // included.
-  for (int j = 1; j <= max_users; j++) {
+  // it got created.
+  for (int present_users = 1; present_users <= max_users; ++present_users) {
     // We simulate the user addition by telling the delegate the new number of
     // users, then change all user tray items and finally tell the tray to
     // re-layout itself.
-    delegate()->set_logged_in_users(j);
+    delegate()->set_logged_in_users(present_users);
     Shell::GetInstance()->system_tray_notifier()->NotifyUserAddedToSession();
     tray()->Layout();
 
     // Check that the tray items are being shown in the reverse order.
-    for (int i = 0; i <= max_users; i++) {
-      gfx::Rect rect = tray()->
-          GetTrayItemViewForTest(tray_user(i))->GetBoundsInScreen();
-      if (max_users - i < j)
+    for (int i = 0; i < max_users; i++) {
+      gfx::Rect rect =
+          tray()->GetTrayItemViewForTest(tray_user(i))->GetBoundsInScreen();
+      if (max_users - 1 - i < present_users)
         EXPECT_FALSE(rect.IsEmpty());
       else
         EXPECT_TRUE(rect.IsEmpty());
@@ -280,14 +286,14 @@ TEST_F(TrayUserTest, CheckTrayUserItems) {
   generator.set_async(false);
 
   // Switch to a new user - again, note that we have to click on the reverse
-  // item in the list (1 -> user 3).
+  // item in the list. Since the first clickable item is 1, we get user #2.
   gfx::Point point =
       tray()->GetTrayItemViewForTest(tray_user(1))->
                          GetBoundsInScreen().CenterPoint();
 
   generator.MoveMouseTo(point.x(), point.y());
   generator.ClickLeftButton();
-  EXPECT_EQ(delegate()->get_activated_user(), delegate()->GetUserID(2));
+  EXPECT_EQ(delegate()->get_activated_user(), delegate()->GetUserID(1));
 }
 
 #endif