permissions: handle request ids for permissions in permission manager

chrome/browser/permissions/permission_manager.cc

 // Copyright 2015 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_manager.h"
 
 #include "base/callback.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/permissions/permission_context.h"
 #include "chrome/browser/permissions/permission_context_base.h"
 #include "chrome/browser/permissions/permission_request_id.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/website_settings/permission_bubble_manager.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 
 using content::PermissionStatus;
 using content::PermissionType;
 
@@ skipping 49 matching lines @@
     case PermissionType::NUM:
       // This will hit the NOTREACHED below.
       break;
   }
 
   NOTREACHED() << "Unknown content setting for permission "
                << static_cast<int>(permission);
   return CONTENT_SETTINGS_TYPE_DEFAULT;
 }
 
-// Helper method that wraps a callback a void(PermissionStatus)
-// callback into a void(ContentSetting) callback.
-void PermissionStatusCallbackWrapper(
-    const base::Callback<void(PermissionStatus)>& callback,
-    ContentSetting content_setting) {
-  callback.Run(ContentSettingToPermissionStatus(content_setting));
-}
-
 // Returns whether the permission has a constant PermissionStatus value (i.e.
 // always approved or always denied)
 // The PermissionTypes for which true is returned should be exactly those which
 // return nullptr in PermissionContext::Get since they don't have a context.
 bool IsConstantPermission(PermissionType type) {
   switch (type) {
     case PermissionType::MIDI:
       return true;
     default:
       return false;
@@ skipping 15 matching lines @@
   }
 }
 
 PermissionStatus GetPermissionStatusForConstantPermission(PermissionType type) {
   return ContentSettingToPermissionStatus(
       GetContentSettingForConstantPermission(type));
 }
 
 }  // anonymous namespace
 
+struct PermissionManager::PendingRequest {
+  PermissionType permission;
+  int render_process_id;
+  int render_frame_id;
+};
+
 struct PermissionManager::Subscription {
   PermissionType permission;
   GURL requesting_origin;
   GURL embedding_origin;
   base::Callback<void(PermissionStatus)> callback;
   ContentSetting current_value;
 };
 
 PermissionManager::PermissionManager(Profile* profile)
-    : profile_(profile) {
+    : profile_(profile),
+      weak_ptr_factory_(this) {
 }
 
 PermissionManager::~PermissionManager() {
   if (!subscriptions_.IsEmpty())
     HostContentSettingsMapFactory::GetForProfile(profile_)
         ->RemoveObserver(this);
 }
 
-void PermissionManager::RequestPermission(
+int PermissionManager::RequestPermission(
     PermissionType permission,
     content::RenderFrameHost* render_frame_host,
-    int request_id,
     const GURL& requesting_origin,
     bool user_gesture,
     const base::Callback<void(PermissionStatus)>& callback) {
   if (IsConstantPermission(permission)) {
     callback.Run(GetPermissionStatusForConstantPermission(permission));
-    return;
+    return kNoPendingRequestOrSubscription;
   }
 
   PermissionContextBase* context = PermissionContext::Get(profile_, permission);
   if (!context) {
     callback.Run(content::PERMISSION_STATUS_DENIED);
-    return;
+    return kNoPendingRequestOrSubscription;
   }
 
   content::WebContents* web_contents =
       content::WebContents::FromRenderFrameHost(render_frame_host);
   if (IsPermissionBubbleManagerMissing(web_contents)) {
     callback.Run(
         GetPermissionStatus(permission, requesting_origin,
                             web_contents->GetLastCommittedURL().GetOrigin()));
-    return;
+    return kNoPendingRequestOrSubscription;
   }
 
   int render_process_id = render_frame_host->GetProcess()->GetID();
   int render_frame_id = render_frame_host->GetRoutingID();
+
+  PendingRequest* pending_request = new PendingRequest();
+  pending_request->render_process_id = render_process_id;
+  pending_request->render_frame_id = render_frame_id;
+  pending_request->permission = permission;

[mlamouri (slow - plz ping), 2015/09/23 14:46:00]

Could you add a ctor to PendingRequest?

[Lalit Maganti, 2015/09/23 15:24:30]

Done.

+
+  int request_id = pending_requests_.Add(pending_request);

[mlamouri (slow - plz ping), 2015/09/23 14:46:00]

... you could even call new PendingRequest() here using the ctor.

[Lalit Maganti, 2015/09/23 15:24:30]

The way I did the constructor means that I need the pointer.

   const PermissionRequestID request(render_process_id,
                                     render_frame_id,
                                     request_id);
 
   context->RequestPermission(
       web_contents, request, requesting_origin, user_gesture,
-      base::Bind(&PermissionStatusCallbackWrapper, callback));
+      base::Bind(&PermissionManager::OnPermissionRequestResponse,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 request_id,
+                 callback));
+  return request_id;
 }
 
-void PermissionManager::CancelPermissionRequest(
-    PermissionType permission,
-    content::RenderFrameHost* render_frame_host,
+void PermissionManager::OnPermissionRequestResponse(
     int request_id,
-    const GURL& requesting_origin) {
-  PermissionContextBase* context = PermissionContext::Get(profile_, permission);
+    const base::Callback<void(PermissionStatus)>& callback,
+    ContentSetting content_setting) {
+  pending_requests_.Remove(request_id);
+  callback.Run(ContentSettingToPermissionStatus(content_setting));
+}
+
+void PermissionManager::CancelPermissionRequest(int request_id) {
+  PendingRequest* pending_request = pending_requests_.Lookup(request_id);
+  if (!pending_request)
+    return;
+
+  PermissionContextBase* context = PermissionContext::Get(
+      profile_, pending_request->permission);
   if (!context)
     return;
 
-  content::WebContents* web_contents =
-      content::WebContents::FromRenderFrameHost(render_frame_host);
-  if (IsPermissionBubbleManagerMissing(web_contents))
+  content::WebContents* web_contents = tab_util::GetWebContentsByFrameID(
+      pending_request->render_process_id, pending_request->render_frame_id);
+  DCHECK(web_contents);
+  if (IsPermissionBubbleManagerMissing(web_contents)) {
+    pending_requests_.Remove(request_id);
     return;
+  }
 
-  int render_process_id = render_frame_host->GetProcess()->GetID();
-  int render_frame_id = render_frame_host->GetRoutingID();
-  const PermissionRequestID request(render_process_id,
-                                    render_frame_id,
+  const PermissionRequestID request(pending_request->render_process_id,
+                                    pending_request->render_frame_id,
                                     request_id);
-
   context->CancelPermissionRequest(web_contents, request);
+  pending_requests_.Remove(request_id);
 }
 
 void PermissionManager::ResetPermission(PermissionType permission,
                                         const GURL& requesting_origin,
                                         const GURL& embedding_origin) {
   PermissionContextBase* context = PermissionContext::Get(profile_, permission);
   if (!context)
     return;
 
   context->ResetPermission(requesting_origin.GetOrigin(),
@@ skipping 107 matching lines @@
     // Add the callback to |callbacks| which will be run after the loop to
     // prevent re-entrance issues.
     callbacks.push_back(
         base::Bind(subscription->callback,
                    ContentSettingToPermissionStatus(new_value)));
   }
 
   for (const auto& callback : callbacks)
     callback.Run();
 }