Add a feature which, when enabled, blocks permissions after X prompt dismissals.

chrome/browser/permissions/permission_util.cc

 // Copyright 2015 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_util.h"
 
+#include <memory>
+
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/values.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/permissions/permission_uma_util.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 "url/gurl.h"
+
+namespace {
+
+// The number of times that users may explicitly dismiss a permission prompt
+// from an origin before it is automatically blocked. Configurable via Finch,
+// and only used if features::kBlockPromptsIfDismissedOften is enabled.
+int gPromptDismissalsBeforeBlock = 3;

[kcarattini, 2016/07/28 08:54:49]

This value isn't used unless it's been updated from the variation param, right?
Why not initialise it to something less meaningful?

[dominickn, 2016/08/01 01:08:51]

This is now a static constant - it's the default value if nothing is sent from
the variation param.

+
+bool gUpdatedFromVariations = false;

[raymes, 2016/07/28 08:07:50]

nit: globals tend to use camel case with a g_ prefix

[kcarattini, 2016/07/28 08:54:49]

Isn't it lowercase with underscores?

[dominickn, 2016/08/01 01:08:51]

Now a constant not a global.

+
+const char kPromptStudyName[] = "PermissionPromptsUX";
+const char kPromptDismissCountKey[] = "dismiss_count";
+
+std::unique_ptr<base::DictionaryValue> GetOriginDict(
+    HostContentSettingsMap* settings,
+    const GURL& origin_url) {
+  if (!settings)
+    return base::WrapUnique(new base::DictionaryValue());
+
+  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));
+  if (!dict)
+    return base::WrapUnique(new base::DictionaryValue());
+
+  return dict;
+}
+
+base::DictionaryValue* GetPermissionDict(base::DictionaryValue* origin_dict,
+                                         const std::string& permission) {
+  base::DictionaryValue* permission_dict = nullptr;
+  if (!origin_dict->GetDictionaryWithoutPathExpansion(permission,
+                                                      &permission_dict)) {
+    permission_dict = new base::DictionaryValue();
+    origin_dict->SetWithoutPathExpansion(permission,
+                                         base::WrapUnique(permission_dict));
+  }
+
+  return permission_dict;
+}
+
+// Records that the user dismissed a permission prompt for type |permission|
+// on |url| to a website setting. Returns the updated number of dismissals.
+int RecordDismissalCount(Profile* profile,
+                         const GURL& url,
+                         content::PermissionType permission) {
+  HostContentSettingsMap* map =
+      HostContentSettingsMapFactory::GetForProfile(profile);
+  std::unique_ptr<base::DictionaryValue> dict = GetOriginDict(map, url);
+  if (!dict.get())
+    return 0;
+
+  base::DictionaryValue* permission_dict = GetPermissionDict(
+      dict.get(), PermissionUtil::GetPermissionString(permission));
+
+  if (!permission_dict)
+    return 0;
+
+  int current_count = 0;
+  permission_dict->GetInteger(kPromptDismissCountKey, &current_count);
+  permission_dict->SetInteger(kPromptDismissCountKey, ++current_count);
+
+  map->SetWebsiteSettingDefaultScope(
+      url, GURL(), CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT,
+      std::string(), std::move(dict));
+
+  return current_count;
+}
+
+}  // anonymous namespace
 
 using content::PermissionType;
 
 std::size_t PermissionTypeHash::operator()(
     const content::PermissionType& type) const {
   return static_cast<size_t>(type);
 }
 
 // The returned strings must match the RAPPOR metrics in rappor.xml,
 // and any Field Trial configs for the Permissions kill switch e.g.
@@ skipping 102 matching lines @@
   ContentSetting final_content_setting = settings_map->GetContentSetting(
       primary_url_, secondary_url_, content_type_, std::string());
   if (final_content_setting != CONTENT_SETTING_ALLOW) {
     PermissionType permission_type;
     if (PermissionUtil::GetPermissionType(content_type_, &permission_type)) {
       PermissionUmaUtil::PermissionRevoked(permission_type, source_ui_,
                                            primary_url_, profile_);
     }
   }
 }
+
+bool PermissionUtil::ShouldChangeDismissalToBlock(
+    Profile* profile,
+    const GURL& url,
+    content::PermissionType permission) {
+  int current_dismissal_count = RecordDismissalCount(profile, url, permission);

[kcarattini, 2016/07/28 08:54:49]

So this count is kept forever, is that correct? It doesn't reset after an
hour/day/week? Is that something we want to consider? When the experiment is
over, do we want to reset the permission for the affected users?

[dominickn, 2016/08/01 01:08:51]

I think that keeping the count forever is correct. When the experiment is over,
I don't think we want to set things back the way they were; that's kind of
painful to do (a one-time task which checks the count, sees if the permission is
denied, and changes it). 

+
+  if (!base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften))
+    return false;
+
+  if (!gUpdatedFromVariations) {
+    int prompt_dismissals = -1;
+    std::string value = variations::GetVariationParamValue(
+        kPromptStudyName, kPromptDismissCountKey);
+    if (base::StringToInt(value, &prompt_dismissals) && prompt_dismissals > 0)
+      gPromptDismissalsBeforeBlock = prompt_dismissals;
+
+    gUpdatedFromVariations = true;
+  }
+
+  return current_dismissal_count >= gPromptDismissalsBeforeBlock;
+}

[raymes, 2016/07/28 08:07:50]

nit: suggestion: since there is a reasonable amount of logic here, along with
the two globals associated with it, perhaps we should put all of this into a
singleton class to make it more self-contained. Take a look at LazyInstance.
This would also make it easy to move if we want to move it to
PermissionContextBase after Tim does refactoring.

[dominickn, 2016/08/01 01:08:51]

Done. Naming is hard, please let me know if you have a better name for this
class.