Add hooks to permission layer for permission action reporting

chrome/browser/permissions/permission_uma_util.cc

Index: chrome/browser/permissions/permission_uma_util.cc
diff --git a/chrome/browser/permissions/permission_uma_util.cc b/chrome/browser/permissions/permission_uma_util.cc
index efbdf5f04b6c65ff92efbb88a5fa6d95f2c44fb1..f2ebf36fe81a64f8e6501e91a8be6b4db0b4d948 100644
--- a/chrome/browser/permissions/permission_uma_util.cc
+++ b/chrome/browser/permissions/permission_uma_util.cc
@@ -6,13 +6,21 @@
 
 #include <utility>
 
+#include "base/command_line.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/permissions/permission_manager.h"
 #include "chrome/browser/permissions/permission_util.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/safe_browsing/ui_manager.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/ui/website_settings/permission_bubble_request.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
+#include "components/browser_sync/browser/profile_sync_service.h"
+#include "components/prefs/pref_service.h"
 #include "components/rappor/rappor_service.h"
 #include "components/rappor/rappor_utils.h"
 #include "content/public/browser/permission_type.h"
@@ -74,9 +82,50 @@ const std::string GetRapporMetric(PermissionType permission,
                             permission_str.c_str(), action_str.c_str());
 }
 
+bool isOptedInPermissionActionReporting(Profile* profile) {

[Nathan Parker, 2016/06/16 21:08:41]

Capital Is

[stefanocs, 2016/06/17 00:42:08]

Done.

+  // Do not report permission action when the permission action reporting
+  // flag is not enabled.
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnablePermissionActionReporting))
+    return false;
+  // Do not report if profile is null.
+  if (!profile)
+    return false;
+  // Do not report when SafeBrowsing is not enabled.
+  if (!profile->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled))

[stefanocs, 2016/06/16 01:58:40]

Hey Nathan, can you check if this is the correct way to check whether the safe
browsing is enabled?

[Nathan Parker, 2016/06/16 21:08:41]

yup

[stefanocs, 2016/06/17 00:42:08]

Acknowledged.

+    return false;
+  // Do not report if profile doesn't have profile sync service.
+  if (!ProfileSyncServiceFactory::HasProfileSyncService(profile))

[Nathan Parker, 2016/06/16 21:08:40]

You probably should check !incognito as well, ya?  e.g.

https://cs.chromium.org/chromium/src/chrome/browser/safe_browsing/safe_browsi...

[stefanocs, 2016/06/17 00:42:08]

Acknowledged.

+    return false;
+
+  ProfileSyncService* profile_sync_service =
+      ProfileSyncServiceFactory::GetForProfile(profile);
+  syncer::ModelTypeSet preferred_data_types =
+      profile_sync_service->GetPreferredDataTypes();
+
+  // Do not report if profile can't start sync immediately.
+  if (!profile_sync_service->CanSyncStart())
+    return false;
+  // Do not report if Chrome Tab Sync is not enabled.
+  if (!preferred_data_types.Has(syncer::PROXY_TABS))
+    return false;
+  // Do not report if Chrome Pref Sync is not enabled.
+  if (!preferred_data_types.Has(syncer::PREFERENCES))

[stefanocs, 2016/06/16 01:58:40]

Hey Pavel, can you check if this is the correct way to check if Chrome Tab Sync
and Chrome Pref Sync are enabled?

[pavely, 2016/06/16 18:59:24]

Yes, this is correct way.

[stefanocs, 2016/06/17 00:42:08]

Acknowledged.

+    return false;
+  return true;
+}
+
 void RecordPermissionAction(PermissionType permission,
                             PermissionAction action,
-                            const GURL& requesting_origin) {
+                            const GURL& requesting_origin,
+                            Profile* profile) {
+  // Send permission action report to opted-in users.
+  if (isOptedInPermissionActionReporting(profile)) {
+    g_browser_process->safe_browsing_service()
+        ->ui_manager()
+        ->ReportPermissionAction(requesting_origin, permission, action);
+  }
+
   bool secure_origin = content::IsOriginSecure(requesting_origin);
 
   switch (permission) {
@@ -280,34 +329,39 @@ void PermissionUmaUtil::PermissionRequested(PermissionType permission,
 }
 
 void PermissionUmaUtil::PermissionGranted(PermissionType permission,
-                                          const GURL& requesting_origin) {
-  RecordPermissionAction(permission, GRANTED, requesting_origin);
+                                          const GURL& requesting_origin,
+                                          Profile* profile) {
+  RecordPermissionAction(permission, GRANTED, requesting_origin, profile);
 }
 
 void PermissionUmaUtil::PermissionDenied(PermissionType permission,
-                                         const GURL& requesting_origin) {
-  RecordPermissionAction(permission, DENIED, requesting_origin);
+                                         const GURL& requesting_origin,
+                                         Profile* profile) {
+  RecordPermissionAction(permission, DENIED, requesting_origin, profile);
 }
 
 void PermissionUmaUtil::PermissionDismissed(PermissionType permission,
-                                            const GURL& requesting_origin) {
-  RecordPermissionAction(permission, DISMISSED, requesting_origin);
+                                            const GURL& requesting_origin,
+                                            Profile* profile) {
+  RecordPermissionAction(permission, DISMISSED, requesting_origin, profile);
 }
 
 void PermissionUmaUtil::PermissionIgnored(PermissionType permission,
-                                          const GURL& requesting_origin) {
-  RecordPermissionAction(permission, IGNORED, requesting_origin);
+                                          const GURL& requesting_origin,
+                                          Profile* profile) {
+  RecordPermissionAction(permission, IGNORED, requesting_origin, profile);
 }
 
 void PermissionUmaUtil::PermissionRevoked(PermissionType permission,
-                                          const GURL& revoked_origin) {
+                                          const GURL& revoked_origin,
+                                          Profile* profile) {
   // TODO(tsergeant): Expand metrics definitions for revocation to include all
   // permissions.
   if (permission == PermissionType::NOTIFICATIONS ||
       permission == PermissionType::GEOLOCATION ||
       permission == PermissionType::AUDIO_CAPTURE ||
       permission == PermissionType::VIDEO_CAPTURE) {
-    RecordPermissionAction(permission, REVOKED, revoked_origin);
+    RecordPermissionAction(permission, REVOKED, revoked_origin, profile);
   }
 }