From 6c6555c13c780b2358b4ed07a38be06eef0d0919 Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Fri, 3 May 2019 11:30:48 -0700 Subject: [PATCH] Convert Callbacks to OnceCallbacks in permissions APIs. https://chromium-review.googlesource.com/c/chromium/src/+/1592356 --- atom/browser/api/atom_api_session.cc | 18 ++++++++-- atom/browser/atom_permission_manager.cc | 48 +++++++++++++------------ atom/browser/atom_permission_manager.h | 32 ++++++++--------- 3 files changed, 56 insertions(+), 42 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index af6a858b654..9f94b09e009 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -452,14 +452,28 @@ void Session::SetCertVerifyProc(v8::Local val, void Session::SetPermissionRequestHandler(v8::Local val, mate::Arguments* args) { - AtomPermissionManager::RequestHandler handler; + using StatusCallback = + base::RepeatingCallback; + using RequestHandler = + base::Callback; + RequestHandler handler; if (!(val->IsNull() || mate::ConvertFromV8(args->isolate(), val, &handler))) { args->ThrowError("Must pass null or function"); return; } auto* permission_manager = static_cast( browser_context()->GetPermissionControllerDelegate()); - permission_manager->SetPermissionRequestHandler(handler); + permission_manager->SetPermissionRequestHandler(base::BindRepeating( + [](const RequestHandler& handler, content::WebContents* web_contents, + content::PermissionType permission_type, + AtomPermissionManager::StatusCallback callback, + const base::DictionaryValue& details) { + handler.Run(web_contents, permission_type, + base::AdaptCallbackForRepeating(std::move(callback)), + details); + }, + base::Passed(std::move(handler)))); } void Session::SetPermissionCheckHandler(v8::Local val, diff --git a/atom/browser/atom_permission_manager.cc b/atom/browser/atom_permission_manager.cc index ffd0797709d..8a571c8affd 100644 --- a/atom/browser/atom_permission_manager.cc +++ b/atom/browser/atom_permission_manager.cc @@ -32,9 +32,9 @@ bool WebContentsDestroyed(int process_id) { } void PermissionRequestResponseCallbackWrapper( - const AtomPermissionManager::StatusCallback& callback, + AtomPermissionManager::StatusCallback callback, const std::vector& vector) { - callback.Run(vector[0]); + std::move(callback).Run(vector[0]); } } // namespace @@ -43,9 +43,9 @@ class AtomPermissionManager::PendingRequest { public: PendingRequest(content::RenderFrameHost* render_frame_host, const std::vector& permissions, - const StatusesCallback& callback) + StatusesCallback callback) : render_process_id_(render_frame_host->GetProcess()->GetID()), - callback_(callback), + callback_(std::move(callback)), permissions_(permissions), results_(permissions.size(), blink::mojom::PermissionStatus::DENIED), remaining_results_(permissions.size()) {} @@ -74,11 +74,15 @@ class AtomPermissionManager::PendingRequest { bool IsComplete() const { return remaining_results_ == 0; } - void RunCallback() const { callback_.Run(results_); } + void RunCallback() { + if (!callback_.is_null()) { + std::move(callback_).Run(results_); + } + } private: int render_process_id_; - const StatusesCallback callback_; + StatusesCallback callback_; std::vector permissions_; std::vector results_; size_t remaining_results_; @@ -91,8 +95,8 @@ AtomPermissionManager::~AtomPermissionManager() {} void AtomPermissionManager::SetPermissionRequestHandler( const RequestHandler& handler) { if (handler.is_null() && !pending_requests_.IsEmpty()) { - for (PendingRequestsMap::const_iterator iter(&pending_requests_); - !iter.IsAtEnd(); iter.Advance()) { + for (PendingRequestsMap::iterator iter(&pending_requests_); !iter.IsAtEnd(); + iter.Advance()) { auto* request = iter.GetCurrentValue(); if (!WebContentsDestroyed(request->render_process_id())) request->RunCallback(); @@ -112,11 +116,10 @@ int AtomPermissionManager::RequestPermission( content::RenderFrameHost* render_frame_host, const GURL& requesting_origin, bool user_gesture, - const base::Callback& - response_callback) { + StatusCallback response_callback) { return RequestPermissionWithDetails(permission, render_frame_host, requesting_origin, user_gesture, nullptr, - response_callback); + std::move(response_callback)); } int AtomPermissionManager::RequestPermissionWithDetails( @@ -125,11 +128,12 @@ int AtomPermissionManager::RequestPermissionWithDetails( const GURL& requesting_origin, bool user_gesture, const base::DictionaryValue* details, - const StatusCallback& response_callback) { + StatusCallback response_callback) { return RequestPermissionsWithDetails( std::vector(1, permission), render_frame_host, requesting_origin, user_gesture, details, - base::Bind(&PermissionRequestResponseCallbackWrapper, response_callback)); + base::BindOnce(PermissionRequestResponseCallbackWrapper, + std::move(response_callback))); } int AtomPermissionManager::RequestPermissions( @@ -137,10 +141,10 @@ int AtomPermissionManager::RequestPermissions( content::RenderFrameHost* render_frame_host, const GURL& requesting_origin, bool user_gesture, - const StatusesCallback& response_callback) { + StatusesCallback response_callback) { return RequestPermissionsWithDetails(permissions, render_frame_host, requesting_origin, user_gesture, nullptr, - response_callback); + std::move(response_callback)); } int AtomPermissionManager::RequestPermissionsWithDetails( @@ -149,9 +153,9 @@ int AtomPermissionManager::RequestPermissionsWithDetails( const GURL& requesting_origin, bool user_gesture, const base::DictionaryValue* details, - const StatusesCallback& response_callback) { + StatusesCallback response_callback) { if (permissions.empty()) { - response_callback.Run(std::vector()); + std::move(response_callback).Run({}); return content::PermissionController::kNoPendingOperation; } @@ -169,20 +173,20 @@ int AtomPermissionManager::RequestPermissionsWithDetails( } statuses.push_back(blink::mojom::PermissionStatus::GRANTED); } - response_callback.Run(statuses); + std::move(response_callback).Run(statuses); return content::PermissionController::kNoPendingOperation; } auto* web_contents = content::WebContents::FromRenderFrameHost(render_frame_host); int request_id = pending_requests_.Add(std::make_unique( - render_frame_host, permissions, response_callback)); + render_frame_host, permissions, std::move(response_callback))); for (size_t i = 0; i < permissions.size(); ++i) { auto permission = permissions[i]; const auto callback = - base::Bind(&AtomPermissionManager::OnPermissionResponse, - base::Unretained(this), request_id, i); + base::BindRepeating(&AtomPermissionManager::OnPermissionResponse, + base::Unretained(this), request_id, i); if (details == nullptr) { request_handler_.Run(web_contents, permission, callback, base::DictionaryValue()); @@ -224,7 +228,7 @@ int AtomPermissionManager::SubscribePermissionStatusChange( content::PermissionType permission, content::RenderFrameHost* render_frame_host, const GURL& requesting_origin, - const base::Callback& callback) { + base::RepeatingCallback callback) { return -1; } diff --git a/atom/browser/atom_permission_manager.h b/atom/browser/atom_permission_manager.h index 70d78a60feb..5e1e326958d 100644 --- a/atom/browser/atom_permission_manager.h +++ b/atom/browser/atom_permission_manager.h @@ -25,12 +25,13 @@ class AtomPermissionManager : public content::PermissionControllerDelegate { AtomPermissionManager(); ~AtomPermissionManager() override; - using StatusCallback = base::Callback; - using StatusesCallback = - base::Callback&)>; + using StatusCallback = + base::OnceCallback; + using StatusesCallback = base::OnceCallback&)>; using RequestHandler = base::Callback; using CheckHandler = base::Callback& callback) - override; + int RequestPermission(content::PermissionType permission, + content::RenderFrameHost* render_frame_host, + const GURL& requesting_origin, + bool user_gesture, + StatusCallback callback) override; int RequestPermissionWithDetails(content::PermissionType permission, content::RenderFrameHost* render_frame_host, const GURL& requesting_origin, bool user_gesture, const base::DictionaryValue* details, - const StatusCallback& callback); + StatusCallback callback); int RequestPermissions( const std::vector& permissions, content::RenderFrameHost* render_frame_host, const GURL& requesting_origin, bool user_gesture, - const base::Callback< - void(const std::vector&)>& callback) - override; + StatusesCallback callback) override; int RequestPermissionsWithDetails( const std::vector& permissions, content::RenderFrameHost* render_frame_host, const GURL& requesting_origin, bool user_gesture, const base::DictionaryValue* details, - const base::Callback< - void(const std::vector&)>& callback); + StatusesCallback callback); blink::mojom::PermissionStatus GetPermissionStatusForFrame( content::PermissionType permission, content::RenderFrameHost* render_frame_host, @@ -98,7 +94,7 @@ class AtomPermissionManager : public content::PermissionControllerDelegate { content::PermissionType permission, content::RenderFrameHost* render_frame_host, const GURL& requesting_origin, - const base::Callback& callback) + base::RepeatingCallback callback) override; void UnsubscribePermissionStatusChange(int subscription_id) override;