Add UMA to track permission changes from the content settings menu

chrome/browser/ui/webui/options/content_settings_handler.cc

Index: chrome/browser/ui/webui/options/content_settings_handler.cc
diff --git a/chrome/browser/ui/webui/options/content_settings_handler.cc b/chrome/browser/ui/webui/options/content_settings_handler.cc
index 5f9ff7c9f4bf888382e1a43d36c4dd577e0d1dec..a01d7d0671af4f8a46d3f894a7404a3d3349dbdc 100644
--- a/chrome/browser/ui/webui/options/content_settings_handler.cc
+++ b/chrome/browser/ui/webui/options/content_settings_handler.cc
@@ -11,6 +11,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
+#include "base/metrics/histogram.h"

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

Ditto

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

Done.

 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ -1418,6 +1419,19 @@ void ContentSettingsHandler::RemoveException(const base::ListValue* args) {
     RemoveMediaException(args);
   else
     RemoveExceptionFromHostContentSettingsMap(args, type);
+
+  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);
+
+  UMA_HISTOGRAM_ENUMERATION(
+      "WebsiteSettings.ContentSettings.PermissionChanged.Reset",
+      histogram_value, CONTENT_SETTINGS_HISTOGRAM_NUM_TYPES);

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

Please call LogPermissionChange() to emit to these histograms, rather than
duplicating (part of) the code here.

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

So the thing is that those two files are a world appart.

website_preference_bridge is android only, it does not even get compiled on
desktop chrome.

similarly content_settings_handler (this file) is desktop specific.


There is no single common ancestor to them, they both get events from the UX and
update content settings independently. This is obviously far from ideal but
refactoring all of it seems way outside the scope of this CL (especially
considering that I am still hoping it will get cherry picked). I am open to
suggestions if you think it can be done differently.

[Ilya Sherman, 2015/02/24 19:13:40]

I'm suggesting moving the helper method out into a content_settings_metrics file
that you could access from both locations.  Does that address your concerns?

 }
 
 void ContentSettingsHandler::SetException(const base::ListValue* args) {