Properly set ShelfAlignment during login

ash/shelf/shelf_layout_manager.cc

Index: ash/shelf/shelf_layout_manager.cc
diff --git a/ash/shelf/shelf_layout_manager.cc b/ash/shelf/shelf_layout_manager.cc
index 5846b634f59b5157c20581b9caee2b3cf7a96291..a63d2c5755ea70d0f809f1a5171ab71c46cf059b 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,25 @@ 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;
+  if (session_state_delegate->GetSessionState() ==

[Mr4D (OOO till 08-26), 2015/02/12 22:04:01]

Could you add a comment towards why _ACTIVE needs to be pre-filtered?

It should not be _ACTIVE with "is_Adding_user_screen" being true. And if I
remember correctly, IsUserSessionBlocked should be false in that case.

[jonross, 2015/02/12 22:21:06]

I will update this with comments. Though I don't believe that _ACTIVE &&
is_adding_user_screen is possible. As the state is tied directly to
_LOGIN_SECONDARY. (see line 1144)

We can have _ACTIVE true, and IsUserSessionBlocked() true. This occurs when a
user first logs in, as it considers a user session blocked until ui/browser has
completed its setup.

[jonross, 2015/02/13 15:29:08]

Done.

+      SessionStateDelegate::SESSION_STATE_ACTIVE) {
+    return false;
+  }
+  if (session_state_delegate->IsUserSessionBlocked() ||
+      state_.is_adding_user_screen) {
+    return true;
+  }
+  return false;
+}
+
 }  // namespace ash