Add UMA to track permission changes from the content settings menu

chrome/browser/android/preferences/website_preference_bridge.cc

Index: chrome/browser/android/preferences/website_preference_bridge.cc
diff --git a/chrome/browser/android/preferences/website_preference_bridge.cc b/chrome/browser/android/preferences/website_preference_bridge.cc
index 64bf3e8909d1c8c6b54a6a3d23345be521fb1c64..fd7a3371563c29b33ad752a24fb8815e7b9ecfab 100644
--- a/chrome/browser/android/preferences/website_preference_bridge.cc
+++ b/chrome/browser/android/preferences/website_preference_bridge.cc
@@ -10,7 +10,7 @@
 #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"
@@ -34,6 +34,37 @@ 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();
@@ -152,6 +183,7 @@ static void SetSettingForOrigin(JNIEnv* env,
       content_type,
       std::string(),
       setting);
+  LogPermissionChange(content_type, setting);
 }
 
 static void GetGeolocationOrigins(JNIEnv* env,
@@ -229,7 +261,7 @@ static void SetPushNotificationSettingForOrigin(JNIEnv* env, jclass clazz,
   // 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(
@@ -237,13 +269,16 @@ static void SetPushNotificationSettingForOrigin(JNIEnv* env, jclass clazz,
       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,
@@ -322,12 +357,15 @@ static void SetCookieSettingForOrigin(JNIEnv* env, jclass clazz,
   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 {