Add UMA for <webview> APIs: a. ClearData, b. when Permission request is allowed.

content/browser/browser_plugin/browser_plugin_guest.cc

Index: content/browser/browser_plugin/browser_plugin_guest.cc
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 4c01545fab9b8916603142e224ed3dfe5d373dfb..7908d3b2562183dd4851e98210683ad56f0d182d 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -52,6 +52,17 @@
 #include "content/browser/browser_plugin/browser_plugin_popup_menu_helper_mac.h"
 #endif
 
+namespace {
+enum UmaType {
+  UMA_TYPE_DOWNLOAD,
+  UMA_TYPE_GEOLOCATION,
+  UMA_TYPE_MEDIA,
+  UMA_TYPE_POINTER_LOCK,
+  UMA_TYPE_NEW_WINDOW,
+  UMA_TYPE_JAVASCRIPT_DIALOG,
+};
+}  // namespace
+
 namespace content {
 
 // static
@@ -62,15 +73,89 @@ BrowserPluginHostFactory* BrowserPluginGuest::factory_ = NULL;
 class BrowserPluginGuest::PermissionRequest :
     public base::RefCounted<BrowserPluginGuest::PermissionRequest> {
  public:
-  virtual void Respond(bool should_allow, const std::string& user_input) = 0;
+  virtual void Respond(bool should_allow,
+                       const std::string& user_input,
+                       bool user_initiated) = 0;
   virtual bool AllowedByDefault() const {
       return false;
   }
+
  protected:
   PermissionRequest() {
     RecordAction(UserMetricsAction("BrowserPlugin.Guest.PermissionRequest"));
   }
   virtual ~PermissionRequest() {}
+
+  // Only record user initiated (i.e. non-default) actions.
+  void MaybeRecordUserMetrics(UmaType uma_type,
+                              bool allow,
+                              bool user_initiated) const {
+    if (!user_initiated)
+      return;
+
+    if (allow) {
+      // Note that |allow| == true means the embedder explicitly allowed the
+      // request. For some requests they might still fail. An example of such
+      // scenario would be: an embedder allows geolocation request but doesn't
+      // have geolocation access on its own.
+      switch (uma_type) {
+        case UMA_TYPE_DOWNLOAD:
+          RecordAction(UserMetricsAction(
+                           "BrowserPlugin.Guest.PermissionAllow.Download"));

[Fady Samuel, 2013/11/13 15:54:05]

All these strings are really similar. Can't you simply use something like this?
StringPrintf("BrowserPlugin.Guest.PermissionAllow.%s",
PermissionTypeToString(permission))?

[lazyboy, 2013/11/13 17:35:33]

As discussed offline and after talking to sadrul@, these need to be literals.

+          break;
+        case UMA_TYPE_GEOLOCATION:
+          RecordAction(UserMetricsAction(
+                           "BrowserPlugin.Guest.PermissionAllow.Geolocation"));
+          break;
+        case UMA_TYPE_MEDIA:
+          RecordAction(UserMetricsAction(
+                           "BrowserPlugin.Guest.PermissionAllow.Media"));
+          break;
+        case UMA_TYPE_POINTER_LOCK:
+          RecordAction(UserMetricsAction(
+                           "BrowserPlugin.Guest.PermissionAllow.PointerLock"));
+          break;
+        case UMA_TYPE_NEW_WINDOW:
+          RecordAction(UserMetricsAction(
+                           "BrowserPlugin.Guest.PermissionAllow.NewWindow"));
+          break;
+        case UMA_TYPE_JAVASCRIPT_DIALOG:
+          RecordAction(
+              UserMetricsAction(
+                  "BrowserPlugin.Guest.PermissionAllow.JavaScriptDialog"));
+          break;
+      }
+    } else {
+      switch (uma_type) {
+        case UMA_TYPE_DOWNLOAD:
+          RecordAction(UserMetricsAction(
+                           "BrowserPlugin.Guest.PermissionDeny.Download"));
+          break;
+        case UMA_TYPE_GEOLOCATION:
+          RecordAction(UserMetricsAction(
+                           "BrowserPlugin.Guest.PermissionDeny.Geolocation"));
+          break;
+        case UMA_TYPE_MEDIA:
+          RecordAction(UserMetricsAction(
+                           "BrowserPlugin.Guest.PermissionDeny.Media"));
+          break;
+        case UMA_TYPE_POINTER_LOCK:
+          RecordAction(UserMetricsAction(
+                           "BrowserPlugin.Guest.PermissionDeny.PointerLock"));
+          break;
+        case UMA_TYPE_NEW_WINDOW:
+          RecordAction(UserMetricsAction(
+                           "BrowserPlugin.Guest.PermissionDeny.NewWindow"));
+          break;
+        case UMA_TYPE_JAVASCRIPT_DIALOG:
+          RecordAction(
+              UserMetricsAction(
+                  "BrowserPlugin.Guest.PermissionDeny.JavaScriptDialog"));
+          break;
+      }
+    }
+  }
+
   // Friend RefCounted so that the dtor can be non-public.
   friend class base::RefCounted<BrowserPluginGuest::PermissionRequest>;
 };
@@ -83,8 +168,10 @@ class BrowserPluginGuest::DownloadRequest : public PermissionRequest {
         UserMetricsAction("BrowserPlugin.Guest.PermissionRequest.Download"));
   }
   virtual void Respond(bool should_allow,
-                       const std::string& user_input) OVERRIDE {
+                       const std::string& user_input,
+                       bool user_initiated) OVERRIDE {
     callback_.Run(should_allow);
+    MaybeRecordUserMetrics(UMA_TYPE_DOWNLOAD, should_allow, user_initiated);
   }
 
  private:
@@ -105,9 +192,11 @@ class BrowserPluginGuest::GeolocationRequest : public PermissionRequest {
   }
 
   virtual void Respond(bool should_allow,
-                       const std::string& user_input) OVERRIDE {
+                       const std::string& user_input,
+                       bool user_initiated) OVERRIDE {
     base::WeakPtr<BrowserPluginGuest> guest(weak_ptr_factory_->GetWeakPtr());
 
+    MaybeRecordUserMetrics(UMA_TYPE_GEOLOCATION, should_allow, user_initiated);
     WebContents* web_contents = guest->embedder_web_contents();
     if (should_allow && web_contents) {
       // If renderer side embedder decides to allow gelocation, we need to check
@@ -159,7 +248,8 @@ class BrowserPluginGuest::MediaRequest : public PermissionRequest {
   }
 
   virtual void Respond(bool should_allow,
-                       const std::string& user_input) OVERRIDE {
+                       const std::string& user_input,
+                       bool user_initiated) OVERRIDE {
     WebContentsImpl* web_contents = guest_->embedder_web_contents();
     if (should_allow && web_contents) {
       // Re-route the request to the embedder's WebContents; the guest gets the
@@ -169,6 +259,7 @@ class BrowserPluginGuest::MediaRequest : public PermissionRequest {
       // Deny the request.
       callback_.Run(MediaStreamDevices(), scoped_ptr<MediaStreamUI>());
     }
+    MaybeRecordUserMetrics(UMA_TYPE_MEDIA, should_allow, user_initiated);
   }
 
  private:
@@ -188,7 +279,8 @@ class BrowserPluginGuest::NewWindowRequest : public PermissionRequest {
   }
 
   virtual void Respond(bool should_allow,
-                       const std::string& user_input) OVERRIDE {
+                       const std::string& user_input,
+                       bool user_initiated) OVERRIDE {
     int embedder_render_process_id =
         guest_->embedder_web_contents()->GetRenderProcessHost()->GetID();
     BrowserPluginGuest* guest =
@@ -199,6 +291,7 @@ class BrowserPluginGuest::NewWindowRequest : public PermissionRequest {
       return;
     }
 
+    MaybeRecordUserMetrics(UMA_TYPE_NEW_WINDOW, should_allow, user_initiated);
     // If we do not destroy the guest then we allow the new window.
     if (!should_allow)
       guest->Destroy();
@@ -220,8 +313,11 @@ class BrowserPluginGuest::JavaScriptDialogRequest : public PermissionRequest {
   }
 
   virtual void Respond(bool should_allow,
-                       const std::string& user_input) OVERRIDE {
+                       const std::string& user_input,
+                       bool user_initiated) OVERRIDE {
     callback_.Run(should_allow, UTF8ToUTF16(user_input));
+    MaybeRecordUserMetrics(UMA_TYPE_JAVASCRIPT_DIALOG, should_allow,
+                           user_initiated);
   }
 
  private:
@@ -238,9 +334,11 @@ class BrowserPluginGuest::PointerLockRequest : public PermissionRequest {
   }
 
   virtual void Respond(bool should_allow,
-                       const std::string& user_input) OVERRIDE {
+                       const std::string& user_input,
+                       bool user_initiated) OVERRIDE {
     guest_->SendMessageToEmbedder(
         new BrowserPluginMsg_SetMouseLock(guest_->instance_id(), should_allow));
+    MaybeRecordUserMetrics(UMA_TYPE_POINTER_LOCK, should_allow, user_initiated);
   }
 
  private:
@@ -409,13 +507,14 @@ void BrowserPluginGuest::LoadURLWithParams(WebContents* web_contents,
 void BrowserPluginGuest::RespondToPermissionRequest(
     int request_id,
     bool should_allow,
-    const std::string& user_input) {
+    const std::string& user_input,
+    bool user_initiated) {
   RequestMap::iterator request_itr = permission_request_map_.find(request_id);
   if (request_itr == permission_request_map_.end()) {
     LOG(INFO) << "Not a valid request ID.";
     return;
   }
-  request_itr->second->Respond(should_allow, user_input);
+  request_itr->second->Respond(should_allow, user_input, user_initiated);
   permission_request_map_.erase(request_itr);
 }
 
@@ -424,7 +523,7 @@ int BrowserPluginGuest::RequestPermission(
     scoped_refptr<BrowserPluginGuest::PermissionRequest> request,
     const base::DictionaryValue& request_info) {
   if (!delegate_) {
-    request->Respond(false, "");
+    request->Respond(false, "", false);
     return browser_plugin::kInvalidPermissionRequestID;
   }
 
@@ -436,10 +535,10 @@ int BrowserPluginGuest::RequestPermission(
                   AsWeakPtr(),
                   request_id);
   // If BrowserPluginGuestDelegate hasn't handled the permission then we simply
-  // reject it immediately.
+  // perform the default action (which is one of allow or reject) immediately.
   if (!delegate_->RequestPermission(
       permission_type, request_info, callback, request->AllowedByDefault())) {
-    callback.Run(request->AllowedByDefault(), "");
+    callback.Run(request->AllowedByDefault(), "", false);
     return browser_plugin::kInvalidPermissionRequestID;
   }