Move BrowsingDataRemoverImpl:: CompletionInhibitor to the public interface

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 <list>
 
 #include "base/bind.h"
+#include "base/callback.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/browsing_data/browsing_data_remover.h"
+#include "chrome/browser/browsing_data/browsing_data_remover_factory.h"
+#include "chrome/browser/browsing_data/browsing_data_remover_test_util.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"
@@ skipping 36 matching lines @@
   EXPECT_EQ(chrome::GetBrowserCount(profile), 0U);
   EXPECT_EQ(chrome::GetTotalBrowserCount(), 1U);
   if (status == Profile::CREATE_STATUS_INITIALIZED)
     base::MessageLoop::current()->QuitWhenIdle();
 }
 
 // An observer that returns back to test code after one or more profiles was
 // deleted. It also create ScopedKeepAlive object to prevent browser shutdown
 // started in case browser has become windowless.
 class MultipleProfileDeletionObserver
-    : public BrowsingDataRemoverImpl::CompletionInhibitor,
-      public ProfileAttributesStorage::Observer {
+    : public ProfileAttributesStorage::Observer {
  public:
   explicit MultipleProfileDeletionObserver(size_t expected_count)
       : expected_count_(expected_count),
         profiles_created_count_(0),
         profiles_data_removed_count_(0) {
     EXPECT_GT(expected_count_, 0u);
-    g_browser_process->profile_manager()->GetProfileAttributesStorage().
-        AddObserver(this);
-    BrowsingDataRemoverImpl::set_completion_inhibitor_for_testing(this);
+    ProfileManager* profile_manager = g_browser_process->profile_manager();
+    profile_manager->GetProfileAttributesStorage().AddObserver(this);
+
+    for (Profile* profile : profile_manager->GetLoadedProfiles()) {

[msramek, 2017/04/10 13:20:08]

I rewrote this test to use BrowsingDataRemoverCompletionInhibitor as well. Since
the inhibition is no longer static, we need to call the appropriate method on
every profile.

I verified that meddling with the int counters above breaks the test, so doing
this for all GetLoadedProfiles() seems to be the right thing to do.

+      inhibitors_.emplace_back(
+          BrowsingDataRemoverFactory::GetForBrowserContext(profile),
+          base::Bind(&MultipleProfileDeletionObserver::

[Bernhard Bauer, 2017/04/10 23:29:37]

Hm, this callback is always identical, right? Couldn't we just directly call
HandleBrowsingDataRemoverWouldComplete() from
CompletionInhibitor::OnBrowsingDataRemoverWouldComplete? And then do we still
need a list of inhibitors? IOW, couldn't we just iterate over the
BrowsingDataRemovers and set the WouldCompleteCallbackForTesting to the same
callback for each of them?

[msramek, 2017/04/11 13:13:03]

Done.

Yes, that's a good idea. I partially reverted this back to the original. I'm
setting the callback directly now (and not unsetting it later, as it doesn't
matter).

+                         HandleBrowsingDataRemoverWouldComplete,
+                     base::Unretained(this)));
+    }
   }
+
   ~MultipleProfileDeletionObserver() override {
     g_browser_process->profile_manager()->GetProfileAttributesStorage().
         RemoveObserver(this);
-    BrowsingDataRemoverImpl::set_completion_inhibitor_for_testing(nullptr);
+    inhibitors_.clear();
   }
+
   void Wait() {
     keep_alive_ = base::MakeUnique<ScopedKeepAlive>(
         KeepAliveOrigin::PROFILE_HELPER, KeepAliveRestartOption::DISABLED);
     loop_.Run();
   }
 
  private:
+  // TODO(https://crbug.com/704601): remove this code when bug is fixed.
+  class CompletionInhibitor : public BrowsingDataRemoverCompletionInhibitor {
+   public:
+    CompletionInhibitor(BrowsingDataRemover* remover,
+                        const base::Closure& callback)
+        : BrowsingDataRemoverCompletionInhibitor(remover),
+          callback_(callback) {}
+    ~CompletionInhibitor() override {}
+
+    // BrowsingDataRemoverCompletionInhibitor:
+    void OnBrowsingDataRemoverWouldComplete(
+        const base::Closure& continue_to_completion) override {
+      BrowsingDataRemoverCompletionInhibitor::
+          OnBrowsingDataRemoverWouldComplete(continue_to_completion);
+      callback_.Run();
+    }
+
+   private:
+    base::Closure callback_;
+
+    DISALLOW_COPY_AND_ASSIGN(CompletionInhibitor);
+  };
+
+  void HandleBrowsingDataRemoverWouldComplete() {
+    profiles_data_removed_count_++;
+    MaybeQuit();
+  }
+
   void OnProfileWillBeRemoved(const base::FilePath& profile_path) override {
     profiles_created_count_++;
     MaybeQuit();
   }
 
-  // TODO(https://crbug.com/704601): remove this code when bug is fixed.
-  void OnBrowsingDataRemoverWouldComplete(
-      BrowsingDataRemoverImpl* remover,
-      const base::Closure& continue_to_completion) override {
-    continue_to_completion.Run();
-    profiles_data_removed_count_++;
-    MaybeQuit();
-  }
-
   void MaybeQuit() {
     DLOG(INFO) << profiles_created_count_
                << " profiles removed, and "
                << profiles_data_removed_count_
                << " profile data removed of expected "
                << expected_count_;
     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();
   }
 
+  std::list<CompletionInhibitor> inhibitors_;
+
   base::RunLoop loop_;
   std::unique_ptr<ScopedKeepAlive> keep_alive_;
   size_t expected_count_;
   size_t profiles_created_count_;
   size_t profiles_data_removed_count_;
 
   DISALLOW_COPY_AND_ASSIGN(MultipleProfileDeletionObserver);
 };
 
 void EphemeralProfileCreationComplete(Profile* profile,
@@ skipping 502 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());
 }