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..e14fa83b97815f4977d6bc3330bc922e8d642f84 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,6 +24,10 @@ bool FilterAll(const GURL& url) {
   return true;
 }
 
+void AutoBlockerCallback(bool expected, bool result) {
+  EXPECT_EQ(expected, result);
+}
+
 }  // namespace
 
 class PermissionDecisionAutoBlockerUnitTest
@@ -44,6 +53,30 @@ class PermissionDecisionAutoBlockerUnitTest
     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, permission, url, nullptr, 2000 /* timeout in ms */, profile(),
+        current_time, base::Bind(&AutoBlockerCallback, expected_result));
+  }
 };
 
 TEST_F(PermissionDecisionAutoBlockerUnitTest, RemoveCountsByUrl) {
@@ -100,3 +133,103 @@ 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 status
+// 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();
+  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) {

[raymes, 2017/01/18 03:15:20]

Does this test anything more than the above test?

[meredithl, 2017/01/18 08:28:16]

Dom and I thought you were looking for a way just to verify that the
PlaceUnderEmbargo test was working correctly, can remove if not
relevant/helpful.

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

I'm not sure I understand completely, but I think the coverage in the above test
is almost equivalent and would allow us to remove the GetEmbargoStatus test
function.

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

Done (removed).

+  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_EQ(1, RecordDismiss(url, content::PermissionType::GEOLOCATION));
+  EXPECT_EQ(2, RecordDismiss(url, content::PermissionType::GEOLOCATION));
+
+  // A request with < 3 prior dismisses should not be automatically blocked.
+  UpdateEmbargoedStatus(content::PermissionType::GEOLOCATION, url, time_now,
+                        false);

[raymes, 2017/01/18 03:15:20]

nit: document booleans

[meredithl, 2017/01/18 08:28:16]

Done.

+
+  // After the 3rd dismiss subsequent permission requests should be autoblocked.
+  EXPECT_EQ(3, RecordDismiss(url, content::PermissionType::GEOLOCATION));
+  UpdateEmbargoedStatus(content::PermissionType::GEOLOCATION, url, time_now,
+                        true);
+
+  // Accelerate time forward, check that the embargo status is lifted and the
+  // request won't be automatically blocked.
+  time_now += base::TimeDelta::FromDays(8);
+  UpdateEmbargoedStatus(content::PermissionType::GEOLOCATION, url, time_now,
+                        false);
+
+  // Record another dismiss, subsequent requests should be autoblocked again.
+  EXPECT_EQ(4, RecordDismiss(url, content::PermissionType::GEOLOCATION));
+  UpdateEmbargoedStatus(content::PermissionType::GEOLOCATION, url, time_now,
+                        true);
+
+  // Accelerate time again, check embargo is lifted and another permission
+  // request is let through.
+  time_now += base::TimeDelta::FromDays(8);
+  UpdateEmbargoedStatus(content::PermissionType::GEOLOCATION, url, time_now,
+                        false);
+}