chromeos: Fix shelf appearing at login screen under mash

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
TEST=ash_unittests and chrome browser_tests

Review-Url: https://codereview.chromium.org/2573703003
Cr-Commit-Position: refs/heads/master@{#439845}

ash/aura/wm_shell_aura.cc

@@ -323,6 +323,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

@@ -97,6 +97,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();
 }

ash/common/shelf/shelf_widget.h

@@ -60,8 +60,6 @@ class ASH_EXPORT ShelfWidget : public views::Widget,
   ShelfView* CreateShelfView();
   void PostCreateShelf();
 
-  // Set visibility of the shelf.
-  void SetShelfVisibility(bool visible);
   bool IsShelfVisible() const;
 
   bool IsShowingAppList() const;

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_root_window_controller.cc

@@ -215,15 +215,6 @@ void WmRootWindowController::CreateShelf() {
   shelf->shelf_widget()->PostCreateShelf();
 }
 
-void WmRootWindowController::ShowShelf() {
-  WmShelf* shelf = GetShelf();
-  if (!shelf->IsShelfInitialized())
-    return;
-  // TODO(jamescook): Move this into WmShelf.
-  shelf->shelf_widget()->SetShelfVisibility(true);
-  shelf->shelf_widget()->status_area_widget()->Show();
-}
-
 SystemTray* WmRootWindowController::GetSystemTray() {
   ShelfWidget* shelf_widget = GetShelf()->shelf_widget();
   if (!shelf_widget || !shelf_widget->status_area_widget())

ash/common/wm_root_window_controller.h

@@ -100,10 +100,6 @@ class ASH_EXPORT WmRootWindowController {
   // 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();
-
   // Returns the system tray controller. May be null for external displays.
   SystemTray* GetSystemTray();
 

ash/common/wm_shell.cc

@@ -151,11 +151,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_)
@@ -272,9 +267,12 @@ WmShell::WmShell(std::unique_ptr<ShellDelegate> shell_delegate)
   keyboard_brightness_control_delegate_.reset(new KeyboardBrightnessController);
   vpn_list_ = base::MakeUnique<VpnList>();
 #endif
+  session_controller_->AddSessionStateObserver(this);
 }
 
-WmShell::~WmShell() {}
+WmShell::~WmShell() {
+  session_controller_->RemoveSessionStateObserver(this);
+}
 
 WmRootWindowController* WmShell::GetPrimaryRootWindowController() {
   return GetPrimaryRootWindow()->GetRootWindowController();
@@ -415,4 +413,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"
@@ -100,7 +101,7 @@ class VpnList;
 #endif
 
 // 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.
@@ -343,9 +344,6 @@ class ASH_EXPORT WmShell {
   // Initializes the appropriate shelves. Does nothing for any existing shelves.
   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.
@@ -446,7 +444,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_;
@@ -474,6 +472,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/ash_test_impl_mus.cc

@@ -4,8 +4,12 @@
 
 #include "ash/mus/test/ash_test_impl_mus.h"
 
+#include "ash/common/session/session_controller.h"
 #include "ash/common/test/ash_test.h"
+#include "ash/common/wm_shell.h"
 #include "ash/mus/bridge/wm_window_mus.h"
+#include "ash/public/cpp/session_types.h"
+#include "ash/public/interfaces/session_controller.mojom.h"
 #include "base/memory/ptr_util.h"
 #include "services/ui/public/cpp/property_type_converters.h"
 #include "services/ui/public/interfaces/window_manager.mojom.h"
@@ -40,6 +44,9 @@ AshTestImplMus::~AshTestImplMus() {}
 
 void AshTestImplMus::SetUp() {
   wm_test_base_->SetUp();
+
+  // Most tests assume the user is logged in (and hence the shelf is created).
+  SimulateUserLogin();
 }
 
 void AshTestImplMus::TearDown() {
@@ -100,6 +107,29 @@ void AshTestImplMus::AddTransientChild(WmWindow* parent, WmWindow* window) {
                           WmWindowAura::GetAuraWindow(window));
 }
 
+void AshTestImplMus::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
 
 // static

ash/mus/test/ash_test_impl_mus.h

@@ -42,6 +42,10 @@ class AshTestImplMus : public AshTestImpl {
   void AddTransientChild(WmWindow* parent, WmWindow* window) override;
 
  private:
+  // Simulates the first user logging in and the session becoming active.
+  // Classic ash handles this via AshTestHelper and TestSessionStateDelegate.
+  void SimulateUserLogin();
+
   // TODO(sky): fold WmTestBase directly into this class when no more subclasses
   // of WmTestBase.
   std::unique_ptr<WmTestBase> wm_test_base_;

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"
@@ -137,9 +138,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::Window* WindowManager::NewTopLevelWindow(
@@ -221,8 +219,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->wm_root_window_controller()->CreateShelf();
+  // Create a shelf if a user is already logged in.
+  if (shell_->session_controller()->NumberOfLoggedInUsers())
+    root_window_controller->wm_root_window_controller()->CreateShelf();
 
   for (auto& observer : observers_)
     observer.OnRootWindowControllerAdded(root_window_controller);

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"
@@ -17,6 +18,7 @@
 #include "ash/common/wm_shell.h"
 #include "ash/common/wm_window.h"
 #include "ash/public/cpp/shell_window_ids.h"
+#include "ash/public/interfaces/session_controller.mojom.h"
 #include "ash/shell.h"
 #include "ash/test/ash_md_test_base.h"
 #include "ash/test/ash_test_base.h"
@@ -975,8 +977,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));
 }
@@ -1010,7 +1012,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

@@ -276,11 +276,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

@@ -197,9 +197,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

@@ -26,6 +26,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/ui/ash/ash_util.h"
+#include "chrome/browser/ui/ash/session_controller_client.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/audio/cras_audio_handler.h"
@@ -133,8 +134,12 @@ void StartRestoreAfterCrashSession(Profile* user_profile,
 
   bool is_running_test = command_line->HasSwitch(::switches::kTestName) ||
                          command_line->HasSwitch(::switches::kTestType);
-
-  if (!is_running_test) {
+  if (is_running_test) {
+    // Some browser tests (e.g. BrowserTest.GetSizeForNewRenderView) require
+    // that ash is synchronously initialized before the first test window opens.
+    // Ensure the session-related mojo messages are sent to ash.
+    SessionControllerClient::FlushForTesting();
+  } else {
     // Enable CrasAudioHandler logging when chrome restarts after crashing.
     if (chromeos::CrasAudioHandler::IsInitialized())
       chromeos::CrasAudioHandler::Get()->LogErrors();

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"
@@ -372,8 +371,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/ui/ash/chrome_shell_delegate.cc

@@ -573,7 +573,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:
@@ -581,7 +580,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

@@ -192,6 +192,7 @@ class LauncherPlatformAppBrowserTest
 
   void RunTestOnMainThreadLoop() override {
     controller_ = GetChromeLauncherControllerImpl();
+    ASSERT_TRUE(controller_);
     return extensions::PlatformAppBrowserTest::RunTestOnMainThreadLoop();
   }
 
@@ -262,6 +263,7 @@ class ShelfAppBrowserTest : public ExtensionBrowserTest {
         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

@@ -36,6 +36,8 @@ namespace {
 // Limits the number of logged in users to 10 due to memory constraints.
 constexpr uint32_t kMaxUsers = 10;
 
+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
@@ -85,9 +87,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;
+
   session_manager::SessionManager::Get()->RemoveObserver(this);
   UserManager::Get()->RemoveSessionStateObserver(this);
 }
@@ -226,6 +234,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/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"
@@ -18,26 +14,21 @@
 #include "chrome/browser/ui/ash/chrome_new_window_client.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) {}
@@ -54,10 +45,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>();
@@ -71,7 +59,6 @@ void ChromeBrowserMainExtraPartsAsh::PreProfileInit() {
   keyboard::InitializeKeyboard();
 
   ui::SelectFileDialog::SetFactory(new SelectFileDialogExtensionFactory);
-#endif  // defined(OS_CHROMEOS)
 }
 
 void ChromeBrowserMainExtraPartsAsh::PostProfileInit() {
@@ -99,7 +86,6 @@ void ChromeBrowserMainExtraPartsAsh::PostProfileInit() {
 }
 
 void ChromeBrowserMainExtraPartsAsh::PostMainMessageLoopRun() {
-#if defined(OS_CHROMEOS)
   vpn_list_forwarder_.reset();
   volume_controller_.reset();
   new_window_client_.reset();
@@ -107,6 +93,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

@@ -22,6 +22,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(