Implement a task scheduler for BrowsingDataRemover

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 <memory>
@@ skipping 1048 matching lines @@
         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->Remove(BrowsingDataRemover::Period(period), remove_mask,
-                    origin_type_mask);
+    remover->RemoveAndReply(BrowsingDataRemover::Period(period), 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(
       browsing_data::TimePeriod period,
       int remove_mask,
       const BrowsingDataFilterBuilder& filter_builder) {
     BrowsingDataRemover* remover =
         BrowsingDataRemoverFactory::GetForBrowserContext(profile_.get());
     TestStoragePartition storage_partition;
     remover->OverrideStoragePartitionForTesting(&storage_partition);
 
-    BrowsingDataRemoverCompletionObserver completion_observer(remover);
+    BrowsingDataRemoverCompletionInhibitor completion_inhibitor;
     remover->RemoveImpl(BrowsingDataRemover::Period(period), remove_mask,
                         filter_builder, BrowsingDataHelper::UNPROTECTED_WEB);
-    completion_observer.BlockUntilCompletion();
+    completion_inhibitor.BlockUntilNearCompletion();
+    completion_inhibitor.ContinueToCompletion();
 
     // Save so we can verify later.
     storage_partition_removal_data_ =
         storage_partition.GetStoragePartitionRemovalData();
   }
 
   TestingProfile* GetProfile() {
     return profile_.get();
   }
 
@@ skipping 1071 matching lines @@
 };
 
 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());
   InspectableCompletionObserver completion_observer(remover);
-  remover->Remove(BrowsingDataRemover::Unbounded(),
-                  BrowsingDataRemover::REMOVE_HISTORY,
-                  BrowsingDataHelper::UNPROTECTED_WEB);
+  remover->RemoveAndReply(BrowsingDataRemover::Unbounded(),
+                          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());
   InspectableCompletionObserver completion_observer(remover);
   BrowsingDataRemoverCompletionInhibitor completion_inhibitor;
-  remover->Remove(BrowsingDataRemover::Unbounded(),
-                  BrowsingDataRemover::REMOVE_HISTORY,
-                  BrowsingDataHelper::UNPROTECTED_WEB);
+  remover->RemoveAndReply(BrowsingDataRemover::Unbounded(),
+                          BrowsingDataRemover::REMOVE_HISTORY,
+                          BrowsingDataHelper::UNPROTECTED_WEB,
+                          &completion_observer);
 
   completion_inhibitor.BlockUntilNearCompletion();
 
   // Verify that the deletion has not yet been completed and the observer has
   // not been called.
   EXPECT_TRUE(remover->is_removing());
   EXPECT_FALSE(completion_observer.called());
 
   // Destroying the profile should trigger the notification.
   DestroyProfile();
@@ skipping 361 matching lines @@
   BrowsingDataRemover::ClearSettingsForOneTypeWithPredicate(
       host_content_settings_map, CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT,
       base::Bind(&MatchPrimaryPattern, http_pattern));
   // Verify we only have one, and it's url1.
   host_content_settings_map->GetSettingsForOneType(
       CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(), &host_settings);
   EXPECT_EQ(1u, host_settings.size());
   EXPECT_EQ(ContentSettingsPattern::FromURLNoWildcard(url1),
             host_settings[0].primary_pattern);
 }
+
+class MultipleTasksObserver {
+ public:
+  // A simple implementation of BrowsingDataRemover::Observer.
+  // MultipleTasksObserver will use several instances of Target to test
+  // that completion callbacks are returned to the correct one.
+  class Target : public BrowsingDataRemover::Observer {
+   public:
+    Target(MultipleTasksObserver* parent, BrowsingDataRemover* remover)
+        : parent_(parent),
+          observer_(this) {
+      observer_.Add(remover);
+    }
+    ~Target() override {}
+
+    void OnBrowsingDataRemoverDone() override {
+      parent_->SetLastCalledTarget(this);
+    }
+
+   private:
+    MultipleTasksObserver* parent_;
+    ScopedObserver<BrowsingDataRemover, BrowsingDataRemover::Observer>
+        observer_;
+  };
+
+  explicit MultipleTasksObserver(BrowsingDataRemover* remover)
+      : target_a_(this, remover),
+        target_b_(this, remover),
+        last_called_target_(nullptr) {}
+  ~MultipleTasksObserver() {}
+
+  void ClearLastCalledTarget() {
+    last_called_target_ = nullptr;
+  }
+
+  Target* GetLastCalledTarget() {
+    return last_called_target_;
+  }
+
+  Target* target_a() { return &target_a_; }
+  Target* target_b() { return &target_b_; }
+
+ private:
+  void SetLastCalledTarget(Target* target) {
+    DCHECK(!last_called_target_)
+        << "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());
+  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.
+  BrowsingDataRemover::RemovalTask tasks[] = {
+    BrowsingDataRemover::RemovalTask(
+        BrowsingDataRemover::Unbounded(),
+        BrowsingDataRemover::REMOVE_HISTORY,
+        BrowsingDataHelper::UNPROTECTED_WEB,
+        base::WrapUnique(new RegistrableDomainFilterBuilder(
+            RegistrableDomainFilterBuilder::BLACKLIST)),
+        observer.target_a()),
+    BrowsingDataRemover::RemovalTask(
+        BrowsingDataRemover::Unbounded(),
+        BrowsingDataRemover::REMOVE_COOKIES,
+        BrowsingDataHelper::PROTECTED_WEB,
+        base::WrapUnique(new RegistrableDomainFilterBuilder(
+            RegistrableDomainFilterBuilder::BLACKLIST)),
+        nullptr),
+    BrowsingDataRemover::RemovalTask(
+        BrowsingDataRemover::TimeRange(base::Time::Now(), base::Time::Max()),
+        BrowsingDataRemover::REMOVE_PASSWORDS,
+        BrowsingDataHelper::ALL,
+        base::WrapUnique(new RegistrableDomainFilterBuilder(
+            RegistrableDomainFilterBuilder::BLACKLIST)),
+        observer.target_b()),
+    BrowsingDataRemover::RemovalTask(
+        BrowsingDataRemover::TimeRange(base::Time(), base::Time::UnixEpoch()),
+        BrowsingDataRemover::REMOVE_PASSWORDS,
+        BrowsingDataHelper::UNPROTECTED_WEB,
+        std::move(filter_builder_1),
+        observer.target_b()),
+    BrowsingDataRemover::RemovalTask(
+        BrowsingDataRemover::TimeRange(
+            base::Time::UnixEpoch(), base::Time::Now()),
+        BrowsingDataRemover::REMOVE_CHANNEL_IDS,
+        BrowsingDataHelper::ALL,
+        std::move(filter_builder_2),
+        nullptr)
+  };
+
+  for (BrowsingDataRemover::RemovalTask& task : tasks) {
+    // All tasks can be directly translated to a RemoveWithFilterAndReply call.
+    // However, to additionally test that the four versions of Remove.* methods
+    // are all correctly reduced to RemoveWithFilterAndReply, we will always
+    // call the simplest one possible.
+    if (!task.observer && task.filter_builder->IsEmptyBlacklist()) {
+      remover->Remove(task.time_range, task.remove_mask, task.origin_type_mask);
+    } else if (task.filter_builder->IsEmptyBlacklist()) {
+      remover->RemoveAndReply(task.time_range, task.remove_mask,
+                              task.origin_type_mask, task.observer);
+    } else if (!task.observer) {
+      remover->RemoveWithFilter(task.time_range, task.remove_mask,
+                                task.origin_type_mask,
+                                std::move(task.filter_builder));
+    } else {
+      remover->RemoveWithFilterAndReply(task.time_range, 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) {
+    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.time_range.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());
+  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,
+      BrowsingDataRemover::REMOVE_HISTORY,
+      BrowsingDataRemover::REMOVE_COOKIES | BrowsingDataRemover::REMOVE_HISTORY,
+      BrowsingDataRemover::REMOVE_COOKIES | BrowsingDataRemover::REMOVE_HISTORY,
+      BrowsingDataRemover::REMOVE_COOKIES |
+          BrowsingDataRemover::REMOVE_HISTORY |
+          BrowsingDataRemover::REMOVE_PASSWORDS,
+      BrowsingDataRemover::REMOVE_PASSWORDS,
+      BrowsingDataRemover::REMOVE_PASSWORDS,
+  };
+
+  for (int removal_mask : test_removal_masks) {
+    remover->Remove(BrowsingDataRemover::Unbounded(), removal_mask,
+                    BrowsingDataHelper::UNPROTECTED_WEB);
+  }
+
+  EXPECT_TRUE(remover->is_removing());
+
+  // Add one more deletion and wait for it.
+  BlockUntilBrowsingDataRemoved(
+      browsing_data::EVERYTHING,
+      BrowsingDataRemover::REMOVE_COOKIES,
+      BrowsingDataHelper::UNPROTECTED_WEB);
+
+  EXPECT_FALSE(remover->is_removing());
+}