Adding ClearBlackList to the PreviewsBlackList and plumbing to UI

components/previews/core/previews_black_list_unittest.cc

Index: components/previews/core/previews_black_list_unittest.cc
diff --git a/components/previews/core/previews_black_list_unittest.cc b/components/previews/core/previews_black_list_unittest.cc
index d9fd376183c263b64f159a2df011c60cb69283f5..d8bc676552be74bf4a37d607f368b373f7e02e43 100644
--- a/components/previews/core/previews_black_list_unittest.cc
+++ b/components/previews/core/previews_black_list_unittest.cc
@@ -50,20 +50,22 @@ class TestPreviewsOptOutStore : public PreviewsOptOutStore {
                             PreviewsType type,
                             base::Time now) override {}
 
   void LoadBlackList(LoadBlackListCallback callback) override {
     std::unique_ptr<BlackListItemMap> black_list_item_map(
         new BlackListItemMap());
     base::MessageLoop::current()->task_runner()->PostTask(
         FROM_HERE, base::Bind(&RunLoadCallback, callback,
                               base::Passed(&black_list_item_map)));
   }
+
+  void ClearBlackList(base::Time begin_time, base::Time end_time) override {}
 };
 
 }  // namespace
 
 TEST_F(PreviewsBlackListTest, BlackListNoStore) {
   // Tests the black list behavior when a null OptOutSture is passed in.
   const GURL url_a("http://www.url_a.com");
   const GURL url_b("http://www.url_b.com");
   const size_t history = 4;
   const int threshold = 2;
@@ -71,21 +73,23 @@ TEST_F(PreviewsBlackListTest, BlackListNoStore) {
   base::FieldTrialList field_trial_list(nullptr);
   std::map<std::string, std::string> params;
   params["stored_history_length"] = base::SizeTToString(history);
   params["opt_out_threshold"] = base::IntToString(threshold);
   params["black_list_duration_in_days"] = base::IntToString(duration_in_days);
   ASSERT_TRUE(
       variations::AssociateVariationParams("ClientSidePreviews", "Enabled",
                                            params) &&
       base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled"));
 
-  base::Clock* test_clock = new base::SimpleTestClock();
+  base::SimpleTestClock* test_clock = new base::SimpleTestClock();
+  base::Time start = test_clock->Now();
+  test_clock->Advance(base::TimeDelta::FromSeconds(1));
 
   std::unique_ptr<PreviewsBlackList> black_list(
       new PreviewsBlackList(nullptr, base::WrapUnique(test_clock)));
 
   EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE));
   EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
 
   black_list->AddPreviewNavigation(url_a, true, PreviewsType::OFFLINE);
   black_list->AddPreviewNavigation(url_a, true, PreviewsType::OFFLINE);
 
@@ -98,20 +102,26 @@ TEST_F(PreviewsBlackListTest, BlackListNoStore) {
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE));
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
 
   black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE);
   black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE);
   black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE);
 
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE));
   EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
 
+  test_clock->Advance(base::TimeDelta::FromSeconds(1));
+  black_list->ClearBlackList(start, test_clock->Now());
+
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE));
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
+
   variations::testing::ClearAllVariationParams();
 }
 
 TEST_F(PreviewsBlackListTest, BlackListWithStore) {
   // Tests the black list behavior when a non-null OptOutSture is passed in.
   const GURL url_a1("http://www.url_a.com/a1");
   const GURL url_a2("http://www.url_a.com/a2");
   const GURL url_b("http://www.url_b.com");
   const size_t history = 4;
   const int threshold = 2;
@@ -121,21 +131,23 @@ TEST_F(PreviewsBlackListTest, BlackListWithStore) {
   params["stored_history_length"] = base::SizeTToString(history);
   params["opt_out_threshold"] = base::IntToString(threshold);
   params["black_list_duration_in_days"] = base::IntToString(duration_in_days);
   ASSERT_TRUE(
       variations::AssociateVariationParams("ClientSidePreviews", "Enabled",
                                            params) &&
       base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled"));
 
   base::MessageLoop loop;
 
-  base::Clock* test_clock = new base::SimpleTestClock();
+  base::SimpleTestClock* test_clock = new base::SimpleTestClock();
+  base::Time start = test_clock->Now();
+  test_clock->Advance(base::TimeDelta::FromSeconds(1));
 
   std::unique_ptr<PreviewsBlackList> black_list(
       new PreviewsBlackList(base::MakeUnique<TestPreviewsOptOutStore>(),
                             base::WrapUnique(test_clock)));
 
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE));
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE));
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
 
   base::RunLoop().RunUntilIdle();
@@ -159,54 +171,83 @@ TEST_F(PreviewsBlackListTest, BlackListWithStore) {
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
 
   black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE);
   black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE);
   black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE);
 
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE));
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE));
   EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
 
+  test_clock->Advance(base::TimeDelta::FromSeconds(1));
+  black_list->ClearBlackList(start, base::Time::Now());
+
+  EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE));
+  EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE));
+  EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
+
+  base::RunLoop().RunUntilIdle();

[tbansal1, 2016/10/03 16:48:12]

What is this message pump doing given that the TestPreviewsOptOutStore has an
empty implementation for ClearBlackList()?

[RyanSturm, 2016/10/03 17:23:13]

ClearBlackList actually calls both ClearBlackList and LoadBlackList from the
store.

I imagine this test would fail if no post task were occuring.

+
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE));
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE));
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
+
   variations::testing::ClearAllVariationParams();
 }
 
 TEST_F(PreviewsBlackListTest, QueueBehavior) {
   // Tests the black list asynchronous queue behavior. Methods called while
   // loading are queued and should run in the order they were queued.
   const GURL url("http://www.url.com");
+  const GURL url2("http://www.url2.com");
   const int duration_in_days = 365;
   base::FieldTrialList field_trial_list(nullptr);
   std::map<std::string, std::string> params;
   params["black_list_duration_in_days"] = base::IntToString(duration_in_days);
   ASSERT_TRUE(
       variations::AssociateVariationParams("ClientSidePreviews", "Enabled",
                                            params) &&
       base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled"));
 
   base::MessageLoop loop;
 
   std::vector<bool> test_opt_out{true, false};
 
   for (auto opt_out : test_opt_out) {
-    base::Clock* test_clock = new base::SimpleTestClock();
+    base::SimpleTestClock* test_clock = new base::SimpleTestClock();
 
     std::unique_ptr<PreviewsBlackList> black_list(
         new PreviewsBlackList(base::MakeUnique<TestPreviewsOptOutStore>(),
                               base::WrapUnique(test_clock)));
 
     EXPECT_FALSE(black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE));
     black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE);
     black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE);
     EXPECT_FALSE(black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE));
     base::RunLoop().RunUntilIdle();
     EXPECT_EQ(!opt_out,
               black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE));
+
+    base::Time start = test_clock->Now();
+    test_clock->Advance(base::TimeDelta::FromSeconds(1));
+    black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE);
+    black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE);
+    test_clock->Advance(base::TimeDelta::FromSeconds(1));
+    black_list->ClearBlackList(
+        start, test_clock->Now() + base::TimeDelta::FromSeconds(1));
+    black_list->AddPreviewNavigation(url2, opt_out, PreviewsType::OFFLINE);
+    black_list->AddPreviewNavigation(url2, opt_out, PreviewsType::OFFLINE);
+    base::RunLoop().RunUntilIdle();
+
+    EXPECT_TRUE(black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE));
+    EXPECT_EQ(!opt_out,
+              black_list->IsLoadedAndAllowed(url2, PreviewsType::OFFLINE));
   }
 
   variations::testing::ClearAllVariationParams();
 }
 
 TEST_F(PreviewsBlackListTest, MaxHosts) {
   // Test that the black list only stores n hosts, and it stores the correct n
   // hosts.
   const GURL url_a("http://www.url_a.com");
   const GURL url_b("http://www.url_b.com");