cros: Fix loading user profile w/o UserSessionManager

chrome/browser/profiles/profile_manager.cc

Index: chrome/browser/profiles/profile_manager.cc
diff --git a/chrome/browser/profiles/profile_manager.cc b/chrome/browser/profiles/profile_manager.cc
index 5f2317b7c992597829e7e6a8f91f739b4878e8ca..ca6cf25f02d19c5792d2b8fef8b0dd840095df47 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -633,8 +633,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);

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

Not insisting (I'm not aware of the general guideline regarding this), but we
could add DumpWithoutCrashing() here and start collecting the data without
disrupting end users / developers with browser crashes.

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

I will fix the offending code and change this to a LOG(FATAL) in a follow up CL.
I intend to merge this CL back to M60 so do not want to have it here.

     }
     return profile->IsGuestSession() ? profile->GetOffTheRecordProfile() :
@@ -1326,12 +1326,21 @@ Profile* ProfileManager::GetActiveUserOrOffTheRecordProfileFromPath(
     return profile;
   }
 
+  // Only allow user profile loading on browser restart, where kLoginUser
+  // switch is present in the command line. For all other cases, the user
+  // profile loading should be from UserSessionManager.
+  const bool allow_user_profile_loading =

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

I'm a bit unhappy with keeping the lazy profile loading here even when limited
to this sole case - this means the improvement only to the non-restarting half
of all cases. Also this complicates the behavior of GetActiveUserProfile() even
more than before.

Couldn't we load the active profile explicitly somewhere in the browser restart
code? For instance, one place for this could be CreatePrimaryProfile() in
chrome_browser_main.cc, where it currently loads the profile by calling
GetActiveUserProfile().

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

Added ProfileManager::CreateInitialProfile() and use it in
CreatePrimaryProfile(). And the |allow_user_profile_loading| check is removed
since CreateInitialProfile() loads the profile explicitly.

+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          chromeos::switches::kLoginUser);
+
   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 is not loaded
+  // and should not be loaded or has not fully loaded yet.
+  if ((!allow_user_profile_loading && !profile_info) ||
+      (profile_info && !profile_info->created)) {
     default_profile_dir = profiles::GetDefaultProfileDir(user_data_dir);
+  }
 
   Profile* profile = GetProfile(default_profile_dir);
   // Some unit tests didn't initialize the UserManager.