Pass errors generated during limited-user registration up to the UI for display.

chrome/browser/ui/webui/options/browser_options_handler.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/ui/webui/options/browser_options_handler.h"
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/memory/singleton.h"
+#include "base/metrics/histogram.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "base/value_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/auto_launch_trial.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_page_zoom.h"
 #include "chrome/browser/custom_home_pages_table_model.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/gpu/gpu_mode_manager.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/printing/cloud_print/cloud_print_proxy_service.h"
 #include "chrome/browser/printing/cloud_print/cloud_print_proxy_service_factory.h"
 #include "chrome/browser/printing/cloud_print/cloud_print_setup_flow.h"
 #include "chrome/browser/printing/cloud_print/cloud_print_url.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_info_util.h"
+#include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/profiles/profile_shortcut_manager.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/service/service_process_control.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
@@ skipping 92 matching lines @@
 }
 
 void CreateDesktopShortcutForProfile(Profile* profile,
                                      Profile::CreateStatus status) {
   ProfileShortcutManager* shortcut_manager =
       g_browser_process->profile_manager()->profile_shortcut_manager();
   if (shortcut_manager)
     shortcut_manager->CreateProfileShortcut(profile->GetPath());
 }
 
+void OnProfileCreatedCallCallbacks(

[Bernhard Bauer, 2013/06/03 10:52:47]

Naming nit: Name this either based on what it does (RunProfileCreationCallbacks)
or on when it's called (OnProfileCreated), but not both? Hm, seeing as there are
too many OnProfileCreated events anyway, probably the latter?

+    const std::vector<ProfileManager::CreateCallback>& callbacks,
+    Profile* profile,
+    Profile::CreateStatus status) {
+  std::vector<ProfileManager::CreateCallback>::const_iterator it;
+  for (it = callbacks.begin(); it != callbacks.end(); ++it) {
+    it->Run(profile, status);
+  }
+}
+
+void OpenNewWindowForProfile(
+    chrome::HostDesktopType desktop_type,
+    Profile* profile,
+    Profile::CreateStatus status) {
+  if (status == Profile::CREATE_STATUS_INITIALIZED) {

[Bernhard Bauer, 2013/06/03 10:52:47]

Early return on other status?

+    ProfileManager::FindOrCreateNewWindowForProfile(
+      profile,
+      chrome::startup::IS_PROCESS_STARTUP,
+      chrome::startup::IS_FIRST_RUN,
+      desktop_type,
+      false);
+  }
+}
+
 }  // namespace
 
 BrowserOptionsHandler::BrowserOptionsHandler()
     : page_initialized_(false),
       template_url_service_(NULL),
       weak_ptr_factory_(this) {
 #if !defined(OS_MACOSX)
   default_browser_worker_ = new ShellIntegration::DefaultBrowserWorker(this);
 #endif
 #if(!defined(GOOGLE_CHROME_BUILD) && defined(OS_WIN))
@@ skipping 373 matching lines @@
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "setDefaultSearchEngine",
       base::Bind(&BrowserOptionsHandler::SetDefaultSearchEngine,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "createProfile",
       base::Bind(&BrowserOptionsHandler::CreateProfile,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
+      "deleteProfile",
+      base::Bind(&BrowserOptionsHandler::DeleteProfile,
+                 base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
       "themesReset",
       base::Bind(&BrowserOptionsHandler::ThemesReset,
                  base::Unretained(this)));
 #if defined(TOOLKIT_GTK)
   web_ui()->RegisterMessageCallback(
       "themesSetGTK",
       base::Bind(&BrowserOptionsHandler::ThemesSetGTK,
                  base::Unretained(this)));
 #endif
   web_ui()->RegisterMessageCallback(
@@ skipping 530 matching lines @@
     return;
 
   Browser* browser =
       chrome::FindBrowserWithWebContents(web_ui()->GetWebContents());
   chrome::HostDesktopType desktop_type = chrome::HOST_DESKTOP_TYPE_NATIVE;
   if (browser)
     desktop_type = browser->host_desktop_type();
 
   string16 name;
   string16 icon;
-  std::vector<ProfileManager::CreateCallback> callbacks;
   bool create_shortcut = false;
   bool managed_user = false;
   if (args->GetString(0, &name) && args->GetString(1, &icon)) {
     if (args->GetBoolean(2, &create_shortcut)) {
       bool success = args->GetBoolean(3, &managed_user);
       DCHECK(success);
     }
   }
 
+  std::vector<ProfileManager::CreateCallback> callbacks;
   if (create_shortcut)
     callbacks.push_back(base::Bind(&CreateDesktopShortcutForProfile));
 
+  ProfileManager::CreateCallback show_user_feedback =
+      base::Bind(&BrowserOptionsHandler::OnProfileCreatedShowUserFeedback,
+                 weak_ptr_factory_.GetWeakPtr(), desktop_type, managed_user);
+
   if (managed_user && ManagedUserService::AreManagedUsersEnabled()) {
 #if defined(ENABLE_MANAGED_USERS)
     ManagedUserRegistrationService* registration_service =
         ManagedUserRegistrationServiceFactory::GetForProfile(profile);
-    callbacks.push_back(registration_service->GetRegistrationAndInitCallback());
+    callbacks.push_back(registration_service->GetRegistrationAndInitCallback(
+        show_user_feedback));
 #else
     NOTREACHED();
 #endif
+  } else {
+    callbacks.push_back(show_user_feedback);
   }
 
+  ProfileMetrics::LogProfileAddNewUser(ProfileMetrics::ADD_NEW_USER_DIALOG);
+
   ProfileManager::CreateMultiProfileAsync(
-      name, icon, base::Bind(&BrowserOptionsHandler::OnProfileCreated,
-                             weak_ptr_factory_.GetWeakPtr(),
-                             desktop_type, managed_user, callbacks),
+      name, icon, base::Bind(&OnProfileCreatedCallCallbacks, callbacks),
       managed_user);
 }
 
-void BrowserOptionsHandler::OnProfileCreated(
+void BrowserOptionsHandler::OnProfileCreatedShowUserFeedback(
     chrome::HostDesktopType desktop_type,
     bool is_managed,
-    const std::vector<ProfileManager::CreateCallback>& callbacks,
     Profile* profile,
     Profile::CreateStatus status) {
-  std::vector<ProfileManager::CreateCallback>::const_iterator it;
-  for (it = callbacks.begin(); it != callbacks.end(); ++it) {
-    it->Run(profile, status);
+  if (status != Profile::CREATE_STATUS_CREATED) {
+    UMA_HISTOGRAM_ENUMERATION("Profile.CreateResult",
+                              status,
+                              Profile::MAX_CREATE_STATUS);
   }
 
   switch (status) {
-    case Profile::CREATE_STATUS_FAIL: {
+    case Profile::CREATE_STATUS_LOCAL_FAIL: {
       web_ui()->CallJavascriptFunction(
           "BrowserOptions.showCreateProfileLocalError");
+      DeleteProfileAtPath(profile->GetPath());
+      break;
+    }
+    case Profile::CREATE_STATUS_REMOTE_FAIL: {
+      web_ui()->CallJavascriptFunction(
+          "BrowserOptions.showCreateProfileRemoteError");
+      DeleteProfileAtPath(profile->GetPath());
       break;
     }
     case Profile::CREATE_STATUS_CREATED: {
       // Do nothing for an intermediate status.
       break;
     }
     case Profile::CREATE_STATUS_INITIALIZED: {
       DictionaryValue dict;
       dict.SetString("name",
                      profile->GetPrefs()->GetString(prefs::kProfileName));
       dict.Set("filePath", base::CreateFilePathValue(profile->GetPath()));
       dict.SetBoolean("isManaged", is_managed);
       web_ui()->CallJavascriptFunction(
           "BrowserOptions.showCreateProfileSuccess", dict);
 
       // If the new profile is a managed user, instead of opening a new window
       // right away, a confirmation overlay will be shown from the creation
       // dialog.
       if (!is_managed) {
         // Opening the new window must be the last action, after all callbacks
         // have been run, to give them a chance to initialize the profile.
-        ProfileManager::FindOrCreateNewWindowForProfile(
-            profile,
-            chrome::startup::IS_PROCESS_STARTUP,
-            chrome::startup::IS_FIRST_RUN,
-            desktop_type,
-            false);
+        OpenNewWindowForProfile(desktop_type,
+                                profile,
+                                Profile::CREATE_STATUS_INITIALIZED);
       }
       break;
     }
+    case Profile::MAX_CREATE_STATUS: {
+      NOTREACHED();
+      break;
+    }
   }
 }
 
+void BrowserOptionsHandler::DeleteProfile(const ListValue* args) {

[Bernhard Bauer, 2013/06/03 10:52:47]

Is there a chance we could keep this in ManageProfileHandler and instead call
that method there if needed? BrowserOptionsHandler is quite big already.

[Pam (message me for reviews), 2013/06/03 13:39:28]

There's quite a bit in BrowserOptionsHandler that I'm not sure belongs there
anymore -- largely historical, I suppose. But I moved this here to match
CreateProfile (or rather, mainly because the CreateProfile flow now calls it
from a few places); it feels strange to me to have them in different classes.

[Bernhard Bauer, 2013/06/03 14:25:01]

Hm. TBH, I'd rather have profile creation and deletion both in
ManageProfileHandler than here, because I figure BrowserOptionsHandler should be
more general options than profile-specific stuff. I won't block this CL on that
particular yak though.

[Pam (message me for reviews), 2013/06/03 14:27:11]

Yeah, so would I, but I didn't want to get involved in that cleanup in this
patch. Thanks.

+  DCHECK(args);
+  const Value* file_path_value;
+  if (!args->Get(0, &file_path_value))
+    return;
+
+  base::FilePath file_path;
+  if (!base::GetValueAsFilePath(*file_path_value, &file_path))
+    return;
+  DeleteProfileAtPath(file_path);
+}
+
+void BrowserOptionsHandler::DeleteProfileAtPath(base::FilePath file_path) {
+#if defined(ENABLE_MANAGED_USERS)
+  // This handler could have been called in managed mode, for example because
+  // the user fiddled with the web inspector. Silently return in this case.
+  ManagedUserService* service =
+      ManagedUserServiceFactory::GetForProfile(Profile::FromWebUI(web_ui()));
+  if (service->ProfileIsManaged())

[Bernhard Bauer, 2013/06/03 10:52:47]

Use the static version? That prevents us from creating the MUService if it isn't
needed.

[Pam (message me for reviews), 2013/06/03 13:39:28]

Changed throughout.

+    return;
+#endif
+
+  if (!ProfileManager::IsMultipleProfilesEnabled())
+    return;
+
+  ProfileMetrics::LogProfileDeleteUser(ProfileMetrics::PROFILE_DELETED);
+
+  Browser* browser =
+      chrome::FindBrowserWithWebContents(web_ui()->GetWebContents());
+  chrome::HostDesktopType desktop_type = chrome::HOST_DESKTOP_TYPE_NATIVE;
+  if (browser)
+    desktop_type = browser->host_desktop_type();
+
+  g_browser_process->profile_manager()->ScheduleProfileForDeletion(
+      file_path,
+      base::Bind(&OpenNewWindowForProfile, desktop_type));
+}
+
 void BrowserOptionsHandler::ObserveThemeChanged() {
   Profile* profile = Profile::FromWebUI(web_ui());
 #if defined(TOOLKIT_GTK)
   GtkThemeService* theme_service = GtkThemeService::GetFrom(profile);
   bool is_gtk_theme = theme_service->UsingNativeTheme();
   base::FundamentalValue gtk_enabled(!is_gtk_theme);
   web_ui()->CallJavascriptFunction("BrowserOptions.setGtkThemeButtonEnabled",
                                    gtk_enabled);
 #else
   ThemeService* theme_service = ThemeServiceFactory::GetForProfile(profile);
@@ skipping 435 matching lines @@
   base::FundamentalValue disabled(profile_pref_registrar_.IsManaged() ||
                                   is_extension_controlled);
   base::FundamentalValue extension_controlled(is_extension_controlled);
   web_ui()->CallJavascriptFunction("BrowserOptions.setupProxySettingsSection",
                                    disabled, extension_controlled);
 
 #endif  // !defined(OS_CHROMEOS)
 }
 
 }  // namespace options