Add revocation reporter to permission util

chrome/browser/permissions/permission_util_unittest.cc

 // Copyright 2016 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 "base/test/histogram_tester.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/permissions/permission_uma_util.h"
 #include "chrome/browser/permissions/permission_util.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 class PermissionUtilTest : public testing::Test {
   content::TestBrowserThreadBundle thread_bundle_;
 };
 
 TEST_F(PermissionUtilTest, SetContentSettingRecordRevocation) {
   TestingProfile profile;
   // TODO(tsergeant): Add more comprehensive tests of PermissionUmaUtil.
   base::HistogramTester histograms;
   HostContentSettingsMap* map =
       HostContentSettingsMapFactory::GetForProfile(&profile);
   GURL host("https://example.com");
   ContentSettingsType type = CONTENT_SETTINGS_TYPE_GEOLOCATION;
 
   // Allow->Block triggers a revocation.
   map->SetContentSettingDefaultScope(host, host, type, std::string(),
                                      CONTENT_SETTING_ALLOW);
-  PermissionUtil::SetContentSettingAndRecordRevocation(
-      &profile, host, host, type, std::string(), CONTENT_SETTING_BLOCK);
+  {
+    PermissionUtil::RevocationReporter scoped_revocation_reporter(
+        &profile, host, host, type, std::string(),
+        PermissionSourceUI::SITE_SETTINGS);
+    map->SetContentSettingDefaultScope(host, host, type, std::string(),
+                                       CONTENT_SETTING_BLOCK);
+  }
   histograms.ExpectBucketCount("Permissions.Action.Geolocation",
                                PermissionAction::REVOKED, 1);
 
   // Block->Allow does not trigger a revocation.
-  PermissionUtil::SetContentSettingAndRecordRevocation(
-      &profile, host, host, type, std::string(), CONTENT_SETTING_ALLOW);
+  {
+    PermissionUtil::RevocationReporter scoped_revocation_reporter(

[raymes, 2016/07/21 23:08:01]

Calling the class ScopedRevocationReporter is actually a good idea I think

[stefanocs, 2016/07/22 00:44:28]

Done.

+        &profile, host, host, type, std::string(),
+        PermissionSourceUI::SITE_SETTINGS);
+    map->SetContentSettingDefaultScope(host, host, type, std::string(),
+                                       CONTENT_SETTING_ALLOW);
+  }
   histograms.ExpectBucketCount("Permissions.Action.Geolocation",
                                PermissionAction::REVOKED, 1);
 
   // Allow->Default triggers a revocation when default is 'ask'.
   map->SetDefaultContentSetting(type, CONTENT_SETTING_ASK);
-  PermissionUtil::SetContentSettingAndRecordRevocation(
-      &profile, host, host, type, std::string(), CONTENT_SETTING_DEFAULT);
+  {
+    PermissionUtil::RevocationReporter scoped_revocation_reporter(
+        &profile, host, host, type, std::string(),
+        PermissionSourceUI::SITE_SETTINGS);
+    map->SetContentSettingDefaultScope(host, host, type, std::string(),
+                                       CONTENT_SETTING_DEFAULT);
+  }
   histograms.ExpectBucketCount("Permissions.Action.Geolocation",
                                PermissionAction::REVOKED, 2);
 
   // Allow->Default does not trigger a revocation when default is 'allow'.
   map->SetDefaultContentSetting(type, CONTENT_SETTING_ALLOW);
-  PermissionUtil::SetContentSettingAndRecordRevocation(
-      &profile, host, host, type, std::string(), CONTENT_SETTING_DEFAULT);
+  {
+    PermissionUtil::RevocationReporter scoped_revocation_reporter(
+        &profile, host, host, type, std::string(),
+        PermissionSourceUI::SITE_SETTINGS);
+    map->SetContentSettingDefaultScope(host, host, type, std::string(),
+                                       CONTENT_SETTING_DEFAULT);

[raymes, 2016/07/21 23:08:01]

These tests don't exercise the new constructor. But I'd suggest we add that when
it is used.

[stefanocs, 2016/07/22 00:44:28]

Acknowledged.

+  }
   histograms.ExpectBucketCount("Permissions.Action.Geolocation",
                                PermissionAction::REVOKED, 2);
 }