Fixed sole profile double deletion.

chrome/browser/profiles/profile_manager.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/profiles/profile_manager.h"
 
 #include <stdint.h>
 
+#include <map>
 #include <set>
 #include <string>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/deferred_sequenced_task_runner.h"
 #include "base/feature_list.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ skipping 105 matching lines @@
 #include "chrome/browser/profiles/profile_statistics.h"
 #include "chrome/browser/profiles/profile_statistics_factory.h"
 #endif
 
 using base::UserMetricsAction;
 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;
 }
 
 int64_t ComputeFilesSize(const base::FilePath& directory,
                          const base::FilePath::StringType& pattern) {
   int64_t running_size = 0;
   base::FileEnumerator iter(directory, false, base::FileEnumerator::FILES,
                             pattern);
   while (!iter.Next().empty())
     running_size += iter.GetInfo().GetSize();
@@ skipping 48 matching lines @@
   size = ComputeFilesSize(path, FILE_PATH_LITERAL("Policy"));
   size_MB = static_cast<int>(size / kBytesInOneMB);
   UMA_HISTOGRAM_COUNTS_10000("Profile.PolicySize", size_MB);
 
   // Count number of enabled apps in this profile, if we know.
   if (enabled_app_count != -1)
     UMA_HISTOGRAM_COUNTS_10000("Profile.AppCount", 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.
 void NukeProfileFromDisk(const base::FilePath& profile_path) {
   // Delete both the profile directory and its corresponding cache.
   base::FilePath cache_path;
   chrome::GetUserCacheDirectory(profile_path, &cache_path);
   base::DeleteFile(profile_path, true);
   base::DeleteFile(cache_path, true);
 }
@@ skipping 104 matching lines @@
   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]);
 }
 #endif
 
 // 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();
 }
 
 // static
 Profile* ProfileManager::GetLastUsedProfile() {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   return profile_manager->GetLastUsedProfile(profile_manager->user_data_dir_);
 }
 
@@ skipping 377 matching lines @@
 
 ProfileShortcutManager* ProfileManager::profile_shortcut_manager() {
   return profile_shortcut_manager_.get();
 }
 
 #if !defined(OS_ANDROID)
 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);
   return true;
 }
 
 void ProfileManager::ScheduleProfileForDeletion(
     const base::FilePath& profile_dir,
     const CreateCallback& callback) {
   DCHECK(profiles::IsMultipleProfilesEnabled());
@@ skipping 878 matching lines @@
 
   FinishDeletingProfile(profile_to_delete_path, new_active_profile_path);
   if (!original_callback.is_null())
     original_callback.Run(loaded_profile, status);
 }
 #endif  // !defined(OS_ANDROID)
 
 ProfileManagerWithoutInit::ProfileManagerWithoutInit(
     const base::FilePath& user_data_dir) : ProfileManager(user_data_dir) {
 }