Skip profiles in GetLastOpenedProfiles that fail to initialize

chrome/browser/profiles/profile_manager.h

Index: chrome/browser/profiles/profile_manager.h
diff --git a/chrome/browser/profiles/profile_manager.h b/chrome/browser/profiles/profile_manager.h
index af7e34bd42c7589fc0c5eea69c7338142fe0ae70..7d1aa743b46913e5e4528dc028b56b8cad173dbb 100644
--- a/chrome/browser/profiles/profile_manager.h
+++ b/chrome/browser/profiles/profile_manager.h
@@ -86,7 +86,9 @@ class ProfileManager : public base::NonThreadSafe,
   // otherwise it will create and manage it.
   // Because this method might synchronously create a new profile, it should
   // only be called for the initial profile or in tests, where blocking is
-  // acceptable.
+  // acceptable. Returns nullptr if creation of the new profile fails.
+  // Callsites should always check for nullptr return before dereferencing. See
+  // crbug.com/383019 and crbug.com/614753 for related crashes.

[Peter Kasting, 2016/05/31 19:12:30]

Nit: This last sentence is unnecessary given that your previous sentence
explicitly says the function can return null.  As I noted before, I'm not a big
fan of bug links in comments, and it doesn't seem like "yes, there have been
crashes in the past" is something so important we need to link to the bugs, so
I'd remove this.

Super picky: I'd also use the word "null" in place of "nullptr" just because it
reads more nicely, here and in the other places you do this below.

[WC Leung, 2016/06/01 18:56:55]

Done. Anyway, I'll likely be as picky as you if I've got a chance to work in
Google.

   // TODO(bauerb): Migrate calls from other code to GetProfileByPath(), then
   // make this method private.
   Profile* GetProfile(const base::FilePath& profile_dir);
@@ -140,7 +142,8 @@ class ProfileManager : public base::NonThreadSafe,
   // were open the last time Chrome was running. The Profiles appear in the
   // order they were opened. The last used profile will be on the list, but its
   // index on the list will depend on when it was opened (it is not necessarily
-  // the last one).
+  // the last one). However profiles that fails to initialize (e.g. due to disk
+  // errors) are skipped.

[Peter Kasting, 2016/05/31 19:12:30]

My comments on this sentence from the last patch still apply.

[WC Leung, 2016/06/01 18:56:55]

Can't find the comment in the last patch. Are you referring to the comment in
another file?

[Peter Kasting, 2016/06/01 19:36:17]

Don't worry, your newest version of the comment is good.

   std::vector<Profile*> GetLastOpenedProfiles(
       const base::FilePath& user_data_dir);
 
@@ -252,7 +255,7 @@ class ProfileManager : public base::NonThreadSafe,
 
   // Creates a new profile by calling into the profile's profile creation
   // method. Virtual so that unittests can return a TestingProfile instead
-  // of the Profile's result.
+  // of the Profile's result. Returns nullptr if creation fails.
   virtual Profile* CreateProfileHelper(const base::FilePath& path);
 
   // Creates a new profile asynchronously by calling into the profile's
@@ -299,7 +302,8 @@ class ProfileManager : public base::NonThreadSafe,
   bool AddProfile(Profile* profile);
 
   // Synchronously creates and returns a profile. This handles both the full
-  // creation and adds it to the set managed by this ProfileManager.
+  // creation and adds it to the set managed by this ProfileManager. Returns
+  // nullptr when fails.

[Peter Kasting, 2016/05/31 19:12:30]

Nit: when -> if creation

   Profile* CreateAndInitializeProfile(const base::FilePath& profile_dir);
 
 #if !defined(OS_ANDROID)