Split BrowsingDataRemover into an abstract interface and implementation.

chrome/browser/browsing_data/browsing_data_remover_unittest.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/browsing_data/browsing_data_remover.h"
-
 #include <stddef.h>
 #include <stdint.h>
 
 #include <list>
 #include <memory>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/guid.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/browsing_data/browsing_data_filter_builder.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.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_impl.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_test_util.h"
 #include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h"
 #include "chrome/browser/browsing_data/registrable_domain_filter_builder.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/domain_reliability/service_factory.h"
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/permissions/permission_decision_auto_blocker.h"
@@ skipping 1035 matching lines @@
 
  private:
   TestingProfile* profile_;
 
   DISALLOW_COPY_AND_ASSIGN(RemovePermissionPromptCountsTest);
 };
 
 #if BUILDFLAG(ENABLE_PLUGINS)
 // A small modification to MockBrowsingDataFlashLSOHelper so that it responds
 // immediately and does not wait for the Notify() call. Otherwise it would
-// deadlock BrowsingDataRemover::RemoveImpl.
+// deadlock BrowsingDataRemoverImpl::RemoveImpl.
 class TestBrowsingDataFlashLSOHelper : public MockBrowsingDataFlashLSOHelper {
  public:
   explicit TestBrowsingDataFlashLSOHelper(TestingProfile* profile)
       : MockBrowsingDataFlashLSOHelper(profile) {}
 
   void StartFetching(const GetSitesWithFlashDataCallback& callback) override {
     MockBrowsingDataFlashLSOHelper::StartFetching(callback);
     Notify();
   }
 
  private:
   ~TestBrowsingDataFlashLSOHelper() override {}
 
   DISALLOW_COPY_AND_ASSIGN(TestBrowsingDataFlashLSOHelper);
 };
 
 class RemovePluginDataTester {
  public:
   explicit RemovePluginDataTester(TestingProfile* profile)
       : helper_(new TestBrowsingDataFlashLSOHelper(profile)) {
-    BrowsingDataRemoverFactory::GetForBrowserContext(profile)
-        ->OverrideFlashLSOHelperForTesting(helper_);
+    static_cast<BrowsingDataRemoverImpl*>(
+        BrowsingDataRemoverFactory::GetForBrowserContext(profile))
+            ->OverrideFlashLSOHelperForTesting(helper_);
   }
 
   void AddDomain(const std::string& domain) {
     helper_->AddFlashLSODomain(domain);
   }
 
   const std::vector<std::string>& GetDomains() {
     // TestBrowsingDataFlashLSOHelper is synchronous, so we can immediately
     // return the fetched domains.
     helper_->StartFetching(
@@ skipping 14 matching lines @@
 };
 #endif
 
 // Test Class ----------------------------------------------------------------
 
 class BrowsingDataRemoverTest : public testing::Test {
  public:
   BrowsingDataRemoverTest()
       : profile_(new TestingProfile()),
         clear_domain_reliability_tester_(GetProfile()) {
-    remover_ =
-        BrowsingDataRemoverFactory::GetForBrowserContext(profile_.get());
+    remover_ = static_cast<BrowsingDataRemoverImpl*>(
+        BrowsingDataRemoverFactory::GetForBrowserContext(profile_.get()));
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
     static_cast<ChromeBrowsingDataRemoverDelegate*>(
-        remover_->get_embedder_delegate())->OverrideWebappRegistryForTesting(
+        remover_->GetEmbedderDelegate())->OverrideWebappRegistryForTesting(
             base::WrapUnique<WebappRegistry>(new TestWebappRegistry()));
 #endif
   }
 
   ~BrowsingDataRemoverTest() override {}
 
   void TearDown() override {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
     mock_policy_ = nullptr;
 #endif
 
     // TestingProfile contains a DOMStorageContext.  BrowserContext's destructor
     // posts a message to the WEBKIT thread to delete some of its member
     // variables. We need to ensure that the profile is destroyed, and that
     // the message loop is cleared out, before destroying the threads and loop.
     // Otherwise we leak memory.
     profile_.reset();
     base::RunLoop().RunUntilIdle();
 
     TestingBrowserProcess::GetGlobal()->SetLocalState(nullptr);
   }
 
   void BlockUntilBrowsingDataRemoved(const base::Time& delete_begin,
                                      const base::Time& delete_end,
                                      int remove_mask,
                                      bool include_protected_origins) {
-    BrowsingDataRemover* remover =
-        BrowsingDataRemoverFactory::GetForBrowserContext(profile_.get());
+    BrowsingDataRemoverImpl* remover = static_cast<BrowsingDataRemoverImpl*>(

[Bernhard Bauer, 2017/01/10 10:39:47]

Don't we already have this in |remover_|?

[msramek, 2017/01/10 12:08:34]

Indeed. This is probably a remnant from the times when BrowsingDataRemover
wasn't a KeyedService and had to be instantiated each time. Replaced here and
below.

+        BrowsingDataRemoverFactory::GetForBrowserContext(profile_.get()));
 
     TestStoragePartition storage_partition;
     remover->OverrideStoragePartitionForTesting(&storage_partition);
 
     int origin_type_mask = BrowsingDataHelper::UNPROTECTED_WEB;
     if (include_protected_origins)
       origin_type_mask |= BrowsingDataHelper::PROTECTED_WEB;
 
     BrowsingDataRemoverCompletionObserver completion_observer(remover);
     remover->RemoveAndReply(
         delete_begin, delete_end, remove_mask, origin_type_mask,
         &completion_observer);
     completion_observer.BlockUntilCompletion();
 
     // Save so we can verify later.
     storage_partition_removal_data_ =
         storage_partition.GetStoragePartitionRemovalData();
   }
 
   void BlockUntilOriginDataRemoved(
       const base::Time& delete_begin,
       const base::Time& delete_end,
       int remove_mask,
       const BrowsingDataFilterBuilder& filter_builder) {
-    BrowsingDataRemover* remover =
-        BrowsingDataRemoverFactory::GetForBrowserContext(profile_.get());
+    BrowsingDataRemoverImpl* remover = static_cast<BrowsingDataRemoverImpl*>(
+        BrowsingDataRemoverFactory::GetForBrowserContext(profile_.get()));
     TestStoragePartition storage_partition;
     remover->OverrideStoragePartitionForTesting(&storage_partition);
 
     BrowsingDataRemoverCompletionInhibitor completion_inhibitor;
     remover->RemoveImpl(delete_begin, delete_end, remove_mask, filter_builder,
                         BrowsingDataHelper::UNPROTECTED_WEB);
     completion_inhibitor.BlockUntilNearCompletion();
     completion_inhibitor.ContinueToCompletion();
 
     // Save so we can verify later.
@@ skipping 49 matching lines @@
 #else
     return true;
 #endif
   }
 
   const ClearDomainReliabilityTester& clear_domain_reliability_tester() {
     return clear_domain_reliability_tester_;
   }
 
  private:
-  // Cached pointer to BrowsingDataRemover for access to testing methods.
-  BrowsingDataRemover* remover_;
+  // Cached pointer to BrowsingDataRemoverImpl for access to testing methods.
+  BrowsingDataRemoverImpl* remover_;
 
   content::TestBrowserThreadBundle thread_bundle_;
   std::unique_ptr<TestingProfile> profile_;
 
   StoragePartitionRemovalData storage_partition_removal_data_;
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   scoped_refptr<MockExtensionSpecialStoragePolicy> mock_policy_;
 #endif
 
@@ skipping 1007 matching lines @@
 
  private:
   bool called_ = false;
 };
 
 TEST_F(BrowsingDataRemoverTest, CompletionInhibition) {
   // The |completion_inhibitor| on the stack should prevent removal sessions
   // from completing until after ContinueToCompletion() is called.
   BrowsingDataRemoverCompletionInhibitor completion_inhibitor;
 
-  BrowsingDataRemover* remover =
-      BrowsingDataRemoverFactory::GetForBrowserContext(GetProfile());
+  BrowsingDataRemoverImpl* remover = static_cast<BrowsingDataRemoverImpl*>(
+      BrowsingDataRemoverFactory::GetForBrowserContext(GetProfile()));
   InspectableCompletionObserver completion_observer(remover);
   remover->RemoveAndReply(base::Time(), base::Time::Max(),
                           BrowsingDataRemover::REMOVE_HISTORY,
                           BrowsingDataHelper::UNPROTECTED_WEB,
                           &completion_observer);
 
   // Process messages until the inhibitor is notified, and then some, to make
   // sure we do not complete asynchronously before ContinueToCompletion() is
   // called.
   completion_inhibitor.BlockUntilNearCompletion();
   base::RunLoop().RunUntilIdle();
 
   // Verify that the removal has not yet been completed and the observer has
   // not been called.
   EXPECT_TRUE(remover->is_removing());
   EXPECT_FALSE(completion_observer.called());
 
   // Now run the removal process until completion, and verify that observers are
   // now notified, and the notifications is sent out.
   completion_inhibitor.ContinueToCompletion();
   completion_observer.BlockUntilCompletion();
 
   EXPECT_FALSE(remover->is_removing());
   EXPECT_TRUE(completion_observer.called());
 }
 
 TEST_F(BrowsingDataRemoverTest, EarlyShutdown) {
-  BrowsingDataRemover* remover =
-      BrowsingDataRemoverFactory::GetForBrowserContext(GetProfile());
+  BrowsingDataRemoverImpl* remover = static_cast<BrowsingDataRemoverImpl*>(
+      BrowsingDataRemoverFactory::GetForBrowserContext(GetProfile()));
   InspectableCompletionObserver completion_observer(remover);
   BrowsingDataRemoverCompletionInhibitor completion_inhibitor;
   remover->RemoveAndReply(base::Time(), base::Time::Max(),
                           BrowsingDataRemover::REMOVE_HISTORY,
                           BrowsingDataHelper::UNPROTECTED_WEB,
                           &completion_observer);
 
   completion_inhibitor.BlockUntilNearCompletion();
 
   // Verify that the deletion has not yet been completed and the observer has
@@ skipping 581 matching lines @@
         << "Call ClearLastCalledTarget() before every removal task.";
     last_called_target_ = target;
   }
 
   Target target_a_;
   Target target_b_;
   Target* last_called_target_;
 };
 
 TEST_F(BrowsingDataRemoverTest, MultipleTasks) {
-  BrowsingDataRemover* remover =
-      BrowsingDataRemoverFactory::GetForBrowserContext(GetProfile());
+  BrowsingDataRemoverImpl* remover = static_cast<BrowsingDataRemoverImpl*>(
+      BrowsingDataRemoverFactory::GetForBrowserContext(GetProfile()));
   EXPECT_FALSE(remover->is_removing());
 
   std::unique_ptr<RegistrableDomainFilterBuilder> filter_builder_1(
       new RegistrableDomainFilterBuilder(
           RegistrableDomainFilterBuilder::WHITELIST));
   std::unique_ptr<RegistrableDomainFilterBuilder> filter_builder_2(
       new RegistrableDomainFilterBuilder(
           RegistrableDomainFilterBuilder::BLACKLIST));
   filter_builder_2->AddRegisterableDomain("example.com");
 
   MultipleTasksObserver observer(remover);
   BrowsingDataRemoverCompletionInhibitor completion_inhibitor;
 
   // Test several tasks with various configuration of masks, filters, and target
   // observers.
-  std::list<BrowsingDataRemover::RemovalTask> tasks;
+  std::list<BrowsingDataRemoverImpl::RemovalTask> tasks;
   tasks.emplace_back(base::Time(), base::Time::Max(),
                      BrowsingDataRemover::REMOVE_HISTORY,
                      BrowsingDataHelper::UNPROTECTED_WEB,
                      base::MakeUnique<RegistrableDomainFilterBuilder>(
                          RegistrableDomainFilterBuilder::BLACKLIST),
                      observer.target_a());
   tasks.emplace_back(base::Time(), base::Time::Max(),
                      BrowsingDataRemover::REMOVE_COOKIES,
                      BrowsingDataHelper::PROTECTED_WEB,
                      base::MakeUnique<RegistrableDomainFilterBuilder>(
@@ skipping 11 matching lines @@
       BrowsingDataHelper::UNPROTECTED_WEB,
       std::move(filter_builder_1),
       observer.target_b());
   tasks.emplace_back(
       base::Time::UnixEpoch(), base::Time::Now(),
       BrowsingDataRemover::REMOVE_CHANNEL_IDS,
       BrowsingDataHelper::ALL,
       std::move(filter_builder_2),
       nullptr);
 
-  for (BrowsingDataRemover::RemovalTask& task : tasks) {
+  for (BrowsingDataRemoverImpl::RemovalTask& task : tasks) {
     // All tasks can be directly translated to a RemoveInternal() call. Since
     // that is a private method, we must call the four public versions of
     // Remove.* instead. This also serves as a test that those methods are all
     // correctly reduced to RemoveInternal().
     if (!task.observer && task.filter_builder->IsEmptyBlacklist()) {
       remover->Remove(task.delete_begin, task.delete_end,
                       task.remove_mask, task.origin_type_mask);
     } else if (task.filter_builder->IsEmptyBlacklist()) {
       remover->RemoveAndReply(task.delete_begin, task.delete_end,
                               task.remove_mask, task.origin_type_mask,
                               task.observer);
     } else if (!task.observer) {
       remover->RemoveWithFilter(task.delete_begin, task.delete_end,
                                 task.remove_mask, task.origin_type_mask,
                                 std::move(task.filter_builder));
     } else {
       remover->RemoveWithFilterAndReply(task.delete_begin, task.delete_end,
                                         task.remove_mask, task.origin_type_mask,
                                         std::move(task.filter_builder),
                                         task.observer);
     }
   }
 
   // Use the inhibitor to stop after every task and check the results.
-  for (BrowsingDataRemover::RemovalTask& task : tasks) {
+  for (BrowsingDataRemoverImpl::RemovalTask& task : tasks) {
     EXPECT_TRUE(remover->is_removing());
     observer.ClearLastCalledTarget();
 
     // Finish the task execution synchronously.
     completion_inhibitor.BlockUntilNearCompletion();
     completion_inhibitor.ContinueToCompletion();
 
     // Observers, if any, should have been called by now (since we call
     // observers on the same thread).
     EXPECT_EQ(task.observer, observer.GetLastCalledTarget());
 
     // TODO(msramek): If BrowsingDataRemover took ownership of the last used
     // filter builder and exposed it, we could also test it here. Make it so.
     EXPECT_EQ(task.remove_mask, GetRemovalMask());
     EXPECT_EQ(task.origin_type_mask, GetOriginTypeMask());
     EXPECT_EQ(task.delete_begin, GetBeginTime());
   }
 
   EXPECT_FALSE(remover->is_removing());
 }
 
 // The previous test, BrowsingDataRemoverTest.MultipleTasks, tests that the
 // tasks are not mixed up and they are executed in a correct order. However,
 // the completion inhibitor kept synchronizing the execution in order to verify
 // the parameters. This test demonstrates that even running the tasks without
 // inhibition is executed correctly and doesn't crash.
 TEST_F(BrowsingDataRemoverTest, MultipleTasksInQuickSuccession) {
-  BrowsingDataRemover* remover =
-      BrowsingDataRemoverFactory::GetForBrowserContext(GetProfile());
+  BrowsingDataRemoverImpl* remover = static_cast<BrowsingDataRemoverImpl*>(
+      BrowsingDataRemoverFactory::GetForBrowserContext(GetProfile()));
   EXPECT_FALSE(remover->is_removing());
 
   int test_removal_masks[] = {
       BrowsingDataRemover::REMOVE_COOKIES,
       BrowsingDataRemover::REMOVE_PASSWORDS,
       BrowsingDataRemover::REMOVE_COOKIES,
       BrowsingDataRemover::REMOVE_COOKIES,
       BrowsingDataRemover::REMOVE_COOKIES,
       BrowsingDataRemover::REMOVE_HISTORY,
       BrowsingDataRemover::REMOVE_HISTORY,
@@ skipping 68 matching lines @@
   // There should be no recently visited bookmarks.
   EXPECT_THAT(ntp_snippets::GetRecentlyVisitedBookmarks(
                   bookmark_model, 2, base::Time::UnixEpoch(),
                   /*consider_visits_from_desktop=*/false),
               IsEmpty());
   EXPECT_THAT(ntp_snippets::GetRecentlyVisitedBookmarks(
                   bookmark_model, 2, base::Time::UnixEpoch(),
                   /*consider_visits_from_desktop=*/true),
               IsEmpty());
 }