Multi-Profiles: Add delete profile command

chrome/browser/profiles/profile_manager.cc

Index: chrome/browser/profiles/profile_manager.cc
diff --git a/chrome/browser/profiles/profile_manager.cc b/chrome/browser/profiles/profile_manager.cc
index e84bbc3fe01843de7c7ef527d2e335e92b579757..a85901efec0a4c2406d7dd917437df96e111d62d 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -41,8 +41,20 @@
 #include "chrome/browser/chromeos/cros/cryptohome_library.h"
 #endif
 
+namespace {
+
+void DeleteProfileDirectories(std::vector<FilePath> paths) {

[willchan no longer on Chromium, 2011/07/07 17:36:27]

const std::vector<FilePath>&

[sail, 2011/07/07 18:00:19]

To make it thread safe I want this to be a copy of the vector. This is vector is
going to be really small so it shouldn't be a performance hit.

[willchan no longer on Chromium, 2011/07/08 08:33:17]

NewRunnableFunction() will take the parameters and copy them. See:

http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/base/task.h&exact_pac...
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/base/task.h&exact_pac...
and in particular:
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/base/task.h&exact_pac...

The last link will show how for const A& a passed as a parameter to
NewRunnableFunction(), NewRunnableFunction() will create a Tuple1<A>, NOT
Tuple1<const A&>. This Tuple1<A> becomes the Params template parameter in the
previous links. The Params parameter becomes a member variable of the
RunnableFunction subclass of Task.

Contrary to popular misunderstanding, the function prototype does not affect
whether or not NewRunnableMethod()/NewRunnableFunction() copies the parameter
into the Task.

So making this function prototype be like this just causes
RunnableFunction::Run() to make an extra unnecessary copy of this vector when it
calls the function pointer.

[sail, 2011/07/08 16:04:33]

Thanks for the explanation! Sent out fix in a separate CL:
http://codereview.chromium.org/7328021

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  for (std::vector<FilePath>::iterator it = paths.begin();
+       it != paths.end(); ++it) {
+    file_util::Delete(*it, true);
+  }
+}
+
+} // namespace
+
 bool ProfileManagerObserver::DeleteAfterCreation() {
-    return false;
+  return false;
 }
 
 // The NewProfileLauncher class is created when to wait for a multi-profile
@@ -99,6 +111,13 @@ ProfileManager::ProfileManager() : logged_in_(false) {
 
 ProfileManager::~ProfileManager() {
   BrowserList::RemoveObserver(this);
+
+  if (profiles_to_delete_.size()) {

[willchan no longer on Chromium, 2011/07/07 17:36:27]

Perhaps we should tag this with TODO: fix crbug.com/88586.

One problem with waiting until shutdown like this is it delays browser process
shutdown and prevents restarting chrome until it finishes.

[Miranda Callahan, 2011/07/07 17:45:55]

Agreed wholeheartedly about tagging; another reason to get Profile shutdown
working nicely soon.

[sail, 2011/07/07 18:00:19]

Done.

+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE,
+        NewRunnableFunction(&DeleteProfileDirectories, profiles_to_delete_));
+    profiles_to_delete_.clear();
+  }
 }
 
 FilePath ProfileManager::GetDefaultProfileDir(
@@ -535,3 +554,11 @@ void ProfileManager::AddProfileToCache(Profile* profile) {
         cache.ChooseAvatarIconIndexForNewProfile());
   }
 }
+
+void ProfileManager::ScheduleProfileForDeletion(const FilePath& profile_dir) {
+  Profile* profile = GetProfileByPath(profile_dir);
+  if (profile)
+    BrowserList::CloseAllBrowsersWithProfile(profile);

[Miranda Callahan, 2011/07/07 17:43:02]

Is there any issue with BackgroundApps that we have to consider here?

[sail, 2011/07/07 18:00:19]

No idea. Would rachel know?

[Miranda Callahan, 2011/07/07 18:24:16]

I can imagine that she would -- adding her in.

[rpetterson, 2011/07/07 18:24:30]

This particular call shouldn't affect BackgroundApps since the actual profile
instance isn't deleted. But when it is deleted, we should make sure to handle it
correctly.

+  profiles_to_delete_.push_back(profile_dir);
+  GetProfileInfoCache().DeleteProfileFromCache(profile_dir);
+}