Re-reland: chromeos: Fix shelf appearing at login screen under mash

The reland adds a fix for a crash in window animation code when restoring
a session that has a minimized window. The animation tries to compute a
shelf item position before the shelf is initialized. The animation is not
visible to the user. Returning an empty rect results in a default target
position, which is fine in this case.

Added guards for access to ShelfView before shelf is initialized.

Original CL description:

Previously chrome --mash would always create the shelf on startup, even at
the login screen. Now it waits until login is complete, like classic ash.

Ash watches for SessionState::ACTIVE via a SessionStateObserver to create the
shelf, rather than NOTIFICATION_LOGIN_USER_PROFILE_PREPARED. For most login
flows this doesn't matter, but for supervised user creation it means that the
shelf is not created until the flow is completed. This is an improvement
because in the old code the shelf would be created too early and had to be
explicitly hidden.

Always create a SessionControllerClient in chrome, even in classic ash. This
allows classic ash to use the mojo pathway via ash::SessionController.

Remove WmShell::ShowShelf(), which was introduced in crrev.com/10693003 to
delay showing the shelf until post-login OOBE (like avatar picture select)
was complete. It does not seem to be needed anymore, either in production or
in tests.

Change AshTestImplMus to simulate a user logging in which is required to
create the shelf on the primary display and also to have a non-zero user
count to create the shelf on additional displays.

Remove some unnecessary OS_CHROMEOS ifdefs in //c/b/ui/ash.

BUG=666021, 679513
TEST=ash_unittests and chrome browser_tests

Committed: https://crrev.com/7f99e933e595824281102ff6737075dccbbc4d5c
Cr-Commit-Position: refs/heads/master@{#439845}

Reverted:

Review-Url: https://codereview.chromium.org/2619943002
Cr-Original-Original-Commit-Position: refs/heads/master@{#442484}
Review-Url: https://codereview.chromium.org/2625733003
Cr-Original-Commit-Position: refs/heads/master@{#443106}
Committed: https://chromium.googlesource.com/chromium/src/+/75298de72cf7c25e137c3c5e63e4d01aa0569c1c
Review-Url: https://codereview.chromium.org/2625733003
Cr-Commit-Position: refs/heads/master@{#443174}

ash/aura/wm_shell_aura.cc

@@ -318,6 +318,15 @@ void WmShellAura::OnAttemptToReactivateWindow(aura::Window* request_active,
   }
 }
 
+void WmShellAura::SessionStateChanged(session_manager::SessionState state) {
+  // Create the shelf if necessary.
+  WmShell::SessionStateChanged(state);
+
+  // Recreate the keyboard after initial login and after multiprofile login.
+  if (state == session_manager::SessionState::ACTIVE)
+    Shell::GetInstance()->CreateKeyboard();
+}
+
 void WmShellAura::OnDisplayConfigurationChanging() {
   for (auto& observer : display_observers_)
     observer.OnDisplayConfigurationChanging();

ash/aura/wm_shell_aura.h

@@ -95,6 +95,9 @@ class ASH_EXPORT WmShellAura : public WmShell,
   void OnAttemptToReactivateWindow(aura::Window* request_active,
                                    aura::Window* actual_active) override;
 
+  // SessionStateObserver:
+  void SessionStateChanged(session_manager::SessionState state) override;
+
   // WindowTreeHostManager::Observer:
   void OnDisplayConfigurationChanging() override;
   void OnDisplayConfigurationChanged() override;

ash/common/shelf/shelf_widget.cc

@@ -331,6 +331,12 @@ void ShelfWidget::PostCreateShelf() {
   // Ensure the newly created |shelf_| gets current values.
   background_animator_.Initialize(this);
 
+  // TODO(jamescook): The IsActiveUserSessionStarted() check may not be needed
+  // because the shelf is only created after the first user session is active.
+  // The ShelfView seems to always be visible after login. At the lock screen
+  // the shelf is hidden because its container is hidden. During auto-hide it is
+  // hidden because ShelfWidget is transparent. Some of the ShelfView visibility
+  // code could be simplified. http://crbug.com/674773
   shelf_view_->SetVisible(
       WmShell::Get()->GetSessionStateDelegate()->IsActiveUserSessionStarted());
   shelf_layout_manager_->LayoutShelf();
@@ -342,11 +348,6 @@ bool ShelfWidget::IsShelfVisible() const {
   return shelf_view_ && shelf_view_->visible();
 }
 
-void ShelfWidget::SetShelfVisibility(bool visible) {
-  if (shelf_view_)
-    shelf_view_->SetVisible(visible);
-}
-
 bool ShelfWidget::IsShowingAppList() const {
   return GetAppListButton() && GetAppListButton()->is_showing_app_list();
 }
@@ -388,6 +389,9 @@ void ShelfWidget::Shutdown() {
 }
 
 void ShelfWidget::UpdateIconPositionForPanel(WmWindow* panel) {
+  if (!shelf_view_)
+    return;
+
   WmWindow* shelf_window = WmLookup::Get()->GetWindowForWidget(this);
   shelf_view_->UpdatePanelIconPosition(
       panel->GetIntProperty(WmWindowProperty::SHELF_ID),
@@ -396,6 +400,11 @@ void ShelfWidget::UpdateIconPositionForPanel(WmWindow* panel) {
 }
 
 gfx::Rect ShelfWidget::GetScreenBoundsOfItemIconForWindow(WmWindow* window) {
+  // Window animations can be triggered during session restore before the shelf
+  // view is created. In that case, return default empty bounds.
+  if (!shelf_view_)
+    return gfx::Rect();
+
   ShelfID id = window->GetIntProperty(WmWindowProperty::SHELF_ID);
   gfx::Rect bounds(shelf_view_->GetIdealBoundsOfItemIcon(id));
   gfx::Point screen_origin;

ash/common/shelf/shelf_widget.h

@@ -29,6 +29,9 @@ class StatusAreaWidget;
 class WmShelf;
 class WmWindow;
 
+// The ShelfWidget manages the shelf view (which contains the shelf icons) and
+// the status area widget. There is one ShelfWidget per display. It is created
+// early during RootWindowController initialization.
 class ASH_EXPORT ShelfWidget : public views::Widget,
                                public views::WidgetObserver,
                                public ShelfBackgroundAnimatorObserver,
@@ -57,11 +60,11 @@ class ASH_EXPORT ShelfWidget : public views::Widget,
   ShelfLayoutManager* shelf_layout_manager() { return shelf_layout_manager_; }
   StatusAreaWidget* status_area_widget() const { return status_area_widget_; }
 
+  // Creates the shelf view and populates it with icons. Called after the user
+  // session is active (and hence the user profile is available).
   ShelfView* CreateShelfView();
   void PostCreateShelf();
 
-  // Set visibility of the shelf.
-  void SetShelfVisibility(bool visible);
   bool IsShelfVisible() const;
 
   bool IsShowingAppList() const;
@@ -115,7 +118,8 @@ class ASH_EXPORT ShelfWidget : public views::Widget,
   // |delegate_view_| is the contents view of this widget and is cleaned up
   // during CloseChildWindows of the associated RootWindowController.
   DelegateView* delegate_view_;
-  // View containing the shelf items. Owned by the views hierarchy.
+  // View containing the shelf items. Owned by the views hierarchy. Null when
+  // at the login screen.
   ShelfView* shelf_view_;
   ShelfBackgroundAnimator background_animator_;
   bool activating_as_fallback_;

ash/common/test/test_session_state_delegate.cc

@@ -161,12 +161,9 @@ session_manager::SessionState TestSessionStateDelegate::GetSessionState()
 }
 
 void TestSessionStateDelegate::SetHasActiveUser(bool has_active_user) {
-  if (!has_active_user) {
-    session_state_ = session_manager::SessionState::LOGIN_PRIMARY;
-  } else {
-    session_state_ = session_manager::SessionState::ACTIVE;
-    WmShell::Get()->ShowShelf();
-  }
+  session_state_ = has_active_user
+                       ? session_manager::SessionState::ACTIVE
+                       : session_manager::SessionState::LOGIN_PRIMARY;
 }
 
 void TestSessionStateDelegate::SetActiveUserSessionStarted(

ash/common/wm_shell.cc

@@ -153,11 +153,6 @@ void WmShell::CreateShelf() {
     root_window->GetRootWindowController()->CreateShelf();
 }
 
-void WmShell::ShowShelf() {
-  for (WmWindow* root_window : GetAllRootWindows())
-    root_window->GetRootWindowController()->ShowShelf();
-}
-
 void WmShell::CreateShelfDelegate() {
   // May be called multiple times as shelves are created and destroyed.
   if (shelf_delegate_)
@@ -274,6 +269,7 @@ WmShell::WmShell(std::unique_ptr<ShellDelegate> shell_delegate)
       window_cycle_controller_(base::MakeUnique<WindowCycleController>()),
       window_selector_controller_(
           base::MakeUnique<WindowSelectorController>()) {
+  session_controller_->AddSessionStateObserver(this);
 
   prefs::mojom::PreferencesManagerPtr pref_manager_ptr;
   // Can be null in tests.
@@ -284,7 +280,9 @@ WmShell::WmShell(std::unique_ptr<ShellDelegate> shell_delegate)
   pref_store_ = new preferences::PrefObserverStore(std::move(pref_manager_ptr));
 }
 
-WmShell::~WmShell() {}
+WmShell::~WmShell() {
+  session_controller_->RemoveSessionStateObserver(this);
+}
 
 RootWindowController* WmShell::GetPrimaryRootWindowController() {
   return GetPrimaryRootWindow()->GetRootWindowController();
@@ -418,4 +416,11 @@ void WmShell::SetAcceleratorController(
   accelerator_controller_ = std::move(accelerator_controller);
 }
 
+void WmShell::SessionStateChanged(session_manager::SessionState state) {
+  // Create the shelf when a session becomes active. It's safe to do this
+  // multiple times (e.g. initial login vs. multiprofile add session).
+  if (state == session_manager::SessionState::ACTIVE)
+    CreateShelf();
+}
+
 }  // namespace ash

ash/common/wm_shell.h

@@ -13,6 +13,7 @@
 #include "ash/ash_export.h"
 #include "ash/common/metrics/gesture_action_type.h"
 #include "ash/common/metrics/user_metrics_action.h"
+#include "ash/common/session/session_state_observer.h"
 #include "ash/common/wm/lock_state_observer.h"
 #include "base/observer_list.h"
 #include "components/ui_devtools/devtools_server.h"
@@ -101,7 +102,7 @@ class WindowState;
 }
 
 // Similar to ash::Shell. Eventually the two will be merged.
-class ASH_EXPORT WmShell {
+class ASH_EXPORT WmShell : public SessionStateObserver {
  public:
   // This is necessary for a handful of places that is difficult to plumb
   // through context.
@@ -353,12 +354,10 @@ class ASH_EXPORT WmShell {
 
   virtual std::unique_ptr<KeyEventWatcher> CreateKeyEventWatcher() = 0;
 
-  // Initializes the appropriate shelves. Does nothing for any existing shelves.
+  // Creates the ShelfView for each display and populates it with items.
+  // TODO(jamescook): Rename this. http://crbug.com/679925
   void CreateShelf();
 
-  // Show shelf view if it was created hidden (before session has started).
-  void ShowShelf();
-
   void CreateShelfDelegate();
 
   // Called after maximize mode has started, windows might still animate though.
@@ -447,7 +446,7 @@ class ASH_EXPORT WmShell {
 
  protected:
   explicit WmShell(std::unique_ptr<ShellDelegate> shell_delegate);
-  virtual ~WmShell();
+  ~WmShell() override;
 
   base::ObserverList<ShellObserver>* shell_observers() {
     return &shell_observers_;
@@ -475,6 +474,9 @@ class ASH_EXPORT WmShell {
   void SetAcceleratorController(
       std::unique_ptr<AcceleratorController> accelerator_controller);
 
+  // SessionStateObserver:
+  void SessionStateChanged(session_manager::SessionState state) override;
+
  private:
   friend class AcceleratorControllerTest;
   friend class ScopedRootWindowForNewWindows;

ash/mus/test/wm_test_base.cc

@@ -7,10 +7,14 @@
 #include <algorithm>
 #include <vector>
 
+#include "ash/common/session/session_controller.h"
+#include "ash/common/wm_shell.h"
 #include "ash/mus/root_window_controller.h"
 #include "ash/mus/test/wm_test_helper.h"
 #include "ash/mus/window_manager.h"
 #include "ash/mus/window_manager_application.h"
+#include "ash/public/cpp/session_types.h"
+#include "ash/public/interfaces/session_controller.mojom.h"
 #include "ash/test/wm_window_aura_test_api.h"
 #include "base/memory/ptr_util.h"
 #include "services/ui/public/cpp/property_type_converters.h"
@@ -165,6 +169,9 @@ void WmTestBase::SetUp() {
       base::MakeUnique<WmWindowAuraTestApi::GlobalMinimumSizeLock>();
   test_helper_.reset(new WmTestHelper);
   test_helper_->Init();
+
+  // Most tests assume the user is logged in (and hence the shelf is created).
+  SimulateUserLogin();
 }
 
 void WmTestBase::TearDown() {
@@ -174,5 +181,29 @@ void WmTestBase::TearDown() {
   zero_duration_mode_.reset();
 }
 
+void WmTestBase::SimulateUserLogin() {
+  SessionController* session_controller = WmShell::Get()->session_controller();
+
+  // Simulate the first user logging in.
+  mojom::UserSessionPtr session = mojom::UserSession::New();
+  session->session_id = 1;
+  session->type = user_manager::USER_TYPE_REGULAR;
+  const std::string email("ash.user@example.com");
+  session->serialized_account_id = AccountId::FromUserEmail(email).Serialize();
+  session->display_name = "Ash User";
+  session->display_email = email;
+  session_controller->UpdateUserSession(std::move(session));
+
+  // Simulate the user session becoming active.
+  mojom::SessionInfoPtr info = mojom::SessionInfo::New();
+  info->max_users = 10;
+  info->can_lock_screen = true;
+  info->should_lock_screen_automatically = false;
+  info->add_user_session_policy = AddUserSessionPolicy::ALLOWED;
+  info->state = session_manager::SessionState::ACTIVE;
+  session_controller->SetSessionInfo(std::move(info));
+}
+
+
 }  // namespace mus
 }  // namespace ash

ash/mus/test/wm_test_base.h

@@ -82,6 +82,10 @@ class WmTestBase : public testing::Test {
  private:
   friend class AshTestImplMus;
 
+  // Simulates the first user logging in and the session becoming active.
+  // Classic ash handles this via AshTestHelper and TestSessionStateDelegate.
+  void SimulateUserLogin();
+
   bool setup_called_ = false;
   bool teardown_called_ = false;
   std::unique_ptr<ui::ScopedAnimationDurationScaleMode> zero_duration_mode_;

ash/mus/window_manager.cc

@@ -8,6 +8,7 @@
 
 #include <utility>
 
+#include "ash/common/session/session_controller.h"
 #include "ash/common/wm/container_finder.h"
 #include "ash/common/wm/window_state.h"
 #include "ash/display/screen_position_controller.h"
@@ -139,9 +140,6 @@ void WindowManager::Init(
                               this, pointer_watcher_event_router_.get()));
   shell_->Initialize(blocking_pool);
   lookup_.reset(new WmLookupMus);
-
-  // TODO: this should be called when logged in. See http://crbug.com/654606.
-  shell_->CreateShelf();
 }
 
 aura::client::ActivationClient* WindowManager::activation_client() {
@@ -228,9 +226,9 @@ RootWindowController* WindowManager::CreateRootWindowController(
       root_window_controller_ptr.get();
   root_window_controllers_.insert(std::move(root_window_controller_ptr));
 
-  // TODO: this should be called when logged in. See http://crbug.com/654606.
-  root_window_controller->ash_root_window_controller()
-      ->CreateShelf();
+  // Create a shelf if a user is already logged in.
+  if (shell_->session_controller()->NumberOfLoggedInUsers())
+    root_window_controller->ash_root_window_controller()->CreateShelf();
 
   for (auto& observer : observers_)
     observer.OnRootWindowControllerAdded(root_window_controller);

ash/root_window_controller.cc

@@ -371,14 +371,6 @@ void RootWindowController::CreateShelf() {
   wm_shelf_->shelf_widget()->PostCreateShelf();
 }
 
-void RootWindowController::ShowShelf() {
-  if (!wm_shelf_->IsShelfInitialized())
-    return;
-  // TODO(jamescook): Move this into WmShelf.
-  wm_shelf_->shelf_widget()->SetShelfVisibility(true);
-  wm_shelf_->shelf_widget()->status_area_widget()->Show();
-}
-
 ShelfLayoutManager* RootWindowController::GetShelfLayoutManager() {
   return wm_shelf_->shelf_layout_manager();
 }

ash/root_window_controller.h

@@ -141,10 +141,6 @@ class ASH_EXPORT RootWindowController : public ShellObserver {
   // Creates the shelf for this root window and notifies observers.
   void CreateShelf();
 
-  // Show shelf view if it was created hidden (before session has started).
-  // TODO(jamescook): Eliminate this and handle show via Shelf.
-  void ShowShelf();
-
   // Get touch HUDs associated with this root window controller.
   TouchHudDebug* touch_hud_debug() const { return touch_hud_debug_; }
   TouchHudProjection* touch_hud_projection() const {

ash/root_window_controller_unittest.cc

@@ -8,6 +8,7 @@
 
 #include "ash/aura/wm_window_aura.h"
 #include "ash/common/material_design/material_design_controller.h"
+#include "ash/common/session/session_controller.h"
 #include "ash/common/session/session_state_delegate.h"
 #include "ash/common/system/tray/system_tray_delegate.h"
 #include "ash/common/wm/system_modal_container_layout_manager.h"
@@ -16,7 +17,6 @@
 #include "ash/common/wm_shell.h"
 #include "ash/common/wm_window.h"
 #include "ash/public/cpp/shell_window_ids.h"
-#include "ash/root_window_controller.h"
 #include "ash/shell.h"
 #include "ash/test/ash_md_test_base.h"
 #include "ash/test/ash_test_base.h"
@@ -970,8 +970,8 @@ TEST_F(VirtualKeyboardRootWindowControllerTest,
   // Track the keyboard container window.
   aura::WindowTracker tracker;
   tracker.Add(keyboard_container);
-  // Mock a login user profile change to reinitialize the keyboard.
-  Shell::GetInstance()->OnLoginUserProfilePrepared();
+  // Reinitialize the keyboard.
+  Shell::GetInstance()->CreateKeyboard();
   // keyboard_container should no longer be present.
   EXPECT_FALSE(tracker.Contains(keyboard_container));
 }
@@ -1005,7 +1005,9 @@ TEST_F(VirtualKeyboardRootWindowControllerTest, RestoreWorkspaceAfterLogin) {
   }
 
   // Mock a login user profile change to reinitialize the keyboard.
-  Shell::GetInstance()->OnLoginUserProfilePrepared();
+  mojom::SessionInfoPtr info = mojom::SessionInfo::New();
+  info->state = session_manager::SessionState::ACTIVE;
+  WmShell::Get()->session_controller()->SetSessionInfo(std::move(info));
   EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().work_area(),
             before);
 }

ash/shell.cc

@@ -273,11 +273,6 @@ void Shell::OnLoginStateChanged(LoginStatus status) {
     observer.OnLoginStateChanged(status);
 }
 
-void Shell::OnLoginUserProfilePrepared() {
-  wm_shell_->CreateShelf();
-  CreateKeyboard();
-}
-
 void Shell::OnAppTerminating() {
   for (auto& observer : *wm_shell_->shell_observers())
     observer.OnAppTerminating();

ash/shell.h

@@ -206,9 +206,6 @@ class ASH_EXPORT Shell : public SystemModalContainerEventFilterDelegate,
   // Called when the user logs in.
   void OnLoginStateChanged(LoginStatus status);
 
-  // Called after the logged-in user's profile is ready.
-  void OnLoginUserProfilePrepared();
-
   // Called when the application is exiting.
   void OnAppTerminating();
 

chrome/browser/chromeos/login/session/chrome_session_manager.cc

@@ -134,7 +134,6 @@ void StartRestoreAfterCrashSession(Profile* user_profile,
 
   bool is_running_test = command_line->HasSwitch(::switches::kTestName) ||
                          command_line->HasSwitch(::switches::kTestType);
-
   if (!is_running_test) {
     // Enable CrasAudioHandler logging when chrome restarts after crashing.
     if (chromeos::CrasAudioHandler::IsInitialized())

chrome/browser/chromeos/login/supervised/supervised_user_creation_screen.cc

@@ -6,7 +6,6 @@
 
 #include <utility>
 
-#include "ash/common/shelf/wm_shelf.h"
 #include "ash/common/wallpaper/wallpaper_controller.h"
 #include "ash/common/wm_shell.h"
 #include "base/memory/ptr_util.h"
@@ -369,8 +368,6 @@ void SupervisedUserCreationScreen::OnManagerFullyAuthenticated(
   // For manager user, move wallpaper to locked container so that windows
   // created during the user image picker step are below it.
   ash::WmShell::Get()->wallpaper_controller()->MoveToLockedContainer();
-  ash::WmShelf::ForWindow(ash::WmShell::Get()->GetPrimaryRootWindow())
-      ->SetAlignment(ash::ShelfAlignment::SHELF_ALIGNMENT_BOTTOM_LOCKED);
 
   controller_->SetManagerProfile(manager_profile);
   if (actor_)

chrome/browser/sessions/session_restore_browsertest_chromeos.cc

@@ -18,10 +18,12 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/test_launcher_utils.h"
 #include "components/prefs/pref_service.h"
 #include "components/sessions/core/serialized_navigation_entry_test_helper.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/test_utils.h"
+#include "ui/wm/core/wm_core_switches.h"
 
 namespace {
 const char* test_app_popup_name1 = "TestApp1";
@@ -33,8 +35,16 @@ class SessionRestoreTestChromeOS : public InProcessBrowserTest {
   ~SessionRestoreTestChromeOS() override {}
 
  protected:
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    InProcessBrowserTest::SetUpCommandLine(command_line);
+  void SetUpDefaultCommandLine(base::CommandLine* command_line) override {
+    base::CommandLine default_command_line(base::CommandLine::NO_PROGRAM);
+    InProcessBrowserTest::SetUpDefaultCommandLine(&default_command_line);
+
+    // Animations have caused crashes in session restore in the past but are
+    // usually disabled in tests. Remove --wm-window-animations-disabled to
+    // re-enable animations.
+    test_launcher_utils::RemoveCommandLineSwitch(
+        default_command_line, wm::switches::kWindowAnimationsDisabled,
+        command_line);
   }
 
   Browser* CreateBrowserWithParams(Browser::CreateParams params) {
@@ -161,3 +171,33 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTestChromeOS, RestoreMaximized) {
   EXPECT_EQ(4u, total_count);
   EXPECT_EQ(2u, maximized_count);
 }
+
+// Test for crash when restoring minimized windows. http://crbug.com/679513.
+IN_PROC_BROWSER_TEST_F(SessionRestoreTestChromeOS, PRE_RestoreMinimized) {
+  // One browser window is always created by default.
+  ASSERT_TRUE(browser());
+  browser()->window()->Minimize();
+
+  Browser* browser2 = CreateBrowserWithParams(Browser::CreateParams(profile()));
+  browser2->window()->Minimize();
+
+  EXPECT_TRUE(browser()->window()->IsMinimized());
+  EXPECT_TRUE(browser2->window()->IsMinimized());
+
+  TurnOnSessionRestore();
+}
+
+IN_PROC_BROWSER_TEST_F(SessionRestoreTestChromeOS, RestoreMinimized) {
+  size_t total_count = 0;
+  size_t minimized_count = 0;
+  for (auto* browser : *BrowserList::GetInstance()) {
+    ++total_count;
+    if (browser->window()->IsMinimized())
+      ++minimized_count;
+  }
+  EXPECT_EQ(2u, total_count);
+  // Chrome OS always activates the last browser window on login, which results
+  // in one window being restored. This seems reasonable as it reminds users
+  // they have a browser running instead of just showing them an empty desktop.
+  EXPECT_EQ(1u, minimized_count);
+}

chrome/browser/ui/ash/chrome_shell_delegate.cc

@@ -565,7 +565,6 @@ void ChromeShellDelegate::Observe(int type,
       // chrome::NOTIFICATION_LOGIN_USER_PROFILE_PREPARED instead.
       if (shelf_delegate_)
         shelf_delegate_->OnUserProfileReadyToSwitch(profile);
-      ash::Shell::GetInstance()->OnLoginUserProfilePrepared();
       break;
     }
     case chrome::NOTIFICATION_SESSION_STARTED:
@@ -573,7 +572,6 @@ void ChromeShellDelegate::Observe(int type,
       // start.
       if (user_manager::UserManager::Get()->GetLoggedInUsers().size() < 2)
         InitAfterFirstSessionStart();
-      ash::WmShell::Get()->ShowShelf();
       break;
     default:
       NOTREACHED() << "Unexpected notification " << type;

chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc

@@ -42,6 +42,7 @@
 #include "chrome/browser/ui/ash/launcher/launcher_application_menu_item_model.h"
 #include "chrome/browser/ui/ash/launcher/launcher_context_menu.h"
 #include "chrome/browser/ui/ash/launcher/launcher_item_controller.h"
+#include "chrome/browser/ui/ash/session_controller_client.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
@@ -192,7 +193,11 @@ class LauncherPlatformAppBrowserTest
   ~LauncherPlatformAppBrowserTest() override {}
 
   void RunTestOnMainThreadLoop() override {
+    // Ensure ash starts the session and creates the shelf and controller.
+    SessionControllerClient::FlushForTesting();
+
     controller_ = GetChromeLauncherControllerImpl();
+    ASSERT_TRUE(controller_);
     return extensions::PlatformAppBrowserTest::RunTestOnMainThreadLoop();
   }
 
@@ -259,10 +264,14 @@ class ShelfAppBrowserTest : public ExtensionBrowserTest {
   ~ShelfAppBrowserTest() override {}
 
   void RunTestOnMainThreadLoop() override {
+    // Ensure ash starts the session and creates the shelf and controller.
+    SessionControllerClient::FlushForTesting();
+
     shelf_ =
         ash::WmShelf::ForWindow(ash::WmShell::Get()->GetPrimaryRootWindow());
     model_ = ash::WmShell::Get()->shelf_model();
     controller_ = GetChromeLauncherControllerImpl();
+    ASSERT_TRUE(controller_);
     return ExtensionBrowserTest::RunTestOnMainThreadLoop();
   }
 

chrome/browser/ui/ash/multi_user/multi_user_window_manager_stub.cc

@@ -10,6 +10,10 @@
 
 namespace chrome {
 
+MultiUserWindowManagerStub::MultiUserWindowManagerStub() {}
+
+MultiUserWindowManagerStub::~MultiUserWindowManagerStub() {}
+
 void MultiUserWindowManagerStub::SetWindowOwner(aura::Window* window,
                                                 const AccountId& account_id) {
   NOTIMPLEMENTED();

chrome/browser/ui/ash/multi_user/multi_user_window_manager_stub.h

@@ -19,8 +19,8 @@ namespace chrome {
 // This is the implementation of MultiUserWindowManager for single user mode.
 class MultiUserWindowManagerStub : public MultiUserWindowManager {
  public:
-  MultiUserWindowManagerStub() {}
-  ~MultiUserWindowManagerStub() override {}
+  MultiUserWindowManagerStub();
+  ~MultiUserWindowManagerStub() override;
 
   // MultiUserWindowManager overrides:
   void SetWindowOwner(aura::Window* window,

chrome/browser/ui/ash/session_controller_client.cc

@@ -34,6 +34,8 @@ using user_manager::UserList;
 
 namespace {
 
+SessionControllerClient* g_instance = nullptr;
+
 uint32_t GetSessionId(const User* user) {
   const UserList logged_in_users = UserManager::Get()->GetLoggedInUsers();
   // TODO(xiyuan): Update with real session id when user session tracking
@@ -83,9 +85,15 @@ SessionControllerClient::SessionControllerClient() : binding_(this) {
   SendSessionInfoIfChanged();
   // User sessions and their order will be sent via UserSessionStateObserver
   // even for crash-n-restart.
+
+  DCHECK(!g_instance);
+  g_instance = this;
 }
 
 SessionControllerClient::~SessionControllerClient() {
+  DCHECK_EQ(this, g_instance);
+  g_instance = nullptr;
+
   SessionManager::Get()->RemoveObserver(this);
   UserManager::Get()->RemoveSessionStateObserver(this);
 }
@@ -225,6 +233,11 @@ void SessionControllerClient::DoCycleActiveUser(bool next_user) {
   DoSwitchActiveUser(account_id);
 }
 
+// static
+void SessionControllerClient::FlushForTesting() {
+  g_instance->session_controller_.FlushForTesting();
+}
+
 void SessionControllerClient::OnSessionStateChanged() {
   SendSessionInfoIfChanged();
 }

chrome/browser/ui/ash/session_controller_client.h

@@ -50,6 +50,9 @@ class SessionControllerClient
   static void DoSwitchActiveUser(const AccountId& account_id);
   static void DoCycleActiveUser(bool next_user);
 
+  // Flushes the mojo pipe to ash.
+  static void FlushForTesting();
+
  private:
   // Connects or reconnects to the |session_controller_| interface and set
   // this object as its client.

chrome/browser/ui/browser_browsertest.cc

@@ -2562,6 +2562,13 @@ IN_PROC_BROWSER_TEST_F(ClickModifierTest, DISABLED_HrefShiftMiddleClickTest) {
 }
 
 IN_PROC_BROWSER_TEST_F(BrowserTest, GetSizeForNewRenderView) {
+  // Force an initial resize. This works around a test-only problem on Chrome OS
+  // where the shelf may not be created before the initial test browser window
+  // opens, which leads to sizing issues in WebContents resize.
+  browser()->window()->SetBounds(gfx::Rect(10, 20, 600, 400));
+  // Let the message loop run so that resize actually takes effect.
+  content::RunAllPendingInMessageLoop();
+
   // The instant extended NTP has javascript that does not work with
   // ui_test_utils::NavigateToURL.  The NTP rvh reloads when the browser tries
   // to navigate away from the page, which causes the WebContents to end up in
@@ -2685,6 +2692,7 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, GetSizeForNewRenderView) {
 #endif
   EXPECT_EQ(exp_commit_size, rwhv_commit_size2);
   EXPECT_EQ(exp_commit_size, wcv_commit_size2);
+
   gfx::Size exp_final_size(initial_wcv_size);
   exp_final_size.Enlarge(wcv_resize_insets.width(),
                          wcv_resize_insets.height() + height_inset);

chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc

@@ -6,11 +6,7 @@
 
 #include "ash/root_window_controller.h"
 #include "ash/shell.h"
-#include "base/command_line.h"
-#include "base/lazy_instance.h"
-#include "base/macros.h"
 #include "base/memory/ptr_util.h"
-#include "build/build_config.h"
 #include "chrome/browser/chrome_browser_main.h"
 #include "chrome/browser/ui/ash/ash_init.h"
 #include "chrome/browser/ui/ash/ash_util.h"
@@ -19,26 +15,21 @@
 #include "chrome/browser/ui/ash/chrome_shell_content_state.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller_mus.h"
 #include "chrome/browser/ui/ash/media_client.h"
+#include "chrome/browser/ui/ash/session_controller_client.h"
+#include "chrome/browser/ui/ash/system_tray_client.h"
+#include "chrome/browser/ui/ash/volume_controller.h"
+#include "chrome/browser/ui/ash/vpn_list_forwarder.h"
 #include "chrome/browser/ui/views/ash/tab_scrubber.h"
 #include "chrome/browser/ui/views/chrome_browser_main_extra_parts_views.h"
 #include "chrome/browser/ui/views/frame/immersive_context_mus.h"
 #include "chrome/browser/ui/views/frame/immersive_handler_factory_mus.h"
-#include "chrome/common/chrome_switches.h"
-#include "ui/aura/env.h"
+#include "chrome/browser/ui/views/select_file_dialog_extension.h"
+#include "chrome/browser/ui/views/select_file_dialog_extension_factory.h"
 #include "ui/keyboard/content/keyboard.h"
 #include "ui/keyboard/keyboard_controller.h"
 #include "ui/wm/core/capture_controller.h"
 #include "ui/wm/core/wm_state.h"
 
-#if defined(OS_CHROMEOS)
-#include "chrome/browser/ui/ash/session_controller_client.h"
-#include "chrome/browser/ui/ash/system_tray_client.h"
-#include "chrome/browser/ui/ash/volume_controller.h"
-#include "chrome/browser/ui/ash/vpn_list_forwarder.h"
-#include "chrome/browser/ui/views/select_file_dialog_extension.h"
-#include "chrome/browser/ui/views/select_file_dialog_extension_factory.h"
-#endif  // defined(OS_CHROMEOS)
-
 ChromeBrowserMainExtraPartsAsh::ChromeBrowserMainExtraPartsAsh(
     ChromeBrowserMainExtraPartsViews* extra_parts_views)
     : extra_parts_views_(extra_parts_views) {}
@@ -55,10 +46,7 @@ void ChromeBrowserMainExtraPartsAsh::PreProfileInit() {
     immersive_handler_factory_ = base::MakeUnique<ImmersiveHandlerFactoryMus>();
   }
 
-#if defined(OS_CHROMEOS)
-  // TODO(xiyuan): Update after SesssionStateDelegate is deprecated.
-  if (chrome::IsRunningInMash())
-    session_controller_client_ = base::MakeUnique<SessionControllerClient>();
+  session_controller_client_ = base::MakeUnique<SessionControllerClient>();
 
   // Must be available at login screen, so initialize before profile.
   system_tray_client_ = base::MakeUnique<SystemTrayClient>();
@@ -72,7 +60,6 @@ void ChromeBrowserMainExtraPartsAsh::PreProfileInit() {
   keyboard::InitializeKeyboard();
 
   ui::SelectFileDialog::SetFactory(new SelectFileDialogExtensionFactory);
-#endif  // defined(OS_CHROMEOS)
 }
 
 void ChromeBrowserMainExtraPartsAsh::PostProfileInit() {
@@ -101,7 +88,6 @@ void ChromeBrowserMainExtraPartsAsh::PostProfileInit() {
 }
 
 void ChromeBrowserMainExtraPartsAsh::PostMainMessageLoopRun() {
-#if defined(OS_CHROMEOS)
   vpn_list_forwarder_.reset();
   volume_controller_.reset();
   new_window_client_.reset();
@@ -109,6 +95,6 @@ void ChromeBrowserMainExtraPartsAsh::PostMainMessageLoopRun() {
   media_client_.reset();
   cast_config_client_media_router_.reset();
   session_controller_client_.reset();
-#endif
+
   chrome::CloseAsh();
 }

chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.h

@@ -23,6 +23,8 @@ class SystemTrayClient;
 class VolumeController;
 class VpnListForwarder;
 
+// Browser initialization for Ash. Only runs on Chrome OS.
+// TODO(jamescook): Fold this into ChromeBrowserMainPartsChromeOS.
 class ChromeBrowserMainExtraPartsAsh : public ChromeBrowserMainExtraParts {
  public:
   explicit ChromeBrowserMainExtraPartsAsh(