cros: Fix loading user profile w/o UserSessionManager

chrome/browser/profiles/profile_manager.cc

 // Copyright (c) 2012 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/profiles/profile_manager.h"
 
 #include <stdint.h>
 
 #include <map>
 #include <set>
@@ skipping 615 matching lines @@
     // since it may refer to profile that has been in use in previous session.
     // That profile dir may not be mounted in this session so instead return
     // active profile from current session.
     profile_dir = chromeos::ProfileHelper::Get()->GetActiveUserProfileDir();
 
     base::FilePath profile_path(user_data_dir);
     Profile* profile = GetProfileByPath(profile_path.Append(profile_dir));
     // 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() :
                                        profile;
   }
 #else
 
   return GetProfile(GetLastUsedProfileDir(user_data_dir));
 #endif
 }
 
@@ skipping 670 matching lines @@
     // For cros, return the OTR profile so we never accidentally keep
     // user data in an unencrypted profile. But doing this makes
     // many of the browser and ui tests fail. We do return the OTR profile
     // if the login-profile switch is passed so that we can test this.
     if (ShouldGoOffTheRecord(profile))
       return profile->GetOffTheRecordProfile();
     DCHECK(!user_manager::UserManager::Get()->IsLoggedInAsGuest());
     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.
   if (user_manager::UserManager::IsInitialized() &&
       user_manager::UserManager::Get()->IsLoggedInAsGuest())
     return profile->GetOffTheRecordProfile();
   return profile;
 #else
   base::FilePath default_profile_dir(user_data_dir);
   default_profile_dir = default_profile_dir.Append(GetInitialProfileDir());
@@ skipping 436 matching lines @@
 
   const base::FilePath new_active_profile_dir =
       found_entry ? found_entry->GetPath() : GenerateNextProfileDirectoryPath();
   FinishDeletingProfile(profile_dir, new_active_profile_dir);
 }
 #endif  // !defined(OS_ANDROID)
 
 ProfileManagerWithoutInit::ProfileManagerWithoutInit(
     const base::FilePath& user_data_dir) : ProfileManager(user_data_dir) {
 }