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

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

Issue 2258763002: Add a feature-controlled persistence checkbox to geolocation prompts on desktop Views platforms. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@permission-infobardelegate-clean
Patch Set: Rebase Created 4 years, 3 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_context_base_unittest.cc
diff --git a/chrome/browser/permissions/permission_context_base_unittest.cc b/chrome/browser/permissions/permission_context_base_unittest.cc
index 3b130f4bb2e49ac06199a5e766806136b1f5ef9b..25cd98b23f15eb2fb9777640f04d08cfe4d2ca70 100644
--- a/chrome/browser/permissions/permission_context_base_unittest.cc
+++ b/chrome/browser/permissions/permission_context_base_unittest.cc
@@ -128,23 +128,23 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
void RespondToPermission(TestPermissionContext* context,
const PermissionRequestID& id,
const GURL& url,
+ bool persist,
ContentSetting response) {
DCHECK(response == CONTENT_SETTING_ALLOW ||
response == CONTENT_SETTING_BLOCK ||
response == CONTENT_SETTING_ASK);
#if defined(OS_ANDROID)
- bool update_content_setting = response != CONTENT_SETTING_ASK;
PermissionAction decision = DISMISSED;
if (response == CONTENT_SETTING_ALLOW)
decision = GRANTED;
else if (response == CONTENT_SETTING_BLOCK)
decision = DENIED;
context->GetInfoBarController()->OnPermissionSet(
- id, url, url, false /* user_gesture */, update_content_setting,
- decision);
+ id, url, url, false /* user_gesture */, persist, decision);
#else
PermissionRequestManager* manager =
PermissionRequestManager::FromWebContents(web_contents());
+ manager->TogglePersist(persist);
switch (response) {
case CONTENT_SETTING_ALLOW:
manager->Accept();
@@ -161,11 +161,13 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
#endif
}
- void TestAskAndGrant_TestContent() {
- TestPermissionContext permission_context(
- profile(), content::PermissionType::NOTIFICATIONS,
- CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
- GURL url("http://www.google.com");
+ void TestAskAndDecide_TestContent(content::PermissionType permission,
+ ContentSettingsType content_settings_type,
+ ContentSetting decision,
+ bool persist) {
+ TestPermissionContext permission_context(profile(), permission,
+ content_settings_type);
+ GURL url("https://www.google.com");
NavigateAndCommit(url);
base::HistogramTester histograms;
@@ -179,48 +181,37 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
base::Bind(&TestPermissionContext::TrackPermissionDecision,
base::Unretained(&permission_context)));
- RespondToPermission(&permission_context, id, url, CONTENT_SETTING_ALLOW);
+ RespondToPermission(&permission_context, id, url, persist, decision);
EXPECT_EQ(1u, permission_context.decisions().size());
- EXPECT_EQ(CONTENT_SETTING_ALLOW, permission_context.decisions()[0]);
+ EXPECT_EQ(decision, permission_context.decisions()[0]);
EXPECT_TRUE(permission_context.tab_context_updated());
- EXPECT_EQ(CONTENT_SETTING_ALLOW,
- permission_context.GetContentSettingFromMap(url, url));
-
- histograms.ExpectUniqueSample(
- "Permissions.Prompt.Accepted.PriorDismissCount.Notifications", 0, 1);
- histograms.ExpectUniqueSample(
- "Permissions.Prompt.Accepted.PriorIgnoreCount.Notifications", 0, 1);
- }
- void TestAskAndDismiss_TestContent() {
- TestPermissionContext permission_context(
- profile(), content::PermissionType::MIDI_SYSEX,
- CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
- GURL url("http://www.google.es");
- NavigateAndCommit(url);
- base::HistogramTester histograms;
-
- const PermissionRequestID id(
- web_contents()->GetRenderProcessHost()->GetID(),
- web_contents()->GetMainFrame()->GetRoutingID(),
- -1);
- permission_context.RequestPermission(
- web_contents(),
- id, url, true /* user_gesture */,
- base::Bind(&TestPermissionContext::TrackPermissionDecision,
- base::Unretained(&permission_context)));
-
- RespondToPermission(&permission_context, id, url, CONTENT_SETTING_ASK);
- EXPECT_EQ(1u, permission_context.decisions().size());
- EXPECT_EQ(CONTENT_SETTING_ASK, permission_context.decisions()[0]);
- EXPECT_TRUE(permission_context.tab_context_updated());
- EXPECT_EQ(CONTENT_SETTING_ASK,
- permission_context.GetContentSettingFromMap(url, url));
+ std::string decision_string = "";
+ if (decision == CONTENT_SETTING_ALLOW)
+ decision_string = "Accepted";
+ else if (decision == CONTENT_SETTING_BLOCK)
+ decision_string = "Denied";
+ else if (decision == CONTENT_SETTING_ASK)
+ decision_string = "Dismissed";
+
+ if (decision_string.size()) {
+ histograms.ExpectUniqueSample(
+ "Permissions.Prompt." + decision_string + ".PriorDismissCount." +
+ PermissionUtil::GetPermissionString(permission),
+ 0, 1);
+ histograms.ExpectUniqueSample(
+ "Permissions.Prompt." + decision_string + ".PriorIgnoreCount." +
+ PermissionUtil::GetPermissionString(permission),
+ 0, 1);
+ }
- histograms.ExpectUniqueSample(
- "Permissions.Prompt.Dismissed.PriorDismissCount.MidiSysEx", 0, 1);
- histograms.ExpectUniqueSample(
- "Permissions.Prompt.Dismissed.PriorIgnoreCount.MidiSysEx", 0, 1);
+ if (persist) {
+ EXPECT_EQ(decision,
+ permission_context.GetContentSettingFromMap(url, url));
+ } else {
+ EXPECT_EQ(CONTENT_SETTING_ASK,
+ permission_context.GetContentSettingFromMap(url, url));
+ }
}
void DismissMultipleTimesAndExpectBlock(
@@ -246,7 +237,8 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
base::Bind(&TestPermissionContext::TrackPermissionDecision,
base::Unretained(&permission_context)));
- RespondToPermission(&permission_context, id, url, CONTENT_SETTING_ASK);
+ RespondToPermission(&permission_context, id, url, false, /* persist */
+ CONTENT_SETTING_ASK);
histograms.ExpectTotalCount(
"Permissions.Prompt.Dismissed.PriorDismissCount." +
PermissionUtil::GetPermissionString(permission_type),
@@ -289,7 +281,8 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
base::Bind(&TestPermissionContext::TrackPermissionDecision,
base::Unretained(&permission_context)));
- RespondToPermission(&permission_context, id, url, CONTENT_SETTING_ASK);
+ RespondToPermission(&permission_context, id, url, false, /* persist */
+ CONTENT_SETTING_ASK);
histograms.ExpectTotalCount(
"Permissions.Prompt.Dismissed.PriorDismissCount.Geolocation",
i + 1);
@@ -379,7 +372,8 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
base::Bind(&TestPermissionContext::TrackPermissionDecision,
base::Unretained(&permission_context)));
- RespondToPermission(&permission_context, id, url, CONTENT_SETTING_ASK);
+ RespondToPermission(&permission_context, id, url, false, /* persist */
+ CONTENT_SETTING_ASK);
EXPECT_EQ(1u, permission_context.decisions().size());
EXPECT_EQ(expected, permission_context.decisions()[0]);
EXPECT_TRUE(permission_context.tab_context_updated());
@@ -446,7 +440,8 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
base::Bind(&TestPermissionContext::TrackPermissionDecision,
base::Unretained(&permission_context)));
- RespondToPermission(&permission_context, id, url, CONTENT_SETTING_ALLOW);
+ RespondToPermission(&permission_context, id, url, true, /* persist */
+ CONTENT_SETTING_ALLOW);
EXPECT_EQ(1u, permission_context.decisions().size());
EXPECT_EQ(CONTENT_SETTING_ALLOW, permission_context.decisions()[0]);
EXPECT_TRUE(permission_context.tab_context_updated());
@@ -509,7 +504,9 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
EXPECT_EQ(0u, permission_context.decisions().size());
- RespondToPermission(&permission_context, id0, url, response);
+ bool persist = (response == CONTENT_SETTING_ALLOW ||
+ response == CONTENT_SETTING_BLOCK);
+ RespondToPermission(&permission_context, id0, url, persist, response);
EXPECT_EQ(2u, permission_context.decisions().size());
EXPECT_EQ(response, permission_context.decisions()[0]);
@@ -535,14 +532,41 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
// Simulates clicking Accept. The permission should be granted and
// saved for future use.
-TEST_F(PermissionContextBaseTests, TestAskAndGrant) {
- TestAskAndGrant_TestContent();
+TEST_F(PermissionContextBaseTests, TestAskAndGrantPersist) {
+ TestAskAndDecide_TestContent(content::PermissionType::NOTIFICATIONS,
+ CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+ CONTENT_SETTING_ALLOW, true);
+}
+
+// Simulates clicking Accept. The permission should be granted, but not
+// persisted.
+TEST_F(PermissionContextBaseTests, TestAskAndGrantNoPersist) {
+ TestAskAndDecide_TestContent(content::PermissionType::NOTIFICATIONS,
+ CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+ CONTENT_SETTING_ALLOW, false);
+}
+
+// Simulates clicking Block. The permission should be denied and
+// saved for future use.
+TEST_F(PermissionContextBaseTests, TestAskAndBlockPersist) {
+ TestAskAndDecide_TestContent(content::PermissionType::GEOLOCATION,
+ CONTENT_SETTINGS_TYPE_GEOLOCATION,
+ CONTENT_SETTING_BLOCK, true);
+}
+
+// Simulates clicking Block. The permission should be denied, but not persisted.
+TEST_F(PermissionContextBaseTests, TestAskAndBlockNoPersist) {
+ TestAskAndDecide_TestContent(content::PermissionType::GEOLOCATION,
+ CONTENT_SETTINGS_TYPE_GEOLOCATION,
+ CONTENT_SETTING_BLOCK, false);
}
// Simulates clicking Dismiss (X) in the infobar/bubble.
// The permission should be denied but not saved for future use.
TEST_F(PermissionContextBaseTests, TestAskAndDismiss) {
- TestAskAndDismiss_TestContent();
+ TestAskAndDecide_TestContent(content::PermissionType::MIDI_SYSEX,
+ CONTENT_SETTINGS_TYPE_MIDI_SYSEX,
+ CONTENT_SETTING_ASK, false);
}
// Simulates clicking Dismiss (X) in the infobar/bubble with the block on too
« no previous file with comments | « chrome/browser/permissions/permission_context_base.cc ('k') | chrome/browser/permissions/permission_request.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698