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

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

Issue 2226633002: Add a feature to display a persistence toggle for permission prompts on Android. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address nits, verbal change to make it geolocation-only Created 4 years, 4 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_queue_controller.cc
diff --git a/chrome/browser/permissions/permission_queue_controller.cc b/chrome/browser/permissions/permission_queue_controller.cc
index 3382aaadc0e6deedeec7514712f664eb18699061..caa45c101fcca4d2dad1af5cac51f553a9c36c0d 100644
--- a/chrome/browser/permissions/permission_queue_controller.cc
+++ b/chrome/browser/permissions/permission_queue_controller.cc
@@ -209,7 +209,7 @@ void PermissionQueueController::OnPermissionSet(const PermissionRequestID& id,
const GURL& embedder,
bool user_gesture,
bool update_content_setting,
- bool allowed) {
+ PermissionAction decision) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
// TODO(miguelg): move the permission persistence to
@@ -217,20 +217,26 @@ void PermissionQueueController::OnPermissionSet(const PermissionRequestID& id,
PermissionRequestGestureType gesture_type =
user_gesture ? PermissionRequestGestureType::GESTURE
: PermissionRequestGestureType::NO_GESTURE;
- if (update_content_setting) {
- UpdateContentSetting(requesting_frame, embedder, allowed);
- if (allowed) {
+ switch (decision) {
+ case GRANTED:
PermissionUmaUtil::PermissionGranted(permission_type_, gesture_type,
requesting_frame, profile_);
- } else {
+ break;
+ case DENIED:
PermissionUmaUtil::PermissionDenied(permission_type_, gesture_type,
requesting_frame, profile_);
- }
- } else {
- PermissionUmaUtil::PermissionDismissed(permission_type_, gesture_type,
- requesting_frame, profile_);
+ break;
+ case DISMISSED:
+ PermissionUmaUtil::PermissionDismissed(permission_type_, gesture_type,
+ requesting_frame, profile_);
+ break;
+ default:
+ NOTREACHED();
}
+ if (update_content_setting)
+ UpdateContentSetting(requesting_frame, embedder, decision);
+
// Cancel this request first, then notify listeners. TODO(pkasting): Why
// is this order important?
PendingInfobarRequests requests_to_notify;
@@ -267,13 +273,16 @@ void PermissionQueueController::OnPermissionSet(const PermissionRequestID& id,
// PermissionContextBase needs to know about the new ContentSetting value,
// CONTENT_SETTING_DEFAULT being the value for nothing happened. The callers
- // of ::OnPermissionSet passes { true, true } for allow, { true, false } for
- // block and { false, * } for dismissed. The tuple being
- // { update_content_setting, allowed }.
+ // of ::OnPermissionSet passes { bool, GRANTED } for allow, { bool, DENIED }
+ // for block and { false, DISMISSED } for dismissed. The tuple being
+ // { update_content_setting, decision }.
ContentSetting content_setting = CONTENT_SETTING_DEFAULT;
- if (update_content_setting) {
- content_setting = allowed ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
- }
+ if (decision == GRANTED)
+ content_setting = CONTENT_SETTING_ALLOW;
+ else if (decision == DENIED)
+ content_setting = CONTENT_SETTING_BLOCK;
+ else
+ DCHECK_EQ(DISMISSED, decision);
// Send out the permission notifications.
for (PendingInfobarRequests::iterator i = requests_to_notify.begin();
@@ -392,7 +401,8 @@ void PermissionQueueController::UnregisterForInfoBarNotifications(
void PermissionQueueController::UpdateContentSetting(
const GURL& requesting_frame,
const GURL& embedder,
- bool allowed) {
+ PermissionAction decision) {
+ DCHECK(decision == GRANTED || decision == DENIED);
if (requesting_frame.GetOrigin().SchemeIsFile()) {
// Chrome can be launched with --disable-web-security which allows
// geolocation requests from file:// URLs. We don't want to store these
@@ -401,7 +411,7 @@ void PermissionQueueController::UpdateContentSetting(
}
ContentSetting content_setting =
- allowed ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
+ (decision == GRANTED) ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
HostContentSettingsMapFactory::GetForProfile(profile_)
->SetContentSettingDefaultScope(

Powered by Google App Engine
This is Rietveld 408576698