Fixed sole profile double deletion.

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 1c992d410357ccbe7f9db73fac8bc686aedbb1fa..070f28fe54478a42f44f462a73ce89aea513fc76 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -6,6 +6,7 @@
 
 #include <stdint.h>
 
+#include <map>
 #include <set>
 #include <string>
 
@@ -131,8 +132,13 @@ using content::BrowserThread;
 namespace {
 
 // Profiles that should be deleted on shutdown.
-std::vector<base::FilePath>& ProfilesToDelete() {
-  CR_DEFINE_STATIC_LOCAL(std::vector<base::FilePath>, profiles_to_delete, ());
+enum class ProfileDeletionStage { SCHEDULED, ABANDONED };
+using ProfileDeletionMap = std::map<base::FilePath, ProfileDeletionStage>;
+// Profile deletion can pass thru two stages: scheduled for deletion when where

[Bernhard Bauer, 2016/08/04 12:24:34]

Nit: "through". Also, would it make sense to move this comment to the enum
definition, and maybe split it up to have a comment for each of the enum values?

+// performing necessary activity prior to deletion, and abandoned stage when it

[Bernhard Bauer, 2016/08/04 12:24:34]

I think we can also go into detail about what the necessary activity is -- it's
creating a new profile as a replacement when the last profile is scheduled to be
deleted, right?

+// can be safely removed from disk.
+ProfileDeletionMap& ProfilesToDelete() {
+  CR_DEFINE_STATIC_LOCAL(ProfileDeletionMap, profiles_to_delete, ());
   return profiles_to_delete;
 }
 
@@ -201,14 +207,25 @@ void ProfileSizeTask(const base::FilePath& path, int enabled_app_count) {
 }
 
 #if !defined(OS_ANDROID)
+// Return true if profile can be scheduled for deletion.

[Bernhard Bauer, 2016/08/04 12:24:34]

I think we should explain in the comment under which conditions the profile can
be scheduled for deletion, and maybe also be more explicit about the action that
is performed. Something like:

// Schedule a profile for deletion if it isn't already scheduled.
// Returns whether the profile has been newly scheduled.
bool MaybeScheduleProfileDirectoryForDeletion()

+bool ScheduleProfileDirectoryForDeletion(const base::FilePath& path) {
+  if (ContainsKey(ProfilesToDelete(), path))
+    return false;
+  ProfilesToDelete()[path] = ProfileDeletionStage::SCHEDULED;
+  return true;
+}
+
 void QueueProfileDirectoryForDeletion(const base::FilePath& path) {

[Bernhard Bauer, 2016/08/04 12:24:34]

Should we maybe use the term "marked" for the second stage? It's already used in
code, and it makes the difference to "scheduled" a bit clearer. Then this method
would be MarkProfileDirectoryForDeletion().

-  ProfilesToDelete().push_back(path);
+  DCHECK(!ContainsKey(ProfilesToDelete(), path) ||
+         ProfilesToDelete()[path] == ProfileDeletionStage::SCHEDULED);
+  ProfilesToDelete()[path] = ProfileDeletionStage::ABANDONED;
 }
 #endif
 
 bool IsProfileMarkedForDeletion(const base::FilePath& profile_path) {

[Bernhard Bauer, 2016/08/04 12:24:34]

For consistency, this method should then probably be renamed to
IsProfileDirectoryMarkedForDeletion().

-  return std::find(ProfilesToDelete().begin(), ProfilesToDelete().end(),
-      profile_path) != ProfilesToDelete().end();
+  auto it = ProfilesToDelete().find(profile_path);
+  return it != ProfilesToDelete().end() &&
+         it->second == ProfileDeletionStage::ABANDONED;
 }
 
 // Physically remove deleted profile directories from disk.
@@ -333,11 +350,9 @@ void ProfileManager::ShutdownSessionServices() {
 
 // static
 void ProfileManager::NukeDeletedProfilesFromDisk() {
-  for (std::vector<base::FilePath>::iterator it =
-          ProfilesToDelete().begin();
-       it != ProfilesToDelete().end();
-       ++it) {
-    NukeProfileFromDisk(*it);
+  for (const auto& item : ProfilesToDelete()) {
+    if (item.second == ProfileDeletionStage::ABANDONED)
+      NukeProfileFromDisk(item.first);
   }
   ProfilesToDelete().clear();
 }
@@ -735,7 +750,7 @@ bool ProfileManager::MaybeScheduleProfileForDeletion(
     const base::FilePath& profile_dir,
     const CreateCallback& callback,
     ProfileMetrics::ProfileDelete deletion_source) {
-  if (IsProfileMarkedForDeletion(profile_dir))
+  if (!ScheduleProfileDirectoryForDeletion(profile_dir))
     return false;
   ScheduleProfileForDeletion(profile_dir, callback);
   ProfileMetrics::LogProfileDeleteUser(deletion_source);