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

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

Issue 2790493002: Implement permissions embargo for prompts which are repeatedly ignored. (Closed)
Patch Set: Not for plugins Created 3 years, 9 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_decision_auto_blocker_unittest.cc
diff --git a/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc b/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
index 849d0b2086349d9d9681f0922895506a4e258408..c6d4e90a14933376a8f4fbf05ef3a97deb380e5a 100644
--- a/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
+++ b/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
@@ -89,6 +89,7 @@ class PermissionDecisionAutoBlockerUnitTest
ChromeRenderViewHostTestHarness::SetUp();
autoblocker_ = PermissionDecisionAutoBlocker::GetForProfile(profile());
feature_list_.InitWithFeatures({features::kBlockPromptsIfDismissedOften,
+ features::kBlockPromptsIfIgnoredOften,
features::kPermissionsBlacklist},
{});
last_embargoed_status_ = false;
@@ -190,14 +191,20 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, RemoveCountsByUrl) {
url1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS));
// Record some ignores.
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
EXPECT_EQ(
- 1, autoblocker()->RecordIgnore(url1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
- EXPECT_EQ(1, autoblocker()->RecordIgnore(
+ 1, autoblocker()->GetIgnoreCount(url1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url1, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE));
+ EXPECT_EQ(1, autoblocker()->GetIgnoreCount(
url1, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE));
- EXPECT_EQ(
- 1, autoblocker()->RecordIgnore(url2, CONTENT_SETTINGS_TYPE_GEOLOCATION));
- EXPECT_EQ(
- 2, autoblocker()->RecordIgnore(url2, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url2, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url2, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+ EXPECT_EQ(2, autoblocker()->GetIgnoreCount(
+ url2, CONTENT_SETTINGS_TYPE_GEOLOCATION));
autoblocker()->RemoveCountsByUrl(base::Bind(&FilterGoogle));
@@ -232,14 +239,22 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, RemoveCountsByUrl) {
EXPECT_EQ(2, autoblocker()->GetDismissCount(
url2, CONTENT_SETTINGS_TYPE_GEOLOCATION));
- EXPECT_EQ(
- 1, autoblocker()->RecordIgnore(url1, CONTENT_SETTINGS_TYPE_GEOLOCATION));
- EXPECT_EQ(1, autoblocker()->RecordIgnore(
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url1, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+ EXPECT_EQ(1, autoblocker()->GetIgnoreCount(
+ url1, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS));
+ EXPECT_EQ(1, autoblocker()->GetIgnoreCount(
url1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS));
- EXPECT_EQ(1, autoblocker()->RecordIgnore(
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url1, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE));
+ EXPECT_EQ(1, autoblocker()->GetIgnoreCount(
url1, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE));
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url2, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
EXPECT_EQ(
- 1, autoblocker()->RecordIgnore(url2, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
+ 1, autoblocker()->GetIgnoreCount(url2, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
// Remove everything and expect that it's all gone.
autoblocker()->RemoveCountsByUrl(base::Bind(&FilterAll));
@@ -287,7 +302,7 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, TestUpdateEmbargoBlacklist) {
}
// Test that an origin that is blacklisted for a permission will not be placed
-// under embargoed for another.
+// under embargo for another permission.
TEST_F(PermissionDecisionAutoBlockerUnitTest, TestRequestNotBlacklisted) {
GURL url("https://www.google.com");
clock()->SetNow(base::Time::Now());
@@ -313,8 +328,10 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, TestRequestNotBlacklisted) {
// Check that we do not apply embargo to the plugins content type, as prompts
// should be triggered only when necessary by Html5ByDefault.
TEST_F(PermissionDecisionAutoBlockerUnitTest,
- PluginsNotEmbargoedByMultipleDismisses) {
+ PluginsNotEmbargoedByMultipleDismissesOrIgnores) {
GURL url("https://www.google.com");
+
+ // Check dismisses first.
autoblocker()->RecordDismissAndEmbargo(url, CONTENT_SETTINGS_TYPE_PLUGINS);
autoblocker()->RecordDismissAndEmbargo(url, CONTENT_SETTINGS_TYPE_PLUGINS);
PermissionResult result =
@@ -327,7 +344,8 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest,
// The third dismiss would normally embargo, but this shouldn't happen for
// plugins.
- autoblocker()->RecordDismissAndEmbargo(url, CONTENT_SETTINGS_TYPE_PLUGINS);
+ EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_PLUGINS));
result = autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_PLUGINS);
EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
@@ -336,13 +354,46 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest,
autoblocker()->GetDismissCount(url, CONTENT_SETTINGS_TYPE_PLUGINS));
// Extra one for sanity checking.
- autoblocker()->RecordDismissAndEmbargo(url, CONTENT_SETTINGS_TYPE_PLUGINS);
+ EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_PLUGINS));
result = autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_PLUGINS);
EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source);
EXPECT_EQ(4,
autoblocker()->GetDismissCount(url, CONTENT_SETTINGS_TYPE_PLUGINS));
+
+ // Check ignores.
+ autoblocker()->RecordIgnoreAndEmbargo(url, CONTENT_SETTINGS_TYPE_PLUGINS);
+ autoblocker()->RecordIgnoreAndEmbargo(url, CONTENT_SETTINGS_TYPE_PLUGINS);
+ autoblocker()->RecordIgnoreAndEmbargo(url, CONTENT_SETTINGS_TYPE_PLUGINS);
+ result = autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_PLUGINS);
+
+ EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
+ EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source);
+ EXPECT_EQ(3,
+ autoblocker()->GetIgnoreCount(url, CONTENT_SETTINGS_TYPE_PLUGINS));
+
+ // The fourth ignore would normally embargo, but this shouldn't happen for
+ // plugins.
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_PLUGINS));
+ result = autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_PLUGINS);
+
+ EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
+ EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source);
+ EXPECT_EQ(4,
+ autoblocker()->GetIgnoreCount(url, CONTENT_SETTINGS_TYPE_PLUGINS));
+
+ // Extra one for sanity checking.
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_PLUGINS));
+ result = autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_PLUGINS);
+
+ EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
+ EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source);
+ EXPECT_EQ(5,
+ autoblocker()->GetIgnoreCount(url, CONTENT_SETTINGS_TYPE_PLUGINS));
}
// Check that GetEmbargoResult returns the correct value when the embargo is set
@@ -473,8 +524,77 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, TestDismissEmbargoBackoff) {
"Permissions.AutoBlocker.SafeBrowsingResponseTime", 0);
}
+// Tests the alternating pattern of the block on multiple ignores behaviour.
+TEST_F(PermissionDecisionAutoBlockerUnitTest, TestIgnoreEmbargoBackoff) {
+ GURL url("https://www.google.com");
+ clock()->SetNow(base::Time::Now());
+ base::HistogramTester histograms;
+
+ // Record some ignores.
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
+
+ // A request with < 4 prior ignores should not be automatically blocked.
+ PermissionResult result =
+ autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
+ EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
+ EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source);
+
+ // After the 4th ignore subsequent permission requests should be autoblocked.
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
+ EXPECT_TRUE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
+ result =
+ autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
+ EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+ EXPECT_EQ(PermissionStatusSource::MULTIPLE_IGNORES, result.source);
+
+ histograms.ExpectTotalCount("Permissions.AutoBlocker.SafeBrowsingResponse",
+ 0);
+ histograms.ExpectTotalCount(
+ "Permissions.AutoBlocker.SafeBrowsingResponseTime", 0);
+ // Accelerate time forward, check that the embargo status is lifted and the
+ // request won't be automatically blocked.
+ clock()->Advance(base::TimeDelta::FromDays(8));
+ result =
+ autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
+ EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
+ EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source);
+
+ // Record another dismiss, subsequent requests should be autoblocked again.
+ EXPECT_TRUE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
+ result =
+ autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
+ EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+ EXPECT_EQ(PermissionStatusSource::MULTIPLE_IGNORES, result.source);
+
+ // Accelerate time again, check embargo is lifted and another permission
+ // request is let through.
+ clock()->Advance(base::TimeDelta::FromDays(8));
+ result =
+ autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
+ EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
+ EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source);
+
+ // Record another dismiss, subsequent requests should be autoblocked again.
+ EXPECT_TRUE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX));
+ result =
+ autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
+ EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+ EXPECT_EQ(PermissionStatusSource::MULTIPLE_IGNORES, result.source);
+ histograms.ExpectTotalCount("Permissions.AutoBlocker.SafeBrowsingResponse",
+ 0);
+ histograms.ExpectTotalCount(
+ "Permissions.AutoBlocker.SafeBrowsingResponseTime", 0);
+}
+
// Test the logic for a combination of blacklisting and dismissal embargo.
-TEST_F(PermissionDecisionAutoBlockerUnitTest, TestExpiredBlacklistEmbargo) {
+TEST_F(PermissionDecisionAutoBlockerUnitTest, TestExpiringOverlappingEmbargo) {
GURL url("https://www.google.com");
clock()->SetNow(base::Time::Now());
@@ -501,6 +621,23 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, TestExpiredBlacklistEmbargo) {
autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_GEOLOCATION);
EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source);
+
+ // Record an ignore embargo.
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+ EXPECT_FALSE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+ EXPECT_TRUE(autoblocker()->RecordIgnoreAndEmbargo(
+ url, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+
+ // Ensure the ignore embargo is still set.
+ clock()->Advance(base::TimeDelta::FromDays(5));
+ result =
+ autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+ EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+ EXPECT_EQ(PermissionStatusSource::MULTIPLE_IGNORES, result.source);
}
TEST_F(PermissionDecisionAutoBlockerUnitTest, TestSafeBrowsingTimeout) {

Powered by Google App Engine
This is Rietveld 408576698