cros: Fix loading user profile w/o UserSessionManager

chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc

 // Copyright (c) 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/chromeos/accessibility/accessibility_manager.h"
 
 #include "ash/accessibility_types.h"
 #include "ash/magnifier/magnification_controller.h"
 #include "ash/shell.h"
 #include "base/command_line.h"
@@ skipping 142 matching lines @@
 
 void SetMonoAudioEnabled(bool enabled) {
   return AccessibilityManager::Get()->EnableMonoAudio(enabled);
 }
 
 bool IsMonoAudioEnabled() {
   return AccessibilityManager::Get()->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;
 }
 
 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);
 }
 
 void SetHighContrastEnabledPref(bool enabled) {
   GetPrefs()->SetBoolean(prefs::kAccessibilityHighContrastEnabled, enabled);
 }
 
 void SetSpokenFeedbackEnabledPref(bool enabled) {
   GetPrefs()->SetBoolean(prefs::kAccessibilitySpokenFeedbackEnabled, enabled);
@@ skipping 50 matching lines @@
       return true;
   }
   return false;
 }
 
 bool IsBrailleImeCurrent() {
   InputMethodManager* imm = InputMethodManager::Get();
   return imm->GetActiveIMEState()->GetCurrentInputMethod().id() ==
          extension_ime_util::kBrailleImeEngineId;
 }
+
 }  // anonymous namespace
 
 class AccessibilityManagerTest : public InProcessBrowserTest {
  protected:
   AccessibilityManagerTest() : default_autoclick_delay_(0) {}
   ~AccessibilityManagerTest() override {}
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitch(chromeos::switches::kLoginManager);
     command_line->AppendSwitchASCII(chromeos::switches::kLoginProfile,
@@ skipping 50 matching lines @@
 
   // Confirms that the features still disabled just after login.
   EXPECT_FALSE(IsLargeCursorEnabled());
   EXPECT_FALSE(IsSpokenFeedbackEnabled());
   EXPECT_FALSE(IsHighContrastEnabled());
   EXPECT_FALSE(IsAutoclickEnabled());
   EXPECT_FALSE(IsVirtualKeyboardEnabled());
   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());
   EXPECT_FALSE(IsSpokenFeedbackEnabled());
   EXPECT_FALSE(IsHighContrastEnabled());
   EXPECT_FALSE(IsAutoclickEnabled());
   EXPECT_FALSE(IsVirtualKeyboardEnabled());
   EXPECT_FALSE(IsMonoAudioEnabled());
   EXPECT_EQ(default_autoclick_delay(), GetAutoclickDelay());
 
@@ skipping 38 matching lines @@
   // Signal the accessibility manager that a braille display was connected.
   SetBrailleDisplayAvailability(true);
   // Confirms that the spoken feedback is enabled.
   EXPECT_TRUE(IsSpokenFeedbackEnabled());
 }
 
 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());
   EXPECT_FALSE(IsSpokenFeedbackEnabled());
   EXPECT_FALSE(IsHighContrastEnabled());
   EXPECT_FALSE(IsAutoclickEnabled());
   EXPECT_EQ(default_autoclick_delay(), GetAutoclickDelay());
   EXPECT_FALSE(IsVirtualKeyboardEnabled());
   EXPECT_FALSE(IsMonoAudioEnabled());
 
@@ skipping 79 matching lines @@
 
   // Sets the pref to enable the on-screen keyboard before login.
   SetVirtualKeyboardEnabledPref(true);
   EXPECT_FALSE(IsVirtualKeyboardEnabled());
 
   // Sets the pref to enable mono audio output before login.
   SetMonoAudioEnabledPref(true);
   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());
   EXPECT_TRUE(IsSpokenFeedbackEnabled());
   EXPECT_TRUE(IsHighContrastEnabled());
   EXPECT_TRUE(IsAutoclickEnabled());
   EXPECT_EQ(kTestAutoclickDelayMs, GetAutoclickDelay());
   EXPECT_TRUE(IsVirtualKeyboardEnabled());
   EXPECT_TRUE(IsMonoAudioEnabled());
 }
 
 IN_PROC_BROWSER_TEST_F(AccessibilityManagerTest,
                        ChangingTypeInvokesNotification) {
   MockAccessibilityObserver observer;
 
   // 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();
 
   SetSpokenFeedbackEnabled(true);
   EXPECT_TRUE(observer.observed());
   EXPECT_TRUE(observer.observed_enabled());
   EXPECT_EQ(observer.observed_type(),
             ACCESSIBILITY_TOGGLE_SPOKEN_FEEDBACK);
   EXPECT_TRUE(IsSpokenFeedbackEnabled());
@@ skipping 55 matching lines @@
   EXPECT_FALSE(IsMonoAudioEnabled());
 }
 
 IN_PROC_BROWSER_TEST_F(AccessibilityManagerTest,
                        ChangingTypePrefInvokesNotification) {
   MockAccessibilityObserver observer;
 
   // 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();
 
   SetSpokenFeedbackEnabledPref(true);
   EXPECT_TRUE(observer.observed());
   EXPECT_TRUE(observer.observed_enabled());
   EXPECT_EQ(observer.observed_type(),
             ACCESSIBILITY_TOGGLE_SPOKEN_FEEDBACK);
   EXPECT_TRUE(IsSpokenFeedbackEnabled());
@@ skipping 100 matching lines @@
   session_manager->CreateSession(account_id, account_id.GetUserEmail());
 
   // Confirms that the features are still enabled just after login.
   EXPECT_TRUE(IsLargeCursorEnabled());
   EXPECT_TRUE(IsSpokenFeedbackEnabled());
   EXPECT_TRUE(IsHighContrastEnabled());
   EXPECT_TRUE(IsAutoclickEnabled());
   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());
   EXPECT_TRUE(IsSpokenFeedbackEnabled());
   EXPECT_TRUE(IsHighContrastEnabled());
   EXPECT_TRUE(IsAutoclickEnabled());
   EXPECT_EQ(kTestAutoclickDelayMs, GetAutoclickDelay());
   EXPECT_TRUE(IsMonoAudioEnabled());
 
   // Confirms that the prefs have been copied to the user's profile.
   EXPECT_TRUE(GetLargeCursorEnabledFromPref());
   EXPECT_TRUE(GetSpokenFeedbackEnabledFromPref());
   EXPECT_TRUE(GetHighContrastEnabledFromPref());
   EXPECT_TRUE(GetAutoclickEnabledFromPref());
   EXPECT_EQ(kTestAutoclickDelayMs, GetAutoclickDelayFromPref());
   EXPECT_TRUE(GetMonoAudioEnabledFromPref());
 }
 
 IN_PROC_BROWSER_TEST_P(AccessibilityManagerUserTypeTest, BrailleWhenLoggedIn) {
   // Logs in.
   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;
   prefs.InitUserPrefsForTesting(
       PrefServiceSyncableFromProfile(GetProfile()),
       user_manager::UserManager::Get()->GetActiveUser(),
       UserSessionManager::GetInstance()->GetDefaultIMEState(GetProfile()));
 
   // Make sure we start in the expected state.
   EXPECT_FALSE(IsBrailleImeActive());
@@ skipping 25 matching lines @@
   // the Braille IME.
   SetBrailleDisplayAvailability(true);
   EXPECT_TRUE(IsSpokenFeedbackEnabled());
   EXPECT_TRUE(IsBrailleImeActive());
 }
 
 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());
   EXPECT_FALSE(IsSpokenFeedbackEnabled());
   EXPECT_FALSE(IsHighContrastEnabled());
   EXPECT_FALSE(IsAutoclickEnabled());
   EXPECT_FALSE(ShouldShowAccessibilityMenu());
   EXPECT_FALSE(IsVirtualKeyboardEnabled());
   EXPECT_FALSE(IsMonoAudioEnabled());
 
@@ skipping 28 matching lines @@
   EXPECT_FALSE(ShouldShowAccessibilityMenu());
 
   // Check mono audio output.
   SetMonoAudioEnabled(true);
   EXPECT_TRUE(ShouldShowAccessibilityMenu());
   SetMonoAudioEnabled(false);
   EXPECT_FALSE(ShouldShowAccessibilityMenu());
 }
 
 }  // namespace chromeos