Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6687)

Unified Diff: chrome/browser/permissions/permission_decision_auto_blocker.cc

Issue 2622983003: Implement embargo in PermissionDecisionAutoBlocker (Closed)
Patch Set: Testing clean up and nits Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..b38ee5edd010edea28f76564337c263b2f75c207 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,21 @@ 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 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;
+
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,
+ std::string(), nullptr));
if (!dict)
return base::MakeUnique<base::DictionaryValue>();
@@ -82,7 +92,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 +111,14 @@ const char PermissionDecisionAutoBlocker::kPromptIgnoreCountKey[] =
"ignore_count";
// static
+const char PermissionDecisionAutoBlocker::kPermissionBlacklistEmbargoKey[] =
+ "blacklisting_embargo_days";
+
+// static
+const char PermissionDecisionAutoBlocker::kPermissionDismissalEmbargoKey[] =
+ "dismissal_embargo_days";
+
+// static
void PermissionDecisionAutoBlocker::RemoveCountsByUrl(
Profile* profile,
base::Callback<bool(const GURL& url)> filter) {
@@ -141,12 +158,22 @@ int PermissionDecisionAutoBlocker::GetIgnoreCount(
}
// static
-int PermissionDecisionAutoBlocker::RecordDismiss(
+bool PermissionDecisionAutoBlocker::RecordDismissAndEmbargo(
const GURL& url,
content::PermissionType permission,
- Profile* profile) {
- return RecordActionInWebsiteSettings(url, permission, kPromptDismissCountKey,
- profile);
+ Profile* profile,
+ base::Time current_time) {
+ int current_dismissal_count = RecordActionInWebsiteSettings(
+ url, permission, kPromptDismissCountKey, profile);
+ if (base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften) &&
+ current_dismissal_count >= g_prompt_dismissals_before_block) {
+ HostContentSettingsMap* map =
+ HostContentSettingsMapFactory::GetForProfile(profile);
+ PlaceUnderEmbargo(permission, url, map, current_time,
+ kPermissionDismissalEmbargoKey);
+ return true;
+ }
+ return false;
}
// static
@@ -163,7 +190,8 @@ bool PermissionDecisionAutoBlocker::ShouldChangeDismissalToBlock(
const GURL& url,
content::PermissionType permission,
Profile* profile) {
- int current_dismissal_count = RecordDismiss(url, permission, profile);
+ int current_dismissal_count =
+ RecordDismissAndEmbargo(url, permission, profile, base::Time::Now());
if (!base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften))
return false;
@@ -174,10 +202,131 @@ bool PermissionDecisionAutoBlocker::ShouldChangeDismissalToBlock(
// static
void PermissionDecisionAutoBlocker::UpdateFromVariations() {
int prompt_dismissals = -1;
- std::string value = variations::GetVariationParamValueByFeature(
+ 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 dismissal_embargo_value =
+ variations::GetVariationParamValueByFeature(
+ features::kBlockPromptsIfDismissedOften,
+ kPermissionDismissalEmbargoKey);
// 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(blacklist_embargo_value, &blacklist_embargo_days) &&
+ blacklist_embargo_days > 0) {
+ g_blacklist_embargo_days = blacklist_embargo_days;
+ }
+ if (base::StringToInt(dismissal_embargo_value, &dismissal_embargo_days) &&
+ dismissal_embargo_days > 0) {
+ g_dismissal_embargo_days = dismissal_embargo_days;
+ }
+}
+
+// static
+// TODO(meredithl): Have PermissionDecisionAutoBlocker handle the database
+// manager, rather than passing it in.
+void PermissionDecisionAutoBlocker::UpdateEmbargoedStatus(
+ 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) {
+ // Check if origin is currently under embargo for the requested permission.
+ if (IsUnderEmbargo(permission, profile, request_origin, current_time)) {
+ callback.Run(true /* permission_blocked */);
+ return;
+ }
+
+ if (base::FeatureList::IsEnabled(features::kPermissionsBlacklist) &&
+ db_manager) {
+ PermissionBlacklistClient::CheckSafeBrowsingBlacklist(
+ db_manager, permission, request_origin, web_contents, timeout,
+ base::Bind(&PermissionDecisionAutoBlocker::CheckSafeBrowsingResult,
+ permission, profile, request_origin, current_time,
+ callback));
+ return;
+ }
+
+ callback.Run(false /* 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_date = -1;
+ bool is_under_dismiss_embargo = false;
+ bool is_under_blacklist_embargo = false;
+ 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)) {
+ is_under_blacklist_embargo = true;
+ }
+ }
+
+ 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)) {
+ is_under_dismiss_embargo = true;
+ }
+ }
+ // If either embargoes is still in effect, return true.
+ return is_under_dismiss_embargo || is_under_blacklist_embargo;
+}
+
+void PermissionDecisionAutoBlocker::PlaceUnderEmbargo(
+ content::PermissionType permission,
+ const GURL& request_origin,
+ HostContentSettingsMap* map,
+ base::Time current_time,
+ const char* key) {
+ std::unique_ptr<base::DictionaryValue> dict =
+ GetOriginDict(map, request_origin);
+ base::DictionaryValue* permission_dict = GetOrCreatePermissionDict(
+ dict.get(), PermissionUtil::GetPermissionString(permission));
+ permission_dict->SetDouble(key, current_time.ToInternalValue());
+ map->SetWebsiteSettingDefaultScope(
+ request_origin, GURL(), CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT,
+ std::string(), std::move(dict));
+}
+
+// static
+void PermissionDecisionAutoBlocker::CheckSafeBrowsingResult(
+ content::PermissionType permission,
+ Profile* profile,
+ const GURL& request_origin,
+ base::Time current_time,
+ 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),
+ current_time, kPermissionBlacklistEmbargoKey);
+ }
+ callback.Run(should_be_embargoed /* permission blocked */);
}

Powered by Google App Engine
This is Rietveld 408576698