Add UMA to track permission changes from the content settings menu

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

 // Copyright (c) 2012 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/ui/webui/options/content_settings_handler.h"
 
 #include <algorithm>
 #include <map>
 #include <vector>
 
 #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"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry_factory.h"
 #include "chrome/browser/extensions/extension_special_storage_policy.h"
 #include "chrome/browser/notifications/desktop_notification_profile_util.h"
@@ skipping 1387 matching lines @@
   if (type_string == kZoomContentType) {
     RemoveZoomLevelException(args);
     return;
   }
 
   ContentSettingsType type = ContentSettingsTypeFromGroupName(type_string);
   if (type == CONTENT_SETTINGS_TYPE_MEDIASTREAM)
     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) {
   std::string type_string;
   CHECK(args->GetString(0, &type_string));
   std::string mode;
   CHECK(args->GetString(1, &mode));
   std::string pattern;
   CHECK(args->GetString(2, &pattern));
   std::string setting;
@@ skipping 164 matching lines @@
 
   // Exceptions apply only when the feature is enabled.
   PrefService* prefs = user_prefs::UserPrefs::Get(GetBrowserContext(web_ui()));
   bool enable_exceptions = prefs->GetBoolean(prefs::kEnableDRM);
   web_ui()->CallJavascriptFunction(
       "ContentSettings.enableProtectedContentExceptions",
       base::FundamentalValue(enable_exceptions));
 }
 
 }  // namespace options