Implement embargo in PermissionDecisionAutoBlocker

chrome/browser/permissions/permission_context_base_unittest.cc

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 9284d1abadc55932bce4451b1ad4d4fd4d790189..a7a973c9e794ccbbcf7e7af2577161dece515b0c 100644
--- a/chrome/browser/permissions/permission_context_base_unittest.cc
+++ b/chrome/browser/permissions/permission_context_base_unittest.cc
@@ -328,9 +328,6 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
     for (uint32_t 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);
@@ -352,19 +349,31 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
           "Permissions.Prompt.Dismissed.PriorDismissCount." +
               PermissionUtil::GetPermissionString(permission_type),
           i, 1);
-
       ASSERT_EQ(1u, permission_context.decisions().size());
-      EXPECT_EQ(expected, permission_context.decisions()[0]);
+      EXPECT_EQ(CONTENT_SETTING_ASK, permission_context.decisions()[0]);
       EXPECT_TRUE(permission_context.tab_context_updated());
-      EXPECT_EQ(expected,
-                permission_context.GetContentSettingFromMap(url, url));
+      EXPECT_EQ(CONTENT_SETTING_ASK,
+                permission_context.GetPermissionStatus(url, url));
     }
 
-    // Ensure that we finish in the block state.
-    TestPermissionContext permission_context(
-        profile(), permission_type, content_settings_type);
+    TestPermissionContext permission_context(profile(), permission_type,
+                                             content_settings_type);
+    const PermissionRequestID id(
+        web_contents()->GetRenderProcessHost()->GetID(),
+        web_contents()->GetMainFrame()->GetRoutingID(), -1);
+
+    permission_context.SetRespondPermissionCallback(
+        base::Bind(&PermissionContextBaseTests::RespondToPermission,
+                   base::Unretained(this), &permission_context, id, url, false,
+                   CONTENT_SETTING_ASK));

[raymes, 2017/01/18 03:15:20]

It looks like it takes one additional request to be blocked now? Is that
intended? Should we just update the loop above to have an extra iteration?

If that's not possible could we just reuse |id| from the loop?

[meredithl, 2017/01/18 08:28:15]

The embargo status wasn't set until the next permission request. We have now
updated it so that it embargoes on the Nth dismissal, rather than waiting until
the next one so this has been taken care of anyway and we've been able to go
back to what we were doing previously, setting the expected value using a
ternary, and not needing the extra request.

+
+    permission_context.RequestPermission(
+        web_contents(), id, url, true /* user_gesture */,
+        base::Bind(&TestPermissionContext::TrackPermissionDecision,
+                   base::Unretained(&permission_context)));
+
     EXPECT_EQ(CONTENT_SETTING_BLOCK,
-              permission_context.GetContentSettingFromMap(url, url));
+              permission_context.GetPermissionStatus(url, url));
   }
 
   void TestBlockOnSeveralDismissals_TestContent() {
@@ -461,9 +470,6 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
       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);
@@ -477,9 +483,9 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
                      base::Unretained(&permission_context)));
 
       EXPECT_EQ(1u, permission_context.decisions().size());
-      ASSERT_EQ(expected, permission_context.decisions()[0]);
+      ASSERT_EQ(CONTENT_SETTING_ASK, permission_context.decisions()[0]);
       EXPECT_TRUE(permission_context.tab_context_updated());
-      EXPECT_EQ(expected,
+      EXPECT_EQ(CONTENT_SETTING_ASK,
                 permission_context.GetContentSettingFromMap(url, url));
 
       histograms.ExpectTotalCount(
@@ -492,8 +498,23 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
     TestPermissionContext permission_context(
         profile(), content::PermissionType::MIDI_SYSEX,
         CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
+
+    const PermissionRequestID id(
+        web_contents()->GetRenderProcessHost()->GetID(),
+        web_contents()->GetMainFrame()->GetRoutingID(), -1);
+
+    permission_context.SetRespondPermissionCallback(
+        base::Bind(&PermissionContextBaseTests::RespondToPermission,
+                   base::Unretained(this), &permission_context, id, url, false,
+                   CONTENT_SETTING_ASK));
+
+    permission_context.RequestPermission(
+        web_contents(), id, url, true /* user_gesture */,
+        base::Bind(&TestPermissionContext::TrackPermissionDecision,
+                   base::Unretained(&permission_context)));

[raymes, 2017/01/18 03:15:20]

Same here

[meredithl, 2017/01/18 08:28:15]

Done.

+
     EXPECT_EQ(CONTENT_SETTING_BLOCK,
-              permission_context.GetContentSettingFromMap(url, url));
+              permission_context.GetPermissionStatus(url, url));
     variations::testing::ClearAllVariationParams();
   }