Add revocation reporter to permission util

chrome/browser/permissions/permission_util.cc

 // Copyright 2015 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_util.h"
 
 #include "base/logging.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/permissions/permission_uma_util.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/browser/permission_type.h"
+#include "url/gurl.h"
 
 using content::PermissionType;
 
 std::size_t PermissionTypeHash::operator()(
     const content::PermissionType& type) const {
   return static_cast<size_t>(type);
 }
 
 // The returned strings must match the RAPPOR metrics in rappor.xml,
 // and any Field Trial configs for the Permissions kill switch e.g.
@@ skipping 49 matching lines @@
 #if defined(OS_ANDROID) || defined(OS_CHROMEOS)
   } else if (type == CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER) {
     *out = PermissionType::PROTECTED_MEDIA_IDENTIFIER;
 #endif
   } else {
     return false;
   }
   return true;
 }
 
-void PermissionUtil::SetContentSettingAndRecordRevocation(
+PermissionUtil::RevocationReporter::RevocationReporter(
     Profile* profile,
     const GURL& primary_url,
     const GURL& secondary_url,
     ContentSettingsType content_type,
     std::string resource_identifier,
-    ContentSetting setting) {
+    PermissionSourceUI source_ui)
+    : profile_(profile),

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

nit: Rather than passing the profile, should we pass the HostContentSettingsMap?
It will probably be needed by the caller anyway and it saves getting to it from
here.

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

We need the profile to check if the user opted into the permission reporting in
permission uma util.

+      primary_url_(primary_url),
+      secondary_url_(secondary_url),
+      content_type_(content_type),
+      resource_identifier_(resource_identifier),
+      source_ui_(source_ui) {
+  if (!primary_url_.is_valid() ||
+      (!secondary_url_.is_valid() && !secondary_url_.is_empty())) {
+    is_initially_allowed_ = false;
+    return;
+  }
   HostContentSettingsMap* map =
-      HostContentSettingsMapFactory::GetForProfile(profile);
-  ContentSetting previous_value = map->GetContentSetting(
-      primary_url, secondary_url, content_type, resource_identifier);
+      HostContentSettingsMapFactory::GetForProfile(profile_);
+  ContentSetting initial_content_setting = map->GetContentSetting(
+      primary_url_, secondary_url_, content_type_, resource_identifier_);
+  is_initially_allowed_ = (initial_content_setting == CONTENT_SETTING_ALLOW);

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

nit: no () needed

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

Done.

+}
 
-  map->SetContentSettingDefaultScope(primary_url, secondary_url, content_type,
-                                     resource_identifier, setting);
+PermissionUtil::RevocationReporter::RevocationReporter(
+    Profile* profile,
+    const ContentSettingsPattern& primary_pattern,
+    const ContentSettingsPattern& secondary_pattern,
+    ContentSettingsType content_type,
+    std::string resource_identifier,
+    PermissionSourceUI source_ui)
+    : RevocationReporter(
+          profile,
+          GURL(primary_pattern.ToString()),
+          GURL((secondary_pattern == ContentSettingsPattern::Wildcard())
+                   ? primary_pattern.ToString()
+                   : secondary_pattern.ToString()),
+          content_type,
+          resource_identifier,
+          source_ui) {}

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

nit: can we leave this constructor out until the next CL?

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

Done.

 
-  ContentSetting final_value = map->GetContentSetting(
-      primary_url, secondary_url, content_type, resource_identifier);
-
-  if (previous_value == CONTENT_SETTING_ALLOW &&
-      final_value != CONTENT_SETTING_ALLOW) {
+PermissionUtil::RevocationReporter::~RevocationReporter() {
+  if (!is_initially_allowed_)
+    return;
+  HostContentSettingsMap* map =
+      HostContentSettingsMapFactory::GetForProfile(profile_);
+  ContentSetting final_content_setting = map->GetContentSetting(
+      primary_url_, secondary_url_, content_type_, resource_identifier_);
+  if (final_content_setting != CONTENT_SETTING_ALLOW) {
     PermissionType permission_type;
-    if (PermissionUtil::GetPermissionType(content_type, &permission_type)) {
-      // TODO(stefanocs): Report revocations from page action as PAGE_ACTION
-      // source UI instead of SITE_SETTINGS source UI.
-      PermissionUmaUtil::PermissionRevoked(permission_type,
-                                           PermissionSourceUI::SITE_SETTINGS,
-                                           primary_url, profile);
+    if (PermissionUtil::GetPermissionType(content_type_, &permission_type)) {
+      PermissionUmaUtil::PermissionRevoked(permission_type, source_ui_,
+                                           primary_url_, profile_);
     }
   }
 }