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;
+
+bool gUpdatedFromVariations = false;
+
+const char kPromptStudyName[] = "PermissionPromptsUX";
+const char kPromptDismissCountKey[] = "dismiss_count";
+
+std::unique_ptr<base::DictionaryValue> GetOriginDict(
+    HostContentSettingsMap* settings,
+    const GURL& origin_url) {
+  if (!settings)

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

I believe this should never be null

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

Done.

+    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));

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

It might be potentially confusing that this Get* function modifies the
origin_dict. Perhaps we can move this line down to RecordDismissalCount? (it
will be called unnecessarily in some cases but it shouldn't be expensive).

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

I've renamed the method to GetOrCreatePermissionDict; is that OK, or do you
prefer doing the setting outside?

+  }
+
+  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())

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

Can this ever be null?

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

Done.

+    return 0;
+
+  base::DictionaryValue* permission_dict = GetPermissionDict(
+      dict.get(), PermissionUtil::GetPermissionString(permission));
+
+  if (!permission_dict)

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

Can this ever be null?

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

Done.

+    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 79 matching lines @@
     PermissionType permission_type;
     if (PermissionUtil::GetPermissionType(content_type, &permission_type)) {
       // TODO(stefanocs): Report revocations from page action as PAGE_ACTION
       // source UI instead of SITE_SETTINGS source UI.
       PermissionUmaUtil::PermissionRevoked(permission_type,
                                            PermissionSourceUI::SITE_SETTINGS,
                                            primary_url, profile);
     }
   }
 }
+
+bool PermissionUtil::ShouldChangeDismissalToBlock(
+    Profile* profile,
+    const GURL& url,
+    content::PermissionType permission) {
+  int current_dismissal_count = RecordDismissalCount(profile, url, permission);

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

Do we want to record the dismissal count even if the experiment isn't enabled?

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

This was a deliberate choice - the overhead seems minimal, and if we want to
promote this experiment to stable then all clients will have the dismissal
counts ready to go. WDYT?

+
+  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)

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

Do you know in what cases this won't succeed? If it's only because of a
programming error, we could just add else NOTREACHED(). But if it could happen
because the user received a broken finch config or something, we should bail
out. In the bail out case should we somehow ensure this function always returns
false?

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

Hmm, catching a potential bad Finch config seems like a good choice. Done. In
the restructured version it will always fall back to the default value (if the
experiment is enabled).

+      gPromptDismissalsBeforeBlock = prompt_dismissals;
+
+    gUpdatedFromVariations = true;
+  }
+
+  return current_dismissal_count >= gPromptDismissalsBeforeBlock;
+}