Implement permissions embargo for prompts which are repeatedly ignored.

chrome/browser/permissions/permission_decision_auto_blocker.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/permissions/permission_decision_auto_blocker.h"
 
 #include <memory>
 
 #include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
@@ skipping 13 matching lines @@
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/web_contents.h"
 #include "url/gurl.h"
 
 namespace {
 
 // The number of times that users may explicitly dismiss a permission prompt
 // from an origin before it is automatically blocked.
 int g_prompt_dismissals_before_block = 3;
 
+// The number of times that users may ignore a permission prompt from an origin
+// before it is automatically blocked.
+int g_prompt_ignores_before_block = 4;
+
+// The number of days that an origin will stay under embargo for a requested
+// permission due to repeated dismissals.
+int g_dismissal_embargo_days = 7;
+
+// The number of days that an origin will stay under embargo for a requested
+// permission due to repeated ignores.
+int g_ignore_embargo_days = 7;
+
 // The number of days that an origin will stay under embargo for a requested
 // permission due to blacklisting.
 int g_blacklist_embargo_days = 7;
 
-// The number of days that an origin will stay under embargo for a requested
-// permission due to repeated dismissals.
-int g_dismissal_embargo_days = 7;
-
 // Maximum time in milliseconds to wait for safe browsing service to check a
 // url for blacklisting. After this amount of time, the check will be aborted
 // and the url will be treated as not safe.
 // TODO(meredithl): Revisit this once UMA metrics have data about request time.
 const int kCheckUrlTimeoutMs = 2000;
 
 std::unique_ptr<base::DictionaryValue> GetOriginDict(
     HostContentSettingsMap* settings,
     const GURL& origin_url) {
   std::unique_ptr<base::DictionaryValue> dict =
@@ skipping 50 matching lines @@
       HostContentSettingsMapFactory::GetForProfile(profile);
   std::unique_ptr<base::DictionaryValue> dict = GetOriginDict(map, url);
   base::DictionaryValue* permission_dict = GetOrCreatePermissionDict(
       dict.get(), PermissionUtil::GetPermissionString(permission));
 
   int current_count = 0;
   permission_dict->GetInteger(key, &current_count);
   return current_count;
 }
 
+bool IsUnderEmbargo(base::DictionaryValue* permission_dict,
+                    const base::Feature& feature,
+                    const char* key,
+                    base::Time current_time,
+                    base::TimeDelta offset) {
+  double embargo_date = -1;
+
+  if (base::FeatureList::IsEnabled(feature) &&
+      permission_dict->GetDouble(key, &embargo_date)) {
+    if (current_time < base::Time::FromInternalValue(embargo_date) + offset)
+      return true;
+  }
+
+  return false;
+}
+
 }  // namespace
 
 // PermissionDecisionAutoBlocker::Factory --------------------------------------
 
 // static
 PermissionDecisionAutoBlocker*
 PermissionDecisionAutoBlocker::Factory::GetForProfile(Profile* profile) {
   return static_cast<PermissionDecisionAutoBlocker*>(
       GetInstance()->GetServiceForBrowserContext(profile, true));
 }
@@ skipping 27 matching lines @@
 
 // static
 const char PermissionDecisionAutoBlocker::kPromptDismissCountKey[] =
     "dismiss_count";
 
 // static
 const char PermissionDecisionAutoBlocker::kPromptIgnoreCountKey[] =
     "ignore_count";
 
 // static
+const char PermissionDecisionAutoBlocker::kPermissionDismissalEmbargoKey[] =
+    "dismissal_embargo_days";
+
+// static
+const char PermissionDecisionAutoBlocker::kPermissionIgnoreEmbargoKey[] =
+    "ignore_embargo_days";
+
+// static
 const char PermissionDecisionAutoBlocker::kPermissionBlacklistEmbargoKey[] =
     "blacklisting_embargo_days";
 
 // static
-const char PermissionDecisionAutoBlocker::kPermissionDismissalEmbargoKey[] =
-    "dismissal_embargo_days";
-
-// static
 PermissionDecisionAutoBlocker* PermissionDecisionAutoBlocker::GetForProfile(
     Profile* profile) {
   return PermissionDecisionAutoBlocker::Factory::GetForProfile(profile);
 }
 
 // static
 void PermissionDecisionAutoBlocker::UpdateFromVariations() {
   int prompt_dismissals = -1;
+  int prompt_ignores = -1;
+  int dismissal_embargo_days = -1;
+  int ignore_embargo_days = -1;
   int blacklist_embargo_days = -1;
-  int dismissal_embargo_days = -1;
+
   std::string dismissals_value = variations::GetVariationParamValueByFeature(
       features::kBlockPromptsIfDismissedOften, kPromptDismissCountKey);
-  std::string blacklist_embargo_value =
-      variations::GetVariationParamValueByFeature(
-          features::kPermissionsBlacklist, kPermissionBlacklistEmbargoKey);
+  std::string ignores_value = variations::GetVariationParamValueByFeature(
+      features::kBlockPromptsIfIgnoredOften, kPromptIgnoreCountKey);
   std::string dismissal_embargo_value =
       variations::GetVariationParamValueByFeature(
           features::kBlockPromptsIfDismissedOften,
           kPermissionDismissalEmbargoKey);
+  std::string ignore_embargo_value =
+      variations::GetVariationParamValueByFeature(
+          features::kBlockPromptsIfIgnoredOften, kPermissionIgnoreEmbargoKey);
+  std::string blacklist_embargo_value =
+      variations::GetVariationParamValueByFeature(
+          features::kPermissionsBlacklist, kPermissionBlacklistEmbargoKey);
+
   // If converting the value fails, stick with the current value.
   if (base::StringToInt(dismissals_value, &prompt_dismissals) &&
       prompt_dismissals > 0) {
     g_prompt_dismissals_before_block = prompt_dismissals;
   }
-  if (base::StringToInt(blacklist_embargo_value, &blacklist_embargo_days) &&
-      blacklist_embargo_days > 0) {
-    g_blacklist_embargo_days = blacklist_embargo_days;
+  if (base::StringToInt(ignores_value, &prompt_ignores) && prompt_ignores > 0) {
+    g_prompt_ignores_before_block = prompt_ignores;
   }
   if (base::StringToInt(dismissal_embargo_value, &dismissal_embargo_days) &&
       dismissal_embargo_days > 0) {
     g_dismissal_embargo_days = dismissal_embargo_days;
   }
+  if (base::StringToInt(ignore_embargo_value, &ignore_embargo_days) &&

[benwells, 2017/03/31 04:47:14]

These variables are all confusingly named. For g_foo, you should use local foo
and foo_val or foo_str or foo_value.

So, g_ignore_embargo_days, ignore_embargo_days, and ignore_embargo_days_value.

You can probs remove the prompt_ from go_prompt_ignores_before_block as well.

[dominickn, 2017/04/02 23:45:57]

Done.

+      ignore_embargo_days > 0) {
+    g_ignore_embargo_days = ignore_embargo_days;
+  }
+  if (base::StringToInt(blacklist_embargo_value, &blacklist_embargo_days) &&
+      blacklist_embargo_days > 0) {
+    g_blacklist_embargo_days = blacklist_embargo_days;
+  }
 }
 
 void PermissionDecisionAutoBlocker::CheckSafeBrowsingBlacklist(
     content::WebContents* web_contents,
     const GURL& request_origin,
     ContentSettingsType permission,
     base::Callback<void(bool)> callback) {
   DCHECK_EQ(CONTENT_SETTING_ASK,
             GetEmbargoResult(request_origin, permission).content_setting);
 
@@ skipping 16 matching lines @@
 
 PermissionResult PermissionDecisionAutoBlocker::GetEmbargoResult(
     const GURL& request_origin,
     ContentSettingsType permission) {
   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_date = -1;
 
   base::Time current_time = clock_->Now();
-  if (base::FeatureList::IsEnabled(features::kPermissionsBlacklist) &&
-      permission_dict->GetDouble(kPermissionBlacklistEmbargoKey,
-                                 &embargo_date)) {
-    if (current_time <
-        base::Time::FromInternalValue(embargo_date) +
-            base::TimeDelta::FromDays(g_blacklist_embargo_days)) {
-      return PermissionResult(CONTENT_SETTING_BLOCK,
-                              PermissionStatusSource::SAFE_BROWSING_BLACKLIST);
-    }
+  if (IsUnderEmbargo(permission_dict, features::kPermissionsBlacklist,

[benwells, 2017/03/31 04:47:14]

Nice factoring :)

[dominickn, 2017/04/02 23:45:57]

:)

+                     kPermissionBlacklistEmbargoKey, current_time,
+                     base::TimeDelta::FromDays(g_blacklist_embargo_days))) {
+    return PermissionResult(CONTENT_SETTING_BLOCK,
+                            PermissionStatusSource::SAFE_BROWSING_BLACKLIST);
   }
 
-  if (base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften) &&
-      permission_dict->GetDouble(kPermissionDismissalEmbargoKey,
-                                 &embargo_date)) {
-    if (current_time <
-        base::Time::FromInternalValue(embargo_date) +
-            base::TimeDelta::FromDays(g_dismissal_embargo_days)) {
-      return PermissionResult(CONTENT_SETTING_BLOCK,
-                              PermissionStatusSource::MULTIPLE_DISMISSALS);
-    }
+  if (IsUnderEmbargo(permission_dict, features::kBlockPromptsIfDismissedOften,
+                     kPermissionDismissalEmbargoKey, current_time,
+                     base::TimeDelta::FromDays(g_dismissal_embargo_days))) {
+    return PermissionResult(CONTENT_SETTING_BLOCK,
+                            PermissionStatusSource::MULTIPLE_DISMISSALS);
+  }
+
+  if (IsUnderEmbargo(permission_dict, features::kBlockPromptsIfIgnoredOften,
+                     kPermissionIgnoreEmbargoKey, current_time,
+                     base::TimeDelta::FromDays(g_ignore_embargo_days))) {
+    return PermissionResult(CONTENT_SETTING_BLOCK,
+                            PermissionStatusSource::MULTIPLE_IGNORES);
   }
 
   return PermissionResult(CONTENT_SETTING_ASK,
                           PermissionStatusSource::UNSPECIFIED);
 }
 
 int PermissionDecisionAutoBlocker::GetDismissCount(
     const GURL& url,
     ContentSettingsType permission) {
   return GetActionCount(url, permission, kPromptDismissCountKey, profile_);
@@ skipping 12 matching lines @@
       url, permission, kPromptDismissCountKey, profile_);
 
   if (base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften) &&
       current_dismissal_count >= g_prompt_dismissals_before_block) {
     PlaceUnderEmbargo(url, permission, kPermissionDismissalEmbargoKey);
     return true;
   }
   return false;
 }
 
-int PermissionDecisionAutoBlocker::RecordIgnore(
+bool PermissionDecisionAutoBlocker::RecordIgnoreAndEmbargo(
     const GURL& url,
     ContentSettingsType permission) {
-  return RecordActionInWebsiteSettings(url, permission, kPromptIgnoreCountKey,
-                                       profile_);
+  int current_ignore_count = RecordActionInWebsiteSettings(
+      url, permission, kPromptIgnoreCountKey, profile_);
+
+  if (base::FeatureList::IsEnabled(features::kBlockPromptsIfIgnoredOften) &&
+      current_ignore_count >= g_prompt_ignores_before_block) {
+    PlaceUnderEmbargo(url, permission, kPermissionIgnoreEmbargoKey);
+    return true;
+  }
+  return false;
 }
 
 void PermissionDecisionAutoBlocker::RemoveCountsByUrl(
     base::Callback<bool(const GURL& url)> filter) {
   HostContentSettingsMap* map =
       HostContentSettingsMapFactory::GetForProfile(profile_);
 
   std::unique_ptr<ContentSettingsForOneType> settings(
       new ContentSettingsForOneType);
   map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_PERMISSION_AUTOBLOCKER_DATA,
@@ skipping 58 matching lines @@
         scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
         int timeout) {
   db_manager_ = db_manager;
   safe_browsing_timeout_ = timeout;
 }
 
 void PermissionDecisionAutoBlocker::SetClockForTesting(
     std::unique_ptr<base::Clock> clock) {
   clock_ = std::move(clock);
 }