Log site engagement scores for permission actions

chrome/browser/permissions/permission_request_manager.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 "chrome/browser/permissions/permission_request_manager.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/feature_list.h"
@@ skipping 13 matching lines @@
 namespace {
 
 class CancelledRequest : public PermissionRequest {
  public:
   explicit CancelledRequest(PermissionRequest* cancelled)
       : icon_(cancelled->GetIconId()),
 #if defined(OS_ANDROID)
         message_(cancelled->GetMessageText()),
 #endif
         message_fragment_(cancelled->GetMessageTextFragment()),
-        origin_(cancelled->GetOrigin()) {
+        origin_(cancelled->GetOrigin()),
+        request_type_(cancelled->GetPermissionRequestType()) {
   }
   ~CancelledRequest() override {}
 
   IconId GetIconId() const override { return icon_; }
 #if defined(OS_ANDROID)
   base::string16 GetMessageText() const override { return message_; }
 #endif
   base::string16 GetMessageTextFragment() const override {
     return message_fragment_;
   }
   GURL GetOrigin() const override { return origin_; }
 
   // These are all no-ops since the placeholder is non-forwarding.
   void PermissionGranted() override {}
   void PermissionDenied() override {}
   void Cancelled() override {}
 
   void RequestFinished() override { delete this; }
 
+  PermissionRequestType GetPermissionRequestType() const override {
+    return request_type_;
+  }
+
  private:
   IconId icon_;
 #if defined(OS_ANDROID)
   base::string16 message_;
 #endif
   base::string16 message_fragment_;
   GURL origin_;
+  PermissionRequestType request_type_;
 };
 
 bool IsMessageTextEqual(PermissionRequest* a,
                         PermissionRequest* b) {
   if (a == b)
     return true;
   if (a->GetMessageTextFragment() == b->GetMessageTextFragment() &&
       a->GetOrigin() == b->GetOrigin()) {
     return true;
   }
@@ skipping 109 matching lines @@
   std::deque<PermissionRequest*>::iterator queued_requests_iter;
   for (queued_requests_iter = queued_requests_.begin();
        queued_requests_iter != queued_requests_.end(); queued_requests_iter++) {
     if (*queued_requests_iter == request) {
       RequestFinishedIncludingDuplicates(*queued_requests_iter);
       queued_requests_.erase(queued_requests_iter);
       return;
     }
   }
 
-  std::vector<PermissionRequest*>::iterator requests_iter;
-  for (requests_iter = requests_.begin(); requests_iter != requests_.end();
-       requests_iter++) {
-    if (*requests_iter != request)
-      continue;
+  if (!requests_.empty() && requests_[0] == request) {
+    // TODO(timloh): We should fix this at some point.
+    // Grouped (mic+camera) requests are currently never cancelled.

[raymes, 2017/07/20 05:21:17]

nit: these comments are probably in the reverse order

[Timothy Loh, 2017/07/20 06:09:33]

Done.

+    DCHECK_EQ(static_cast<size_t>(1), requests_.size());

[raymes, 2017/07/20 05:21:17]

nit: 1u?

optional: may want to move this up above the if-statement (and check that it's 0
or 1) just so it's checked regardless of whether the condition is matched

[Timothy Loh, 2017/07/20 06:09:33]

Don't think 1u works on 64 bit platforms? But we can get into a state where we
have mic+camera showing but we cancel a different request, so I can't move the
check up.

 
-    // We can simply erase the current entry in the request table if we aren't
-    // showing the dialog (because we switched tabs with an active prompt), or
-    // if we are showing it and it can accept the update.
+    // We can finalize the prompt if we aren't showing the dialog (because we
+    // switched tabs with an active prompt), or if we are showing it and it
+    // can accept the update.
     if (!view_ || view_->CanAcceptRequestUpdate()) {
-      // TODO(timloh): We should fix this at some point.
-      // Grouped (mic+camera) requests are currently never cancelled.
-      DCHECK_EQ(static_cast<size_t>(1), requests_.size());
-
-      if (view_)
-        DeleteBubble();
-
-      RequestFinishedIncludingDuplicates(*requests_iter);
-      requests_.erase(requests_iter);
-
-      DequeueRequestsAndShowBubble();
+      FinalizeBubble(PermissionAction::IGNORED);
       return;
     }
 
     // Cancel the existing request and replace it with a dummy.
-    PermissionRequest* cancelled_request =
-        new CancelledRequest(*requests_iter);
-    RequestFinishedIncludingDuplicates(*requests_iter);
-    *requests_iter = cancelled_request;
+    PermissionRequest* cancelled_request = new CancelledRequest(request);
+    RequestFinishedIncludingDuplicates(request);
+    requests_[0] = cancelled_request;
     return;
   }
 
   // Since |request| wasn't found in queued_requests_ or
   // requests_ it must have been marked as a duplicate. We can't search
   // duplicate_requests_ by value, so instead use GetExistingRequest to find the
   // key (request it was duped against), and iterate through duplicates of that.
   PermissionRequest* existing_request = GetExistingRequest(request);
   auto range = duplicate_requests_.equal_range(existing_request);
   for (auto it = range.first; it != range.second; ++it) {
@@ skipping 93 matching lines @@
 
 const std::vector<PermissionRequest*>& PermissionRequestManager::Requests() {
   return requests_;
 }
 
 void PermissionRequestManager::TogglePersist(bool new_value) {
   persist_ = new_value;
 }
 
 void PermissionRequestManager::Accept() {
-  PermissionUmaUtil::PermissionPromptAccepted(requests_);
-
   std::vector<PermissionRequest*>::iterator requests_iter;
   for (requests_iter = requests_.begin(); requests_iter != requests_.end();
        requests_iter++) {
     PermissionGrantedIncludingDuplicates(*requests_iter);
   }
-  FinalizeBubble();
+  FinalizeBubble(PermissionAction::GRANTED);
 }
 
 void PermissionRequestManager::Deny() {
-  PermissionUmaUtil::PermissionPromptDenied(requests_);
-
   std::vector<PermissionRequest*>::iterator requests_iter;
   for (requests_iter = requests_.begin();
        requests_iter != requests_.end();
        requests_iter++) {
     PermissionDeniedIncludingDuplicates(*requests_iter);
   }
-  FinalizeBubble();
+  FinalizeBubble(PermissionAction::DENIED);
 }
 
 void PermissionRequestManager::Closing() {
 #if defined(OS_MACOSX)
   // Mac calls this whenever you press Esc.
   if (!view_)
     return;
 #endif
+
   std::vector<PermissionRequest*>::iterator requests_iter;
   for (requests_iter = requests_.begin();
        requests_iter != requests_.end();
        requests_iter++) {
     CancelledIncludingDuplicates(*requests_iter);
   }
-  FinalizeBubble();
+  FinalizeBubble(PermissionAction::DISMISSED);
 }
 
 void PermissionRequestManager::ScheduleShowBubble() {
   // ::ScheduleShowBubble() will be called again when the main frame will be
   // loaded.
   if (!main_frame_has_fully_loaded_)
     return;
 
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
@@ skipping 35 matching lines @@
   // If in testing mode, automatically respond to the bubble that was shown.
   if (auto_response_for_test_ != NONE)
     DoAutoResponseForTesting();
 }
 
 void PermissionRequestManager::DeleteBubble() {
   DCHECK(view_);
   view_.reset();
 }
 
-void PermissionRequestManager::FinalizeBubble() {
-  DCHECK(view_);

[raymes, 2017/07/20 05:21:17]

nit: I think it would be useful to hoist this DCHECK up into the callsites of
this function where it still holds.

[Timothy Loh, 2017/07/20 06:09:33]

Done.

+void PermissionRequestManager::FinalizeBubble(
+    PermissionAction permission_action) {
   DCHECK(!requests_.empty());
 
-  DeleteBubble();
+  if (view_)
+    DeleteBubble();
+
+  PermissionUmaUtil::PermissionPromptResolved(requests_, web_contents(),
+                                              permission_action);
 
   std::vector<PermissionRequest*>::iterator requests_iter;
   for (requests_iter = requests_.begin();
        requests_iter != requests_.end();
        requests_iter++) {
     RequestFinishedIncludingDuplicates(*requests_iter);
   }
   requests_.clear();
   if (queued_requests_.size())
     DequeueRequestsAndShowBubble();
 }
 
 void PermissionRequestManager::CleanUpRequests() {
   std::deque<PermissionRequest*>::iterator requests_iter;
   for (requests_iter = queued_requests_.begin();
        requests_iter != queued_requests_.end();
        requests_iter++) {
     RequestFinishedIncludingDuplicates(*requests_iter);
   }
   queued_requests_.clear();
 
   if (view_)
-    FinalizeBubble();
+    FinalizeBubble(PermissionAction::IGNORED);
 }
 
 PermissionRequest* PermissionRequestManager::GetExistingRequest(
     PermissionRequest* request) {
   for (PermissionRequest* existing_request : requests_)
     if (IsMessageTextEqual(existing_request, request))
       return existing_request;
   for (PermissionRequest* existing_request : queued_requests_)
     if (IsMessageTextEqual(existing_request, request))
       return existing_request;
@@ skipping 71 matching lines @@
     case DENY_ALL:
       Deny();
       break;
     case DISMISS:
       Closing();
       break;
     case NONE:
       NOTREACHED();
   }
 }