Skip profiles in GetLastOpenedProfiles that fail to initialize

chrome/browser/profiles/profile_manager.h

 // 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.
 
 // This class keeps track of the currently-active profiles in the runtime.
 
 #ifndef CHROME_BROWSER_PROFILES_PROFILE_MANAGER_H_
 #define CHROME_BROWSER_PROFILES_PROFILE_MANAGER_H_
 
 #include <stddef.h>
@@ skipping 68 matching lines @@
   // This function is temporary and will soon be moved to ash. As such avoid
   // using it at all cost.
   // TODO(skuhne): Move into ash's new user management function.
   static Profile* GetActiveUserProfile();
 
   // 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.
   // 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);
 
   // Returns total number of profiles available on this machine.
   size_t GetNumberOfProfiles();
 
   // Asynchronously loads an existing profile given its |profile_name| within
   // the user data directory, optionally in |incognito| mode. The |callback|
   // will be called with the Profile when it has been loaded, or with a nullptr
@@ skipping 33 matching lines @@
   base::FilePath GetLastUsedProfileDir(const base::FilePath& user_data_dir);
 
   // Get the name of the last used profile, or if that's undefined, the default
   // profile.
   std::string GetLastUsedProfileName();
 
   // Get the Profiles which are currently open, i.e., have open browsers, or
   // 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);
 
   // Returns created and fully initialized profiles. Note, profiles order is NOT
   // guaranteed to be related with the creation order.
   std::vector<Profile*> GetLoadedProfiles() const;
 
   // If a profile with the given path is currently managed by this object and
   // fully initialized, return a pointer to the corresponding Profile object;
   // otherwise return null.
@@ skipping 91 matching lines @@
                         bool is_new_profile) override;
 
  protected:
   // Does final initial actions.
   virtual void DoFinalInit(Profile* profile, bool go_off_the_record);
   virtual void DoFinalInitForServices(Profile* profile, bool go_off_the_record);
   virtual void DoFinalInitLogging(Profile* profile);
 
   // 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
   // asynchronous profile creation method. Virtual so that unittests can return
   // a TestingProfile instead of the Profile's result.
   virtual Profile* CreateProfileAsyncHelper(const base::FilePath& path,
                                             Delegate* delegate);
 
  private:
   friend class TestingProfileManager;
@@ skipping 26 matching lines @@
   Profile* GetActiveUserOrOffTheRecordProfileFromPath(
       const base::FilePath& user_data_dir);
 
   // Adds a pre-existing Profile object to the set managed by this
   // ProfileManager. This ProfileManager takes ownership of the Profile.
   // The Profile should not already be managed by this ProfileManager.
   // Returns true if the profile was added, false otherwise.
   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)
   // Schedules the profile at the given path to be deleted on shutdown,
   // and marks the new profile as active.
   void FinishDeletingProfile(const base::FilePath& profile_dir,
                              const base::FilePath& new_active_profile_dir);
 #endif
 
   // Registers profile with given info. Returns pointer to created ProfileInfo
@@ skipping 104 matching lines @@
 class ProfileManagerWithoutInit : public ProfileManager {
  public:
   explicit ProfileManagerWithoutInit(const base::FilePath& user_data_dir);
 
  protected:
   void DoFinalInitForServices(Profile*, bool) override {}
   void DoFinalInitLogging(Profile*) override {}
 };
 
 #endif  // CHROME_BROWSER_PROFILES_PROFILE_MANAGER_H_