Collecting statistics on iframe permissions use.

chrome/browser/content_settings/permission_context_uma_util.cc

 // Copyright 2014 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 "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/permission_context_uma_util.h"
+#include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/rappor/rappor_utils.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/common/origin_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);     \
@@ skipping 113 matching lines @@
         NOTREACHED() << "PERMISSION " << permission << " not accounted for";
     }
 
     const std::string& rappor_metric = GetRapporMetric(permission, action);
     if (!rappor_metric.empty())
       rappor::SampleDomainAndRegistryFromGURL(
           g_browser_process->rappor_service(), rappor_metric,
           requesting_origin);
 }
 
-void RecordPermissionRequest(ContentSettingsType permission,
-                             const GURL& requesting_origin) {
+std::string PermissionTypeToString(PermissionType permission_type) {
+  switch (permission_type) {
+    case PermissionType::MIDI_SYSEX:
+      return "MidiSysex";
+    case PermissionType::PUSH_MESSAGING:
+      return "PushMessaging";
+    case PermissionType::NOTIFICATIONS:
+      return "Notifications";
+    case PermissionType::GEOLOCATION:
+      return "Geolocation";
+    case PermissionType::PROTECTED_MEDIA_IDENTIFIER:
+      return "ProtectedMediaIdentifier";
+    case PermissionType::NUM:
+      NOTREACHED();
+  }
+  return std::string();

[mlamouri (slow - plz ping), 2015/06/30 10:27:20]

nit: could you add a NOTREACHED() here too? Sorry, I should have pointed that
before.

[keenanb, 2015/06/30 21:20:07]

might as well just move NOTREACHED.

+}
+
+void RecordPermissionRequest(
+    ContentSettingsType permission,
+    const GURL& requesting_origin,
+    const GURL& embedding_origin,
+    HostContentSettingsMap* host_content_settings_map) {
   bool secure_origin = content::IsOriginSecure(requesting_origin);
   PermissionType type;
   switch (permission) {
     case CONTENT_SETTINGS_TYPE_GEOLOCATION:
       type = PermissionType::GEOLOCATION;
       rappor::SampleDomainAndRegistryFromGURL(
           g_browser_process->rappor_service(),
           "ContentSettings.PermissionRequested.Geolocation.Url",
           requesting_origin);
       break;
@@ skipping 27 matching lines @@
     UMA_HISTOGRAM_ENUMERATION(
         "ContentSettings.PermissionRequested_SecureOrigin",
         static_cast<base::HistogramBase::Sample>(type),
         static_cast<base::HistogramBase::Sample>(PermissionType::NUM));
   } else {
     UMA_HISTOGRAM_ENUMERATION(
         "ContentSettings.PermissionRequested_InsecureOrigin",
         static_cast<base::HistogramBase::Sample>(type),
         static_cast<base::HistogramBase::Sample>(PermissionType::NUM));
   }
+
+  // We have dicided not to worry about it,
+  // but statistics will get skewed if, for example,
+  // before the user has made a permission decision,
+  // getCurrentPosition gets called ten times each second
+  // and generates ten permission requests each second.
+  // That may be a job for RAPPOR.

[mlamouri (slow - plz ping), 2015/06/30 10:27:20]

nit: the wrapping of that comment block seems off. It should wrap at 80
characters.

Also, I think I would leave a comment like this instead:
"""
In order to improve the security models for iframes around permissions,
we need to know the ratio between same origin and cross origin iframes
requesting permissions and the permission state of their embedder when
cross origin.
This ration might be biased if an iframe makes multiple requests that
end up being saved but it was considered as statically insignificant.
"""

[keenanb, 2015/06/30 21:20:08]

Done.

+  if (requesting_origin.GetOrigin() != embedding_origin.GetOrigin()) {
+    ContentSetting embedding_content_setting =
+        host_content_settings_map->GetContentSetting(
+            embedding_origin, embedding_origin, permission, std::string());

[mlamouri (slow - plz ping), 2015/06/30 10:27:20]

Could you get the profile instead of HostContentSettingsMap and call
GetPermissionStatus()? Then, you print the PermissionStatus.

[keenanb, 2015/06/30 21:20:08]

Profile does not expose GetPermissionStatus; it's PermissionContext that does. i
chose not to pass either of those in order to avoid circular dependency.

let me know what you think.

[mlamouri (slow - plz ping), 2015/07/01 11:23:05]

You can get the PermissionManager from a profile
(Profile::GetPermissionManager()).

+    UMA_HISTOGRAM_ENUMERATION("ContentSettings.PermissionRequested.OffOrigin_" +
+                                  PermissionTypeToString(type),
+                              embedding_content_setting,
+                              CONTENT_SETTING_NUM_SETTINGS);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION(
+        "ContentSettings.PermissionRequested.SameOrigin",
+        static_cast<base::HistogramBase::Sample>(type),
+        static_cast<base::HistogramBase::Sample>(PermissionType::NUM));
+  }
 }
 
 } // namespace
 
 // Make sure you update histograms.xml permission histogram_suffix if you
 // add new permission
 void PermissionContextUmaUtil::PermissionRequested(
-    ContentSettingsType permission, const GURL& requesting_origin) {
-  RecordPermissionRequest(permission, requesting_origin);
+    ContentSettingsType permission,
+    const GURL& requesting_origin,
+    const GURL& embedding_origin,
+    HostContentSettingsMap* host_content_settings_map) {
+  RecordPermissionRequest(permission, requesting_origin, embedding_origin,
+                          host_content_settings_map);
 }
 
 void PermissionContextUmaUtil::PermissionGranted(
     ContentSettingsType permission, const GURL& requesting_origin) {
   RecordPermissionAction(permission, GRANTED, requesting_origin);
 }
 
 void PermissionContextUmaUtil::PermissionDenied(
     ContentSettingsType permission, const GURL& requesting_origin) {
   RecordPermissionAction(permission, DENIED, requesting_origin);
 }
 
 void PermissionContextUmaUtil::PermissionDismissed(
     ContentSettingsType permission, const GURL& requesting_origin) {
   RecordPermissionAction(permission, DISMISSED, requesting_origin);
 }
 
 void PermissionContextUmaUtil::PermissionIgnored(
     ContentSettingsType permission, const GURL& requesting_origin) {
   RecordPermissionAction(permission, IGNORED, requesting_origin);
 }