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),
+      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;
+  }
   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);
+}
 
-  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()),

[tsergeant, 2016/07/21 01:32:52]

Is this version of the constructor currently used? It seems useful, but I'd be
worried about just converting the pattern to a string. What happens if the
pattern has a wildcard in it?

[stefanocs, 2016/07/21 01:45:46]

Not in this cl, but we are planning to use this for recording revocation in
content settings (I'm sure if this constructor should be added on that cl
instead). The revocation should only be recorded if the pattern string itself is
a valid url (checked in the first constructor).

[tsergeant, 2016/07/21 07:10:56]

Alright, sounds good to me.

+          GURL((secondary_pattern == ContentSettingsPattern::Wildcard())
+                   ? primary_pattern.ToString()
+                   : secondary_pattern.ToString()),
+          content_type,
+          resource_identifier,
+          source_ui) {}
 
-  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_);
     }
   }
 }