Added profile deletion browsertests.

chrome/browser/profiles/profile_manager_browsertest.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 <stddef.h>
 
 #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"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/common/pref_names.h"
@@ skipping 30 matching lines @@
 void ProfileCreationComplete(Profile* profile, Profile::CreateStatus status) {
   ASSERT_NE(status, Profile::CREATE_STATUS_LOCAL_FAIL);
   ASSERT_NE(status, Profile::CREATE_STATUS_REMOTE_FAIL);
   // No browser should have been created for this profile yet.
   EXPECT_EQ(chrome::GetBrowserCount(profile), 0U);
   EXPECT_EQ(chrome::GetTotalBrowserCount(), 1U);
   if (status == Profile::CREATE_STATUS_INITIALIZED)
     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)
     profile->GetPrefs()->SetBoolean(prefs::kForceEphemeralProfiles, true);
   ProfileCreationComplete(profile, status);
 }
 
 class ProfileRemovalObserver : public ProfileAttributesStorage::Observer {
  public:
   ProfileRemovalObserver() {
@@ skipping 73 matching lines @@
 class ProfileManagerBrowserTest : public InProcessBrowserTest {
  protected:
   void SetUpCommandLine(base::CommandLine* command_line) override {
 #if defined(OS_CHROMEOS)
     command_line->AppendSwitch(
         chromeos::switches::kIgnoreUserProfileMappingForTests);
 #endif
   }
 };
 
-#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) {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   ProfileAttributesStorage& storage =
       profile_manager->GetProfileAttributesStorage();
   ProfileRemovalObserver observer;
 
   // We should start out with 1 profile.
   ASSERT_EQ(1u, storage.GetNumberOfProfiles());
 
   // Delete singleton profile.
   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());
   base::FilePath new_profile_path =
       storage.GetAllProfilesAttributes().front()->GetPath();
   EXPECT_NE(new_profile_path.value(), singleton_profile_path.value());
 
   // Make sure that last used profile preference is set correctly.
   Profile* last_used = ProfileManager::GetLastUsedProfile();
   EXPECT_EQ(new_profile_path.value(), last_used->GetPath().value());
 
   // Make sure the last used profile was set correctly before the notification
   // was sent.
   std::string last_used_profile_name =
       last_used->GetPath().BaseName().MaybeAsASCII();
   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();
 
   // 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 the message loop to allow profile creation to take place; the loop is
   // terminated by OnUnblockOnProfileCreation when the profile is created.
   run_loop.Run();
 
   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;
   for (ProfileAttributesEntry* entry : entries) {
     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());
   base::FilePath new_profile_path =
       storage.GetAllProfilesAttributes().front()->GetPath();
   for (const base::FilePath& old_profile_path : old_profile_paths)
     EXPECT_NE(old_profile_path, new_profile_path);
 
   // Make sure that last used profile preference is set correctly.
   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)
 
 class ProfileManagerCrOSBrowserTest : public ProfileManagerBrowserTest {
  protected:
   void SetUpCommandLine(base::CommandLine* command_line) override {
     // Use a user hash other than the default chrome::kTestUserProfileDir
     // so that the prefix case is tested.
     command_line->AppendSwitchASCII(chromeos::switches::kLoginProfile,
                                     "test-user-hash");
@@ skipping 245 matching lines @@
 
   EXPECT_FALSE(profile->HasOffTheRecordProfile());
   EXPECT_FALSE(profile_manager->IsValidProfile(incognito_profile));
   EXPECT_EQ(initial_profile_count, profile_manager->GetNumberOfProfiles());
   // After destroying the incognito profile incognito preferences should be
   // cleared so the default save path should be taken from the main profile.
   EXPECT_FALSE(profile->GetOffTheRecordPrefs()
                    ->GetFilePath(prefs::kSaveFileDefaultDirectory)
                    .empty());
 }