cros: Fix loading user profile w/o UserSessionManager

The fallback logic in GetActiveUserOrOffTheRecordProfileFromPath
should cover the case when the user profile loading has not started.
So that it does not creates/loads user profile accidentally without
going through UserSessionManager.

BUG=728683,718734
TEST=ProfileManagerTest.UserProfileLoading

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

chrome/browser/chrome_browser_main.cc

@@ -427,11 +427,11 @@ Profile* CreatePrimaryProfile(const content::MainFunctionParams& parameters,
   Profile* profile = nullptr;
 #if defined(OS_CHROMEOS) || defined(OS_ANDROID)
   // On ChromeOS and Android the ProfileManager will use the same path as the
-  // one we got passed. GetActiveUserProfile will therefore use the correct path
+  // one we got passed. CreateInitialProfile will therefore use the correct path
   // automatically.
   DCHECK_EQ(user_data_dir.value(),
             g_browser_process->profile_manager()->user_data_dir().value());
-  profile = ProfileManager::GetActiveUserProfile();
+  profile = ProfileManager::CreateInitialProfile();
 
   // TODO(port): fix this. See comments near the definition of |user_data_dir|.
   // It is better to CHECK-fail here than it is to silently exit because of

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 ProfileHelper::GetProfileByUserIdHashForTest() to
+  // do an explicit load.
+  Profile* const profile = ProfileHelper::GetProfileByUserIdHashForTest(
+      user_manager::UserManager::Get()->GetActiveUser()->username_hash());
+
   DCHECK(profile);
   return profile;
 }
@@ -169,6 +174,15 @@ PrefService* GetPrefs() {
   return GetProfile()->GetPrefs();
 }
 
+// Simulates how UserSessionManager starts a user session by loading user
+// profile and marking session as started.
+void StartUserSession(const AccountId& account_id) {
+  ProfileHelper::GetProfileByUserIdHashForTest(
+      user_manager::UserManager::Get()->FindUser(account_id)->username_hash());
+
+  session_manager::SessionManager::Get()->SessionStarted();
+}
+
 void SetLargeCursorEnabledPref(bool enabled) {
   GetPrefs()->SetBoolean(prefs::kAccessibilityLargeCursorEnabled, enabled);
 }
@@ -239,6 +253,7 @@ bool IsBrailleImeCurrent() {
   return imm->GetActiveIMEState()->GetCurrentInputMethod().id() ==
          extension_ime_util::kBrailleImeEngineId;
 }
+
 }  // anonymous namespace
 
 class AccessibilityManagerTest : public InProcessBrowserTest {
@@ -309,7 +324,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 +383,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 +483,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 +502,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 +578,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 +699,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 +723,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 +769,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());

chrome/browser/chromeos/accessibility/magnification_manager_browsertest.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/accessibility/magnification_manager.h"
 #include "chrome/browser/chromeos/login/helper.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/chrome_switches.h"
@@ -101,8 +102,19 @@ void PrepareNonNewProfile(const AccountId& account_id) {
   // To prepare a non-new profile for tests, we must ensure the profile
   // directory and the preference files are created, because that's what
   // Profile::IsNewProfile() checks. CreateSession(), however, does not yet
-  // create the profile directory until GetActiveUserProfile() is called.
-  ProfileManager::GetActiveUserProfile();
+  // create the profile directory until GetProfileByUserIdHashForTest() is
+  // called.
+  ProfileHelper::Get()->GetProfileByUserIdHashForTest(
+      account_id.GetUserEmail());
+}
+
+// Simulates how UserSessionManager starts a user session by loading user
+// profile and mark session as started.
+void StartUserSession(const AccountId& account_id) {
+  ProfileHelper::GetProfileByUserIdHashForTest(
+      user_manager::UserManager::Get()->FindUser(account_id)->username_hash());
+
+  session_manager::SessionManager::Get()->SessionStarted();
 }
 
 }  // namespace
@@ -193,7 +205,7 @@ IN_PROC_BROWSER_TEST_F(MagnificationManagerTest, LoginOffToOff) {
   // Confirms that magnifier is still disabled just after login.
   EXPECT_FALSE(IsMagnifierEnabled());
 
-  session_manager::SessionManager::Get()->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that magnifier is still disabled just after session starts.
   EXPECT_FALSE(IsMagnifierEnabled());
@@ -234,7 +246,7 @@ IN_PROC_BROWSER_TEST_F(MagnificationManagerTest, LoginFullToOff) {
   EXPECT_TRUE(IsMagnifierEnabled());
   EXPECT_EQ(ash::MAGNIFIER_FULL, GetMagnifierType());
 
-  session_manager::SessionManager::Get()->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that magnifier is disabled just after session start.
   EXPECT_FALSE(IsMagnifierEnabled());
@@ -263,7 +275,7 @@ IN_PROC_BROWSER_TEST_F(MagnificationManagerTest, LoginOffToFull) {
   // Confirms that magnifier is keeping disabled.
   EXPECT_FALSE(IsMagnifierEnabled());
 
-  session_manager::SessionManager::Get()->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that the magnifier is enabled and configured according to the
   // explicitly set prefs just after session start.
@@ -300,7 +312,7 @@ IN_PROC_BROWSER_TEST_F(MagnificationManagerTest, LoginFullToFull) {
   EXPECT_TRUE(IsMagnifierEnabled());
   EXPECT_EQ(ash::MAGNIFIER_FULL, GetMagnifierType());
 
-  session_manager::SessionManager::Get()->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that the magnifier is enabled and configured according to the
   // explicitly set prefs just after session start.
@@ -330,7 +342,7 @@ IN_PROC_BROWSER_TEST_F(MagnificationManagerTest, LoginFullToUnset) {
   EXPECT_TRUE(IsMagnifierEnabled());
   EXPECT_EQ(ash::MAGNIFIER_FULL, GetMagnifierType());
 
-  session_manager::SessionManager::Get()->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that magnifier is disabled.
   EXPECT_FALSE(IsMagnifierEnabled());
@@ -351,7 +363,7 @@ IN_PROC_BROWSER_TEST_F(MagnificationManagerTest, LoginAsNewUserOff) {
   // Confirms that magnifier is keeping disabled.
   EXPECT_FALSE(IsMagnifierEnabled());
 
-  session_manager::SessionManager::Get()->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that magnifier is keeping disabled.
   EXPECT_FALSE(IsMagnifierEnabled());
@@ -375,7 +387,7 @@ IN_PROC_BROWSER_TEST_F(MagnificationManagerTest, LoginAsNewUserFull) {
   EXPECT_TRUE(IsMagnifierEnabled());
   EXPECT_EQ(ash::MAGNIFIER_FULL, GetMagnifierType());
 
-  session_manager::SessionManager::Get()->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that magnifier keeps enabled.
   EXPECT_TRUE(IsMagnifierEnabled());
@@ -395,7 +407,7 @@ IN_PROC_BROWSER_TEST_F(MagnificationManagerTest, LoginAsNewUserUnset) {
   // Confirms that magnifier is keeping disabled.
   EXPECT_FALSE(IsMagnifierEnabled());
 
-  session_manager::SessionManager::Get()->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that magnifier is keeping disabled.
   EXPECT_FALSE(IsMagnifierEnabled());
@@ -463,7 +475,7 @@ IN_PROC_BROWSER_TEST_F(MagnificationManagerTest, TypePref) {
   // Logs in
   session_manager::SessionManager::Get()->CreateSession(test_account_id_,
                                                         kTestUserName);
-  session_manager::SessionManager::Get()->SessionStarted();
+  StartUserSession(test_account_id_);
 
   // Confirms that magnifier is disabled just after login.
   EXPECT_FALSE(IsMagnifierEnabled());

chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

@@ -615,8 +615,9 @@ class MultiProfileFileManagerBrowserTest : public FileManagerBrowserTestBase {
 
   // Returns primary profile (if it is already created.)
   Profile* profile() override {
-    Profile* const profile = chromeos::ProfileHelper::GetProfileByUserIdHash(
-        kTestAccounts[PRIMARY_ACCOUNT_INDEX].hash);
+    Profile* const profile =
+        chromeos::ProfileHelper::GetProfileByUserIdHashForTest(
+            kTestAccounts[PRIMARY_ACCOUNT_INDEX].hash);
     return profile ? profile : FileManagerBrowserTestBase::profile();
   }
 
@@ -634,7 +635,7 @@ class MultiProfileFileManagerBrowserTest : public FileManagerBrowserTestBase {
     user_manager::UserManager::Get()->SaveUserDisplayName(
         account_id, base::UTF8ToUTF16(info.display_name));
     SigninManagerFactory::GetForProfile(
-        chromeos::ProfileHelper::GetProfileByUserIdHash(info.hash))
+        chromeos::ProfileHelper::GetProfileByUserIdHashForTest(info.hash))
         ->SetAuthenticatedAccountInfo(info.gaia_id, info.email);
   }
 

chrome/browser/chromeos/file_manager/path_util_unittest.cc

@@ -100,14 +100,14 @@ TEST(FileManagerPathUtilTest, ConvertPathToArcUrl) {
 
   // Conversion of paths under the primary profile's downloads folder.
   const base::FilePath downloads = GetDownloadsFolderForProfile(
-      chromeos::ProfileHelper::Get()->GetProfileByUserIdHash(
+      chromeos::ProfileHelper::Get()->GetProfileByUserIdHashForTest(
           "user@gmail.com-hash"));
   EXPECT_TRUE(ConvertPathToArcUrl(downloads.AppendASCII("a/b/c"), &url));
   EXPECT_EQ(GURL("file:///sdcard/Download/a/b/c"), url);
 
   // Non-primary profile's downloads folder is not supported for ARC yet.
   const base::FilePath downloads2 = GetDownloadsFolderForProfile(
-      chromeos::ProfileHelper::Get()->GetProfileByUserIdHash(
+      chromeos::ProfileHelper::Get()->GetProfileByUserIdHashForTest(
           "user2@gmail.com-hash"));
   EXPECT_FALSE(ConvertPathToArcUrl(downloads2.AppendASCII("a/b/c"), &url));
 }

chrome/browser/chromeos/login/crash_restore_browsertest.cc

@@ -29,9 +29,10 @@ namespace chromeos {
 
 namespace {
 
-const char kUserId1[] = "user1@example.com";
-const char kUserId2[] = "user2@example.com";
-const char kUserId3[] = "user3@example.com";
+// Use consumer.example.com to keep policy code out of the tests.
+constexpr char kUserId1[] = "user1@consumer.example.com";
+constexpr char kUserId2[] = "user2@consumer.example.com";
+constexpr char kUserId3[] = "user3@consumer.example.com";
 
 }  // namespace
 

chrome/browser/chromeos/login/session/user_session_manager.cc

@@ -360,6 +360,13 @@ void RestartOnTimeout() {
   chrome::AttemptRestart();
 }
 
+bool IsRunningTest() {
+  return base::CommandLine::ForCurrentProcess()->HasSwitch(
+             ::switches::kTestName) ||
+         base::CommandLine::ForCurrentProcess()->HasSwitch(
+             ::switches::kTestType);
+}
+
 }  // namespace
 
 UserSessionManagerDelegate::~UserSessionManagerDelegate() {
@@ -902,15 +909,11 @@ void UserSessionManager::OnSessionRestoreStateChanged(
 
 void UserSessionManager::OnConnectionTypeChanged(
     net::NetworkChangeNotifier::ConnectionType type) {
-  bool is_running_test =
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          ::switches::kTestName) ||
-      base::CommandLine::ForCurrentProcess()->HasSwitch(::switches::kTestType);
   user_manager::UserManager* user_manager = user_manager::UserManager::Get();
   if (type == net::NetworkChangeNotifier::CONNECTION_NONE ||
       !user_manager->IsUserLoggedIn() ||
       !user_manager->IsLoggedInAsUserWithGaiaAccount() ||
-      user_manager->IsLoggedInAsStub() || is_running_test) {
+      user_manager->IsLoggedInAsStub() || IsRunningTest()) {
     return;
   }
 
@@ -942,8 +945,7 @@ void UserSessionManager::OnConnectionTypeChanged(
 
 void UserSessionManager::OnProfilePrepared(Profile* profile,
                                            bool browser_launched) {
-  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          ::switches::kTestName)) {
+  if (!IsRunningTest()) {
     // Did not log in (we crashed or are debugging), need to restore Sync.
     // TODO(nkostylev): Make sure that OAuth state is restored correctly for all
     // users once it is fully multi-profile aware. http://crbug.com/238987
@@ -1078,6 +1080,11 @@ void UserSessionManager::InitProfilePreferences(
       const AccountInfo info = account_tracker->FindAccountInfoByEmail(
           user_context.GetAccountId().GetUserEmail());
       gaia_id = info.gaia;
+
+      // Use a fake gaia id for tests that do not have it.
+      if (IsRunningTest() && gaia_id.empty())
+        gaia_id = "fake_gaia_id_" + user_context.GetAccountId().GetUserEmail();
+
       DCHECK(!gaia_id.empty());
     }
 

chrome/browser/chromeos/login/users/user_manager_unittest.cc

@@ -227,7 +227,7 @@ TEST_F(UserManagerTest, ScreenLockAvailability) {
   user_manager::User* const user =
       user_manager::UserManager::Get()->GetActiveUser();
   Profile* const profile =
-      ProfileHelper::GetProfileByUserIdHash(user->username_hash());
+      ProfileHelper::GetProfileByUserIdHashForTest(user->username_hash());
 
   // Verify that the user is allowed to lock the screen.
   EXPECT_TRUE(user_manager::UserManager::Get()->CanCurrentUserLock());

chrome/browser/chromeos/profiles/profile_helper.cc

@@ -64,6 +64,13 @@ class UsernameHashMatcher {
   const std::string& username_hash;
 };
 
+// Internal helper to get an already-loaded user profile by user id hash. Return
+// nullptr if the user profile is not yet loaded.
+Profile* GetProfileByUserIdHash(const std::string& user_id_hash) {
+  return g_browser_process->profile_manager()->GetProfileByPath(
+      ProfileHelper::GetProfilePathByUserIdHash(user_id_hash));
+}
+
 }  // anonymous namespace
 
 // static
@@ -93,10 +100,10 @@ ProfileHelper* ProfileHelper::Get() {
 }
 
 // static
-Profile* ProfileHelper::GetProfileByUserIdHash(
+Profile* ProfileHelper::GetProfileByUserIdHashForTest(
     const std::string& user_id_hash) {
-  ProfileManager* profile_manager = g_browser_process->profile_manager();
-  return profile_manager->GetProfile(GetProfilePathByUserIdHash(user_id_hash));
+  return g_browser_process->profile_manager()->GetProfile(
+      ProfileHelper::GetProfilePathByUserIdHash(user_id_hash));
 }
 
 // static
@@ -292,8 +299,7 @@ Profile* ProfileHelper::GetProfileByUser(const user_manager::User* user) {
 
   if (!user->is_profile_created())
     return NULL;
-  Profile* profile =
-      ProfileHelper::GetProfileByUserIdHash(user->username_hash());
+  Profile* profile = GetProfileByUserIdHash(user->username_hash());
 
   // GetActiveUserProfile() or GetProfileByUserIdHash() returns a new instance
   // of ProfileImpl(), but actually its OffTheRecordProfile() should be used.
@@ -313,7 +319,7 @@ Profile* ProfileHelper::GetProfileByUserUnsafe(const user_manager::User* user) {
 
   Profile* profile = NULL;
   if (user->is_profile_created()) {
-    profile = ProfileHelper::GetProfileByUserIdHash(user->username_hash());
+    profile = GetProfileByUserIdHash(user->username_hash());
   } else {
     LOG(ERROR) << "ProfileHelper::GetProfileByUserUnsafe is called when "
                   "|user|'s profile is not created. It probably means that "

chrome/browser/chromeos/profiles/profile_helper.h

@@ -76,8 +76,13 @@ class ProfileHelper
   // knowledge in one place.
   static ProfileHelper* Get();
 
-  // Returns Profile instance that corresponds to |user_id_hash|.
-  static Profile* GetProfileByUserIdHash(const std::string& user_id_hash);
+  // Loads and returns Profile instance that corresponds to |user_id_hash| for
+  // test. It should not be used in production code because it could load a
+  // not-yet-loaded user profile and skip the user profile initialization code
+  // in UserSessionManager.
+  // See http://crbug.com/728683 and http://crbug.com/718734.
+  static Profile* GetProfileByUserIdHashForTest(
+      const std::string& user_id_hash);
 
   // Returns profile path that corresponds to a given |user_id_hash|.
   static base::FilePath GetProfilePathByUserIdHash(

chrome/browser/chromeos/status/data_promo_notification_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/run_loop.h"
 #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "chromeos/chromeos_switches.h"
@@ -103,6 +104,11 @@ class DataPromoNotificationTest : public testing::Test {
         new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
     ASSERT_TRUE(profile_manager_->SetUp());
     profile_manager_->SetLoggedIn(true);
+
+    ProfileHelper::GetProfileByUserIdHashForTest(
+        ProfileHelper::GetUserIdHashByUserIdForTesting(
+            test_account_id.GetUserEmail()));
+
     ASSERT_TRUE(user_manager::UserManager::Get()->GetPrimaryUser());
 
     LoginState::Initialize();

chrome/browser/download/notification/download_notification_browsertest.cc

@@ -1176,7 +1176,7 @@ class MultiProfileDownloadNotificationTest
   }
 
   Profile* GetProfileByIndex(int index) {
-    return chromeos::ProfileHelper::GetProfileByUserIdHash(
+    return chromeos::ProfileHelper::GetProfileByUserIdHashForTest(
         kTestAccounts[index].hash);
   }
 
@@ -1190,8 +1190,8 @@ class MultiProfileDownloadNotificationTest
         AccountId::FromUserEmailGaiaId(info.email, info.gaia_id),
         base::UTF8ToUTF16(info.display_name));
     SigninManagerFactory::GetForProfile(
-        chromeos::ProfileHelper::GetProfileByUserIdHash(info.hash))
-            ->SetAuthenticatedAccountInfo(info.gaia_id, info.email);
+        chromeos::ProfileHelper::GetProfileByUserIdHashForTest(info.hash))
+        ->SetAuthenticatedAccountInfo(info.gaia_id, info.email);
   }
 };
 

chrome/browser/extensions/api/braille_display_private/braille_display_private_apitest.cc

@@ -7,6 +7,7 @@
 #include <deque>
 
 #include "base/bind.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/login/lock/screen_locker.h"
@@ -336,6 +337,8 @@ IN_PROC_BROWSER_TEST_F(BrailleDisplayPrivateAPIUserTest,
   // Log in.
   session_manager::SessionManager::Get()->CreateSession(
       AccountId::FromUserEmail(kTestUserName), kTestUserName);
+  g_browser_process->profile_manager()->GetProfile(
+      ProfileHelper::Get()->GetProfilePathByUserIdHash(kTestUserName));
   session_manager::SessionManager::Get()->SessionStarted();
   Profile* profile = ProfileManager::GetActiveUserProfile();
   ASSERT_FALSE(

chrome/browser/profiles/profile_manager.cc

@@ -487,6 +487,13 @@ Profile* ProfileManager::GetActiveUserProfile() {
   return profile;
 }
 
+// static
+Profile* ProfileManager::CreateInitialProfile() {
+  ProfileManager* const profile_manager = g_browser_process->profile_manager();
+  return profile_manager->GetProfile(profile_manager->user_data_dir().Append(
+      profile_manager->GetInitialProfileDir()));
+}
+
 Profile* ProfileManager::GetProfile(const base::FilePath& profile_dir) {
   TRACE_EVENT0("browser", "ProfileManager::GetProfile");
 
@@ -638,8 +645,8 @@ Profile* ProfileManager::GetLastUsedProfile(
     // If we get here, it means the user has logged in but the profile has not
     // finished initializing, so treat the user as not having logged in.
     if (!profile) {
-      DLOG(WARNING) << "Calling GetLastUsedProfile() before profile "
-                    << "initialization is completed. Returning login profile.";
+      LOG(WARNING) << "Calling GetLastUsedProfile() before profile "
+                   << "initialization is completed. Returning login profile.";
       return GetActiveUserOrOffTheRecordProfileFromPath(user_data_dir);
     }
     return profile->IsGuestSession() ? profile->GetOffTheRecordProfile() :
@@ -1322,9 +1329,9 @@ Profile* ProfileManager::GetActiveUserOrOffTheRecordProfileFromPath(
 
   default_profile_dir = default_profile_dir.Append(GetInitialProfileDir());
   ProfileInfo* profile_info = GetProfileInfoByPath(default_profile_dir);
-  // Fallback to default off-the-record profile, if user profile has not fully
-  // loaded yet.
-  if (profile_info && !profile_info->created)
+  // Fallback to default off-the-record profile, if user profile has not started
+  // loading or has not fully loaded yet.
+  if (!profile_info || !profile_info->created)
     default_profile_dir = profiles::GetDefaultProfileDir(user_data_dir);
 
   Profile* profile = GetProfile(default_profile_dir);

chrome/browser/profiles/profile_manager.h

@@ -80,6 +80,12 @@ class ProfileManager : public content::NotificationObserver,
   // TODO(skuhne): Move into ash's new user management function.
   static Profile* GetActiveUserProfile();
 
+  // Load and return the initial profile for browser. On ChromeOS, this returns
+  // either the sign-in profile or the active user profile depending on whether
+  // browser is started normally or is restarted after crash. On other
+  // platforms, this returns the default profile.
+  static Profile* CreateInitialProfile();
+
   // Returns a profile for a specific profile directory within the user data
   // dir. This will return an existing profile it had already been created,
   // otherwise it will create and manage it.

chrome/browser/profiles/profile_manager_unittest.cc

@@ -290,6 +290,54 @@ TEST_F(ProfileManagerTest, LoggedInProfileDir) {
                  .value();
 }
 
+// Test Get[ActiveUser|PrimaryUser|LastUsed]Profile does not load user profile.
+TEST_F(ProfileManagerTest, UserProfileLoading) {
+  using chromeos::ProfileHelper;
+
+  Profile* const signin_profile = ProfileHelper::GetSigninProfile();
+
+  // Get[Active|Primary|LastUsed]Profile return the sign-in profile before login
+  // happens. IsSameProfile() is used to properly test against TestProfile whose
+  // OTR version uses a different temp path.
+  EXPECT_TRUE(
+      ProfileManager::GetActiveUserProfile()->IsSameProfile(signin_profile));
+  EXPECT_TRUE(
+      ProfileManager::GetPrimaryUserProfile()->IsSameProfile(signin_profile));
+  EXPECT_TRUE(
+      ProfileManager::GetLastUsedProfile()->IsSameProfile(signin_profile));
+
+  // User signs in but user profile loading has not started.
+  const std::string user_id = "test-user@example.com";
+  const std::string user_id_hash =
+      ProfileHelper::Get()->GetUserIdHashByUserIdForTesting(user_id);
+  user_manager::UserManager::Get()->UserLoggedIn(
+      AccountId::FromUserEmail(user_id), user_id_hash, false);
+
+  // Sign-in profile should be returned at this stage. Otherwise, login code
+  // ends up in an invalid state. Strange things as in http://crbug.com/728683
+  // and http://crbug.com/718734 happens.
+  EXPECT_TRUE(
+      ProfileManager::GetActiveUserProfile()->IsSameProfile(signin_profile));
+  EXPECT_TRUE(
+      ProfileManager::GetPrimaryUserProfile()->IsSameProfile(signin_profile));
+  EXPECT_TRUE(
+      ProfileManager::GetLastUsedProfile()->IsSameProfile(signin_profile));
+
+  // Simulate UserSessionManager loads the profile.
+  Profile* const user_profile =
+      g_browser_process->profile_manager()->GetProfile(
+          ProfileHelper::Get()->GetProfilePathByUserIdHash(user_id_hash));
+  ASSERT_FALSE(user_profile->IsSameProfile(signin_profile));
+
+  // User profile is returned thereafter.
+  EXPECT_TRUE(
+      ProfileManager::GetActiveUserProfile()->IsSameProfile(user_profile));
+  EXPECT_TRUE(
+      ProfileManager::GetPrimaryUserProfile()->IsSameProfile(user_profile));
+  EXPECT_TRUE(
+      ProfileManager::GetLastUsedProfile()->IsSameProfile(user_profile));
+}
+
 #endif
 
 TEST_F(ProfileManagerTest, CreateAndUseTwoProfiles) {
@@ -677,7 +725,7 @@ TEST_F(ProfileManagerTest, GetLastUsedProfileAllowedByPolicy) {
   ASSERT_TRUE(profile_manager);
 
 #if defined(OS_CHROMEOS)
-  // On CrOS, profile returned by GetLastUsedProfile is a singin profile that
+  // On CrOS, profile returned by GetLastUsedProfile is a sign-in profile that
   // is forced to be incognito. That's why we need to create at least one user
   // to get a regular profile.
   RegisterUser("test-user@example.com");

chrome/browser/ui/ash/chrome_new_window_client_browsertest.cc

@@ -5,6 +5,7 @@
 #include "ash/new_window_controller.h"
 #include "ash/shell.h"
 #include "ash/wm/window_util.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -19,6 +20,17 @@ namespace {
 const char kTestUserName1[] = "test1@test.com";
 const char kTestUserName2[] = "test2@test.com";
 
+void CreateAndStartUserSession(const AccountId& account_id) {
+  using chromeos::ProfileHelper;
+  using session_manager::SessionManager;
+
+  const std::string user_id_hash =
+      ProfileHelper::GetUserIdHashByUserIdForTesting(account_id.GetUserEmail());
+  SessionManager::Get()->CreateSession(account_id, user_id_hash);
+  ProfileHelper::GetProfileByUserIdHashForTest(user_id_hash);
+  SessionManager::Get()->SessionStarted();
+}
+
 }  // namespace
 
 using ChromeNewWindowClientBrowserTest = InProcessBrowserTest;
@@ -28,8 +40,7 @@ using ChromeNewWindowClientBrowserTest = InProcessBrowserTest;
 // should open a new window.
 IN_PROC_BROWSER_TEST_F(ChromeNewWindowClientBrowserTest,
                        NewWindowForActiveWindowProfileTest) {
-  session_manager::SessionManager::Get()->CreateSession(
-      AccountId::FromUserEmail(kTestUserName1), kTestUserName1);
+  CreateAndStartUserSession(AccountId::FromUserEmail(kTestUserName1));
   Profile* profile1 = ProfileManager::GetActiveUserProfile();
   Browser* browser1 = CreateBrowser(profile1);
   // The newly created window should be created for the current active profile.
@@ -39,8 +50,7 @@ IN_PROC_BROWSER_TEST_F(ChromeNewWindowClientBrowserTest,
       profile1);
 
   // Login another user and make sure the current active user changes.
-  session_manager::SessionManager::Get()->CreateSession(
-      AccountId::FromUserEmail(kTestUserName2), kTestUserName2);
+  CreateAndStartUserSession(AccountId::FromUserEmail(kTestUserName2));
   Profile* profile2 = ProfileManager::GetActiveUserProfile();
   EXPECT_NE(profile1, profile2);