Added profile deletion browsertests.

chrome/browser/profiles/profile_manager_browsertest.cc

Index: chrome/browser/profiles/profile_manager_browsertest.cc
diff --git a/chrome/browser/profiles/profile_manager_browsertest.cc b/chrome/browser/profiles/profile_manager_browsertest.cc
index f0287d73c6b953d7a3c65518adb11d0e0178910c..27f5f0440cbd7926808231f9c7d98211f2ed6a8e 100644
--- a/chrome/browser/profiles/profile_manager_browsertest.cc
+++ b/chrome/browser/profiles/profile_manager_browsertest.cc
@@ -7,8 +7,13 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
+#include "chrome/browser/browsing_data/browsing_data_remover_impl.h"
+#include "chrome/browser/lifetime/keep_alive_types.h"
+#include "chrome/browser/lifetime/scoped_keep_alive.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
@@ -59,6 +64,71 @@ void ProfileCreationComplete(Profile* profile, Profile::CreateStatus status) {
     base::MessageLoop::current()->QuitWhenIdle();
 }
 
+// An observer returns back to test code after one or more profiles was deleted.
+// It has ScopedKeepAlive object to prevent browser shutdown started in case
+// browser become windowless.
+class MultipleProfileDeletionObserver
+    : public BrowsingDataRemoverImpl::CompletionInhibitor {
+ public:
+  explicit MultipleProfileDeletionObserver(size_t expected_count)
+      : expected_count_(expected_count),
+        profiles_created_count_(0),
+        profiles_data_removed_count_(0) {
+    EXPECT_LT(0u, expected_count_);

[Bernhard Bauer, 2017/04/03 10:57:39]

Nit: I would do EXPECT_GT(expected_count_, 0u) -- if it's not EQ, we don't get a
better error message by putting the expected value first, and `expected_count_ >
0` reads more straightforward to me than the other way around.

+    BrowsingDataRemoverImpl::set_completion_inhibitor_for_testing(this);
+  }
+  ~MultipleProfileDeletionObserver() {
+    BrowsingDataRemoverImpl::set_completion_inhibitor_for_testing(nullptr);
+  }
+  ProfileManager::CreateCallback OnProfileCreatedClosure() {
+    return base::Bind(&MultipleProfileDeletionObserver::OnProfileCreated,
+                      base::Unretained(this));
+  }
+  void Wait() {
+    keep_alive_ = base::MakeUnique<ScopedKeepAlive>(
+        KeepAliveOrigin::PROFILE_HELPER, KeepAliveRestartOption::DISABLED);
+    loop_.Run();
+  }
+
+ private:
+  void OnProfileCreated(Profile* profile, Profile::CreateStatus status) {
+    EXPECT_EQ(Profile::CREATE_STATUS_INITIALIZED, status);
+    ++profiles_created_count_;

[Bernhard Bauer, 2017/04/03 10:57:39]

Nit: I would use postincrement -- the performance difference is negligible, and
again it reads a bit more straightforward to me.

+    MaybeQuit();
+  }
+
+  // TODO(bug 704601): remove it when bug is fixed.

[Bernhard Bauer, 2017/04/03 10:57:39]

Nit: Use TODO(704601) or TODO(https://crbug.com/704601).

+  void OnBrowsingDataRemoverWouldComplete(
+      BrowsingDataRemoverImpl* remover,
+      const base::Closure& continue_to_completion) override {
+    continue_to_completion.Run();
+
+    ++profiles_data_removed_count_;
+    MaybeQuit();
+  }
+
+  void MaybeQuit() {
+    LOG(INFO) << profiles_created_count_ << " profiles created & "

[Bernhard Bauer, 2017/04/03 10:57:39]

Make this a DLOG.

+              << profiles_data_removed_count_ << " profile data removed of "
+              << expected_count_ << " expected.";
+    if (profiles_created_count_ < expected_count_ ||
+        profiles_data_removed_count_ < expected_count_)
+      return;
+
+    EXPECT_EQ(expected_count_, profiles_created_count_);
+    EXPECT_EQ(expected_count_, profiles_data_removed_count_);
+
+    keep_alive_.reset();
+    loop_.Quit();
+  }
+
+  base::RunLoop loop_;
+  std::unique_ptr<ScopedKeepAlive> keep_alive_;
+  size_t expected_count_;
+  size_t profiles_created_count_;
+  size_t profiles_data_removed_count_;
+};
+
 void EphemeralProfileCreationComplete(Profile* profile,
                                       Profile::CreateStatus status) {
   if (status == Profile::CREATE_STATUS_INITIALIZED)
@@ -152,7 +222,9 @@ class ProfileManagerBrowserTest : public InProcessBrowserTest {
   }
 };
 
-#if defined(OS_MACOSX)
+// Android does not support multi-profiles, and CrOS multi-profiles
+// implementation is too different for these tests.
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 
 // Delete single profile and make sure a new one is created.
 IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteSingletonProfile) {
@@ -168,14 +240,14 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteSingletonProfile) {
   base::FilePath singleton_profile_path =
       storage.GetAllProfilesAttributes().front()->GetPath();
   EXPECT_FALSE(singleton_profile_path.empty());
-  base::RunLoop run_loop;
+  MultipleProfileDeletionObserver profile_deletion_observer(1u);
   profile_manager->ScheduleProfileForDeletion(
       singleton_profile_path,
-      base::Bind(&OnUnblockOnProfileCreation, &run_loop));
+      profile_deletion_observer.OnProfileCreatedClosure());
 
   // Run the message loop until the profile is actually deleted (as indicated
   // by the callback above being called).
-  run_loop.Run();
+  profile_deletion_observer.Wait();
 
   // Make sure a new profile was created automatically.
   EXPECT_EQ(1u, storage.GetNumberOfProfiles());
@@ -194,10 +266,80 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteSingletonProfile) {
   EXPECT_EQ(last_used_profile_name, observer.last_used_profile_name());
 }
 
+// Delete inactive profile in a multi profile setup and make sure current
+// browser is not affected.
+IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteInactiveProfile) {
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  ProfileAttributesStorage& storage =
+      profile_manager->GetProfileAttributesStorage();
+  base::FilePath current_profile_path = browser()->profile()->GetPath();
+
+  // Create an additional profile.
+  base::FilePath new_path = profile_manager->GenerateNextProfileDirectoryPath();
+  base::RunLoop run_loop;
+  profile_manager->CreateProfileAsync(
+      new_path, base::Bind(&OnUnblockOnProfileCreation, &run_loop),
+      base::string16(), std::string(), std::string());
+  run_loop.Run();
+
+  ASSERT_EQ(2u, storage.GetNumberOfProfiles());
+
+  // Delete inactive profile.
+  MultipleProfileDeletionObserver profile_deletion_observer(1u);
+  profile_manager->ScheduleProfileForDeletion(
+      new_path, profile_deletion_observer.OnProfileCreatedClosure());
+  // Profile will not be created, call OnProfileCreatedClosure() directly to
+  // fulfill |profile_deletion_observer| expectataion.

[Bernhard Bauer, 2017/04/03 10:57:39]

This seems a bit strange. If a new profile is not created in all cases, why have
the deletion observer expect that? Would it make more sense to pass in one value
for the expected number of deletions and one for additions?

+  profile_deletion_observer.OnProfileCreatedClosure().Run(
+      browser()->profile(), Profile::CREATE_STATUS_INITIALIZED);
+  profile_deletion_observer.Wait();
+
+  // Make sure there only preexisted profile left.
+  EXPECT_EQ(1u, storage.GetNumberOfProfiles());
+  EXPECT_EQ(current_profile_path,
+            storage.GetAllProfilesAttributes().front()->GetPath());
+
+  // Make sure that last used profile preference is set correctly.
+  Profile* last_used = ProfileManager::GetLastUsedProfile();
+  EXPECT_EQ(current_profile_path, last_used->GetPath());
+}
+
+// Delete current profile in a multi profile setup and make sure an existing one
+// is loaded.
+IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteCurrentProfile) {
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  ProfileAttributesStorage& storage =
+      profile_manager->GetProfileAttributesStorage();
+
+  // Create an additional profile.
+  base::FilePath new_path = profile_manager->GenerateNextProfileDirectoryPath();
+  base::RunLoop run_loop;
+  profile_manager->CreateProfileAsync(
+      new_path, base::Bind(&OnUnblockOnProfileCreation, &run_loop),
+      base::string16(), std::string(), std::string());
+  run_loop.Run();
+
+  ASSERT_EQ(2u, storage.GetNumberOfProfiles());
+
+  // Delete current profile.
+  MultipleProfileDeletionObserver profile_deletion_observer(1u);
+  profile_manager->ScheduleProfileForDeletion(
+      browser()->profile()->GetPath(),
+      profile_deletion_observer.OnProfileCreatedClosure());
+  profile_deletion_observer.Wait();
+
+  // Make sure a profile created earlier become the only profile.
+  EXPECT_EQ(1u, storage.GetNumberOfProfiles());
+  EXPECT_EQ(new_path, storage.GetAllProfilesAttributes().front()->GetPath());
+
+  // Make sure that last used profile preference is set correctly.
+  Profile* last_used = ProfileManager::GetLastUsedProfile();
+  EXPECT_EQ(new_path, last_used->GetPath());
+}
+
 // Delete all profiles in a multi profile setup and make sure a new one is
 // created.
-// Crashes/CHECKs. See crbug.com/104851
-IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DISABLED_DeleteAllProfiles) {
+IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteAllProfiles) {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   ProfileAttributesStorage& storage =
       profile_manager->GetProfileAttributesStorage();
@@ -216,6 +358,7 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DISABLED_DeleteAllProfiles) {
   ASSERT_EQ(2u, storage.GetNumberOfProfiles());
 
   // Delete all profiles.
+  MultipleProfileDeletionObserver profile_deletion_observer(2u);
   std::vector<ProfileAttributesEntry*> entries =
       storage.GetAllProfilesAttributes();
   std::vector<base::FilePath> old_profile_paths;
@@ -223,12 +366,10 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DISABLED_DeleteAllProfiles) {
     base::FilePath profile_path = entry->GetPath();
     EXPECT_FALSE(profile_path.empty());
     profile_manager->ScheduleProfileForDeletion(
-        profile_path, ProfileManager::CreateCallback());
+        profile_path, profile_deletion_observer.OnProfileCreatedClosure());
     old_profile_paths.push_back(profile_path);
   }
-
-  // Spin things so deletion can take place.
-  content::RunAllPendingInMessageLoop();
+  profile_deletion_observer.Wait();
 
   // Make sure a new profile was created automatically.
   EXPECT_EQ(1u, storage.GetNumberOfProfiles());
@@ -241,7 +382,7 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DISABLED_DeleteAllProfiles) {
   Profile* last_used = ProfileManager::GetLastUsedProfile();
   EXPECT_EQ(new_profile_path, last_used->GetPath());
 }
-#endif  // OS_MACOSX
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 
 #if defined(OS_CHROMEOS)