Implement embargo in PermissionDecisionAutoBlocker

chrome/browser/permissions/permission_decision_auto_blocker.cc

Index: chrome/browser/permissions/permission_decision_auto_blocker.cc
diff --git a/chrome/browser/permissions/permission_decision_auto_blocker.cc b/chrome/browser/permissions/permission_decision_auto_blocker.cc
index 309f1230eb131912b562c3ba1fad4d67559403ca..d3bb458ecc7d25819eb0506128de39bbdd8041f9 100644
--- a/chrome/browser/permissions/permission_decision_auto_blocker.cc
+++ b/chrome/browser/permissions/permission_decision_auto_blocker.cc
@@ -10,13 +10,16 @@
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/browser/permissions/permission_blacklist_client.h"
 #include "chrome/browser/permissions/permission_util.h"
 #include "chrome/common/chrome_features.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/permission_type.h"
+#include "content/public/browser/web_contents.h"
 #include "url/gurl.h"
 
 namespace {
@@ -25,14 +28,17 @@ namespace {
 // from an origin before it is automatically blocked.
 int g_prompt_dismissals_before_block = 3;
 
+// The number of days that an origin will stay under embargo for a requested
+// permission.
+int g_embargo_days = 7;
+
 std::unique_ptr<base::DictionaryValue> GetOriginDict(
     HostContentSettingsMap* settings,
     const GURL& origin_url) {
   std::unique_ptr<base::DictionaryValue> dict =
       base::DictionaryValue::From(settings->GetWebsiteSetting(
-          origin_url, origin_url,
-          CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT, std::string(),
-          nullptr));
+          origin_url, GURL(), CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT,

[dominickn, 2017/01/11 07:52:13]

Do you want to rename the content setting in this CL too? raymes is the relevant
OWNER for all of that code as well (aside from webui which you can TBR)

[meredithl, 2017/01/11 23:22:28]

Raymes left a comment on the design doc pointing out that it may be difficult to
change the name due to underlying prefs, and we may want to deprecate this one
and adding a new one.

+          std::string(), nullptr));
   if (!dict)
     return base::MakeUnique<base::DictionaryValue>();
 
@@ -82,7 +88,6 @@ int GetActionCount(const GURL& url,
   HostContentSettingsMap* map =
       HostContentSettingsMapFactory::GetForProfile(profile);
   std::unique_ptr<base::DictionaryValue> dict = GetOriginDict(map, url);
-
   base::DictionaryValue* permission_dict = GetOrCreatePermissionDict(
       dict.get(), PermissionUtil::GetPermissionString(permission));
 
@@ -102,6 +107,10 @@ const char PermissionDecisionAutoBlocker::kPromptIgnoreCountKey[] =
     "ignore_count";
 
 // static
+const char PermissionDecisionAutoBlocker::kPermissionOriginEmbargoKey[] =
+    "embargo_days";
+
+// static
 void PermissionDecisionAutoBlocker::RemoveCountsByUrl(
     Profile* profile,
     base::Callback<bool(const GURL& url)> filter) {
@@ -174,10 +183,108 @@ bool PermissionDecisionAutoBlocker::ShouldChangeDismissalToBlock(
 // static
 void PermissionDecisionAutoBlocker::UpdateFromVariations() {
   int prompt_dismissals = -1;
-  std::string value = variations::GetVariationParamValueByFeature(
+  int embargo_time = -1;

[dominickn, 2017/01/11 07:52:13]

Nit: call this embargo_days?

[meredithl, 2017/01/11 23:22:28]

Done.

+  std::string dismissals_value = variations::GetVariationParamValueByFeature(
       features::kBlockPromptsIfDismissedOften, kPromptDismissCountKey);
-
+  std::string embargo_value = variations::GetVariationParamValueByFeature(
+      features::kPermissionsBlacklist, kPermissionOriginEmbargoKey);
   // If converting the value fails, stick with the current value.
-  if (base::StringToInt(value, &prompt_dismissals) && prompt_dismissals > 0)
+  if (base::StringToInt(dismissals_value, &prompt_dismissals) &&
+      prompt_dismissals > 0)
     g_prompt_dismissals_before_block = prompt_dismissals;
+  if (base::StringToInt(embargo_value, &embargo_time) && embargo_time > 0)
+    g_embargo_days = embargo_time;
+}
+
+// static
+// TODO(meredithl): Have PermissionDecisionAutoBlocker handle the database
+// manager, rather than passing it in.
+void PermissionDecisionAutoBlocker::ShouldAutomaticallyBlock(
+    scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
+    content::PermissionType permission,
+    const GURL& request_origin,
+    content::WebContents* web_contents,
+    int timeout,
+    Profile* profile,
+    base::Time current_time,
+    base::Callback<void(bool)> callback) {
+  if (!base::FeatureList::IsEnabled(features::kPermissionsBlacklist)) {
+    callback.Run(false /* permission blocked */);
+    return;
+  }
+
+  else if (IsUnderEmbargo(permission, profile, request_origin, current_time)) {
+    callback.Run(true /* permission blocked */);
+    return;
+  }
+
+  else if (!db_manager) {
+    callback.Run(false /* permission blocked */);
+    return;
+  }
+
+  // Request origin is not yet under embargo, so we check it with Safe Browsing.
+  // TODO(meredithl): Implement incremental back off for checking blacklist.
+  PermissionBlacklistClient::CheckSafeBrowsingBlacklist(
+      db_manager, permission, request_origin, web_contents, timeout,
+      base::Bind(&PermissionDecisionAutoBlocker::CheckSafeBrowsingResult,
+                 permission, profile, request_origin, callback));
+}
+
+// static
+void PermissionDecisionAutoBlocker::CheckSafeBrowsingResult(
+    content::PermissionType permission,
+    Profile* profile,
+    const GURL& request_origin,
+    base::Callback<void(bool)> callback,
+    bool should_be_embargoed) {
+  if (should_be_embargoed) {
+    // Requesting site is blacklisted for this permission, update the content
+    // setting to place it under embargo.
+    PlaceUnderEmbargo(permission, request_origin,
+                      HostContentSettingsMapFactory::GetForProfile(profile),
+                      base::Time::Now());
+  }
+  callback.Run(should_be_embargoed /* permission blocked */);
+}
+
+// static
+bool PermissionDecisionAutoBlocker::IsUnderEmbargo(
+    content::PermissionType permission,
+    Profile* profile,
+    const GURL& request_origin,
+    base::Time current_time) {
+  HostContentSettingsMap* map =
+      HostContentSettingsMapFactory::GetForProfile(profile);
+  std::unique_ptr<base::DictionaryValue> dict =
+      GetOriginDict(map, request_origin);
+  base::DictionaryValue* permission_dict = GetOrCreatePermissionDict(
+      dict.get(), PermissionUtil::GetPermissionString(permission));
+  double embargo_time = -1;
+  if ((permission_dict->GetDouble(kPermissionOriginEmbargoKey,
+                                  &embargo_time))) {
+    if (current_time < base::Time::FromInternalValue(embargo_time)) {

[dominickn, 2017/01/11 07:52:13]

Nit: remove braces from this if

[meredithl, 2017/01/11 23:22:28]

Done.

+      return true;
+    }
+  }
+  return false;
+}
+
+// static
+void PermissionDecisionAutoBlocker::PlaceUnderEmbargo(
+    content::PermissionType permission,
+    const GURL& request_origin,
+    HostContentSettingsMap* map,
+    base::Time current_time) {
+  std::unique_ptr<base::DictionaryValue> dict =
+      GetOriginDict(map, request_origin);
+  base::DictionaryValue* permission_dict = GetOrCreatePermissionDict(
+      dict.get(), PermissionUtil::GetPermissionString(permission));
+  double embargo_end =

[dominickn, 2017/01/11 07:52:13]

Minor nit: I'd find this clearer as:

base::Time embargo_end = current_time +
base::TimeDelta::FromDays(g_embargo_days);
permission_dict->SetDouble(kPermissionOriginEmbargoKey,
embargo_end.ToInternalValue())

[meredithl, 2017/01/11 23:22:28]

Done.

+      (current_time + base::TimeDelta::FromDays(g_embargo_days))
+          .ToInternalValue();
+  permission_dict->SetDouble(kPermissionOriginEmbargoKey, embargo_end);
+  map->SetWebsiteSettingDefaultScope(
+      request_origin, GURL(), CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT,
+      std::string(), std::move(dict));
 }