[ImportantSites] Implementing dialog level blacklisting and blacklist expiration

chrome/browser/engagement/important_sites_util_unittest.cc

 // Copyright 2016 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/engagement/important_sites_util.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/files/scoped_temp_dir.h"
@@ skipping 240 matching lines @@
   std::vector<GURL> expected_sorted_origins = {url1, url4};
   ExpectImportantResultsEqualUnordered(
       expected_sorted_domains, expected_sorted_origins, important_sites);
 }
 
 TEST_F(ImportantSitesUtilTest, Blacklisting) {
   SiteEngagementService* service = SiteEngagementService::Get(profile());
   ASSERT_TRUE(service);
 
   GURL url1("http://www.google.com/");
+  GURL url2("http://www.gmail.com/");
 
   // Set a bunch of positive signals.
   service->ResetScoreForURL(url1, 5);
-  AddBookmark(url1);
+  AddBookmark(url2);
   AddContentSetting(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, CONTENT_SETTING_ALLOW,
                     url1);
 
   // Important fetch 1.
   std::vector<ImportantDomainInfo> important_sites =
       ImportantSitesUtil::GetImportantRegisterableDomains(profile(),
                                                           kNumImportantSites);
-  std::vector<std::string> expected_sorted_domains = {"google.com"};
-  std::vector<GURL> expected_sorted_origins = {url1};
+  std::vector<std::string> expected_sorted_domains = {"google.com",
+                                                      "gmail.com"};
+  std::vector<GURL> expected_sorted_origins = {url1, url2};
   ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins,
                            important_sites);
-  ASSERT_EQ(1u, important_sites.size());
+  ASSERT_EQ(2u, important_sites.size());
   // Record ignore twice.
   ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
-      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      profile(), {"gmail.com"}, {important_sites[1].reason_bitfield},
       {"google.com"}, {important_sites[0].reason_bitfield});
   ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
-      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      profile(), {"gmail.com"}, {important_sites[1].reason_bitfield},
       {"google.com"}, {important_sites[0].reason_bitfield});
 
   // Important fetch 2.
   important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
       profile(), kNumImportantSites);
   ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins,
                            important_sites);
   // We shouldn't blacklist after first two times.
-  ASSERT_EQ(1u, important_sites.size());
+  ASSERT_EQ(2u, important_sites.size());
 
   // Record ignore 3rd time.
   ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
-      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      profile(), {"gmail.com"}, {important_sites[1].reason_bitfield},
       {"google.com"}, {important_sites[0].reason_bitfield});
 
-  // Important fetch 3. We should be blacklisted now.
+  // Important fetch 3. Google.com should be blacklisted now.
   important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
       profile(), kNumImportantSites);
-  ASSERT_EQ(0u, important_sites.size());
+
+  ASSERT_EQ(1u, important_sites.size());
+  expected_sorted_domains = {"gmail.com"};
+  expected_sorted_origins = {url2};
+  ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins,
+                           important_sites);
 }
 
 TEST_F(ImportantSitesUtilTest, BlacklistingReset) {
   SiteEngagementService* service = SiteEngagementService::Get(profile());
   ASSERT_TRUE(service);
 
   GURL url1("http://www.google.com/");
+  GURL url2("http://www.gmail.com/");
 
   // Set a bunch of positive signals.
   service->ResetScoreForURL(url1, 5);
-  AddBookmark(url1);
+  AddBookmark(url2);
   AddContentSetting(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, CONTENT_SETTING_ALLOW,
                     url1);
 
+  // Important fetch 1.
   std::vector<ImportantDomainInfo> important_sites =
       ImportantSitesUtil::GetImportantRegisterableDomains(profile(),
                                                           kNumImportantSites);
-
-  // Record ignored twice.
-  ASSERT_EQ(1u, important_sites.size());
+  ASSERT_EQ(2u, important_sites.size());
+  // Record ignore twice.
   ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
-      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      profile(), {"gmail.com"}, {important_sites[1].reason_bitfield},
       {"google.com"}, {important_sites[0].reason_bitfield});
   ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
-      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      profile(), {"gmail.com"}, {important_sites[1].reason_bitfield},
       {"google.com"}, {important_sites[0].reason_bitfield});
 
   // Important fetch, we should still be there.
   important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
       profile(), kNumImportantSites);
-  std::vector<std::string> expected_sorted_domains = {"google.com"};
-  std::vector<GURL> expected_sorted_origins = {url1};
-  ASSERT_EQ(1u, important_sites.size());
+  std::vector<std::string> expected_sorted_domains = {"google.com",
+                                                      "gmail.com"};
+  std::vector<GURL> expected_sorted_origins = {url1, url2};
+  ASSERT_EQ(2u, important_sites.size());
   ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins,
                            important_sites);
 
   // Record NOT ignored.
   ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
-      profile(), {"google.com"}, {important_sites[0].reason_bitfield},
+      profile(), {"google.com", "gmail.com"},
+      {important_sites[0].reason_bitfield, important_sites[1].reason_bitfield},
       std::vector<std::string>(), std::vector<int32_t>());
 
-  // Record ignored twice again.
+  // Record ignored twice again
   ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
-      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      profile(), {"gmail.com"}, {important_sites[1].reason_bitfield},
       {"google.com"}, {important_sites[0].reason_bitfield});
   ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
-      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      profile(), {"gmail.com"}, {important_sites[1].reason_bitfield},
       {"google.com"}, {important_sites[0].reason_bitfield});
 
   // Important fetch, we should still be there.
   important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
       profile(), kNumImportantSites);
   ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins,
                            important_sites);
 
   // Record ignored 3rd time in a row.
   ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
-      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      profile(), {"gmail.com"}, {important_sites[1].reason_bitfield},
       {"google.com"}, {important_sites[0].reason_bitfield});
 
   // Blacklisted now.
   important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
       profile(), kNumImportantSites);
-  EXPECT_EQ(0u, important_sites.size());
+  ASSERT_EQ(1u, important_sites.size());
+  expected_sorted_domains = {"gmail.com"};
+  expected_sorted_origins = {url2};
+  ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins,
+                           important_sites);
 }
 
 TEST_F(ImportantSitesUtilTest, Metrics) {
   SiteEngagementService* service = SiteEngagementService::Get(profile());
   ASSERT_TRUE(service);
   base::HistogramTester histogram_tester;
 
   GURL url1("http://www.google.com/");
   service->ResetScoreForURL(url1, 5);
   AddContentSetting(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, CONTENT_SETTING_ALLOW,
@@ skipping 24 matching lines @@
       histogram_tester.GetAllSamples("Storage.ImportantSites.CBDIgnoredReason"),
       testing::ElementsAre(base::Bucket(BOOKMARKS, 1)));
 
   // Bookmarks are "unknown", as they were added after the crossed reasons.
   EXPECT_THAT(histogram_tester.GetAllSamples(
                   "Storage.BlacklistedImportantSites.Reason"),
               testing::ElementsAre(
                   base::Bucket(CROSSED_NOTIFICATIONS_AND_ENGAGEMENT, 1),
                   base::Bucket(CROSSED_REASON_UNKNOWN, 1)));
 }
+
+TEST_F(ImportantSitesUtilTest, DialogBlacklisting) {
+  SiteEngagementService* service = SiteEngagementService::Get(profile());
+  ASSERT_TRUE(service);
+
+  GURL url1("http://www.google.com/");
+  GURL url2("http://www.yahoo.com/");
+
+  // Set a bunch of positive signals.
+  service->ResetScoreForURL(url2, 5);
+  AddBookmark(url1);
+  AddContentSetting(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, CONTENT_SETTING_ALLOW,
+                    url1);
+
+  // Start off not disabled.
+  EXPECT_FALSE(ImportantSitesUtil::IsDialogDisabled(profile()));
+
+  // Important fetch 1.
+  std::vector<ImportantDomainInfo> important_sites =
+      ImportantSitesUtil::GetImportantRegisterableDomains(profile(),
+                                                          kNumImportantSites);
+  std::vector<std::string> expected_sorted_domains = {"google.com",
+                                                      "yahoo.com"};
+  std::vector<GURL> expected_sorted_origins = {url1, url2};
+  ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins,
+                           important_sites);
+  ASSERT_EQ(2u, important_sites.size());
+  // Ignore all sites 3 times.

[dominickn, 2017/02/03 21:14:51]

Nit: we ignore twice first

[dmurph, 2017/02/04 00:52:25]

Done.

+  ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
+      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      {"google.com", "yahoo.com"},
+      {important_sites[0].reason_bitfield, important_sites[1].reason_bitfield});
+  ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(
+      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      {"google.com", "yahoo.com"},
+      {important_sites[0].reason_bitfield, important_sites[1].reason_bitfield});
+
+  // Still not disabled...
+  EXPECT_FALSE(ImportantSitesUtil::IsDialogDisabled(profile()));
+
+  ImportantSitesUtil::RecordBlacklistedAndIgnoredImportantSites(

[dominickn, 2017/02/03 21:14:51]

Nit: this is the third ignore

[dmurph, 2017/02/04 00:52:25]

Done.

+      profile(), std::vector<std::string>(), std::vector<int32_t>(),
+      {"google.com", "yahoo.com"},
+      {important_sites[0].reason_bitfield, important_sites[1].reason_bitfield});
+
+  // Items should still be present.
+  important_sites = ImportantSitesUtil::GetImportantRegisterableDomains(
+      profile(), kNumImportantSites);
+  ExpectImportantResultsEq(expected_sorted_domains, expected_sorted_origins,
+                           important_sites);
+
+  // Dialog should be blacklisted.
+  EXPECT_TRUE(ImportantSitesUtil::IsDialogDisabled(profile()));
+}