Permissions: Clear embargo if user changes an embargoed permission's setting.

chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc

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..bf1a9d8174986fa501a5ddbb1a04da49ee91c168 100644
--- a/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
+++ b/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
@@ -159,6 +159,123 @@ class PermissionDecisionAutoBlockerUnitTest
   bool callback_was_run_;
 };
 
+// Check removing the the embargo for a single permission on a site works, and
+// that it doesn't interfere with other embargoed permissions or the same
+// permission embargoed on other sites.
+TEST_F(PermissionDecisionAutoBlockerUnitTest, RemoveEmbargoByUrl) {
+  GURL url1("https://www.google.com");
+  GURL url2("https://www.example.com");
+
+  // Record dismissals for location and notifications in |url1|.
+  EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
+      url1, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+  EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
+      url1, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+  EXPECT_TRUE(autoblocker()->RecordDismissAndEmbargo(
+      url1, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+  EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
+      url1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS));
+  EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
+      url1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS));
+  EXPECT_TRUE(autoblocker()->RecordDismissAndEmbargo(
+      url1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS));
+  // Record dismissals for location in |url2|.
+  EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
+      url2, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+  EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
+      url2, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+  EXPECT_TRUE(autoblocker()->RecordDismissAndEmbargo(
+      url2, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+
+  // Verify all dismissals recorded above resulted in embargo.
+  PermissionResult result =
+      autoblocker()->GetEmbargoResult(url1, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source);
+  result = autoblocker()->GetEmbargoResult(url1,
+                                           CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source);
+  result =
+      autoblocker()->GetEmbargoResult(url2, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source);
+
+  // Remove the embargo on notifications. Verify it is no longer under embargo,
+  // but location still is.
+  autoblocker()->RemoveEmbargoByUrl(url1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
+  result =
+      autoblocker()->GetEmbargoResult(url1, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source);
+  // Notification's default setting is |CONTENT_SETTING_ASK|.

[raymes, 2017/04/06 04:45:35]

nit: I think it's not that notifications default is CONTENT_SETTING_ASK but
GetEmbargoResult will always return ASK if it's not embargoed

[Patti Lor, 2017/04/06 05:31:46]

Oh, you're totally right - thanks. Updated the comment.

+  result = autoblocker()->GetEmbargoResult(url1,
+                                           CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
+  EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source);
+  // Verify |url2|'s embargo is still intact as well.
+  result =
+      autoblocker()->GetEmbargoResult(url2, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source);
+}
+
+// Test that removing embargo from blacklisted permissions also works.
+TEST_F(PermissionDecisionAutoBlockerUnitTest,
+       RemoveEmbargoByUrlForBlacklistedPermission) {
+  GURL url("https://www.example.com");
+
+  // Place under embargo and verify.
+  PlaceUnderBlacklistEmbargo(url, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  PermissionResult result =
+      autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::SAFE_BROWSING_BLACKLIST, result.source);
+
+  // Remove embargo and verify.
+  autoblocker()->RemoveEmbargoByUrl(url, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  result =
+      autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source);
+}
+
+// Test it still only takes one more dismissal to re-trigger embargo after
+// removing the embargo status for a site.
+TEST_F(PermissionDecisionAutoBlockerUnitTest,
+       DismissAfterRemovingEmbargoByURL) {
+  GURL url("https://www.example.com");
+
+  // Record dismissals for location.
+  EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
+      url, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+  EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
+      url, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+  EXPECT_TRUE(autoblocker()->RecordDismissAndEmbargo(
+      url, CONTENT_SETTINGS_TYPE_GEOLOCATION));
+
+  // Verify location is under embargo.
+  PermissionResult result =
+      autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source);
+
+  // Remove embargo and verify this is true.
+  autoblocker()->RemoveEmbargoByUrl(url, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  result =
+      autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source);
+
+  // Record another dismissal and verify location is under embargo again.
+  autoblocker()->RecordDismissAndEmbargo(url,
+                                         CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  result =
+      autoblocker()->GetEmbargoResult(url, CONTENT_SETTINGS_TYPE_GEOLOCATION);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting);
+  EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source);
+}
+
 TEST_F(PermissionDecisionAutoBlockerUnitTest, RemoveCountsByUrl) {
   GURL url1("https://www.google.com");
   GURL url2("https://www.example.com");