Implement embargo in PermissionDecisionAutoBlocker

chrome/browser/permissions/permission_context_base_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/permissions/permission_context_base.h"
 
 #include <map>
 #include <set>
 #include <string>
 #include <utility>
@@ skipping 310 matching lines @@
       content::PermissionType permission_type,
       ContentSettingsType content_settings_type,
       uint32_t iterations) {
     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 (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);
 
       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)));
       histograms.ExpectTotalCount(
           "Permissions.Prompt.Dismissed.PriorDismissCount." +
               PermissionUtil::GetPermissionString(permission_type),
           i + 1);
       histograms.ExpectBucketCount(
           "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() {
     GURL url("https://www.google.com");
     NavigateAndCommit(url);
     base::HistogramTester histograms;
 
     // First, ensure that > 3 dismissals behaves correctly.
     for (uint32_t i = 0; i < 4; ++i) {
       TestPermissionContext permission_context(
@@ skipping 76 matching lines @@
       std::map<std::string, std::string> actual_params;
       EXPECT_TRUE(variations::GetVariationParamsByFeature(
           features::kBlockPromptsIfDismissedOften, &actual_params));
       EXPECT_EQ(params, actual_params);
     }
 
     for (uint32_t 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.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)));
 
       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(
           "Permissions.Prompt.Dismissed.PriorDismissCount.MidiSysEx", i + 1);
       histograms.ExpectBucketCount(
           "Permissions.Prompt.Dismissed.PriorDismissCount.MidiSysEx", i, 1);
     }
 
     // Ensure that we finish in the block state.
     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();
   }
 
   void TestRequestPermissionInvalidUrl(
       content::PermissionType permission_type,
       ContentSettingsType content_settings_type) {
     TestPermissionContext permission_context(profile(), permission_type,
                                              content_settings_type);
     GURL url;
     ASSERT_FALSE(url.is_valid());
@@ skipping 346 matching lines @@
       new MockSafeBrowsingDatabaseManager(false /* perform_callback */);
   const GURL url("https://www.example.com");
   std::set<std::string> blacklisted_permissions{
       PermissionUtil::GetPermissionString(
           content::PermissionType::GEOLOCATION)};
   db_manager->BlacklistUrlPermissions(url, blacklisted_permissions);
   TestPermissionsBlacklisting(content::PermissionType::GEOLOCATION,
                               CONTENT_SETTINGS_TYPE_GEOLOCATION, db_manager,
                               url, 0 /* timeout in ms */, CONTENT_SETTING_ASK);
 }