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..f9fa41470702cbf1ccecc9dd3921975986a72849 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,17 @@ 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. |
+int g_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 +88,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 +107,10 @@ const char PermissionDecisionAutoBlocker::kPromptIgnoreCountKey[] = |
"ignore_count"; |
// static |
+const char PermissionDecisionAutoBlocker::kPermissionOriginEmbargoKey[] = |
+ "embargo_days"; |
+ |
+// static |
void PermissionDecisionAutoBlocker::RemoveCountsByUrl( |
Profile* profile, |
base::Callback<bool(const GURL& url)> filter) { |
@@ -174,10 +183,104 @@ bool PermissionDecisionAutoBlocker::ShouldChangeDismissalToBlock( |
// static |
void PermissionDecisionAutoBlocker::UpdateFromVariations() { |
int prompt_dismissals = -1; |
- std::string value = variations::GetVariationParamValueByFeature( |
+ int embargo_days = -1; |
+ std::string dismissals_value = variations::GetVariationParamValueByFeature( |
features::kBlockPromptsIfDismissedOften, kPromptDismissCountKey); |
- |
+ std::string embargo_value = variations::GetVariationParamValueByFeature( |
+ features::kPermissionsBlacklist, kPermissionOriginEmbargoKey); |
// 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(embargo_value, &embargo_days) && embargo_days > 0) |
+ g_embargo_days = embargo_days; |
+} |
+ |
+// static |
+// TODO(meredithl): Have PermissionDecisionAutoBlocker handle the database |
+// manager, rather than passing it in. |
+void PermissionDecisionAutoBlocker::ShouldAutomaticallyBlock( |
raymes
2017/01/12 02:34:10
A slightly alternative design would be to have thi
meredithl
2017/01/12 05:59:19
I'm not sure I understand what you mean. Permissio
raymes
2017/01/16 02:23:42
Yep - that's what I meant. I don't necessarily thi
meredithl
2017/01/16 04:28:19
Agreed, the name is bad. I called it this to tie i
|
+ 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) { |
+ if (!base::FeatureList::IsEnabled(features::kPermissionsBlacklist)) { |
+ callback.Run(false /* permission blocked */); |
+ return; |
+ } else if (IsUnderEmbargo(permission, profile, request_origin, |
+ current_time)) { |
+ callback.Run(true /* permission blocked */); |
+ return; |
+ } else if (!db_manager) { |
raymes
2017/01/12 00:53:14
Can this be incorporated into the feature enabled
meredithl
2017/01/12 05:59:19
The only reason I didn't was if blacklisting is en
raymes
2017/01/16 02:23:42
That makes sense, it might be worth adding a comme
meredithl
2017/01/16 04:28:19
This has since changed. Dominick suggested that I
|
+ callback.Run(false /* permission blocked */); |
+ return; |
raymes
2017/01/12 00:53:15
Rather than all the return statements, the below s
meredithl
2017/01/12 05:59:19
Done.
|
+ } |
+ |
+ // Request origin is not yet under embargo, so we check it with Safe Browsing. |
+ // TODO(meredithl): Implement incremental back off for checking blacklist. |
+ PermissionBlacklistClient::CheckSafeBrowsingBlacklist( |
+ db_manager, permission, request_origin, web_contents, timeout, |
+ base::Bind(&PermissionDecisionAutoBlocker::CheckSafeBrowsingResult, |
+ permission, profile, request_origin, callback)); |
+} |
+ |
+// static |
+void PermissionDecisionAutoBlocker::CheckSafeBrowsingResult( |
+ content::PermissionType permission, |
+ Profile* profile, |
+ const GURL& request_origin, |
+ 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), |
+ base::Time::Now()); |
+ } |
+ callback.Run(should_be_embargoed /* 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_days = -1; |
raymes
2017/01/12 02:34:10
nit: would this be better named embargo_time?
meredithl
2017/01/12 05:59:19
Dom had me change this from embargo_time to embarg
raymes
2017/01/16 02:23:42
I see you've changed it to embargo_date which make
meredithl
2017/01/16 04:28:19
Yes, as per your other suggestion to store the dat
|
+ if ((permission_dict->GetDouble(kPermissionOriginEmbargoKey, |
+ &embargo_days))) { |
+ if (current_time < base::Time::FromInternalValue(embargo_days)) |
raymes
2017/01/12 02:34:10
Another question is how the embargo period will re
raymes
2017/01/12 03:19:30
Dom, Ben and I chatted about this and concluded th
meredithl
2017/01/12 05:59:19
Sgtm, I'll make sure to work it into the design do
|
+ return true; |
+ } |
+ return false; |
+} |
+ |
+// static |
+void PermissionDecisionAutoBlocker::PlaceUnderEmbargo( |
+ content::PermissionType permission, |
+ const GURL& request_origin, |
+ HostContentSettingsMap* map, |
+ base::Time current_time) { |
+ std::unique_ptr<base::DictionaryValue> dict = |
+ GetOriginDict(map, request_origin); |
+ base::DictionaryValue* permission_dict = GetOrCreatePermissionDict( |
+ dict.get(), PermissionUtil::GetPermissionString(permission)); |
+ base::Time embargo_end = |
+ current_time + base::TimeDelta::FromDays(g_embargo_days); |
raymes
2017/01/12 03:19:30
Would it be better to just store the current times
meredithl
2017/01/12 05:59:19
Done.
|
+ permission_dict->SetDouble(kPermissionOriginEmbargoKey, |
+ embargo_end.ToInternalValue()); |
+ map->SetWebsiteSettingDefaultScope( |
+ request_origin, GURL(), CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT, |
raymes
2017/01/12 00:53:15
I don't quite understand what the end state of the
meredithl
2017/01/12 05:59:19
The idea that Dom and I are going with at the mome
raymes
2017/01/16 02:23:42
Ok - I still have some questions about how the dis
meredithl
2017/01/16 04:28:19
Sgtm!
|
+ std::string(), std::move(dict)); |
} |