Replace ProfileManagerObserver with callback

chrome/browser/profiles/profile_manager.cc

 // Copyright (c) 2011 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 <set>
 
 #include "chrome/browser/profiles/profile_manager.h"
 
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
-#include "base/stl_util.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/default_apps_trial.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/sessions/session_service_factory.h"
@@ skipping 110 matching lines @@
                                      kDefaultAppsTrialName),
           extension_count_);
     }
   }
 }
 
 void QueueProfileDirectoryForDeletion(const FilePath& path) {
   ProfilesToDelete().push_back(path);
 }
 
+// Called upon completion of profile creation. This function takes care of
+// launching a new browser window and signing the user in to their Google
+// account.
+void OnOpenWindowForNewProfile(Profile* profile,
+                               Profile::CreateStatus status) {
+  if (status == Profile::CREATE_STATUS_INITIALIZED) {
+    ProfileManager::NewWindowWithProfile(profile,
+                                         BrowserInit::IS_PROCESS_STARTUP,
+                                         BrowserInit::IS_FIRST_RUN);
+  }
+}
+
 } // namespace
 
-bool ProfileManagerObserver::DeleteAfter() {
-  return false;
-}
-
-// The NewProfileLauncher class is created when to wait for a multi-profile
-// to be created asynchronously. Upon completion of profile creation, the
-// NPL takes care of launching a new browser window and signing the user
-// in to their Google account.
-class NewProfileLauncher : public ProfileManagerObserver {
- public:
-  virtual void OnProfileCreated(Profile* profile, Status status) {
-    if (status == STATUS_INITIALIZED) {
-      ProfileManager::NewWindowWithProfile(profile,
-                                           BrowserInit::IS_PROCESS_STARTUP,
-                                           BrowserInit::IS_FIRST_RUN);
-    }
-  }
-
-  virtual bool DeleteAfter() OVERRIDE { return true; }
-};
-
 // static
 void ProfileManager::ShutdownSessionServices() {
   ProfileManager* pm = g_browser_process->profile_manager();
   if (!pm)  // Is NULL when running unit tests.
     return;
   std::vector<Profile*> profiles(pm->GetLoadedProfiles());
   for (size_t i = 0; i < profiles.size(); ++i)
     SessionServiceFactory::ShutdownForProfile(profiles[i]);
 }
 
@@ skipping 147 matching lines @@
 
   profile = CreateProfileHelper(profile_dir);
   DCHECK(profile);
   if (profile) {
     bool result = AddProfile(profile);
     DCHECK(result);
   }
   return profile;
 }
 
-void ProfileManager::CreateProfileAsync(const FilePath& user_data_dir,
-                                        ProfileManagerObserver* observer) {
+void ProfileManager::CreateProfileAsync(const FilePath& profile_path,
+                                        CreateCallback callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  ProfilesInfoMap::iterator iter = profiles_info_.find(user_data_dir);
+  ProfilesInfoMap::iterator iter = profiles_info_.find(profile_path);
   if (iter != profiles_info_.end()) {
     ProfileInfo* info = iter->second.get();
     if (info->created) {
       // Profile has already been created. Call observer immediately.
-      observer->OnProfileCreated(
-          info->profile.get(), ProfileManagerObserver::STATUS_INITIALIZED);
-      if (observer->DeleteAfter())
-        delete observer;
+      callback.Run(info->profile.get(), Profile::CREATE_STATUS_INITIALIZED);
     } else {
-      // Profile is being created. Add observer to list.
-      info->observers.push_back(observer);
+      // Profile is being created. Add callback to list.
+      info->callbacks.push_back(callback);
     }
   } else {
     // Initiate asynchronous creation process.
     ProfileInfo* info =
-        RegisterProfile(CreateProfileAsyncHelper(user_data_dir, this),
-                        false);
-    info->observers.push_back(observer);
+        RegisterProfile(CreateProfileAsyncHelper(profile_path, this), false);
+    info->callbacks.push_back(callback);
   }
 }
 
 // static
-void ProfileManager::CreateDefaultProfileAsync(
-    ProfileManagerObserver* observer) {
+void ProfileManager::CreateDefaultProfileAsync(CreateCallback callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   ProfileManager* profile_manager = g_browser_process->profile_manager();
 
   FilePath default_profile_dir = profile_manager->user_data_dir_;
   // TODO(mirandac): current directory will not always be default in the future
   default_profile_dir = default_profile_dir.Append(
       profile_manager->GetInitialProfileDir());
 
   profile_manager->CreateProfileAsync(default_profile_dir,
-                                      observer);
+                                      callback);

[Robert Sesek, 2011/12/07 16:36:13]

nit: Does this fit on one line now?

[sail, 2011/12/07 17:18:13]

Done.

 }
 
 bool ProfileManager::AddProfile(Profile* profile) {
   DCHECK(profile);
 
   // Make sure that we're not loading a profile with the same ID as a profile
   // that's already loaded.
   if (GetProfileByPath(profile->GetPath())) {
     NOTREACHED() << "Attempted to add profile with the same path (" <<
                     profile->GetPath().value() <<
@@ skipping 113 matching lines @@
   return Profile::CreateProfileAsync(path, delegate);
 }
 
 void ProfileManager::OnProfileCreated(Profile* profile, bool success) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   ProfilesInfoMap::iterator iter = profiles_info_.find(profile->GetPath());
   DCHECK(iter != profiles_info_.end());
   ProfileInfo* info = iter->second.get();
 
-  std::vector<ProfileManagerObserver*> observers;
-  info->observers.swap(observers);
+  std::vector<CreateCallback> callbacks;
+  info->callbacks.swap(callbacks);
 
+  // Invoke CREATED callback for normal profiles.
   bool go_off_the_record = ShouldGoOffTheRecord();
+  if (success & !go_off_the_record) {

[Robert Sesek, 2011/12/07 16:36:13]

&&

[sail, 2011/12/07 17:18:13]

Done.

+    for (size_t i = 0; i < callbacks.size(); ++i)
+      callbacks[i].Run(profile, Profile::CREATE_STATUS_CREATED);
+  }
+
   if (success) {

[Robert Sesek, 2011/12/07 16:36:13]

I think this if-block structure is less clear than the nested if's. At first
glance, it seems like this should be an else if.

[sail, 2011/12/07 17:18:13]

I found the original code very confusing which is why I change the structure a
bit. I agree the new code isn't perfect but I find it much easier to break down
into logical groups. I added a comment here to help out.

-    if (!go_off_the_record) {
-      for (size_t i = 0; i < observers.size(); ++i) {
-        observers[i]->OnProfileCreated(
-            profile, ProfileManagerObserver::STATUS_CREATED);
-      }
-    }
     DoFinalInit(profile, go_off_the_record);
+    if (go_off_the_record)
+      profile = profile->GetOffTheRecordProfile();
     info->created = true;
   } else {
     profile = NULL;
     profiles_info_.erase(iter);
   }
 
-  std::vector<ProfileManagerObserver*> observers_to_delete;
-
-  for (size_t i = 0; i < observers.size(); ++i) {
-    if (profile && go_off_the_record) {
-      profile = profile->GetOffTheRecordProfile();
-      DCHECK(profile);
-      observers[i]->OnProfileCreated(
-          profile, ProfileManagerObserver::STATUS_CREATED);
-    }
-    observers[i]->OnProfileCreated(
-        profile, profile ? ProfileManagerObserver::STATUS_INITIALIZED :
-            ProfileManagerObserver::STATUS_FAIL);
-    if (observers[i]->DeleteAfter())
-      observers_to_delete.push_back(observers[i]);
+  // Invoke CREATED callback for incognito profiles.
+  if (profile && go_off_the_record) {
+    for (size_t i = 0; i < callbacks.size(); ++i)
+      callbacks[i].Run(profile, Profile::CREATE_STATUS_CREATED);

[Robert Sesek, 2011/12/07 16:36:13]

What's the point of the CREATED status? It seems like if you send INITIALIZED
for all profiles right after this, CREATED doesn't do much.

[sail, 2011/12/07 17:18:13]

It looks like CREATED was added for the ChromeOS code. It needs to be called
before DoFinalInit() is called.

   }
 
-  STLDeleteElements(&observers_to_delete);
+  // Invoke INITIALIZED or FAIL for all profiles.
+  for (size_t i = 0; i < callbacks.size(); ++i) {
+    callbacks[i].Run(profile, profile ? Profile::CREATE_STATUS_INITIALIZED :
+                                        Profile::CREATE_STATUS_FAIL);
+  }
 }
 
 FilePath ProfileManager::GenerateNextProfileDirectoryPath() {
   PrefService* local_state = g_browser_process->local_state();
   DCHECK(local_state);
 
   // Create the next profile in the next available directory slot.
   int next_directory = local_state->GetInteger(prefs::kProfilesNumCreated);
   std::string profile_name = chrome::kMultiProfileDirPrefix;
   profile_name.append(base::IntToString(next_directory));
   FilePath new_path = user_data_dir_;
 #if defined(OS_WIN)
   new_path = new_path.Append(ASCIIToUTF16(profile_name));
 #else
   new_path = new_path.Append(profile_name);
 #endif
   local_state->SetInteger(prefs::kProfilesNumCreated, ++next_directory);
   return new_path;
 }
 
 // static
 void ProfileManager::CreateMultiProfileAsync() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   ProfileManager* profile_manager = g_browser_process->profile_manager();
 
   FilePath new_path = profile_manager->GenerateNextProfileDirectoryPath();
 
-  // The launcher is deleted by the manager when profile creation is finished.
-  NewProfileLauncher* launcher = new NewProfileLauncher();
-  profile_manager->CreateProfileAsync(new_path, launcher);
+  profile_manager->CreateProfileAsync(new_path,
+                                      base::Bind(&OnOpenWindowForNewProfile));
 }
 
 // static
 void ProfileManager::RegisterPrefs(PrefService* prefs) {
   prefs->RegisterStringPref(prefs::kProfileLastUsed, "");
   prefs->RegisterIntegerPref(prefs::kProfilesNumCreated, 1);
 }
 
 size_t ProfileManager::GetNumberOfProfiles() {
   return GetProfileInfoCache().GetNumberOfProfiles();
@@ skipping 64 matching lines @@
   return go_off_the_record;
 }
 
 void ProfileManager::ScheduleProfileForDeletion(const FilePath& profile_dir) {
   // If we're deleting the last profile, then create a new profile in its
   // place.
   ProfileInfoCache& cache = GetProfileInfoCache();
   if (cache.GetNumberOfProfiles() == 1) {
     FilePath new_path = GenerateNextProfileDirectoryPath();
 
-    // The launcher is deleted by the manager when profile creation is finished.
-    NewProfileLauncher* launcher = new NewProfileLauncher();
-    CreateProfileAsync(new_path, launcher);
+    CreateProfileAsync(new_path, base::Bind(&OnOpenWindowForNewProfile));
   }
 
   // Update the last used profile pref before closing browser windows. This way
   // the correct last used profile is set for any notification observers.
   PrefService* local_state = g_browser_process->local_state();
   std::string last_profile = local_state->GetString(prefs::kProfileLastUsed);
   if (profile_dir.BaseName().MaybeAsASCII() == last_profile) {
     for (size_t i = 0; i < cache.GetNumberOfProfiles(); ++i) {
       FilePath cur_path = cache.GetPathOfProfileAtIndex(i);
       if (cur_path != profile_dir) {
@@ skipping 50 matching lines @@
   RegisterProfile(profile, true);
   if (add_to_cache)
     AddProfileToCache(profile);
 }
 
 #if defined(OS_WIN)
 void ProfileManager::RemoveProfileShortcutManagerForTesting() {
   profile_info_cache_->RemoveObserver(profile_shortcut_manager_.get());
 }
 #endif