Add fallback behavior if the last used profile cannot initialize

chrome/browser/chrome_browser_main.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/chrome_browser_main.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <set>
@@ skipping 72 matching lines @@
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/tracing/navigation_tracing.h"
 #include "chrome/browser/translate/translate_service.h"
 #include "chrome/browser/ui/app_list/app_list_service.h"
 #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"
 #include "chrome/browser/ui/uma_browsing_activity_observer.h"
 #include "chrome/browser/ui/webui/chrome_web_ui_controller_factory.h"
 #include "chrome/common/channel_info.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_result_codes.h"
@@ skipping 279 matching lines @@
     profiles::SetLastUsedProfile(
         parsed_command_line.GetSwitchValueASCII(switches::kProfileDirectory));
     // Clear kProfilesLastActive since the user only wants to launch a specific
     // profile.
     ListPrefUpdate update(g_browser_process->local_state(),
                           prefs::kProfilesLastActive);
     base::ListValue* profile_list = update.Get();
     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
   // automatically.
   DCHECK_EQ(user_data_dir.value(),
             g_browser_process->profile_manager()->user_data_dir().value());
   profile = ProfileManager::GetActiveUserProfile();
 #else
   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(
         "Startup.CreateFirstProfile", base::Time::Now() - start);
     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)
 OSStatus KeychainCallback(SecKeychainEvent keychain_event,
                           SecKeychainCallbackInfo* info, void* context) {
   return noErr;
 }
 #endif  // defined(OS_MACOSX)
 
 void RegisterComponentsForUpdate() {
@@ skipping 1545 matching lines @@
   chromeos::CrosSettings::Shutdown();
 #endif  // defined(OS_CHROMEOS)
 #endif  // defined(OS_ANDROID)
 }
 
 // Public members:
 
 void ChromeBrowserMainParts::AddParts(ChromeBrowserMainExtraParts* parts) {
   chrome_extra_parts_.push_back(parts);
 }