Chromium Code Reviews| 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) { |