Multi-Profiles: Add delete profile command

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"
@@ skipping 23 matching lines @@
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_job.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #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
 // 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) {
     Browser* browser = Browser::Create(profile);
@@ skipping 36 matching lines @@
 #if defined(OS_CHROMEOS)
   registrar_.Add(
       this,
       NotificationType::LOGIN_USER_CHANGED,
       NotificationService::AllSources());
 #endif
 }
 
 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(
     const FilePath& user_data_dir) {
   FilePath default_profile_dir(user_data_dir);
   default_profile_dir =
       default_profile_dir.AppendASCII(chrome::kNotSignedInProfile);
   return default_profile_dir;
 }
 
@@ skipping 416 matching lines @@
     cache.AddProfileToCache(
         profile->GetPath(),
         l10n_util::GetStringUTF16(IDS_DEFAULT_PROFILE_NAME), 0);
   } else {
     cache.AddProfileToCache(
         profile->GetPath(),
         cache.ChooseNameForNewProfile(),
         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);
+}