Add UMA to track permission changes from the content settings menu

chrome/browser/android/preferences/website_preference_bridge.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/android/preferences/website_preference_bridge.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
-#include "base/strings/string_util.h"
+#include "base/metrics/histogram.h"

[Ilya Sherman, 2015/02/23 22:55:20]

nit: Please include histogram_macros instead.

[Miguel Garcia, 2015/02/24 13:25:24]

Done.

 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/browsing_data_local_storage_helper.h"
 #include "chrome/browser/browsing_data/cookies_tree_model.h"
 #include "chrome/browser/browsing_data/local_data_container.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/notifications/desktop_notification_profile_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
 #include "jni/WebsitePreferenceBridge_jni.h"
 #include "storage/browser/quota/quota_client.h"
 #include "storage/browser/quota/quota_manager.h"
 #include "url/url_constants.h"
 
 using base::android::ConvertJavaStringToUTF8;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::JavaRef;
 using base::android::ScopedJavaGlobalRef;
 using base::android::ScopedJavaLocalRef;
 using content::BrowserThread;
 
+namespace {
+
+static void LogPermissionChange(ContentSettingsType type,

[Ilya Sherman, 2015/02/23 22:55:20]

nit: No need for "static" in an anonymous namespace.

[Miguel Garcia, 2015/02/24 13:25:24]

Done.

+                                ContentSetting setting) {
+  ContentSettingsTypeHistogram histogram_value =
+      ContentSettingTypeToHistogramValue(type);
+  DCHECK_NE(histogram_value, CONTENT_SETTINGS_TYPE_HISTOGRAM_INVALID)
+      << "Invalid content setting type specified.";

[Ilya Sherman, 2015/02/23 22:55:20]

nit: Please omit this string.  It adds weight to the binary, without adding
developer benefit beyond what a comment can provide.

[Miguel Garcia, 2015/02/24 13:25:24]

Done.

+  UMA_HISTOGRAM_ENUMERATION("WebsiteSettings.Menu.PermissionChanged",
+                            histogram_value,
+                            CONTENT_SETTINGS_HISTOGRAM_NUM_TYPES);
+
+  if (setting == ContentSetting::CONTENT_SETTING_ALLOW) {
+    UMA_HISTOGRAM_ENUMERATION("WebsiteSettings.Menu.PermissionChanged.Allowed",
+                              histogram_value,
+                              CONTENT_SETTINGS_HISTOGRAM_NUM_TYPES);
+  } else if (setting == ContentSetting::CONTENT_SETTING_BLOCK) {
+    UMA_HISTOGRAM_ENUMERATION("WebsiteSettings.Menu.PermissionChanged.Blocked",
+                              histogram_value,
+                              CONTENT_SETTINGS_HISTOGRAM_NUM_TYPES);
+  } else if (setting == ContentSetting::CONTENT_SETTING_DEFAULT) {
+    UMA_HISTOGRAM_ENUMERATION("WebsiteSettings.Menu.PermissionChanged.Reset",
+                              histogram_value,
+                              CONTENT_SETTINGS_HISTOGRAM_NUM_TYPES);
+  } else {
+    NOTREACHED() << "Requested to log permission change " << type << " to "
+                 << setting;
+  }
+}
+}  // namespace
+
 static HostContentSettingsMap* GetHostContentSettingsMap() {
   Profile* profile = ProfileManager::GetActiveUserProfile();
   return profile->GetHostContentSettingsMap();
 }
 
 static void GetOrigins(JNIEnv* env,
                        ContentSettingsType content_type,
                        jobject list,
                        jboolean managedOnly) {
   ContentSettingsForOneType all_settings;
@@ skipping 98 matching lines @@
     case 2: setting = CONTENT_SETTING_BLOCK; break;
     default:
       NOTREACHED();
   }
   GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(url),
       secondary_pattern,
       content_type,
       std::string(),
       setting);
+  LogPermissionChange(content_type, setting);
 }
 
 static void GetGeolocationOrigins(JNIEnv* env,
                                   jclass clazz,
                                   jobject list,
                                   jboolean managedOnly) {
   GetOrigins(env, CONTENT_SETTINGS_TYPE_GEOLOCATION, list, managedOnly);
 }
 
 static jint GetGeolocationSettingForOrigin(JNIEnv* env, jclass clazz,
@@ skipping 57 matching lines @@
       ProfileManager::GetActiveUserProfile(),
       GURL(ConvertJavaStringToUTF8(env, origin)));
 }
 
 static void SetPushNotificationSettingForOrigin(JNIEnv* env, jclass clazz,
     jstring origin, jstring embedder, jint value) {
   // TODO(peter): Web Notification permission behaves differently from all other
   // permission types. See https://crbug.com/416894.
   Profile* profile = ProfileManager::GetActiveUserProfile();
   GURL url = GURL(ConvertJavaStringToUTF8(env, origin));
-
+  ContentSetting setting = CONTENT_SETTING_DEFAULT;
   switch (value) {
     case -1:
       DesktopNotificationProfileUtil::ClearSetting(
           profile, ContentSettingsPattern::FromURLNoWildcard(url));
       break;
     case 1:
       DesktopNotificationProfileUtil::GrantPermission(profile, url);
+      setting = CONTENT_SETTING_ALLOW;
       break;
     case 2:
       DesktopNotificationProfileUtil::DenyPermission(profile, url);
+      setting = CONTENT_SETTING_BLOCK;
       break;
     default:
       NOTREACHED();
   }
+  LogPermissionChange(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, setting);
 }
 
 static void GetVoiceAndVideoCaptureOrigins(JNIEnv* env,
                                            jclass clazz,
                                            jobject list,
                                            jboolean managedOnly) {
   GetOrigins(env, CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC, list, managedOnly);
   GetOrigins(env, CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA, list, managedOnly);
 }
 
@@ skipping 58 matching lines @@
   return GetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_COOKIES, origin,
       embedder);
 }
 
 static void SetCookieSettingForOrigin(JNIEnv* env, jclass clazz,
     jstring origin, jstring embedder, jint value) {
   GURL url(ConvertJavaStringToUTF8(env, origin));
   ContentSettingsPattern primary_pattern(
       ContentSettingsPattern::FromURLNoWildcard(url));
   ContentSettingsPattern secondary_pattern(ContentSettingsPattern::Wildcard());
+  ContentSetting setting = CONTENT_SETTING_DEFAULT;
   if (value == -1) {
     GetCookieSettings()->ResetCookieSetting(primary_pattern, secondary_pattern);
   } else {
+    setting = value ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
     GetCookieSettings()->SetCookieSetting(primary_pattern, secondary_pattern,
         value ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK);

[Finnur, 2015/02/23 14:47:19]

nit: Probably more readable to use |setting| here.

[Miguel Garcia, 2015/02/24 13:25:24]

Done.

   }
+  LogPermissionChange(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, setting);
 }
 
 namespace {
 
 class SiteDataDeleteHelper :
       public base::RefCountedThreadSafe<SiteDataDeleteHelper>,
       public CookiesTreeModel::Observer {
  public:
   SiteDataDeleteHelper(Profile* profile, const GURL& domain)
       : profile_(profile), domain_(domain), ending_batch_processing_(false) {
@@ skipping 305 matching lines @@
   GURL url(ConvertJavaStringToUTF8(env, jorigin));
   scoped_refptr<SiteDataDeleteHelper> site_data_deleter(
       new SiteDataDeleteHelper(profile, url));
   site_data_deleter->Run();
 }
 
 // Register native methods
 bool RegisterWebsitePreferenceBridge(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }