Upgrade the generic permission UMA

Records actions per origin type (secure or insecure)

BUG=419731

Review URL: https://codereview.chromium.org/630793003

Cr-Commit-Position: refs/heads/master@{#298894}

6 files changed, 101 insertions, 41 deletions

diff --git a/chrome/browser/content_settings/permission_bubble_request_impl.cc b/chrome/browser/content_settings/permission_bubble_request_impl.cc
index db10bf7..5359d65 100644
--- a/chrome/browser/content_settings/permission_bubble_request_impl.cc
+++ b/chrome/browser/content_settings/permission_bubble_request_impl.cc
@@ -31,7 +31,7 @@ PermissionBubbleRequestImpl::PermissionBubbleRequestImpl(
 PermissionBubbleRequestImpl::~PermissionBubbleRequestImpl() {
   DCHECK(is_finished_);
   if (!action_taken_)
-    PermissionContextUmaUtil::PermissionIgnored(type_);
+    PermissionContextUmaUtil::PermissionIgnored(type_, request_origin_);
 }
 
 int PermissionBubbleRequestImpl::GetIconID() const {
diff --git a/chrome/browser/content_settings/permission_context_base.cc b/chrome/browser/content_settings/permission_context_base.cc
index e82419d..405f45e 100644
--- a/chrome/browser/content_settings/permission_context_base.cc
+++ b/chrome/browser/content_settings/permission_context_base.cc
@@ -93,7 +93,8 @@ void PermissionContextBase::DecidePermission(
       break;
   }
 
-  PermissionContextUmaUtil::PermissionRequested(permission_type_);
+  PermissionContextUmaUtil::PermissionRequested(
+      permission_type_, requesting_origin);
 
   if (PermissionBubbleManager::Enabled()) {
     PermissionBubbleManager* bubble_manager =
@@ -152,11 +153,14 @@ void PermissionContextBase::PermissionDecided(
   if (PermissionBubbleManager::Enabled()) {
     if (persist) {
       if (allowed)
-        PermissionContextUmaUtil::PermissionGranted(permission_type_);
+        PermissionContextUmaUtil::PermissionGranted(permission_type_,
+                                                    requesting_origin);
       else
-        PermissionContextUmaUtil::PermissionDenied(permission_type_);
+        PermissionContextUmaUtil::PermissionDenied(permission_type_,
+                                                   requesting_origin);
     } else {
-      PermissionContextUmaUtil::PermissionDismissed(permission_type_);
+      PermissionContextUmaUtil::PermissionDismissed(permission_type_,
+                                                    requesting_origin);
     }
   }
 
diff --git a/chrome/browser/content_settings/permission_context_uma_util.cc b/chrome/browser/content_settings/permission_context_uma_util.cc
index b76d541..d288367 100644
--- a/chrome/browser/content_settings/permission_context_uma_util.cc
+++ b/chrome/browser/content_settings/permission_context_uma_util.cc
@@ -4,6 +4,27 @@
 
 #include "base/metrics/histogram.h"
 #include "chrome/browser/content_settings/permission_context_uma_util.h"
+#include "url/gurl.h"
+
+
+// UMA keys need to be statically initialized so plain function would not
+// work. Use a Macro instead.
+#define PERMISSION_ACTION_UMA(secure_origin,                                   \
+            permission, permission_secure, permission_insecure, action)        \
+    UMA_HISTOGRAM_ENUMERATION(                                                 \
+        permission,                                                            \
+        action,                                                                \
+        PERMISSION_ACTION_NUM);                                                \
+    if (secure_origin) {                                                       \
+        UMA_HISTOGRAM_ENUMERATION(permission_secure,                           \
+                                  action,                                      \
+                                  PERMISSION_ACTION_NUM);                      \
+    } else {                                                                   \
+        UMA_HISTOGRAM_ENUMERATION(permission_insecure,                         \
+                                  action,                                      \
+                                  PERMISSION_ACTION_NUM);                      \
+    }
+
 
 namespace {
 
@@ -35,30 +56,41 @@ enum PermissionType {
 };
 
 void RecordPermissionAction(
-      ContentSettingsType permission, PermissionAction action) {
+      ContentSettingsType permission,
+      PermissionAction action,
+      bool secure_origin) {
   switch (permission) {
       case CONTENT_SETTINGS_TYPE_GEOLOCATION:
-        UMA_HISTOGRAM_ENUMERATION(
-                   "ContentSettings.PermissionActions_Geolocation",
-                   action,
-                   PERMISSION_ACTION_NUM);
+        PERMISSION_ACTION_UMA(
+            secure_origin,
+            "ContentSettings.PermissionActions_Geolocation",
+            "ContentSettings.PermissionActionsSecureOrigin_Geolocation",
+            "ContentSettings.PermissionActionsInsecureOrigin_Geolocation",
+            action);
         break;
       case CONTENT_SETTINGS_TYPE_NOTIFICATIONS:
-        UMA_HISTOGRAM_ENUMERATION(
+        PERMISSION_ACTION_UMA(
+            secure_origin,
             "ContentSettings.PermissionActions_Notifications",
-            action,
-            PERMISSION_ACTION_NUM);
+            "ContentSettings.PermissionActionsSecureOrigin_Notifications",
+            "ContentSettings.PermissionActionsInsecureOrigin_Notifications",
+            action);
         break;
       case CONTENT_SETTINGS_TYPE_MIDI_SYSEX:
-        UMA_HISTOGRAM_ENUMERATION("ContentSettings.PermissionActions_MidiSysEx",
-                                  action,
-                                  PERMISSION_ACTION_NUM);
+        PERMISSION_ACTION_UMA(
+            secure_origin,
+            "ContentSettings.PermissionActions_MidiSysEx",
+            "ContentSettings.PermissionActionsSecureOrigin_MidiSysEx",
+            "ContentSettings.PermissionActionsInsecureOrigin_MidiSysEx",
+            action);
         break;
       case CONTENT_SETTINGS_TYPE_PUSH_MESSAGING:
-        UMA_HISTOGRAM_ENUMERATION(
+        PERMISSION_ACTION_UMA(
+            secure_origin,
             "ContentSettings.PermissionActions_PushMessaging",
-            action,
-            PERMISSION_ACTION_NUM);
+            "ContentSettings.PermissionActionsSecureOrigin_PushMessaging",
+            "ContentSettings.PermissionActionsInsecureOrigin_PushMessaging",
+            action);
         break;
 #if defined(OS_ANDROID)
       case CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER:
@@ -72,7 +104,7 @@ void RecordPermissionAction(
 }
 
 void RecordPermissionRequest(
-    ContentSettingsType permission) {
+    ContentSettingsType permission, bool secure_origin) {
   PermissionType type;
   switch (permission) {
     case CONTENT_SETTINGS_TYPE_GEOLOCATION:
@@ -91,8 +123,21 @@ void RecordPermissionRequest(
       NOTREACHED() << "PERMISSION " << permission << " not accounted for";
       type = PERMISSION_UNKNOWN;
   }
-  UMA_HISTOGRAM_ENUMERATION("ContentSettings.PermissionRequested", type,
-                            PERMISSION_NUM);
+  UMA_HISTOGRAM_ENUMERATION(
+      "ContentSettings.PermissionRequested",
+      type,
+      PERMISSION_NUM);
+  if (secure_origin) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "ContentSettings.PermissionRequested_SecureOrigin",
+        type,
+        PERMISSION_NUM);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION(
+        "ContentSettings.PermissionRequested_InsecureOrigin",
+        type,
+        PERMISSION_NUM);
+  }
 }
 
 } // namespace
@@ -100,26 +145,30 @@ void RecordPermissionRequest(
 // Make sure you update histograms.xml permission histogram_suffix if you
 // add new permission
 void PermissionContextUmaUtil::PermissionRequested(
-    ContentSettingsType permission) {
-  RecordPermissionRequest(permission);
+    ContentSettingsType permission, const GURL& requesting_origin) {
+  RecordPermissionRequest(permission, requesting_origin.SchemeIsSecure());
 }
 
 void PermissionContextUmaUtil::PermissionGranted(
-    ContentSettingsType permission) {
-  RecordPermissionAction(permission, GRANTED);
+    ContentSettingsType permission, const GURL& requesting_origin) {
+  RecordPermissionAction(permission, GRANTED,
+                         requesting_origin.SchemeIsSecure());
 }
 
 void PermissionContextUmaUtil::PermissionDenied(
-    ContentSettingsType permission) {
-  RecordPermissionAction(permission, DENIED);
+    ContentSettingsType permission, const GURL& requesting_origin) {
+  RecordPermissionAction(permission, DENIED,
+                         requesting_origin.SchemeIsSecure());
 }
 
 void PermissionContextUmaUtil::PermissionDismissed(
-    ContentSettingsType permission) {
-  RecordPermissionAction(permission, DISMISSED);
+    ContentSettingsType permission, const GURL& requesting_origin) {
+  RecordPermissionAction(permission, DISMISSED,
+                         requesting_origin.SchemeIsSecure());
 }
 
 void PermissionContextUmaUtil::PermissionIgnored(
-    ContentSettingsType permission) {
-  RecordPermissionAction(permission, IGNORED);
+    ContentSettingsType permission, const GURL& requesting_origin) {
+  RecordPermissionAction(permission, IGNORED,
+                         requesting_origin.SchemeIsSecure());
 }
diff --git a/chrome/browser/content_settings/permission_context_uma_util.h b/chrome/browser/content_settings/permission_context_uma_util.h
index d364efb..8d769ae 100644
--- a/chrome/browser/content_settings/permission_context_uma_util.h
+++ b/chrome/browser/content_settings/permission_context_uma_util.h
@@ -8,15 +8,22 @@
 #include "base/logging.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 
+class GURL;
+
 // Provides a convenient way of logging UMA for permission related
 // operations.
 class PermissionContextUmaUtil {
  public:
-  static void PermissionGranted(ContentSettingsType permission);
-  static void PermissionDenied(ContentSettingsType permission);
-  static void PermissionDismissed(ContentSettingsType permission);
-  static void PermissionIgnored(ContentSettingsType permission);
-  static void PermissionRequested(ContentSettingsType permission);
+  static void PermissionGranted(ContentSettingsType permission,
+                                const GURL& requesting_origin);
+  static void PermissionDenied(ContentSettingsType permission,
+                               const GURL& requesting_origin);
+  static void PermissionDismissed(ContentSettingsType permission,
+                                  const GURL& requesting_origin);
+  static void PermissionRequested(ContentSettingsType permission,
+                                  const GURL& requesting_origin);
+  static void PermissionIgnored(ContentSettingsType permission,
+                                const GURL& requesting_origin);
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(PermissionContextUmaUtil);
diff --git a/chrome/browser/content_settings/permission_infobar_delegate.cc b/chrome/browser/content_settings/permission_infobar_delegate.cc
index 7f41598..e70f879 100644
--- a/chrome/browser/content_settings/permission_infobar_delegate.cc
+++ b/chrome/browser/content_settings/permission_infobar_delegate.cc
@@ -12,7 +12,7 @@
 
 PermissionInfobarDelegate::~PermissionInfobarDelegate() {
   if (!action_taken_)
-    PermissionContextUmaUtil::PermissionIgnored(type_);
+    PermissionContextUmaUtil::PermissionIgnored(type_, requesting_origin_);
 }
 
 PermissionInfobarDelegate::PermissionInfobarDelegate(
diff --git a/chrome/browser/content_settings/permission_queue_controller.cc b/chrome/browser/content_settings/permission_queue_controller.cc
index c97b625..59e2f7e3 100644
--- a/chrome/browser/content_settings/permission_queue_controller.cc
+++ b/chrome/browser/content_settings/permission_queue_controller.cc
@@ -200,11 +200,11 @@ void PermissionQueueController::OnPermissionSet(
   if (update_content_setting) {
     UpdateContentSetting(requesting_frame, embedder, allowed);
     if (allowed)
-      PermissionContextUmaUtil::PermissionGranted(type_);
+      PermissionContextUmaUtil::PermissionGranted(type_, requesting_frame);
     else
-      PermissionContextUmaUtil::PermissionDenied(type_);
+      PermissionContextUmaUtil::PermissionDenied(type_, requesting_frame);
   } else {
-    PermissionContextUmaUtil::PermissionDismissed(type_);
+    PermissionContextUmaUtil::PermissionDismissed(type_, requesting_frame);
   }
 
   // Cancel this request first, then notify listeners.  TODO(pkasting): Why