Add fallback behavior if the last used profile cannot initialize

chrome/browser/chrome_browser_main.cc

Index: chrome/browser/chrome_browser_main.cc
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index c496ffe34032dc98f91d26d70326ab24f6248eb8..4978a45e85e727ec24d72190a5805dba304194ff 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -90,6 +90,8 @@
 #include "chrome/browser/ui/app_modal/chrome_javascript_native_dialog_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/profile_error_dialog.h"
+#include "chrome/browser/ui/simple_message_box.h"
 #include "chrome/browser/ui/startup/bad_flags_prompt.h"
 #include "chrome/browser/ui/startup/default_browser_prompt.h"
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
@@ -389,7 +391,7 @@ Profile* CreatePrimaryProfile(const content::MainFunctionParams& parameters,
     profile_list->Clear();
   }
 
-  Profile* profile = NULL;
+  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
@@ -401,25 +403,87 @@ Profile* CreatePrimaryProfile(const content::MainFunctionParams& parameters,
   base::FilePath profile_path =
       GetStartupProfilePath(user_data_dir, parsed_command_line);
 
-  profile = g_browser_process->profile_manager()->GetProfile(
-      profile_path);
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  profile = profile_manager->GetProfile(profile_path);
 
-  // If we're using the --new-profile-management flag and this profile is
-  // signed out, then we should show the user manager instead. By switching
-  // the active profile to the guest profile we ensure that no
-  // browser windows will be opened for the guest profile.
+  // If we're using the --new-profile-management flag and this profile is signed
+  // out, then we should show the user manager instead. By switching the active
+  // profile to the guest profile we ensure that no browser windows will be
+  // opened for the guest profile. The initialization of guest profile is
+  // possible to fail.
   if (switches::IsNewProfileManagement() &&
       profile &&
       !profile->IsGuestSession()) {
     ProfileAttributesEntry* entry;
-    bool has_entry = g_browser_process->profile_manager()->
-                         GetProfileAttributesStorage().
-                         GetProfileAttributesWithPath(profile_path, &entry);
+    bool has_entry = g_browser_process->profile_manager()
+                         ->GetProfileAttributesStorage()
+                         .GetProfileAttributesWithPath(profile_path, &entry);
     if (has_entry && entry->IsSigninRequired()) {
-      profile = g_browser_process->profile_manager()->GetProfile(
-          ProfileManager::GetGuestProfilePath());
+      profile = profile_manager->GetProfile(
+                                     ProfileManager::GetGuestProfilePath());
     }
   }
+
+  // If the user specified a profile and it fails to initialize, do not start
+  // chromium.
+  if (profiles::IsMultipleProfilesEnabled() &&
+      parsed_command_line.HasSwitch(switches::kProfileDirectory) &&
+      !profile) {
+    ShowProfileErrorDialog(PROFILE_ERROR_CREATE_FAILURE_SPECIFIED,
+                           IDS_COULDNT_OPEN_SPECIFIED_PROFILE_ERROR);
+    return nullptr;
+  }
+
+  // If the last used profile is unable to initialize, see if any of other last

[brettw, 2016/06/14 00:02:07]

Can the code added from here be moved to a helper function "PickFallbackProfile"
(or something) to avoid making this existing function so large? Potentially then
you could factor it so that the error dialog code only happens once (you
conditionally call the fallback when no profile is explicitly specified, then
unconditionally fail at the end if profile is still null).

[WC Leung, 2016/06/15 17:44:08]

SGTM. Agree that the current function is too long, so I've refactored it a
little bit (now some code is moved to startup_browser_creator.cc). Now the code
should be much more readable!

+  // opened profiles can initialize successfully. At this stage we assume
+  // creation of all new profile (including guest profile) to fail.
+  if (!profile) {
+    std::vector<Profile*> last_opened_profiles =
+        ProfileManager::GetLastOpenedProfiles();
+    // Get the first profile that is not signed out..

[brettw, 2016/06/14 00:02:07]

nit: remove extra "."

[WC Leung, 2016/06/15 17:44:08]

Oops. Thanks for catching. Done.

+    if (last_opened_profiles.size()) {
+      for (Profile* last_opened_profile : last_opened_profiles) {
+        ProfileAttributesEntry* entry;
+        bool has_entry =
+            g_browser_process->profile_manager()
+                             ->GetProfileAttributesStorage()
+                             .GetProfileAttributesWithPath(profile->GetPath(),

[WC Leung, 2016/06/15 17:44:08]

profile should be last_opened_profile instead. Oops! Thanks for the refactoring
the bug was spotted easily with shorter functions.

+                                                           &entry);
+        if (!has_entry || !entry->IsSigninRequired()) {
+          profile = last_opened_profile;
+          break;
+        }
+      }
+    }
+  }
+
+  // If it still fails, try to create a guest profile. Show the user manager
+  // if this is successful.
+  if (!profile) {
+    profile = profile_manager->GetProfile(
+                                   ProfileManager::GetGuestProfilePath());
+  }
+
+  // If still fails, try to open any non-signin profile.
+  if (!profile) {
+    std::vector<ProfileAttributesEntry*> entries =
+        profile_manager->GetProfileAttributesStorage()
+                       .GetAllProfilesAttributes();
+    for (ProfileAttributesEntry* entry : entries) {
+      if (!entry->IsSigninRequired()) {
+        profile = profile_manager->GetProfile(entry->GetPath());
+        if (profile)
+          break;
+      }
+    }
+  }
+
+  // If it continues to fail, we have to show an error message and give up.
+  if (!profile) {
+    ShowProfileErrorDialog(PROFILE_ERROR_CREATE_FAILURE_ALL,
+                           IDS_COULDNT_OPEN_ANY_PROFILE_ERROR);
+    return nullptr;
+  }
 #endif  // defined(OS_CHROMEOS) || defined(OS_ANDROID)
   if (profile) {
     UMA_HISTOGRAM_LONG_TIMES(
@@ -427,14 +491,12 @@ Profile* CreatePrimaryProfile(const content::MainFunctionParams& parameters,
     return profile;
   }
 
-#if !defined(OS_WIN)
   // 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 missing code in the above test.
   CHECK(profile) << "Cannot get default profile.";
-#endif  // !defined(OS_WIN)
 
-  return NULL;
+  return nullptr;
 }
 
 #if defined(OS_MACOSX)