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

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

Issue 2184823007: Add a feature which, when enabled, blocks permissions after X prompt dismissals. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Unify implementation in permission_context_base, make log static 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_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 e351822da4006e7b839894a0a44ecc18916560a8..f6b3844f2ef29a65623227b174b514f2da88b11f 100644
--- a/chrome/browser/permissions/permission_context_base_unittest.cc
+++ b/chrome/browser/permissions/permission_context_base_unittest.cc
@@ -8,15 +8,19 @@
#include "base/bind.h"
#include "base/command_line.h"
+#include "base/feature_list.h"
#include "base/macros.h"
#include "base/metrics/field_trial.h"
+#include "base/test/histogram_tester.h"
#include "base/test/mock_entropy_provider.h"
#include "build/build_config.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/infobars/infobar_service.h"
+#include "chrome/browser/permissions/permission_prompt_decision_log.h"
#include "chrome/browser/permissions/permission_queue_controller.h"
#include "chrome/browser/permissions/permission_request_id.h"
#include "chrome/browser/permissions/permission_util.h"
+#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/testing_profile.h"
@@ -41,6 +45,10 @@ const char* kPermissionsKillSwitchFieldStudy =
const char* kPermissionsKillSwitchBlockedValue =
PermissionContextBase::kPermissionsKillSwitchBlockedValue;
const char kPermissionsKillSwitchTestGroup[] = "TestGroup";
+const char* kPromptGroupName = kPermissionsKillSwitchTestGroup;
+const char kPromptTrialName[] = "PermissionPromptsUX";
+const char* kPromptDismissCountKey =
+ PermissionPromptDecisionLog::kPromptDismissCountKey;
class TestPermissionContext : public PermissionContextBase {
public:
@@ -190,6 +198,169 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
permission_context.GetContentSettingFromMap(url, url));
}
+ void DismissMultipleTimesAndExpectBlock(
+ const GURL& url,
+ content::PermissionType permission_type,
+ ContentSettingsType content_settings_type,
+ unsigned int iterations) {
raymes 2016/08/05 03:27:48 nit: I think uint32/64_t is preferred over unsigne
dominickn 2016/08/05 04:29:30 Done.
+ base::HistogramTester histograms;
+
+ // Dismiss |iterations| times. The final dismiss should change the decision
+ // from dismiss to block, and hence change the persisted content setting.
+ for (unsigned int i = 0; i < iterations; ++i) {
+ TestPermissionContext permission_context(
+ profile(), permission_type, content_settings_type);
+ ContentSetting expected =
+ (i < (iterations - 1)) ? CONTENT_SETTING_ASK : CONTENT_SETTING_BLOCK;
+
+ const PermissionRequestID id(
+ web_contents()->GetRenderProcessHost()->GetID(),
+ web_contents()->GetMainFrame()->GetRoutingID(), i);
+ 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);
+ histograms.ExpectTotalCount(
+ "Permissions.DismissCount." +
+ PermissionUtil::GetPermissionString(permission_type),
+ i + 1);
+
+ EXPECT_EQ(1u, permission_context.decisions().size());
+ EXPECT_EQ(expected, permission_context.decisions()[0]);
+ EXPECT_TRUE(permission_context.tab_context_updated());
+ EXPECT_EQ(expected,
+ permission_context.GetContentSettingFromMap(url, url));
+ }
raymes 2016/08/05 03:27:47 nit: as a sanity check against an off-by-one bug i
dominickn 2016/08/05 04:29:30 Done.
+ }
+
+ void TestBlockOnSeveralDismissals_TestContent() {
+ GURL url("https://www.google.com");
+ NavigateAndCommit(url);
+ base::HistogramTester histograms;
+
+ // First, ensure that > 3 dismissals behaves correctly.
+ for (unsigned int i = 0; i < 4; ++i) {
+ TestPermissionContext permission_context(
+ profile(), content::PermissionType::GEOLOCATION,
+ CONTENT_SETTINGS_TYPE_GEOLOCATION);
+
+ const PermissionRequestID id(
+ web_contents()->GetRenderProcessHost()->GetID(),
+ web_contents()->GetMainFrame()->GetRoutingID(), i);
+ 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);
+ histograms.ExpectTotalCount("Permissions.DismissCount.Geolocation",
+ i + 1);
+ 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));
+ }
+
+ // Flush the dismissal counts. Enable the block on too many dismissals
+ // feature, which is disabled by default.
+ HostContentSettingsMapFactory::GetForProfile(profile())
+ ->ClearSettingsForOneType(
+ CONTENT_SETTINGS_TYPE_PROMPT_NO_DECISION_COUNT);
+
+ {
+ // Set up the custom parameter. This must be within a scope to correctly
+ // instantiate the field trial global.
+ base::FieldTrialList field_trials_(nullptr);
+
+ base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial(
+ kPromptTrialName, kPromptGroupName);
+ base::FeatureList::ClearInstanceForTesting();
+ std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
+ feature_list->RegisterFieldTrialOverride(
+ features::kBlockPromptsIfDismissedOften.name,
+ base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial);
+ base::FeatureList::SetInstance(std::move(feature_list));
+ EXPECT_EQ(base::FeatureList::GetFieldTrial(
+ features::kBlockPromptsIfDismissedOften),
+ trial);
+ }
+
+ EXPECT_TRUE(
+ base::FeatureList::IsEnabled(features::kBlockPromptsIfDismissedOften));
+
+ // Sanity check independence per permission type by checking two of them.
+ DismissMultipleTimesAndExpectBlock(url,
+ content::PermissionType::GEOLOCATION,
+ CONTENT_SETTINGS_TYPE_GEOLOCATION, 3);
+ DismissMultipleTimesAndExpectBlock(url,
+ content::PermissionType::NOTIFICATIONS,
+ CONTENT_SETTINGS_TYPE_NOTIFICATIONS, 3);
+ base::FeatureList::ClearInstanceForTesting();
+ }
+
+ void TestVariationBlockOnSeveralDismissals_TestContent() {
+ GURL url("https://www.google.com");
+ NavigateAndCommit(url);
+
+ {
+ // Set up the custom parameter. This must be within a scope to correctly
+ // instantiate the field trial global.
+ base::FieldTrialList field_trials_(nullptr);
+
+ base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial(
+ kPromptTrialName, kPromptGroupName);
+ std::map<std::string, std::string> params;
+ params[kPromptDismissCountKey] = "5";
+ ASSERT_TRUE(variations::AssociateVariationParams(
+ kPromptTrialName, kPromptGroupName, params));
+
+ base::FeatureList::ClearInstanceForTesting();
+ std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
+ feature_list->RegisterFieldTrialOverride(
+ features::kBlockPromptsIfDismissedOften.name,
+ base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial);
+ base::FeatureList::SetInstance(std::move(feature_list));
+ EXPECT_EQ(base::FeatureList::GetFieldTrial(
+ features::kBlockPromptsIfDismissedOften),
+ trial);
+
+ std::map<std::string, std::string> actualParams;
+ EXPECT_TRUE(variations::GetVariationParamsByFeature(
+ features::kBlockPromptsIfDismissedOften, &actualParams));
+ EXPECT_EQ(params, actualParams);
+
+ PermissionContextBase::prompt_decision_log_.Get().UpdateFromVariations();
+ }
+
+ for (unsigned int i = 0; i < 5; ++i) {
+ TestPermissionContext permission_context(
+ profile(), content::PermissionType::MIDI_SYSEX,
+ CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
+
+ ContentSetting expected =
+ (i < 4) ? CONTENT_SETTING_ASK : CONTENT_SETTING_BLOCK;
+ const PermissionRequestID id(
+ web_contents()->GetRenderProcessHost()->GetID(),
+ web_contents()->GetMainFrame()->GetRoutingID(), i);
+ 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(expected, permission_context.decisions()[0]);
+ EXPECT_TRUE(permission_context.tab_context_updated());
+ EXPECT_EQ(expected,
+ permission_context.GetContentSettingFromMap(url, url));
+ }
raymes 2016/08/05 03:27:48 A similar sanity check here would be good.
dominickn 2016/08/05 04:29:30 Done.
+ base::FeatureList::ClearInstanceForTesting();
+ variations::testing::ClearAllVariationParams();
+ }
+
void TestRequestPermissionInvalidUrl(
content::PermissionType permission_type,
ContentSettingsType content_settings_type) {
@@ -333,6 +504,18 @@ TEST_F(PermissionContextBaseTests, TestAskAndDismiss) {
TestAskAndDismiss_TestContent();
}
+// Simulates clicking Dismiss (X) in the infobar/bubble with the block on too
+// many dismissals feature active.
+// The permission should be blocked after several dismissals.
raymes 2016/08/05 03:27:47 nit: fill 80 chars on previous line or add a newli
dominickn 2016/08/05 04:29:30 Done.
+TEST_F(PermissionContextBaseTests, TestDismissUntilBlocked) {
+ TestBlockOnSeveralDismissals_TestContent();
+}
+
+// Test setting a custom number of dismissals before block via variations.
+TEST_F(PermissionContextBaseTests, TestDismissVariations) {
+ TestVariationBlockOnSeveralDismissals_TestContent();
+}
+
// Simulates non-valid requesting URL.
// The permission should be denied but not saved for future use.
TEST_F(PermissionContextBaseTests, TestNonValidRequestingUrl) {

Powered by Google App Engine
This is Rietveld 408576698