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

chrome/browser/permissions/permission_prompt_decision_log.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_prompt_decision_log.h"
+
+#include <memory>
+
+#include "base/feature_list.h"
+#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 "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 "url/gurl.h"
+
+namespace {
+
+// The default number of times that users may explicitly dismiss a permission
+// prompt from an origin before it is automatically blocked.
+int kPromptDismissalsBeforeBlock = 3;

[raymes, 2016/08/05 03:27:48]

nit: const int

[dominickn, 2016/08/05 04:29:30]

Done.

+
+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));
+  if (!dict)
+    return base::WrapUnique(new base::DictionaryValue());
+
+  return dict;
+}
+
+base::DictionaryValue* GetOrCreatePermissionDict(
+    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;
+}
+
+}  // namespace
+
+// static
+const char PermissionPromptDecisionLog::kPromptDismissCountKey[] =
+    "dismiss_count";
+
+// static
+void PermissionPromptDecisionLog::RemoveCountsByUrl(
+    Profile* profile,
+    base::Callback<bool(const GURL& url)> filter) {
+  HostContentSettingsMap* map =
+      HostContentSettingsMapFactory::GetForProfile(profile);
+
+  std::unique_ptr<ContentSettingsForOneType> settings(
+      new ContentSettingsForOneType);
+  map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT,
+                             std::string(), settings.get());
+
+  for (const auto& site : *settings) {
+    GURL origin(site.primary_pattern.ToString());
+
+    if (origin.is_valid() && filter.Run(origin)) {
+      map->SetWebsiteSettingDefaultScope(
+          origin, GURL(), CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT,
+          std::string(), nullptr);
+    }
+  }
+}
+
+PermissionPromptDecisionLog::PermissionPromptDecisionLog()
+    : prompt_dismissals_before_block_(kPromptDismissalsBeforeBlock) {
+  UpdateFromVariations();
+}
+
+bool PermissionPromptDecisionLog::ShouldChangeDismissalToBlock(
+    Profile* profile,
+    const GURL& url,
+    content::PermissionType permission) {
+  int current_dismissal_count = RecordDismissalCount(profile, url, permission);
+
+  // |current_dismissal_count| is always (count_before_this_dismissal + 1)

[raymes, 2016/08/05 03:27:48]

nit: What's count_before_this_dismissal? Maybe just change the _ to spaces so it
doesn't sound like a variable name?

[dominickn, 2016/08/05 04:29:30]

Done.

+  PermissionUmaUtil::PermissionPromptDismissCount(permission,
+                                                  current_dismissal_count - 1);
+
+  if (!base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften))
+    return false;
+
+  return current_dismissal_count >= prompt_dismissals_before_block_;
+}
+
+// static
+int PermissionPromptDecisionLog::GetDismissalCount(
+    Profile* profile,
+    const GURL& url,
+    content::PermissionType permission) {

[raymes, 2016/08/05 03:27:48]

Since this code is only used in the test, I'd suggest just moving the
implementation into the test (always better to keep test code separate from
production code). The only thing needed would be kPromptDismissCountKey which I
think is available in the test.

[dominickn, 2016/08/05 04:29:30]

Both GetOriginDict() and GetOrCreatePermissionDict() are defined only in the
anonymous namespace in this file, and both of them are required for this method.
Either those would need to be exposed in this class (worse), or they'd need to
be duplicated in the test (bad), so I think the best option is to keep this
method here.

[raymes, 2016/08/06 00:50:21]

Ah that makes sense. In that case we always suffix test-only functions with
ForTest. So this should be called GetDismissalCountForTest

[dominickn, 2016/08/08 02:18:28]

Done.

+  HostContentSettingsMap* map =
+      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(
+      PermissionPromptDecisionLog::kPromptDismissCountKey, &current_count);

[raymes, 2016/08/05 03:27:48]

nit: shouldn't need PermissionPromptDecisionLog:: here I don't think

[dominickn, 2016/08/05 04:29:30]

Done.

+  return current_count;
+}
+
+// static
+int PermissionPromptDecisionLog::RecordDismissalCount(
+    Profile* profile,
+    const GURL& url,
+    content::PermissionType permission) {
+  HostContentSettingsMap* map =
+      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(
+      PermissionPromptDecisionLog::kPromptDismissCountKey, &current_count);
+  permission_dict->SetInteger(
+      PermissionPromptDecisionLog::kPromptDismissCountKey, ++current_count);

[raymes, 2016/08/05 03:27:48]

nit: same here and above

[dominickn, 2016/08/05 04:29:30]

Done.

+
+  map->SetWebsiteSettingDefaultScope(
+      url, GURL(), CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT,
+      std::string(), std::move(dict));
+
+  return current_count;
+}
+
+void PermissionPromptDecisionLog::UpdateFromVariations() {
+  int prompt_dismissals = -1;
+  std::string value = variations::GetVariationParamValueByFeature(
+      features::kBlockPromptsIfDismissedOften, kPromptDismissCountKey);
+
+  // If converting the value fails, stick with the default value.
+  if (base::StringToInt(value, &prompt_dismissals) && prompt_dismissals > 0)
+    prompt_dismissals_before_block_ = prompt_dismissals;
+}