cros: Fix loading user profile w/o UserSessionManager

chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc

Index: chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
diff --git a/chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc b/chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
index 817565a5cb7a05f04cb4a74f78157e2b18fe2d7e..8db43685000d518546889beb052f9a88f58db04a 100644
--- a/chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
+++ b/chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
@@ -160,7 +160,12 @@ bool IsMonoAudioEnabled() {
 }
 
 Profile* GetProfile() {
-  Profile* profile = ProfileManager::GetActiveUserProfile();
+  // ProfileManager::GetActiveUserProfile() does not load user profile
+  // implicitly any more. Use GetProfile() to do an explicit load.
+  Profile* profile = g_browser_process->profile_manager()->GetProfile(

[emaxx, 2017/06/06 12:11:23]

This construction - loading the profile for the active user - is repeated
several times in this CL. So maybe it's better to introduce the new method in
ProfileManager/ProfileHelper for doing this operation?

[xiyuan, 2017/06/06 20:29:39]

Explicit user profile loading should be banned in production code. It skips
UserSessionManager and is the reason why we had problem now.

The code here is only for tests so that we can exclude UserSessionManager from
tests and keep them simple.

Found we have a ProfileHelper::GetProfileByUserIdHash call. Renamed to
GetProfileByUserIdHashForTest and changed to use it here. 

+      ProfileHelper::Get()->GetProfilePathByUserIdHash(
+          user_manager::UserManager::Get()->GetActiveUser()->username_hash()));
+
   DCHECK(profile);
   return profile;
 }
@@ -169,6 +174,16 @@ PrefService* GetPrefs() {
   return GetProfile()->GetPrefs();
 }
 
+// Simulates how UserSessionManager starts a user session by loading user
+// profile and mark session as started.
+void StartUserSession(const AccountId& account_id) {
+  g_browser_process->profile_manager()->GetProfile(
+      ProfileHelper::Get()->GetProfilePathByUserIdHash(

[emaxx, 2017/06/06 12:11:23]

Is it really fine to pass UserEmail to this function instead of UserIdHash?
I think ProfileHelper::GetUserIdHashByUserIdForTesting() is used for this
transformation from UserEmail in other tests.

[xiyuan, 2017/06/06 20:29:39]

This is because the tests are using the email as hash when CreateSession.
Changed to look up the User and use the username_hash in User so that it looks
more correct.

+          account_id.GetUserEmail()));
+
+  session_manager::SessionManager::Get()->SessionStarted();
+}
+
 void SetLargeCursorEnabledPref(bool enabled) {
   GetPrefs()->SetBoolean(prefs::kAccessibilityLargeCursorEnabled, enabled);
 }
@@ -239,6 +254,7 @@ bool IsBrailleImeCurrent() {
   return imm->GetActiveIMEState()->GetCurrentInputMethod().id() ==
          extension_ime_util::kBrailleImeEngineId;
 }
+
 }  // anonymous namespace
 
 class AccessibilityManagerTest : public InProcessBrowserTest {
@@ -309,7 +325,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityManagerTest, Login) {
   EXPECT_FALSE(IsMonoAudioEnabled());
   EXPECT_EQ(default_autoclick_delay(), GetAutoclickDelay());
 
-  session_manager->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that the features are still disabled just after login.
   EXPECT_FALSE(IsLargeCursorEnabled());
@@ -368,7 +384,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityManagerTest, TypePref) {
   // Logs in.
   auto* session_manager = session_manager::SessionManager::Get();
   session_manager->CreateSession(test_account_id_, kTestUserName);
-  session_manager->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that the features are disabled just after login.
   EXPECT_FALSE(IsLargeCursorEnabled());
@@ -468,7 +484,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityManagerTest, ResumeSavedPref) {
   EXPECT_FALSE(IsMonoAudioEnabled());
 
   // Logs in.
-  session_manager->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that features are enabled by restoring from pref just after login.
   EXPECT_TRUE(IsLargeCursorEnabled());
@@ -487,7 +503,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityManagerTest,
   // Logs in.
   auto* session_manager = session_manager::SessionManager::Get();
   session_manager->CreateSession(test_account_id_, kTestUserName);
-  session_manager->SessionStarted();
+  StartUserSession(test_account_id_);
 
   EXPECT_FALSE(observer.observed());
   observer.reset();
@@ -563,7 +579,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityManagerTest,
   // Logs in.
   auto* session_manager = session_manager::SessionManager::Get();
   session_manager->CreateSession(test_account_id_, kTestUserName);
-  session_manager->SessionStarted();
+  StartUserSession(test_account_id_);
 
   EXPECT_FALSE(observer.observed());
   observer.reset();
@@ -684,7 +700,7 @@ IN_PROC_BROWSER_TEST_P(AccessibilityManagerUserTypeTest,
   EXPECT_EQ(kTestAutoclickDelayMs, GetAutoclickDelay());
   EXPECT_TRUE(IsMonoAudioEnabled());
 
-  session_manager->SessionStarted();
+  StartUserSession(account_id);
 
   // Confirms that the features keep enabled after session starts.
   EXPECT_TRUE(IsLargeCursorEnabled());
@@ -708,7 +724,7 @@ IN_PROC_BROWSER_TEST_P(AccessibilityManagerUserTypeTest, BrailleWhenLoggedIn) {
   const AccountId account_id = AccountId::FromUserEmail(GetParam());
   auto* session_manager = session_manager::SessionManager::Get();
   session_manager->CreateSession(account_id, account_id.GetUserEmail());
-  session_manager->SessionStarted();
+  StartUserSession(account_id);
   // This object watches for IME preference changes and reflects those in
   // the IME framework state.
   chromeos::Preferences prefs;
@@ -754,7 +770,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityManagerTest, AccessibilityMenuVisibility) {
   // Log in.
   auto* session_manager = session_manager::SessionManager::Get();
   session_manager->CreateSession(test_account_id_, kTestUserName);
-  session_manager->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that the features are disabled.
   EXPECT_FALSE(IsLargeCursorEnabled());