Fix cancelling permission requests on Android when the PermissionRequestManager is enabled

chrome/browser/permissions/permission_request_manager.cc

Index: chrome/browser/permissions/permission_request_manager.cc
diff --git a/chrome/browser/permissions/permission_request_manager.cc b/chrome/browser/permissions/permission_request_manager.cc
index 2a7cf3ac5729cd357ac67051284a3872ce8cc374..b5f02fc17a1b3ff30357eec53e346008d5a036b4 100644
--- a/chrome/browser/permissions/permission_request_manager.cc
+++ b/chrome/browser/permissions/permission_request_manager.cc
@@ -188,26 +188,24 @@ void PermissionRequestManager::CancelRequest(PermissionRequest* request) {
       continue;
 
     // We can simply erase the current entry in the request table if we aren't
-    // showing the dialog, or if we are showing it and it can accept the update.
-    bool can_erase = !IsBubbleVisible() || view_->CanAcceptRequestUpdate();
-    if (can_erase) {
+    // showing the dialog
+    if (!IsBubbleVisible()) {
       RequestFinishedIncludingDuplicates(*requests_iter);
       requests_.erase(requests_iter);
       accept_states_.erase(accepts_iter);
-
-      if (IsBubbleVisible()) {
-        view_->Hide();
-        // Will redraw the bubble if it is being shown.
-        TriggerShowBubble();
-      }
-      return;
+    } else if (view_->MaybeCancelRequest()) {

[raymes, 2017/05/01 04:23:35]

I think we could document each of these cases better. Firstly, maybe we should
talk about hiding the prompt here rather than cancelling the request, because
this won't actually modify the request? Secondly, the prompt will be hidden if
the user isn't interacting with the prompt. So maybe we should call the function
something like:
HideIfUserNotInteracting ?

+      RequestFinishedIncludingDuplicates(*requests_iter);
+      requests_.erase(requests_iter);
+      accept_states_.erase(accepts_iter);
+      TriggerShowBubble();
+    } else {
+      // Cancel the existing request and replace it with a dummy.

[raymes, 2017/05/01 04:23:35]

I think we can clarify this comment a bit.
// If the user is still interacting with the prompt, we don't 
// hide it but we still cancel the permission request and replace
// it with a dummy request that will do nothing when the permission
// prompt is answered.

+      PermissionRequest* cancelled_request =
+          new CancelledRequest(*requests_iter);
+      RequestFinishedIncludingDuplicates(*requests_iter);
+      *requests_iter = cancelled_request;
+      view_->Show(requests_, accept_states_);

[raymes, 2017/05/01 04:23:35]

As discussed I think it would be preferable to allow access to the vector of
requests via the delegate, which would remove the need for calling Show() again
and also ensure that we don't have the 2 lists getting out of sync,

     }
-
-    // Cancel the existing request and replace it with a dummy.
-    PermissionRequest* cancelled_request =
-        new CancelledRequest(*requests_iter);
-    RequestFinishedIncludingDuplicates(*requests_iter);
-    *requests_iter = cancelled_request;
     return;
   }