Add metrics for user manually added exceptions

components/content_settings/core/browser/content_settings_pref.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 "components/content_settings/core/browser/content_settings_pref.h"
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_split.h"
 #include "base/time/clock.h"
 #include "components/content_settings/core/browser/content_settings_info.h"
 #include "components/content_settings/core/browser/content_settings_registry.h"
 #include "components/content_settings/core/browser/content_settings_rule.h"
 #include "components/content_settings/core/browser/content_settings_utils.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
 #include "components/content_settings/core/common/pref_names.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "url/gurl.h"
 
 namespace {
 
 const char kSettingPath[] = "setting";
 const char kPerResourceIdentifierPrefName[] = "per_resource";
 const char kLastUsed[] = "last_used";
 
+struct ExceptionUmaStats {
+  ContentSettingsType settings_type;
+  const char* allow_exception_metrics_name;
+  size_t allow_exception_count;
+  const char* block_exception_metrics_name;
+  size_t block_exception_count;
+  const char* ask_exception_metrics_name;
+  size_t ask_exception_count;
+  const char* important_content_exception_metrics_name;
+  size_t important_content_exception_count;
+  const char* session_only_exception_metrics_name;
+  size_t session_only_exception_count;
+};
+
+ExceptionUmaStats ContentSettingExceptionUmaMap[] = {
+    {
+        CONTENT_SETTINGS_TYPE_COOKIES,
+        "ContentSettings.NumberOfAllowCookiesExceptions", 0,
+        "ContentSettings.NumberOfBlockCookiesExceptions", 0, "", 0, "", 0,
+        "ContentSettings.NumberOfSessionOnlyCookiesExceptions", 0,
+    },
+    {
+        CONTENT_SETTINGS_TYPE_IMAGES,
+        "ContentSettings.NumberOfAllowImagesExceptions", 0,
+        "ContentSettings.NumberOfBlockImagesExceptions", 0, "", 0, "", 0, "", 0,
+    },
+    {
+        CONTENT_SETTINGS_TYPE_JAVASCRIPT,
+        "ContentSettings.NumberOfAllowJavaScriptExceptions", 0,
+        "ContentSettings.NumberOfBlockJavaScriptExceptions", 0, "", 0, "", 0,
+        "", 0,
+    },
+    {
+        CONTENT_SETTINGS_TYPE_PLUGINS,
+        "ContentSettings.NumberOfAllowPluginsExceptions", 0,
+        "ContentSettings.NumberOfBlockPluginsExceptions", 0,
+        "ContentSettings.NumberOfAskPluginsExceptions", 0,
+        "ContentSettings.NumberOfDetectImportantContentPluginsExceptions", 0,
+        "", 0,
+    },
+    {
+        CONTENT_SETTINGS_TYPE_POPUPS,
+        "ContentSettings.NumberOfAllowPopupsExceptions", 0,
+        "ContentSettings.NumberOfBlockPopupsExceptions", 0, "", 0, "", 0, "", 0,
+    },
+    {
+        CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
+        "ContentSettings.NumberOfAllowAutomaticDownloadsExceptions", 0,
+        "ContentSettings.NumberOfBlockAutomaticDownloadsExceptions", 0,
+        "ContentSettings.NumberOfAskAutomaticDownloadsExceptions", 0, "", 0, "",
+        0,
+    }};
+
 // If the given content type supports resource identifiers in user preferences,
 // returns true and sets |pref_key| to the key in the content settings
 // dictionary under which per-resource content settings are stored.
 // Otherwise, returns false.
 bool SupportsResourceIdentifiers(ContentSettingsType content_type) {
   return content_type == CONTENT_SETTINGS_TYPE_PLUGINS;
 }
 
 bool IsValueAllowedForType(const base::Value* value, ContentSettingsType type) {
   const content_settings::ContentSettingsInfo* info =
@@ skipping 233 matching lines @@
   if (!is_incognito_) {
     mutable_settings = update.Get();
   } else {
     // Create copy as we do not want to persist anything in OTR prefs.
     mutable_settings = all_settings_dictionary->DeepCopy();
     mutable_settings_scope.reset(mutable_settings);
   }
   // Convert all Unicode patterns into punycode form, then read.
   CanonicalizeContentSettingsExceptions(mutable_settings);
 
-  size_t cookies_block_exception_count = 0;
-  size_t cookies_allow_exception_count = 0;
-  size_t cookies_session_only_exception_count = 0;
   for (base::DictionaryValue::Iterator i(*mutable_settings); !i.IsAtEnd();
        i.Advance()) {
     const std::string& pattern_str(i.key());
     std::pair<ContentSettingsPattern, ContentSettingsPattern> pattern_pair =
         ParsePatternString(pattern_str);
     if (!pattern_pair.first.IsValid() ||
         !pattern_pair.second.IsValid()) {
       // TODO: Change this to DFATAL when crbug.com/132659 is fixed.
       LOG(ERROR) << "Invalid pattern strings: " << pattern_str;
       continue;
@@ skipping 31 matching lines @@
     const base::Value* value = nullptr;
     settings_dictionary->GetWithoutPathExpansion(kSettingPath, &value);
 
     if (value) {
       DCHECK(IsValueAllowedForType(value, content_type_));
       value_map_.SetValue(pattern_pair.first,
                           pattern_pair.second,
                           content_type_,
                           ResourceIdentifier(),
                           value->DeepCopy());
-      if (content_type_ == CONTENT_SETTINGS_TYPE_COOKIES) {
-        ContentSetting s = ValueToContentSetting(value);
-        switch (s) {
-          case CONTENT_SETTING_ALLOW :
-            ++cookies_allow_exception_count;
-            break;
-          case CONTENT_SETTING_BLOCK :
-            ++cookies_block_exception_count;
-            break;
-          case CONTENT_SETTING_SESSION_ONLY :
-            ++cookies_session_only_exception_count;
-            break;
-          default:
-            NOTREACHED();
-            break;
+
+      for (ExceptionUmaStats stats : ContentSettingExceptionUmaMap) {

[raymes, 2016/04/07 04:57:59]

I think that perhaps we can just measure all types in ContentSettingsRegistry.
We can generate the histogram name based on the name of the content setting.

[raymes, 2016/04/07 04:57:59]

Can we move this code higher up the stack? Perhaps into the
HostContentSettingsMap constructor?

[lshang, 2016/04/11 06:31:07]

Done.

[lshang, 2016/04/11 06:31:07]

Done.

+        if (stats.settings_type == content_type_) {
+          ContentSetting s = ValueToContentSetting(value);
+          switch (s) {
+            case CONTENT_SETTING_ALLOW:
+              ++stats.allow_exception_count;
+              break;
+            case CONTENT_SETTING_BLOCK:
+              ++stats.block_exception_count;

[raymes, 2016/04/07 04:57:59]

I think we should probably only worry about allow/block (or possibly even just
measure the total number as a starting point).

[lshang, 2016/04/11 06:31:07]

Done.

+              break;
+            case CONTENT_SETTING_ASK:
+              ++stats.ask_exception_count;
+              break;
+            case CONTENT_SETTING_DETECT_IMPORTANT_CONTENT:
+              ++stats.important_content_exception_count;
+              break;
+            case CONTENT_SETTING_SESSION_ONLY:
+              ++stats.session_only_exception_count;
+              break;
+            default:
+              NOTREACHED();
+              break;
+          }
         }
       }
     }
 
   }
 
-  if (content_type_ == CONTENT_SETTINGS_TYPE_COOKIES) {
-    UMA_HISTOGRAM_COUNTS("ContentSettings.NumberOfBlockCookiesExceptions",
-                         cookies_block_exception_count);
-    UMA_HISTOGRAM_COUNTS("ContentSettings.NumberOfAllowCookiesExceptions",
-                         cookies_allow_exception_count);
-    UMA_HISTOGRAM_COUNTS("ContentSettings.NumberOfSessionOnlyCookiesExceptions",
-                         cookies_session_only_exception_count);
+  for (ExceptionUmaStats stats : ContentSettingExceptionUmaMap) {
+    if (stats.settings_type == content_type_) {
+      UMA_HISTOGRAM_COUNTS(stats.allow_exception_metrics_name,
+                           stats.allow_exception_count);
+      UMA_HISTOGRAM_COUNTS(stats.block_exception_metrics_name,
+                           stats.block_exception_count);
+      if (content_type_ == CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS) {
+        UMA_HISTOGRAM_COUNTS(stats.ask_exception_metrics_name,
+                             stats.ask_exception_count);
+      } else if (content_type_ == CONTENT_SETTINGS_TYPE_PLUGINS) {
+        UMA_HISTOGRAM_COUNTS(stats.ask_exception_metrics_name,
+                             stats.ask_exception_count);
+        UMA_HISTOGRAM_COUNTS(stats.important_content_exception_metrics_name,
+                             stats.important_content_exception_count);
+      } else if (content_type_ == CONTENT_SETTINGS_TYPE_COOKIES) {
+        UMA_HISTOGRAM_COUNTS(stats.session_only_exception_metrics_name,
+                             stats.session_only_exception_count);
+      }
+    }
   }
 }
 
 void ContentSettingsPref::OnPrefChanged() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (updating_preferences_)
     return;
 
   ReadContentSettingsFromPref();
@@ skipping 133 matching lines @@
 
 void ContentSettingsPref::AssertLockNotHeld() const {
 #if !defined(NDEBUG)
   // |Lock::Acquire()| will assert if the lock is held by this thread.
   lock_.Acquire();
   lock_.Release();
 #endif
 }
 
 }  // namespace content_settings