Commit 2fb5288e authored by Clark DuVall's avatar Clark DuVall Committed by Commit Bot

Clean up some //components/permissions code

GetAutoApprovalOrigin() does not need the request parameter. Also
removed some unnecessary namespace qualifications.

Bug: 1025609
Change-Id: If40143d7c6b743328167afc5b95af3b6b49616cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066102
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: default avatarBalazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743018}
parent 12639cce
...@@ -101,8 +101,7 @@ void ChromePermissionsClient::OnPromptResolved( ...@@ -101,8 +101,7 @@ void ChromePermissionsClient::OnPromptResolved(
} }
} }
base::Optional<url::Origin> ChromePermissionsClient::GetAutoApprovalOrigin( base::Optional<url::Origin> ChromePermissionsClient::GetAutoApprovalOrigin() {
const permissions::PermissionRequest* request) {
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
// In web kiosk mode, all permission requests are auto-approved for the origin // In web kiosk mode, all permission requests are auto-approved for the origin
// of the main app. // of the main app.
......
...@@ -31,8 +31,7 @@ class ChromePermissionsClient : public permissions::PermissionsClient { ...@@ -31,8 +31,7 @@ class ChromePermissionsClient : public permissions::PermissionsClient {
void OnPromptResolved(content::BrowserContext* browser_context, void OnPromptResolved(content::BrowserContext* browser_context,
permissions::PermissionRequestType request_type, permissions::PermissionRequestType request_type,
permissions::PermissionAction action) override; permissions::PermissionAction action) override;
base::Optional<url::Origin> GetAutoApprovalOrigin( base::Optional<url::Origin> GetAutoApprovalOrigin() override;
const permissions::PermissionRequest* request) override;
private: private:
friend base::NoDestructor<ChromePermissionsClient>; friend base::NoDestructor<ChromePermissionsClient>;
......
...@@ -293,15 +293,13 @@ std::set<GURL> PermissionDecisionAutoBlocker::GetEmbargoedOrigins( ...@@ -293,15 +293,13 @@ std::set<GURL> PermissionDecisionAutoBlocker::GetEmbargoedOrigins(
std::set<GURL> origins; std::set<GURL> origins;
for (const auto& e : embargo_settings) { for (const auto& e : embargo_settings) {
for (auto content_type : content_types) { for (auto content_type : content_types) {
if (!permissions::PermissionUtil::IsPermission(content_type)) if (!PermissionUtil::IsPermission(content_type))
continue; continue;
const GURL url(e.primary_pattern.ToString()); const GURL url(e.primary_pattern.ToString());
permissions::PermissionResult result = PermissionResult result =
GetEmbargoResult(settings_map_, url, content_type, clock_->Now()); GetEmbargoResult(settings_map_, url, content_type, clock_->Now());
if (result.source == if (result.source == PermissionStatusSource::MULTIPLE_DISMISSALS ||
permissions::PermissionStatusSource::MULTIPLE_DISMISSALS || result.source == PermissionStatusSource::MULTIPLE_IGNORES) {
result.source ==
permissions::PermissionStatusSource::MULTIPLE_IGNORES) {
origins.insert(url); origins.insert(url);
break; break;
} }
......
...@@ -108,7 +108,7 @@ void PermissionRequestManager::AddRequest(PermissionRequest* request) { ...@@ -108,7 +108,7 @@ void PermissionRequestManager::AddRequest(PermissionRequest* request) {
.IsSameOriginWith(url::Origin::Create(request->GetOrigin())); .IsSameOriginWith(url::Origin::Create(request->GetOrigin()));
base::Optional<url::Origin> auto_approval_origin = base::Optional<url::Origin> auto_approval_origin =
PermissionsClient::Get()->GetAutoApprovalOrigin(request); PermissionsClient::Get()->GetAutoApprovalOrigin();
if (auto_approval_origin) { if (auto_approval_origin) {
if (url::Origin::Create(request->GetOrigin()) == if (url::Origin::Create(request->GetOrigin()) ==
auto_approval_origin.value()) { auto_approval_origin.value()) {
......
...@@ -63,46 +63,43 @@ namespace { ...@@ -63,46 +63,43 @@ namespace {
const int kPriorCountCap = 10; const int kPriorCountCap = 10;
std::string GetPermissionRequestString( std::string GetPermissionRequestString(PermissionRequestType type) {
permissions::PermissionRequestType type) {
switch (type) { switch (type) {
case permissions::PermissionRequestType::MULTIPLE: case PermissionRequestType::MULTIPLE:
return "AudioAndVideoCapture"; return "AudioAndVideoCapture";
case permissions::PermissionRequestType::QUOTA: case PermissionRequestType::QUOTA:
return "Quota"; return "Quota";
case permissions::PermissionRequestType::DOWNLOAD: case PermissionRequestType::DOWNLOAD:
return "MultipleDownload"; return "MultipleDownload";
case permissions::PermissionRequestType::REGISTER_PROTOCOL_HANDLER: case PermissionRequestType::REGISTER_PROTOCOL_HANDLER:
return "RegisterProtocolHandler"; return "RegisterProtocolHandler";
case permissions::PermissionRequestType::PERMISSION_GEOLOCATION: case PermissionRequestType::PERMISSION_GEOLOCATION:
return "Geolocation"; return "Geolocation";
case permissions::PermissionRequestType::PERMISSION_MIDI_SYSEX: case PermissionRequestType::PERMISSION_MIDI_SYSEX:
return "MidiSysEx"; return "MidiSysEx";
case permissions::PermissionRequestType::PERMISSION_NOTIFICATIONS: case PermissionRequestType::PERMISSION_NOTIFICATIONS:
return "Notifications"; return "Notifications";
case permissions::PermissionRequestType:: case PermissionRequestType::PERMISSION_PROTECTED_MEDIA_IDENTIFIER:
PERMISSION_PROTECTED_MEDIA_IDENTIFIER:
return "ProtectedMedia"; return "ProtectedMedia";
case permissions::PermissionRequestType::PERMISSION_FLASH: case PermissionRequestType::PERMISSION_FLASH:
return "Flash"; return "Flash";
case permissions::PermissionRequestType::PERMISSION_MEDIASTREAM_MIC: case PermissionRequestType::PERMISSION_MEDIASTREAM_MIC:
return "AudioCapture"; return "AudioCapture";
case permissions::PermissionRequestType::PERMISSION_MEDIASTREAM_CAMERA: case PermissionRequestType::PERMISSION_MEDIASTREAM_CAMERA:
return "VideoCapture"; return "VideoCapture";
case permissions::PermissionRequestType:: case PermissionRequestType::PERMISSION_SECURITY_KEY_ATTESTATION:
PERMISSION_SECURITY_KEY_ATTESTATION:
return "SecurityKeyAttestation"; return "SecurityKeyAttestation";
case permissions::PermissionRequestType::PERMISSION_PAYMENT_HANDLER: case PermissionRequestType::PERMISSION_PAYMENT_HANDLER:
return "PaymentHandler"; return "PaymentHandler";
case permissions::PermissionRequestType::PERMISSION_NFC: case PermissionRequestType::PERMISSION_NFC:
return "Nfc"; return "Nfc";
case permissions::PermissionRequestType::PERMISSION_CLIPBOARD_READ_WRITE: case PermissionRequestType::PERMISSION_CLIPBOARD_READ_WRITE:
return "ClipboardReadWrite"; return "ClipboardReadWrite";
case permissions::PermissionRequestType::PERMISSION_VR: case PermissionRequestType::PERMISSION_VR:
return "VR"; return "VR";
case permissions::PermissionRequestType::PERMISSION_AR: case PermissionRequestType::PERMISSION_AR:
return "AR"; return "AR";
case permissions::PermissionRequestType::PERMISSION_STORAGE_ACCESS: case PermissionRequestType::PERMISSION_STORAGE_ACCESS:
return "StorageAccess"; return "StorageAccess";
default: default:
NOTREACHED(); NOTREACHED();
...@@ -110,14 +107,12 @@ std::string GetPermissionRequestString( ...@@ -110,14 +107,12 @@ std::string GetPermissionRequestString(
} }
} }
void RecordEngagementMetric( void RecordEngagementMetric(const std::vector<PermissionRequest*>& requests,
const std::vector<permissions::PermissionRequest*>& requests,
content::WebContents* web_contents, content::WebContents* web_contents,
const std::string& action) { const std::string& action) {
permissions::PermissionRequestType type = PermissionRequestType type = requests[0]->GetPermissionRequestType();
requests[0]->GetPermissionRequestType();
if (requests.size() > 1) if (requests.size() > 1)
type = permissions::PermissionRequestType::MULTIPLE; type = PermissionRequestType::MULTIPLE;
DCHECK(action == "Accepted" || action == "Denied" || action == "Dismissed" || DCHECK(action == "Accepted" || action == "Denied" || action == "Dismissed" ||
action == "Ignored"); action == "Ignored");
...@@ -403,21 +398,19 @@ void PermissionUmaUtil::RecordMissingPermissionInfobarShouldShow( ...@@ -403,21 +398,19 @@ void PermissionUmaUtil::RecordMissingPermissionInfobarShouldShow(
for (const auto& content_settings_type : content_settings_types) { for (const auto& content_settings_type : content_settings_types) {
base::UmaHistogramBoolean( base::UmaHistogramBoolean(
"Permissions.MissingOSLevelPermission.ShouldShow." + "Permissions.MissingOSLevelPermission.ShouldShow." +
permissions::PermissionUtil::GetPermissionString( PermissionUtil::GetPermissionString(content_settings_type),
content_settings_type),
should_show); should_show);
} }
} }
void PermissionUmaUtil::RecordMissingPermissionInfobarAction( void PermissionUmaUtil::RecordMissingPermissionInfobarAction(
permissions::PermissionAction action, PermissionAction action,
const std::vector<ContentSettingsType>& content_settings_types) { const std::vector<ContentSettingsType>& content_settings_types) {
for (const auto& content_settings_type : content_settings_types) { for (const auto& content_settings_type : content_settings_types) {
base::UmaHistogramEnumeration( base::UmaHistogramEnumeration(
"Permissions.MissingOSLevelPermission.Action." + "Permissions.MissingOSLevelPermission.Action." +
permissions::PermissionUtil::GetPermissionString( PermissionUtil::GetPermissionString(content_settings_type),
content_settings_type), action, PermissionAction::NUM);
action, permissions::PermissionAction::NUM);
} }
} }
...@@ -483,7 +476,7 @@ void PermissionUmaUtil::RecordPermissionAction( ...@@ -483,7 +476,7 @@ void PermissionUmaUtil::RecordPermissionAction(
const GURL& requesting_origin, const GURL& requesting_origin,
const content::WebContents* web_contents, const content::WebContents* web_contents,
content::BrowserContext* browser_context) { content::BrowserContext* browser_context) {
permissions::PermissionDecisionAutoBlocker* autoblocker = PermissionDecisionAutoBlocker* autoblocker =
PermissionsClient::Get()->GetPermissionDecisionAutoBlocker( PermissionsClient::Get()->GetPermissionDecisionAutoBlocker(
browser_context); browser_context);
int dismiss_count = int dismiss_count =
...@@ -557,7 +550,7 @@ void PermissionUmaUtil::RecordPermissionAction( ...@@ -557,7 +550,7 @@ void PermissionUmaUtil::RecordPermissionAction(
break; break;
case ContentSettingsType::STORAGE_ACCESS: case ContentSettingsType::STORAGE_ACCESS:
base::UmaHistogramEnumeration("Permissions.Action.StorageAccess", action, base::UmaHistogramEnumeration("Permissions.Action.StorageAccess", action,
permissions::PermissionAction::NUM); PermissionAction::NUM);
break; break;
// The user is not prompted for these permissions, thus there is no // The user is not prompted for these permissions, thus there is no
// permission action recorded for them. // permission action recorded for them.
......
...@@ -149,7 +149,7 @@ class PermissionUmaUtil { ...@@ -149,7 +149,7 @@ class PermissionUmaUtil {
bool should_show, bool should_show,
const std::vector<ContentSettingsType>& content_settings_types); const std::vector<ContentSettingsType>& content_settings_types);
static void RecordMissingPermissionInfobarAction( static void RecordMissingPermissionInfobarAction(
permissions::PermissionAction action, PermissionAction action,
const std::vector<ContentSettingsType>& content_settings_types); const std::vector<ContentSettingsType>& content_settings_types);
// A scoped class that will check the current resolved content setting on // A scoped class that will check the current resolved content setting on
......
...@@ -44,9 +44,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) { ...@@ -44,9 +44,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) {
map->SetContentSettingDefaultScope(host, host, type, std::string(), map->SetContentSettingDefaultScope(host, host, type, std::string(),
CONTENT_SETTING_BLOCK); CONTENT_SETTING_BLOCK);
} }
histograms.ExpectBucketCount( histograms.ExpectBucketCount("Permissions.Action.Geolocation",
"Permissions.Action.Geolocation", static_cast<int>(PermissionAction::REVOKED), 1);
static_cast<int>(permissions::PermissionAction::REVOKED), 1);
// Block->Allow does not trigger a revocation. // Block->Allow does not trigger a revocation.
{ {
...@@ -55,9 +54,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) { ...@@ -55,9 +54,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) {
map->SetContentSettingDefaultScope(host, host, type, std::string(), map->SetContentSettingDefaultScope(host, host, type, std::string(),
CONTENT_SETTING_ALLOW); CONTENT_SETTING_ALLOW);
} }
histograms.ExpectBucketCount( histograms.ExpectBucketCount("Permissions.Action.Geolocation",
"Permissions.Action.Geolocation", static_cast<int>(PermissionAction::REVOKED), 1);
static_cast<int>(permissions::PermissionAction::REVOKED), 1);
// Allow->Default triggers a revocation when default is 'ask'. // Allow->Default triggers a revocation when default is 'ask'.
map->SetDefaultContentSetting(type, CONTENT_SETTING_ASK); map->SetDefaultContentSetting(type, CONTENT_SETTING_ASK);
...@@ -67,9 +65,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) { ...@@ -67,9 +65,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) {
map->SetContentSettingDefaultScope(host, host, type, std::string(), map->SetContentSettingDefaultScope(host, host, type, std::string(),
CONTENT_SETTING_DEFAULT); CONTENT_SETTING_DEFAULT);
} }
histograms.ExpectBucketCount( histograms.ExpectBucketCount("Permissions.Action.Geolocation",
"Permissions.Action.Geolocation", static_cast<int>(PermissionAction::REVOKED), 2);
static_cast<int>(permissions::PermissionAction::REVOKED), 2);
// Allow->Default does not trigger a revocation when default is 'allow'. // Allow->Default does not trigger a revocation when default is 'allow'.
map->SetDefaultContentSetting(type, CONTENT_SETTING_ALLOW); map->SetDefaultContentSetting(type, CONTENT_SETTING_ALLOW);
...@@ -79,9 +76,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) { ...@@ -79,9 +76,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) {
map->SetContentSettingDefaultScope(host, host, type, std::string(), map->SetContentSettingDefaultScope(host, host, type, std::string(),
CONTENT_SETTING_DEFAULT); CONTENT_SETTING_DEFAULT);
} }
histograms.ExpectBucketCount( histograms.ExpectBucketCount("Permissions.Action.Geolocation",
"Permissions.Action.Geolocation", static_cast<int>(PermissionAction::REVOKED), 2);
static_cast<int>(permissions::PermissionAction::REVOKED), 2);
// Allow->Block with url pattern string triggers a revocation. // Allow->Block with url pattern string triggers a revocation.
map->SetContentSettingDefaultScope(host, host, type, std::string(), map->SetContentSettingDefaultScope(host, host, type, std::string(),
...@@ -92,9 +88,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) { ...@@ -92,9 +88,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) {
map->SetContentSettingCustomScope(host_pattern, host_pattern, type, map->SetContentSettingCustomScope(host_pattern, host_pattern, type,
std::string(), CONTENT_SETTING_BLOCK); std::string(), CONTENT_SETTING_BLOCK);
} }
histograms.ExpectBucketCount( histograms.ExpectBucketCount("Permissions.Action.Geolocation",
"Permissions.Action.Geolocation", static_cast<int>(PermissionAction::REVOKED), 3);
static_cast<int>(permissions::PermissionAction::REVOKED), 3);
// Allow->Block with non url pattern string does not trigger a revocation. // Allow->Block with non url pattern string does not trigger a revocation.
map->SetContentSettingDefaultScope(host, host, type, std::string(), map->SetContentSettingDefaultScope(host, host, type, std::string(),
...@@ -107,9 +102,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) { ...@@ -107,9 +102,8 @@ TEST_F(PermissionUmaUtilTest, ScopedevocationReporter) {
host_pattern, type, std::string(), host_pattern, type, std::string(),
CONTENT_SETTING_BLOCK); CONTENT_SETTING_BLOCK);
} }
histograms.ExpectBucketCount( histograms.ExpectBucketCount("Permissions.Action.Geolocation",
"Permissions.Action.Geolocation", static_cast<int>(PermissionAction::REVOKED), 3);
static_cast<int>(permissions::PermissionAction::REVOKED), 3);
} }
} // namespace permissions } // namespace permissions
...@@ -65,8 +65,7 @@ void PermissionsClient::OnPromptResolved( ...@@ -65,8 +65,7 @@ void PermissionsClient::OnPromptResolved(
PermissionRequestType request_type, PermissionRequestType request_type,
PermissionAction action) {} PermissionAction action) {}
base::Optional<url::Origin> PermissionsClient::GetAutoApprovalOrigin( base::Optional<url::Origin> PermissionsClient::GetAutoApprovalOrigin() {
const PermissionRequest* request) {
return base::nullopt; return base::nullopt;
} }
......
...@@ -80,8 +80,7 @@ class PermissionsClient { ...@@ -80,8 +80,7 @@ class PermissionsClient {
// If the embedder returns an origin here, any requests matching that origin // If the embedder returns an origin here, any requests matching that origin
// will be approved. Requests that do not match the returned origin will // will be approved. Requests that do not match the returned origin will
// immediately be finished without granting/denying the permission. // immediately be finished without granting/denying the permission.
virtual base::Optional<url::Origin> GetAutoApprovalOrigin( virtual base::Optional<url::Origin> GetAutoApprovalOrigin();
const PermissionRequest* request);
private: private:
PermissionsClient(const PermissionsClient&) = delete; PermissionsClient(const PermissionsClient&) = delete;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment