Adding a short blacklist period after every previews opt out

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 6f98c122b016f94e8f3651566c760aa418e0961d..737d8aa09eda0db4a9d09efed94ab80e8f280a16 100644
--- a/components/previews/core/previews_black_list_unittest.cc
+++ b/components/previews/core/previews_black_list_unittest.cc
@@ -70,25 +70,28 @@ class TestPreviewsOptOutStore : public PreviewsOptOutStore {
 
 }  // 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;
   const int duration_in_days = 365;
+  const int single_opt_out_duration = 0;
   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);
+  params["single_opt_out_duration_in_seconds"] =
+      base::IntToString(single_opt_out_duration);

[tbansal1, 2016/10/21 23:15:33]

Can you add a comment that it is set to 0 so that single opt-out does not kick
in.

[RyanSturm, 2016/10/24 21:18:25]

Done.

   ASSERT_TRUE(
       variations::AssociateVariationParams("ClientSidePreviews", "Enabled",
                                            params) &&
       base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled"));
 
   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(
@@ -126,25 +129,28 @@ TEST_F(PreviewsBlackListTest, BlackListNoStore) {
 }
 
 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;
   const int duration_in_days = 365;
+  const int single_opt_out_duration = 0;
   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);
+  params["single_opt_out_duration_in_seconds"] =
+      base::IntToString(single_opt_out_duration);
   ASSERT_TRUE(
       variations::AssociateVariationParams("ClientSidePreviews", "Enabled",
                                            params) &&
       base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled"));
 
   base::MessageLoop loop;
 
   base::SimpleTestClock* test_clock = new base::SimpleTestClock();
   base::Time start = test_clock->Now();
   test_clock->Advance(base::TimeDelta::FromSeconds(1));
@@ -204,23 +210,26 @@ TEST_F(PreviewsBlackListTest, BlackListWithStore) {
 
   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;
+  const int single_opt_out_duration = 0;
   base::FieldTrialList field_trial_list(nullptr);
   std::map<std::string, std::string> params;
   params["black_list_duration_in_days"] = base::IntToString(duration_in_days);
+  params["single_opt_out_duration_in_seconds"] =
+      base::IntToString(single_opt_out_duration);
   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) {
@@ -265,29 +274,32 @@ 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");
   const GURL url_c("http://www.url_c.com");
   const GURL url_d("http://www.url_d.com");
   const GURL url_e("http://www.url_e.com");
   const size_t stored_history_length = 1;
   const int opt_out_threshold = 1;
   const int black_list_duration_in_days = 365;
+  const int single_opt_out_duration = 0;
   const size_t max_hosts_in_blacklist = 2;
   base::FieldTrialList field_trial_list(nullptr);
   std::map<std::string, std::string> params;
   params["stored_history_length"] = base::SizeTToString(stored_history_length);
   params["opt_out_threshold"] = base::IntToString(opt_out_threshold);
   params["black_list_duration_in_days"] =
       base::IntToString(black_list_duration_in_days);
   params["max_hosts_in_blacklist"] =
       base::SizeTToString(max_hosts_in_blacklist);
+  params["single_opt_out_duration_in_seconds"] =
+      base::IntToString(single_opt_out_duration);
   ASSERT_TRUE(
       variations::AssociateVariationParams("ClientSidePreviews", "Enabled",
                                            params) &&
       base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled"));
 
   base::MessageLoop loop;
 
   base::SimpleTestClock* test_clock = new base::SimpleTestClock();
 
   std::unique_ptr<PreviewsBlackList> black_list(
@@ -308,11 +320,65 @@ TEST_F(PreviewsBlackListTest, MaxHosts) {
   test_clock->Advance(base::TimeDelta::FromSeconds(1));
   black_list->AddPreviewNavigation(url_e, true, PreviewsType::OFFLINE);
   // url_d and url_e should remain in the map, but url_a should be evicted.
   EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE));
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_d, PreviewsType::OFFLINE));
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_e, PreviewsType::OFFLINE));
 
   variations::testing::ClearAllVariationParams();
 }
 
+TEST_F(PreviewsBlackListTest, SingleOptOut) {
+  // Test that the black list only stores n hosts, and it stores the correct n

[tbansal1, 2016/10/21 23:15:33]

Comment looks stale.

[RyanSturm, 2016/10/24 21:18:25]

Done.

+  // hosts.
+  const GURL url_a("http://www.url_a.com");
+  const GURL url_b("http://www.url_b.com");
+  const GURL url_c("http://www.url_c.com");
+  const size_t stored_history_length = 1;
+  const int opt_out_threshold = 2;
+  const int black_list_duration_in_days = 365;
+  const int single_opt_out_duration = 5;
+  const size_t max_hosts_in_blacklist = 10;
+  base::FieldTrialList field_trial_list(nullptr);
+  std::map<std::string, std::string> params;
+  params["stored_history_length"] = base::SizeTToString(stored_history_length);
+  params["opt_out_threshold"] = base::IntToString(opt_out_threshold);
+  params["black_list_duration_in_days"] =
+      base::IntToString(black_list_duration_in_days);
+  params["max_hosts_in_blacklist"] =
+      base::SizeTToString(max_hosts_in_blacklist);
+  params["single_opt_out_duration_in_seconds"] =
+      base::IntToString(single_opt_out_duration);
+  ASSERT_TRUE(
+      variations::AssociateVariationParams("ClientSidePreviews", "Enabled",
+                                           params) &&
+      base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled"));
+
+  base::SimpleTestClock* test_clock = new base::SimpleTestClock();
+
+  std::unique_ptr<PreviewsBlackList> black_list(
+      new PreviewsBlackList(nullptr, base::WrapUnique(test_clock)));
+
+  black_list->AddPreviewNavigation(url_a, false, PreviewsType::OFFLINE);
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE));
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE));
+
+  test_clock->Advance(base::TimeDelta::FromSeconds(10));
+
+  black_list->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE);
+  EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
+  EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE));
+
+  test_clock->Advance(base::TimeDelta::FromSeconds(1));

[tbansal1, 2016/10/21 23:15:33]

s/1/single_opt_out_duration - 1/

[RyanSturm, 2016/10/24 21:18:25]

Done.

+
+  EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
+  EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE));
+
+  test_clock->Advance(base::TimeDelta::FromSeconds(10));

[tbansal1, 2016/10/21 23:15:33]

s/10/single_opt_out_duration + 1/
to make it clearer that 10 is related to the value of |single_opt_out_duration|

[RyanSturm, 2016/10/24 21:18:25]

Done.

+
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE));
+
+  variations::testing::ClearAllVariationParams();
+}
+
 }  // namespace previews