Implement RequestPermissions using request handler

atom/browser/atom_permission_manager.cc

@@ -26,10 +26,52 @@ bool WebContentsDestroyed(int process_id) {
   return contents->IsBeingDestroyed();
 }
 
+void PermissionRequestResponseCallbackWrapper(
+    const AtomPermissionManager::StatusCallback& callback,
+    const std::vector<blink::mojom::PermissionStatus>& vector) {
+  callback.Run(vector[0]);
+}
+
 }  // namespace
 
-AtomPermissionManager::AtomPermissionManager()
+class AtomPermissionManager::PendingRequest {
-    : request_id_(0) {
+ public:
+  PendingRequest(content::RenderFrameHost* render_frame_host,
+                 const std::vector<content::PermissionType>& permissions,
+                 const StatusesCallback& callback)
+      : render_process_id_(render_frame_host->GetProcess()->GetID()),
+        callback_(callback),
+        results_(permissions.size(), blink::mojom::PermissionStatus::DENIED),
+        remaining_results_(permissions.size()) {}
+
+  void SetPermissionStatus(int permission_id,
+                           blink::mojom::PermissionStatus status) {
+    DCHECK(!IsComplete());
+
+    results_[permission_id] = status;
+    --remaining_results_;
+  }
+
+  int render_process_id() const {
+    return render_process_id_;
+  }
+
+  bool IsComplete() const {
+    return remaining_results_ == 0;
+  }
+
+  void RunCallback() const {
+    callback_.Run(results_);
+  }
+
+ private:
+  int render_process_id_;
+  const StatusesCallback callback_;
+  std::vector<blink::mojom::PermissionStatus> results_;
+  size_t remaining_results_;
+};
+
+AtomPermissionManager::AtomPermissionManager() {
 }
 
 AtomPermissionManager::~AtomPermissionManager() {
@@ -37,12 +79,14 @@ AtomPermissionManager::~AtomPermissionManager() {
 
 void AtomPermissionManager::SetPermissionRequestHandler(
     const RequestHandler& handler) {
-  if (handler.is_null() && !pending_requests_.empty()) {
+  if (handler.is_null() && !pending_requests_.IsEmpty()) {
-    for (const auto& request : pending_requests_) {
+    for (PendingRequestsMap::const_iterator iter(&pending_requests_);
-      if (!WebContentsDestroyed(request.second.render_process_id))
+         !iter.IsAtEnd(); iter.Advance()) {
-        request.second.callback.Run(blink::mojom::PermissionStatus::DENIED);
+      auto request = iter.GetCurrentValue();
+      if (!WebContentsDestroyed(request->render_process_id()))
+        request->RunCallback();
     }
-    pending_requests_.clear();
+    pending_requests_.Clear();
   }
   request_handler_ = handler;
 }
@@ -52,30 +96,13 @@ int AtomPermissionManager::RequestPermission(
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin,
     bool user_gesture,
-    const ResponseCallback& response_callback) {
+    const StatusCallback& response_callback) {
-  int process_id = render_frame_host->GetProcess()->GetID();
+  return RequestPermissions(
-
+      std::vector<content::PermissionType>(1, permission),
-  if (permission == content::PermissionType::MIDI_SYSEX) {
+      render_frame_host,
-    content::ChildProcessSecurityPolicy::GetInstance()->
+      requesting_origin,
-        GrantSendMidiSysExMessage(process_id);
+      user_gesture,
-  }
+      base::Bind(&PermissionRequestResponseCallbackWrapper, response_callback));
-
-  if (!request_handler_.is_null()) {
-    auto web_contents =
-        content::WebContents::FromRenderFrameHost(render_frame_host);
-    ++request_id_;
-    auto callback = base::Bind(&AtomPermissionManager::OnPermissionResponse,
-                               base::Unretained(this),
-                               request_id_,
-                               requesting_origin,
-                               response_callback);
-    pending_requests_[request_id_] = { process_id, callback };
-    request_handler_.Run(web_contents, permission, callback);
-    return request_id_;
-  }
-
-  response_callback.Run(blink::mojom::PermissionStatus::GRANTED);
-  return kNoPendingOperation;
 }
 
 int AtomPermissionManager::RequestPermissions(
@@ -83,41 +110,68 @@ int AtomPermissionManager::RequestPermissions(
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin,
     bool user_gesture,
-    const base::Callback<void(
+    const StatusesCallback& response_callback) {
-        const std::vector<blink::mojom::PermissionStatus>&)>& callback) {
+  if (permissions.empty()) {
-  // FIXME(zcbenz): Just ignore multiple permissions request for now.
+    response_callback.Run(std::vector<blink::mojom::PermissionStatus>());
-  std::vector<blink::mojom::PermissionStatus> permissionStatuses;
+    return kNoPendingOperation;
-  for (auto permission : permissions) {
+  }
+
+  if (request_handler_.is_null()) {
+    std::vector<blink::mojom::PermissionStatus> statuses;
+    for (auto permission : permissions) {
+      if (permission == content::PermissionType::MIDI_SYSEX) {
+        content::ChildProcessSecurityPolicy::GetInstance()->
+            GrantSendMidiSysExMessage(render_frame_host->GetProcess()->GetID());
+      }
+      statuses.push_back(blink::mojom::PermissionStatus::GRANTED);
+    }
+    response_callback.Run(statuses);
+    return kNoPendingOperation;
+  }
+
+  auto web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host);
+  int request_id = pending_requests_.Add(new PendingRequest(
+      render_frame_host, permissions, response_callback));
+
+  for (size_t i = 0; i < permissions.size(); ++i) {
+    auto permission = permissions[i];
     if (permission == content::PermissionType::MIDI_SYSEX) {
       content::ChildProcessSecurityPolicy::GetInstance()->
           GrantSendMidiSysExMessage(render_frame_host->GetProcess()->GetID());
     }
-    permissionStatuses.push_back(blink::mojom::PermissionStatus::GRANTED);
+    const auto callback =
+        base::Bind(&AtomPermissionManager::OnPermissionResponse,
+                   base::Unretained(this), request_id, i);
+    request_handler_.Run(web_contents, permission, callback);
   }
-  callback.Run(permissionStatuses);
+
-  return kNoPendingOperation;
+  return request_id;
 }
 
 void AtomPermissionManager::OnPermissionResponse(
     int request_id,
-    const GURL& origin,
+    int permission_id,
-    const ResponseCallback& callback,
     blink::mojom::PermissionStatus status) {
-  auto request = pending_requests_.find(request_id);
+  auto pending_request = pending_requests_.Lookup(request_id);
-  if (request != pending_requests_.end()) {
+  if (!pending_request)
-    if (!WebContentsDestroyed(request->second.render_process_id))
+    return;
-      callback.Run(status);
+
-    pending_requests_.erase(request);
+  pending_request->SetPermissionStatus(permission_id, status);
+  if (pending_request->IsComplete()) {
+    pending_request->RunCallback();
+    pending_requests_.Remove(request_id);
   }
 }
 
 void AtomPermissionManager::CancelPermissionRequest(int request_id) {
-  auto request = pending_requests_.find(request_id);
+  auto pending_request = pending_requests_.Lookup(request_id);
-  if (request != pending_requests_.end()) {
+  if (!pending_request)
-    if (!WebContentsDestroyed(request->second.render_process_id))
+    return;
-      request->second.callback.Run(blink::mojom::PermissionStatus::DENIED);
+
-    pending_requests_.erase(request);
+  if (!WebContentsDestroyed(pending_request->render_process_id()))
-  }
+    pending_request->RunCallback();
+  pending_requests_.Remove(request_id);
 }
 
 void AtomPermissionManager::ResetPermission(
@@ -143,7 +197,7 @@ int AtomPermissionManager::SubscribePermissionStatusChange(
     content::PermissionType permission,
     const GURL& requesting_origin,
     const GURL& embedding_origin,
-    const ResponseCallback& callback) {
+    const StatusCallback& callback) {
   return -1;
 }
 

atom/browser/atom_permission_manager.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "base/id_map.h"
 #include "content/public/browser/permission_manager.h"
 
 namespace content {
@@ -22,12 +23,14 @@ class AtomPermissionManager : public content::PermissionManager {
   AtomPermissionManager();
   ~AtomPermissionManager() override;
 
-  using ResponseCallback =
+  using StatusCallback =
       base::Callback<void(blink::mojom::PermissionStatus)>;
+  using StatusesCallback =
+      base::Callback<void(const std::vector<blink::mojom::PermissionStatus>&)>;
   using RequestHandler =
       base::Callback<void(content::WebContents*,
                           content::PermissionType,
-                          const ResponseCallback&)>;
+                          const StatusCallback&)>;
 
   // Handler to dispatch permission requests in JS.
   void SetPermissionRequestHandler(const RequestHandler& handler);
@@ -51,8 +54,7 @@ class AtomPermissionManager : public content::PermissionManager {
 
  protected:
   void OnPermissionResponse(int request_id,
-                            const GURL& url,
+                            int permission_id,
-                            const ResponseCallback& callback,
                             blink::mojom::PermissionStatus status);
 
   // content::PermissionManager:
@@ -76,16 +78,12 @@ class AtomPermissionManager : public content::PermissionManager {
   void UnsubscribePermissionStatusChange(int subscription_id) override;
 
  private:
-  struct RequestInfo {
+  class PendingRequest;
-    int render_process_id;
+  using PendingRequestsMap = IDMap<PendingRequest, IDMapOwnPointer>;
-    ResponseCallback callback;
-  };
 
   RequestHandler request_handler_;
 
-  std::map<int, RequestInfo> pending_requests_;
+  PendingRequestsMap pending_requests_;
-
-  int request_id_;
 
   DISALLOW_COPY_AND_ASSIGN(AtomPermissionManager);
 };