cros: Clean up SessionStateDelegate refs in Chrome

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.h"
 
 #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"
 #include "ash/common/wm_window.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/shell.h"
 #include "ash/wm/window_state_aura.h"
 #include "base/auto_reset.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #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"
 #include "chrome/browser/ui/browser_window.h"
 #include "content/public/browser/notification_service.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/browser/app_window/app_window_registry.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_event_dispatcher.h"
@@ skipping 156 matching lines @@
   if (animation_.get())
     animation_->CancelAnimation();
 
   // Remove all window observers.
   WindowToEntryMap::iterator window = window_to_entry_.begin();
   while (window != window_to_entry_.end()) {
     OnWindowDestroyed(window->first);
     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();
   // might be nullptr in unit tests.
   if (!profile_manager)
     return;
 
   std::vector<Profile*> profiles = profile_manager->GetLoadedProfiles();
   for (auto it = profiles.begin(); it != profiles.end(); ++it) {
     const AccountId account_id = multi_user_util::GetAccountIdFromProfile(*it);
     AccountIdToAppWindowObserver::iterator app_observer_iterator =
         account_id_to_app_observer_.find(account_id);
     if (app_observer_iterator != account_id_to_app_observer_.end()) {
       extensions::AppWindowRegistry::Get(*it)->RemoveObserver(
           app_observer_iterator->second);
       delete app_observer_iterator->second;
       account_id_to_app_observer_.erase(app_observer_iterator);
     }
   }
 }
 
 void MultiUserWindowManagerChromeOS::Init() {
   // Since we are setting the SessionStateObserver and adding the user, this
   // function should get called only once.
   DCHECK(account_id_to_app_observer_.find(current_account_id_) ==
          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.
   registrar_.Add(this, NOTIFICATION_BROWSER_WINDOW_READY,
                  content::NotificationService::AllSources());
 
   // Add an app window observer & all already running apps.
   Profile* profile =
       multi_user_util::GetProfileFromAccountId(current_account_id_);
   if (profile)
@@ skipping 47 matching lines @@
   const AccountId previous_owner(GetUserPresentingWindow(window));
   if (!ShowWindowForUserIntern(window, account_id))
     return;
   // The window switched to a new desktop and we have to switch to that desktop,
   // but only when it was on the visible desktop and the the target is not the
   // visible desktop.
   if (account_id == current_account_id_ ||
       previous_owner != current_account_id_)
     return;
 
-  ash::WmShell::Get()->GetSessionStateDelegate()->SwitchActiveUser(account_id);
+  SessionControllerClient::DoSwitchActiveUser(account_id);
 }
 
 bool MultiUserWindowManagerChromeOS::AreWindowsSharedAmongUsers() const {
   WindowToEntryMap::const_iterator it = window_to_entry_.begin();
   for (; it != window_to_entry_.end(); ++it) {
     if (it->second->owner() != it->second->show_for_user())
       return true;
   }
   return false;
 }
@@ skipping 58 matching lines @@
 
 void MultiUserWindowManagerChromeOS::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);
 }
 
 void MultiUserWindowManagerChromeOS::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(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
   // constructed will hide all windows of the old user in the first step of the
   // 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();
 }
 
 void MultiUserWindowManagerChromeOS::OnWindowDestroyed(aura::Window* window) {
   if (GetWindowOwner(window).empty()) {
     // This must be a window in the transient chain - remove it and its
     // children from the owner.
     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];
   window_to_entry_.erase(window);
 
   // Notify entry change.
   for (Observer& observer : observers_)
     observer.OnOwnerEntryRemoved(window);
 }
 
@@ skipping 143 matching lines @@
     if (HasSystemModalTransientChildWindow(window)) {
       // The window is system modal and we need to find the parent which owns
       // it so that we can switch to the desktop accordingly.
       AccountId account_id = GetUserPresentingWindow(window);
       if (!account_id.is_valid()) {
         aura::Window* owning_window = GetOwningWindowInTransientChain(window);
         DCHECK(owning_window);
         account_id = GetUserPresentingWindow(owning_window);
         DCHECK(account_id.is_valid());
       }
-      ash::WmShell::Get()->GetSessionStateDelegate()->SwitchActiveUser(
-          account_id);
+      SessionControllerClient::DoSwitchActiveUser(account_id);
       return;
     }
   }
 
   // To avoid that these commands are recorded as any other commands, we are
   // suppressing any window entry changes while this is going on.
   base::AutoReset<bool> suppressor(&suppress_visibility_changes_, true);
 
   if (visible)
     ShowWithTransientChildrenRecursive(window, animation_time_in_ms);
@@ skipping 123 matching lines @@
     window->Hide();
 }
 
 int MultiUserWindowManagerChromeOS::GetAdjustedAnimationTimeInMS(
     int default_time_in_ms) const {
   return animation_speed_ == ANIMATION_SPEED_NORMAL ? default_time_in_ms :
       (animation_speed_ == ANIMATION_SPEED_FAST ? 10 : 0);
 }
 
 }  // namespace chrome