Properly set ShelfAlignment during login

ShelfLayoutManager has three methods from which the ShelfAlignment can be
applied. For supressed user sessions the shelf is forced to the bottom, however
this was only addressing locked screens, and adding new users.

When the first login occurs a different state transition takes place, where the
SessionState becomes active while the user session is still supressed.
ShelfLayoutManager was not applying alignment changes made during this time.

Update ShelfLayoutManager to unify the logic for overriding the user set
ShelfAlignment. Update this logic to account for having to apply alignment
during the initial login.

Manual testing was also done for: logging in; locking screen; unlocking screen;
adding a new user; switching users.

TEST=ShelfLayoutManagerTest.SideAlignmentInteractionWithLockScreen, ShelfLayoutManagerTest.SideAlignmentInteractionWithAddUserScreen, ShelfLayoutManagerTest.SideAlignmentInteractionWithLoginScreen
BUG=456199

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

Cr-Commit-Position: refs/heads/master@{#316673}

7 files changed, 80 insertions, 22 deletions

diff --git a/ash/shelf/shelf_layout_manager.cc b/ash/shelf/shelf_layout_manager.cc
index 5846b63..0aa077a 100644
--- a/ash/shelf/shelf_layout_manager.cc
+++ b/ash/shelf/shelf_layout_manager.cc
@@ -246,13 +246,11 @@ bool ShelfLayoutManager::SetAlignment(ShelfAlignment alignment) {
     return false;
 
   alignment_ = alignment;
-  if (Shell::GetInstance()->session_state_delegate()->IsUserSessionBlocked() ||
-      state_.is_adding_user_screen) {
-    // The shelf will itself move to the bottom while locked. If a request is
-    // sent to move while being locked, we postpone the move until the lock
-    // screen goes away.
+  // The shelf will itself move to the bottom while locked or obscured by user
+  // login. If a request is sent to move while being obscured, we postpone the
+  // move until the user session is resumed.
+  if (IsAlignmentLocked())
     return false;
-  }
 
   // This should not be called during the lock screen transitions.
   shelf_->SetAlignment(alignment);
@@ -262,11 +260,8 @@ bool ShelfLayoutManager::SetAlignment(ShelfAlignment alignment) {
 
 ShelfAlignment ShelfLayoutManager::GetAlignment() const {
   // When the screen is locked or a user gets added, the shelf is forced into
-  // bottom alignment. Note: We cannot use state_.is_screen_locked here since
-  // that flag gets set later than the SessionStateDelegate reports a locked
-  // screen which leads in
-  if (Shell::GetInstance()->session_state_delegate()->IsUserSessionBlocked() ||
-      state_.is_adding_user_screen)
+  // bottom alignment.
+  if (IsAlignmentLocked())
     return SHELF_ALIGNMENT_BOTTOM;
   return alignment_;
 }
@@ -1159,10 +1154,28 @@ void ShelfLayoutManager::SessionStateChanged(
 }
 
 void ShelfLayoutManager::UpdateShelfVisibilityAfterLoginUIChange() {
-  shelf_->SetAlignment(state_.is_adding_user_screen || state_.is_screen_locked ?
-                           SHELF_ALIGNMENT_BOTTOM : alignment_);
+  shelf_->SetAlignment(GetAlignment());
   UpdateVisibilityState();
   LayoutShelf();
 }
 
+bool ShelfLayoutManager::IsAlignmentLocked() const {
+  SessionStateDelegate* session_state_delegate =
+      Shell::GetInstance()->session_state_delegate();
+  if (state_.is_screen_locked)
+    return true;
+  // The session state becomes active at the start of transitioning to a user
+  // session, however the session is considered blocked until the full UI is
+  // ready. Exit early to allow for proper layout.
+  if (session_state_delegate->GetSessionState() ==
+      SessionStateDelegate::SESSION_STATE_ACTIVE) {
+    return false;
+  }
+  if (session_state_delegate->IsUserSessionBlocked() ||
+      state_.is_adding_user_screen) {
+    return true;
+  }
+  return false;
+}
+
 }  // namespace ash
diff --git a/ash/shelf/shelf_layout_manager.h b/ash/shelf/shelf_layout_manager.h
index 7e0118a..6f91547 100644
--- a/ash/shelf/shelf_layout_manager.h
+++ b/ash/shelf/shelf_layout_manager.h
@@ -326,6 +326,11 @@ class ASH_EXPORT ShelfLayoutManager
   // Called when the LoginUI changes from visible to invisible.
   void UpdateShelfVisibilityAfterLoginUIChange();
 
+  // Returns true when |alignment_| is locked. This can be caused by the screen
+  // being locked, or when adding a user. Returns false when transitioning to a
+  // user session, and while the session is active.
+  bool IsAlignmentLocked() const;
+
   // The RootWindow is cached so that we don't invoke Shell::GetInstance() from
   // our destructor. We avoid that as at the time we're deleted Shell is being
   // deleted too.
diff --git a/ash/shelf/shelf_layout_manager_unittest.cc b/ash/shelf/shelf_layout_manager_unittest.cc
index f4a8cb8..1ad9007 100644
--- a/ash/shelf/shelf_layout_manager_unittest.cc
+++ b/ash/shelf/shelf_layout_manager_unittest.cc
@@ -380,6 +380,7 @@ class ShelfLayoutManagerTest : public ash::test::AshTestBase {
 
   // Open the add user screen if |show| is true, otherwise end it.
   void ShowAddUserScreen(bool show) {
+    SetUserAddingScreenRunning(show);
     ShelfLayoutManager* manager = GetShelfWidget()->shelf_layout_manager();
     manager->SessionStateChanged(
         show ? SessionStateDelegate::SESSION_STATE_LOGIN_SECONDARY :
@@ -779,6 +780,28 @@ TEST_F(ShelfLayoutManagerTest, SideAlignmentInteractionWithAddUserScreen) {
   EXPECT_EQ(SHELF_ALIGNMENT_LEFT, manager->GetAlignment());
 }
 
+// Makes sure shelf alignment is correct for login screen.
+TEST_F(ShelfLayoutManagerTest, SideAlignmentInteractionWithLoginScreen) {
+  ShelfLayoutManager* manager = GetShelfWidget()->shelf_layout_manager();
+  ASSERT_EQ(SHELF_ALIGNMENT_BOTTOM, manager->GetAlignment());
+  SetUserLoggedIn(false);
+  SetSessionStarted(false);
+
+  // The test session state delegate does not fire state changes.
+  SetSessionStarting();
+  manager->SessionStateChanged(
+      Shell::GetInstance()->session_state_delegate()->GetSessionState());
+
+  // Login sets alignment preferences before the session completes startup.
+  manager->SetAlignment(SHELF_ALIGNMENT_LEFT);
+  SetUserLoggedIn(true);
+  SetSessionStarted(true);
+
+  EXPECT_EQ(SHELF_ALIGNMENT_LEFT, manager->GetAlignment());
+  // Ensure that the shelf has been notified.
+  EXPECT_EQ(SHELF_ALIGNMENT_LEFT, GetShelfWidget()->shelf()->alignment());
+}
+
 // Makes sure LayoutShelf invoked while animating cleans things up.
 TEST_F(ShelfLayoutManagerTest, LayoutShelfWhileAnimating) {
   ShelfWidget* shelf = GetShelfWidget();
diff --git a/ash/test/ash_test_base.cc b/ash/test/ash_test_base.cc
index e367ee7..d202b1c 100644
--- a/ash/test/ash_test_base.cc
+++ b/ash/test/ash_test_base.cc
@@ -292,6 +292,12 @@ void AshTestBase::SetSessionStarted(bool session_started) {
       SetActiveUserSessionStarted(session_started);
 }
 
+void AshTestBase::SetSessionStarting() {
+  ash_test_helper_->test_shell_delegate()
+      ->test_session_state_delegate()
+      ->set_session_state(SessionStateDelegate::SESSION_STATE_ACTIVE);
+}
+
 void AshTestBase::SetUserLoggedIn(bool user_logged_in) {
   ash_test_helper_->test_shell_delegate()->test_session_state_delegate()->
       SetHasActiveUser(user_logged_in);
diff --git a/ash/test/ash_test_base.h b/ash/test/ash_test_base.h
index 875013b..e877eec 100644
--- a/ash/test/ash_test_base.h
+++ b/ash/test/ash_test_base.h
@@ -117,6 +117,7 @@ class AshTestBase : public testing::Test {
   // Utility methods to emulate user logged in or not, session started or not
   // and user able to lock screen or not cases.
   void SetSessionStarted(bool session_started);
+  void SetSessionStarting();
   void SetUserLoggedIn(bool user_logged_in);
   void SetCanLockScreen(bool can_lock_screen);
   void SetShouldLockScreenBeforeSuspending(bool should_lock);
diff --git a/ash/test/test_session_state_delegate.cc b/ash/test/test_session_state_delegate.cc
index ebdad58..0d31d45 100644
--- a/ash/test/test_session_state_delegate.cc
+++ b/ash/test/test_session_state_delegate.cc
@@ -70,7 +70,8 @@ TestSessionStateDelegate::TestSessionStateDelegate()
       screen_locked_(false),
       user_adding_screen_running_(false),
       logged_in_users_(1),
-      active_user_index_(0) {
+      active_user_index_(0),
+      session_state_(SESSION_STATE_LOGIN_PRIMARY) {
   user_list_.push_back(
       new MockUserInfo("First@tray"));  // This is intended to be capitalized.
   user_list_.push_back(
@@ -145,30 +146,30 @@ bool TestSessionStateDelegate::IsUserSessionBlocked() const {
 
 SessionStateDelegate::SessionState TestSessionStateDelegate::GetSessionState()
     const {
-  if (user_adding_screen_running_)
-    return SESSION_STATE_LOGIN_SECONDARY;
-
-  // Assuming that if session is not active we're at login.
-  return IsActiveUserSessionStarted() ?
-      SESSION_STATE_ACTIVE : SESSION_STATE_LOGIN_PRIMARY;
+  return session_state_;
 }
 
 void TestSessionStateDelegate::SetHasActiveUser(bool has_active_user) {
   has_active_user_ = has_active_user;
-  if (!has_active_user)
+  if (!has_active_user) {
     active_user_session_started_ = false;
-  else
+    session_state_ = SESSION_STATE_LOGIN_PRIMARY;
+  } else {
     Shell::GetInstance()->ShowShelf();
+  }
 }
 
 void TestSessionStateDelegate::SetActiveUserSessionStarted(
     bool active_user_session_started) {
   active_user_session_started_ = active_user_session_started;
   if (active_user_session_started) {
+    session_state_ = SESSION_STATE_ACTIVE;
     has_active_user_ = true;
     Shell::GetInstance()->CreateShelf();
     Shell::GetInstance()->UpdateAfterLoginStatusChange(
         user::LOGGED_IN_USER);
+  } else {
+    session_state_ = SESSION_STATE_LOGIN_PRIMARY;
   }
 }
 
@@ -184,6 +185,8 @@ void TestSessionStateDelegate::SetShouldLockScreenBeforeSuspending(
 void TestSessionStateDelegate::SetUserAddingScreenRunning(
     bool user_adding_screen_running) {
   user_adding_screen_running_ = user_adding_screen_running;
+  if (user_adding_screen_running_)
+    session_state_ = SESSION_STATE_LOGIN_SECONDARY;
 }
 
 void TestSessionStateDelegate::SetUserImage(
diff --git a/ash/test/test_session_state_delegate.h b/ash/test/test_session_state_delegate.h
index a77b3cd..b35f864 100644
--- a/ash/test/test_session_state_delegate.h
+++ b/ash/test/test_session_state_delegate.h
@@ -23,6 +23,9 @@ class TestSessionStateDelegate : public SessionStateDelegate {
   ~TestSessionStateDelegate() override;
 
   void set_logged_in_users(int users) { logged_in_users_ = users; }
+  void set_session_state(SessionState session_state) {
+    session_state_ = session_state;
+  }
   void AddUser(const std::string user_id);
   const user_manager::UserInfo* GetActiveUserInfo() const;
 
@@ -111,6 +114,10 @@ class TestSessionStateDelegate : public SessionStateDelegate {
 
   std::vector<MockUserInfo*> user_list_;
 
+  // The current state of the login screen. |session_state_| becomes active
+  // before the profile and browser UI are available.
+  SessionState session_state_;
+
   DISALLOW_COPY_AND_ASSIGN(TestSessionStateDelegate);
 };