Implement embargo in PermissionDecisionAutoBlocker

chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc

Index: chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
diff --git a/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc b/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
index 852696e16d94cb56fbd1af3605ddc2b8ac315513..7b4108bae6766d58e3c16a217da6fc10673e3e30 100644
--- a/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
+++ b/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
@@ -5,8 +5,13 @@
 #include "chrome/browser/permissions/permission_decision_auto_blocker.h"
 
 #include "base/bind.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/time/time.h"
+#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/safe_browsing_db/test_database_manager.h"
 #include "content/public/browser/permission_type.h"
 
 namespace {
@@ -19,44 +24,89 @@ bool FilterAll(const GURL& url) {
   return true;
 }
 
+void AutoBlockerCallback(bool expected, bool result) {
+  EXPECT_EQ(expected, result);
+}
+
 }  // namespace
 
 class PermissionDecisionAutoBlockerUnitTest
     : public ChromeRenderViewHostTestHarness {
  protected:
-  int GetDismissalCount(const GURL& url,
-                        content::PermissionType permission) {
-    return PermissionDecisionAutoBlocker::GetDismissCount(
-        url, permission, profile());
+  int GetDismissalCount(const GURL& url, content::PermissionType permission) {
+    return PermissionDecisionAutoBlocker::GetDismissCount(url, permission,
+                                                          profile());
   }
 
   int GetIgnoreCount(const GURL& url, content::PermissionType permission) {
-    return PermissionDecisionAutoBlocker::GetIgnoreCount(
-        url, permission, profile());
+    return PermissionDecisionAutoBlocker::GetIgnoreCount(url, permission,
+                                                         profile());
   }
 
-  int RecordDismiss(const GURL& url, content::PermissionType permission) {
-    return PermissionDecisionAutoBlocker::RecordDismiss(url, permission,
-                                                        profile());
+  int RecordDismissAndEmbargo(const GURL& url,
+                              content::PermissionType permission,
+                              base::Time current_time) {
+    return PermissionDecisionAutoBlocker::RecordDismissAndEmbargo(
+        url, permission, profile(), current_time);
   }
 
   int RecordIgnore(const GURL& url, content::PermissionType permission) {
     return PermissionDecisionAutoBlocker::RecordIgnore(url, permission,
                                                        profile());
   }
+
+  void PlaceUnderEmbargo(content::PermissionType permission,
+                         const GURL& url,
+                         HostContentSettingsMap* map,
+                         base::Time current_time) {
+    PermissionDecisionAutoBlocker::PlaceUnderEmbargoForTest(permission, url,
+                                                            map, current_time);
+  }
+
+  bool GetEmbargoStatus(content::PermissionType permission,
+                        const GURL& url,
+                        HostContentSettingsMap* map) {
+    return PermissionDecisionAutoBlocker::GetEmbargoStatusForTest(permission,
+                                                                  url, map);
+  }
+
+  void UpdateEmbargoedStatus(content::PermissionType permission,
+                             const GURL& url,
+                             base::Time current_time,
+                             bool expected_result) {
+    PermissionDecisionAutoBlocker::UpdateEmbargoedStatus(
+        nullptr /* db manager */, permission, url, nullptr /* web contents */,
+        2000 /* timeout in ms */, profile(), current_time,
+        base::Bind(&AutoBlockerCallback, expected_result));
+  }
 };
 
 TEST_F(PermissionDecisionAutoBlockerUnitTest, RemoveCountsByUrl) {
   GURL url1("https://www.google.com");
   GURL url2("https://www.example.com");
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kBlockPromptsIfDismissedOften);
 
   // Record some dismissals.
-  EXPECT_EQ(1, RecordDismiss(url1, content::PermissionType::GEOLOCATION));
-  EXPECT_EQ(2, RecordDismiss(url1, content::PermissionType::GEOLOCATION));
-  EXPECT_EQ(3, RecordDismiss(url1, content::PermissionType::GEOLOCATION));
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url1, content::PermissionType::GEOLOCATION, base::Time::Now()));
+  EXPECT_EQ(1, GetDismissalCount(url1, content::PermissionType::GEOLOCATION));
+
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url1, content::PermissionType::GEOLOCATION, base::Time::Now()));
+  EXPECT_EQ(2, GetDismissalCount(url1, content::PermissionType::GEOLOCATION));
+
+  EXPECT_TRUE(RecordDismissAndEmbargo(
+      url1, content::PermissionType::GEOLOCATION, base::Time::Now()));
+  EXPECT_EQ(3, GetDismissalCount(url1, content::PermissionType::GEOLOCATION));
+
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url2, content::PermissionType::GEOLOCATION, base::Time::Now()));
+  EXPECT_EQ(1, GetDismissalCount(url2, content::PermissionType::GEOLOCATION));
 
-  EXPECT_EQ(1, RecordDismiss(url2, content::PermissionType::GEOLOCATION));
-  EXPECT_EQ(1, RecordDismiss(url1, content::PermissionType::NOTIFICATIONS));
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url1, content::PermissionType::NOTIFICATIONS, base::Time::Now()));
+  EXPECT_EQ(1, GetDismissalCount(url1, content::PermissionType::NOTIFICATIONS));
 
   // Record some ignores.
   EXPECT_EQ(1, RecordIgnore(url1, content::PermissionType::MIDI_SYSEX));
@@ -77,9 +127,17 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, RemoveCountsByUrl) {
   EXPECT_EQ(2, GetIgnoreCount(url2, content::PermissionType::GEOLOCATION));
 
   // Add some more actions.
-  EXPECT_EQ(1, RecordDismiss(url1, content::PermissionType::GEOLOCATION));
-  EXPECT_EQ(1, RecordDismiss(url1, content::PermissionType::NOTIFICATIONS));
-  EXPECT_EQ(2, RecordDismiss(url2, content::PermissionType::GEOLOCATION));
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url1, content::PermissionType::GEOLOCATION, base::Time::Now()));
+  EXPECT_EQ(1, GetDismissalCount(url1, content::PermissionType::GEOLOCATION));
+
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url1, content::PermissionType::NOTIFICATIONS, base::Time::Now()));
+  EXPECT_EQ(1, GetDismissalCount(url1, content::PermissionType::NOTIFICATIONS));
+
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url2, content::PermissionType::GEOLOCATION, base::Time::Now()));
+  EXPECT_EQ(2, GetDismissalCount(url2, content::PermissionType::GEOLOCATION));
 
   EXPECT_EQ(1, RecordIgnore(url1, content::PermissionType::GEOLOCATION));
   EXPECT_EQ(1, RecordIgnore(url1, content::PermissionType::NOTIFICATIONS));
@@ -100,3 +158,156 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, RemoveCountsByUrl) {
   EXPECT_EQ(0, GetIgnoreCount(url2, content::PermissionType::DURABLE_STORAGE));
   EXPECT_EQ(0, GetIgnoreCount(url2, content::PermissionType::MIDI_SYSEX));
 }
+
+// Check that IsUnderEmbargo returns the correct value when the embargo is set
+// and expires.
+TEST_F(PermissionDecisionAutoBlockerUnitTest, CheckEmbargoStatus) {
+  GURL url("https://www.google.com");
+  auto* map = HostContentSettingsMapFactory::GetForProfile(profile());
+  base::Time time_now = base::Time::Now();
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kPermissionsBlacklist);
+
+  EXPECT_TRUE(base::FeatureList::IsEnabled(features::kPermissionsBlacklist));
+
+  PlaceUnderEmbargo(content::PermissionType::GEOLOCATION, url, map, time_now);
+
+  EXPECT_TRUE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url, time_now));
+
+  // Check that the origin is not under embargo for another permission.
+  EXPECT_FALSE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::NOTIFICATIONS, profile(), url, time_now));
+
+  EXPECT_TRUE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url,
+      time_now + base::TimeDelta::FromDays(5)));
+
+  // Check embargo is lifted on expiry day. A small offset after the exact
+  // embargo expiration date has been added to account for any precision errors
+  // when removing the date stored as a double from the permission dictionary.
+  EXPECT_FALSE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url,
+      time_now + base::TimeDelta::FromHours(7 * 24 + 1)));
+
+  // Check embargo is lifted well after the expiry day.
+  EXPECT_FALSE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url,
+      time_now + base::TimeDelta::FromDays(8)));
+
+  // Place under embargo again.
+  time_now = base::Time::Now();
+  PlaceUnderEmbargo(content::PermissionType::NOTIFICATIONS, url, map, time_now);
+
+  EXPECT_TRUE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::NOTIFICATIONS, profile(), url, time_now));
+}
+
+// Check that the embargo status is correctly recorded in PlaceUnderEmbargo.
+TEST_F(PermissionDecisionAutoBlockerUnitTest, TestUpdatingEmbargoStatus) {
+  GURL url("https://www.google.com");
+  auto* map = HostContentSettingsMapFactory::GetForProfile(profile());
+
+  EXPECT_FALSE(
+      GetEmbargoStatus(content::PermissionType::GEOLOCATION, url, map));
+
+  PlaceUnderEmbargo(content::PermissionType::GEOLOCATION, url, map,
+                    base::Time::Now());
+
+  EXPECT_TRUE(GetEmbargoStatus(content::PermissionType::GEOLOCATION, url, map));
+}
+
+// Tests the alternating pattern of the block on multiple dismiss behaviour. On
+// N prior dismissals, the N+1th request will cause the origin to be embargoed
+// for the requested permission and automatically blocked. Each time the embargo
+// is lifted, the site gets another chance to request the permission, but if it
+// is again dismissed it is placed under embargo again and the request
+// automatically blocked for that period.
+TEST_F(PermissionDecisionAutoBlockerUnitTest, TestDismissEmbargo) {
+  GURL url("https://www.google.com");
+  base::Time time_now = base::Time::Now();
+  // Enable the autoblocking feature, which is disabled by default.
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kBlockPromptsIfDismissedOften);
+
+  EXPECT_TRUE(
+      base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften));
+
+  // Record some dismisses.
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url, content::PermissionType::GEOLOCATION, time_now));
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url, content::PermissionType::GEOLOCATION, time_now));
+
+  // A request with < 3 prior dismisses should not be automatically blocked.
+  EXPECT_FALSE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url, time_now));
+
+  // After the 3rd dismiss subsequent permission requests should be autoblocked.
+  EXPECT_TRUE(RecordDismissAndEmbargo(url, content::PermissionType::GEOLOCATION,
+                                      time_now));
+  EXPECT_TRUE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url, time_now));
+
+  // Accelerate time forward, check that the embargo status is lifted and the
+  // request won't be automatically blocked.
+  time_now += base::TimeDelta::FromDays(8);
+  EXPECT_FALSE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url, time_now));
+
+  // Record another dismiss, subsequent requests should be autoblocked again.
+  EXPECT_TRUE(RecordDismissAndEmbargo(url, content::PermissionType::GEOLOCATION,
+                                      time_now));
+  EXPECT_TRUE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url, time_now));
+
+  // Accelerate time again, check embargo is lifted and another permission
+  // request is let through.
+  time_now += base::TimeDelta::FromDays(8);
+  EXPECT_FALSE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url, time_now));
+}
+
+// Test the logic for different combinations of embargo.
+TEST_F(PermissionDecisionAutoBlockerUnitTest, TestExpiredBlacklistEmbargo) {
+  GURL url("https://www.google.com");
+  base::Time time_now = base::Time::Now();
+  auto* map = HostContentSettingsMapFactory::GetForProfile(profile());
+
+  // Enable both dismissals and permissions blacklisting features.
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitWithFeatures({features::kBlockPromptsIfDismissedOften,
+                                 features::kPermissionsBlacklist},
+                                {});

[raymes, 2017/01/18 23:35:00]

nit: it might be worth just setting up these feature lists in the SetUp function
of PermissionDecisionAutoBlockerUnitTest so we don't need to in each test. It's
not too important to do in this CL.

[meredithl, 2017/01/19 02:11:43]

Acknowledged.

+
+  EXPECT_TRUE(
+      base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften));
+  EXPECT_TRUE(base::FeatureList::IsEnabled(features::kPermissionsBlacklist));
+
+  // Place under blacklist embargo and check the status.
+  PlaceUnderEmbargo(content::PermissionType::GEOLOCATION, url, map,
+                    base::Time::Now());
+  EXPECT_TRUE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url,
+      time_now + base::TimeDelta::FromDays(5)));
+
+  // Accelerate time to a point where the blacklist embargo should be expired.
+  time_now += base::TimeDelta::FromDays(8);
+
+  EXPECT_FALSE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url,
+      time_now + base::TimeDelta::FromDays(5)));
+
+  // Record dismisses to place it under dismissal embargo.
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url, content::PermissionType::GEOLOCATION, time_now));
+  EXPECT_FALSE(RecordDismissAndEmbargo(
+      url, content::PermissionType::GEOLOCATION, time_now));
+  EXPECT_TRUE(RecordDismissAndEmbargo(url, content::PermissionType::GEOLOCATION,
+                                      time_now));
+
+  // Check that it still returns true, even though the previous embargo for
+  // blacklisting has expired.
+  EXPECT_TRUE(PermissionDecisionAutoBlocker::IsUnderEmbargo(
+      content::PermissionType::GEOLOCATION, profile(), url, time_now));
+}