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"
 #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,
+                                ContentSetting setting) {
+  ContentSettingsTypeHistogram histogram_value =
+      ContentSettingTypeToHistogramValue(type);
+  DCHECK_NE(histogram_value, CONTENT_SETTINGS_TYPE_HISTOGRAM_INVALID)
+      << "Invalid content setting type specified.";
+  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 ||
+             setting == ContentSetting::CONTENT_SETTING_ASK) {
+    UMA_HISTOGRAM_ENUMERATION("WebsiteSettings.Menu.PermissionChanged.Ask",

[Finnur, 2015/02/23 14:15:27]

'Ask' is, as I recall, not something we can set for individual origins (it is
only available for global defaults), so I'm surprised to see CONTENT_SETTING_ASK
handled here. In what cases do you need to handle 'Ask' (for origins
specifically)?

I would like to think of this in terms of Allowed/Block/Reset to default, which
better reflects behind the scenes what happens (and is more future-proof, if we
change the global default for a category to be something other than 'Ask').

Therefore, I'd remove line 58 and change the UMA histogram name to be
WebsiteSettings.Menu.PermissionChanged.Reset, or something to that effect.

[Miguel Garcia, 2015/02/23 14:35:21]

Done.

+                              histogram_value,
+                              CONTENT_SETTINGS_HISTOGRAM_NUM_TYPES);
+  } else {
+    NOTREACHED();
+  }
+}
+}  // 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 63 matching lines @@
     jstring origin, jstring embedder, jint value) {
   GURL url(ConvertJavaStringToUTF8(env, origin));
   ContentSettingsPattern primary_pattern(
       ContentSettingsPattern::FromURLNoWildcard(url));
   ContentSettingsPattern secondary_pattern(ContentSettingsPattern::Wildcard());
   if (value == -1) {
     GetCookieSettings()->ResetCookieSetting(primary_pattern, secondary_pattern);
   } else {
     GetCookieSettings()->SetCookieSetting(primary_pattern, secondary_pattern,
         value ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK);
   }

[Finnur, 2015/02/23 14:15:27]

What about adding... 
   ContentSetting setting = CONTENT_SETTING_DEFAULT;
... at line 360. Then change this else to:

} else {
   setting = value ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
   GetCookieSettings()->SetCookieSetting(primary_pattern, setting); 
}

LogPermissionChange(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, setting);

[Miguel Garcia, 2015/02/23 14:35:21]

Good catch thanks.

On 2015/02/23 14:15:27, Finnur wrote:

 }
 
 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);
 }