cros: Clean up SessionStateDelegate refs in Chrome

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

Index: chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
diff --git a/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc b/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
index c454ba5eac4e03bb1650df376fe3352e80763436..4d5da0bafaa1c280bf6381f7867a4c39441f6642 100644
--- a/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
+++ b/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
@@ -6,7 +6,6 @@
 
 #include "ash/common/media_controller.h"
 #include "ash/common/multi_profile_uma.h"
-#include "ash/common/session/session_state_delegate.h"
 #include "ash/common/wm/maximize_mode/maximize_mode_controller.h"
 #include "ash/common/wm/window_state.h"
 #include "ash/common/wm_shell.h"
@@ -25,6 +24,7 @@
 #include "chrome/browser/ui/ash/multi_user/multi_user_notification_blocker_chromeos.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
 #include "chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.h"
+#include "chrome/browser/ui/ash/session_controller_client.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -201,10 +201,8 @@ MultiUserWindowManagerChromeOS::~MultiUserWindowManagerChromeOS() {
     window = window_to_entry_.begin();
   }
 
-  if (ash::WmShell::HasInstance()) {
-    ash::WmShell::Get()->GetSessionStateDelegate()->RemoveSessionStateObserver(
-        this);
-  }
+  if (user_manager::UserManager::IsInitialized())
+    user_manager::UserManager::Get()->RemoveSessionStateObserver(this);
 
   // Remove all app observers.
   ProfileManager* profile_manager = g_browser_process->profile_manager();
@@ -233,10 +231,8 @@ void MultiUserWindowManagerChromeOS::Init() {
          account_id_to_app_observer_.end());
 
   // Add a session state observer to be able to monitor session changes.
-  if (ash::WmShell::HasInstance()) {
-    ash::WmShell::Get()->GetSessionStateDelegate()->AddSessionStateObserver(
-        this);
-  }
+  if (user_manager::UserManager::IsInitialized())
+    user_manager::UserManager::Get()->AddSessionStateObserver(this);
 
   // The BrowserListObserver would have been better to use then the old
   // notification system, but that observer fires before the window got created.
@@ -304,7 +300,7 @@ void MultiUserWindowManagerChromeOS::ShowWindowForUser(
       previous_owner != current_account_id_)
     return;
 
-  ash::WmShell::Get()->GetSessionStateDelegate()->SwitchActiveUser(account_id);
+  SessionControllerClient::DoSwitchActiveUser(account_id);
 }
 
 bool MultiUserWindowManagerChromeOS::AreWindowsSharedAmongUsers() const {
@@ -383,9 +379,9 @@ void MultiUserWindowManagerChromeOS::RemoveObserver(Observer* observer) {
 }
 
 void MultiUserWindowManagerChromeOS::ActiveUserChanged(
-    const AccountId& account_id) {
+    const user_manager::User* active_user) {
   // This needs to be set before the animation starts.
-  current_account_id_ = account_id;
+  current_account_id_ = active_user->GetAccountId();
 
   // Here to avoid a very nasty race condition, we must destruct any previously
   // created animation before creating a new one. Otherwise, the newly
@@ -393,7 +389,8 @@ void MultiUserWindowManagerChromeOS::ActiveUserChanged(
   // animation only to be reshown again by the destructor of the old animation.
   animation_.reset();
   animation_.reset(new UserSwitchAnimatorChromeOS(
-      this, account_id, GetAdjustedAnimationTimeInMS(kUserFadeTimeMS)));
+      this, current_account_id_,
+      GetAdjustedAnimationTimeInMS(kUserFadeTimeMS)));
   // Call notifier here instead of observing ActiveUserChanged because
   // this must happen after MultiUserWindowManagerChromeOS is notified.
   ash::WmShell::Get()->media_controller()->RequestCaptureState();
@@ -406,6 +403,8 @@ void MultiUserWindowManagerChromeOS::OnWindowDestroyed(aura::Window* window) {
     RemoveTransientOwnerRecursive(window);
     return;
   }
+
+  window->RemoveObserver(this);

[Mr4D (OOO till 08-26), 2017/02/28 04:56:56]

This is odd. Why wasn't this needed before?

[xiyuan, 2017/02/28 18:35:50]

Got the CHECK failure in WindowObserver dtor [1] when working on the CL. It is
related to switching from TestSessionStateDelegate's default user to the
FakeUserManager's default user [2]. Somehow, I could no longer repro it now. :(

I believe we still have problems though. The RemoveObserver call was in this
OnWindowDestroyed before. It was removed in
https://codereview.chromium.org/101013002 where aura::Window now calls
RemoveObserver before notifying observers about OnWindowDestroyed. However,
MultiUserWindowManagerChromeOS's destructor still calls its OnWindowDestroyed
and expects it would do RemoveObserver on the window. This is broken and causing
the CHECK.

Based on what I have learned now, I move this RemoevObserver call to the
MultiUserWindowManagerChromeOS's dtor and added a comment there.

[1]:
https://cs.chromium.org/chromium/src/ui/aura/window_observer.cc?rcl=96a656266...
[2]:
https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/multi_user/multi_u...

   wm::TransientWindowManager::Get(window)->RemoveObserver(this);
   // Remove the window from the owners list.
   delete window_to_entry_[window];
@@ -569,8 +568,7 @@ void MultiUserWindowManagerChromeOS::SetWindowVisibility(
         account_id = GetUserPresentingWindow(owning_window);
         DCHECK(account_id.is_valid());
       }
-      ash::WmShell::Get()->GetSessionStateDelegate()->SwitchActiveUser(
-          account_id);
+      SessionControllerClient::DoSwitchActiveUser(account_id);
       return;
     }
   }