Refactor chromeos::PermissionBrokerClient to use base::OnceCallback

Call sites are also updated to use base::BindOnce() instead of Bind()
or BindRepeating(), and remove AdaptCallbackForRepeating() where
possible.

Bug: 950118
Change-Id: I98449b98842a9c75f4ec670777cf245a51c683fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1555854
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649156}

chromeos/dbus/permission_broker/fake_permission_broker_client.cc

@@ -29,8 +29,8 @@ FakePermissionBrokerClient* g_instance = nullptr;
 // permission broker by opening the path specified and returning the resulting
 // file descriptor.
 void OpenPath(const std::string& path,
-              const PermissionBrokerClient::OpenPathCallback& callback,
-              const PermissionBrokerClient::ErrorCallback& error_callback,
+              PermissionBrokerClient::OpenPathCallback callback,
+              PermissionBrokerClient::ErrorCallback error_callback,
               scoped_refptr<base::TaskRunner> task_runner) {
   base::ScopedFD fd(HANDLE_EINTR(open(path.c_str(), O_RDWR)));
   if (!fd.is_valid()) {
@@ -38,14 +38,15 @@ void OpenPath(const std::string& path,
     task_runner->PostTask(
         FROM_HERE,
         base::BindOnce(
-            error_callback, kOpenFailedError,
+            std::move(error_callback), kOpenFailedError,
             base::StringPrintf(
                 "Failed to open '%s': %s", path.c_str(),
                 logging::SystemErrorCodeToString(error_code).c_str())));
     return;
   }
 
-  task_runner->PostTask(FROM_HERE, base::BindOnce(callback, std::move(fd)));
+  task_runner->PostTask(FROM_HERE,
+                        base::BindOnce(std::move(callback), std::move(fd)));
 }
 
 }  // namespace
@@ -66,19 +67,19 @@ FakePermissionBrokerClient* FakePermissionBrokerClient::Get() {
   return g_instance;
 }
 
-void FakePermissionBrokerClient::CheckPathAccess(
-    const std::string& path,
-    const ResultCallback& callback) {
-  callback.Run(true);
+void FakePermissionBrokerClient::CheckPathAccess(const std::string& path,
+                                                 ResultCallback callback) {
+  std::move(callback).Run(true);
 }
 
 void FakePermissionBrokerClient::OpenPath(const std::string& path,
-                                          const OpenPathCallback& callback,
-                                          const ErrorCallback& error_callback) {
+                                          OpenPathCallback callback,
+                                          ErrorCallback error_callback) {
   base::PostTaskWithTraits(
       FROM_HERE,
       {base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
-      base::BindOnce(&chromeos::OpenPath, path, callback, error_callback,
+      base::BindOnce(&chromeos::OpenPath, path, std::move(callback),
+                     std::move(error_callback),
                      base::ThreadTaskRunnerHandle::Get()));
 }
 
@@ -86,8 +87,8 @@ void FakePermissionBrokerClient::RequestTcpPortAccess(
     uint16_t port,
     const std::string& interface,
     int lifeline_fd,
-    const ResultCallback& callback) {
-  callback.Run(
+    ResultCallback callback) {
+  std::move(callback).Run(
       RequestPortImpl(port, interface, tcp_deny_rule_set_, &tcp_hole_set_));
 }
 
@@ -95,23 +96,21 @@ void FakePermissionBrokerClient::RequestUdpPortAccess(
     uint16_t port,
     const std::string& interface,
     int lifeline_fd,
-    const ResultCallback& callback) {
-  callback.Run(
+    ResultCallback callback) {
+  std::move(callback).Run(
       RequestPortImpl(port, interface, udp_deny_rule_set_, &udp_hole_set_));
 }
 
-void FakePermissionBrokerClient::ReleaseTcpPort(
-    uint16_t port,
-    const std::string& interface,
-    const ResultCallback& callback) {
-  callback.Run(tcp_hole_set_.erase(std::make_pair(port, interface)));
+void FakePermissionBrokerClient::ReleaseTcpPort(uint16_t port,
+                                                const std::string& interface,
+                                                ResultCallback callback) {
+  std::move(callback).Run(tcp_hole_set_.erase(std::make_pair(port, interface)));
 }
 
-void FakePermissionBrokerClient::ReleaseUdpPort(
-    uint16_t port,
-    const std::string& interface,
-    const ResultCallback& callback) {
-  callback.Run(udp_hole_set_.erase(std::make_pair(port, interface)));
+void FakePermissionBrokerClient::ReleaseUdpPort(uint16_t port,
+                                                const std::string& interface,
+                                                ResultCallback callback) {
+  std::move(callback).Run(udp_hole_set_.erase(std::make_pair(port, interface)));
 }
 
 void FakePermissionBrokerClient::AddTcpDenyRule(uint16_t port,

chromeos/dbus/permission_broker/fake_permission_broker_client.h

@@ -27,24 +27,24 @@ class COMPONENT_EXPORT(PERMISSION_BROKER) FakePermissionBrokerClient
   static FakePermissionBrokerClient* Get();
 
   void CheckPathAccess(const std::string& path,
-                       const ResultCallback& callback) override;
+                       ResultCallback callback) override;
   void OpenPath(const std::string& path,
-                const OpenPathCallback& callback,
-                const ErrorCallback& error_callback) override;
+                OpenPathCallback callback,
+                ErrorCallback error_callback) override;
   void RequestTcpPortAccess(uint16_t port,
                             const std::string& interface,
                             int lifeline_fd,
-                            const ResultCallback& callback) override;
+                            ResultCallback callback) override;
   void RequestUdpPortAccess(uint16_t port,
                             const std::string& interface,
                             int lifeline_fd,
-                            const ResultCallback& callback) override;
+                            ResultCallback callback) override;
   void ReleaseTcpPort(uint16_t port,
                       const std::string& interface,
-                      const ResultCallback& callback) override;
+                      ResultCallback callback) override;
   void ReleaseUdpPort(uint16_t port,
                       const std::string& interface,
-                      const ResultCallback& callback) override;
+                      ResultCallback callback) override;
 
   // Add a rule to have RequestTcpPortAccess fail.
   void AddTcpDenyRule(uint16_t port, const std::string& interface);

chromeos/dbus/permission_broker/permission_broker_client.cc

@@ -42,34 +42,35 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
   ~PermissionBrokerClientImpl() override = default;
 
   void CheckPathAccess(const std::string& path,
-                       const ResultCallback& callback) override {
+                       ResultCallback callback) override {
     dbus::MethodCall method_call(kPermissionBrokerInterface, kCheckPathAccess);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(path);
     proxy_->CallMethod(
         &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::BindOnce(&PermissionBrokerClientImpl::OnResponse,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
   void OpenPath(const std::string& path,
-                const OpenPathCallback& callback,
-                const ErrorCallback& error_callback) override {
+                OpenPathCallback callback,
+                ErrorCallback error_callback) override {
     dbus::MethodCall method_call(kPermissionBrokerInterface, kOpenPath);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(path);
     proxy_->CallMethodWithErrorCallback(
         &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::BindOnce(&PermissionBrokerClientImpl::OnOpenPathResponse,
-                       weak_ptr_factory_.GetWeakPtr(), callback),
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)),
         base::BindOnce(&PermissionBrokerClientImpl::OnError,
-                       weak_ptr_factory_.GetWeakPtr(), error_callback));
+                       weak_ptr_factory_.GetWeakPtr(),
+                       std::move(error_callback)));
   }
 
   void RequestTcpPortAccess(uint16_t port,
                             const std::string& interface,
                             int lifeline_fd,
-                            const ResultCallback& callback) override {
+                            ResultCallback callback) override {
     dbus::MethodCall method_call(kPermissionBrokerInterface,
                                  kRequestTcpPortAccess);
     dbus::MessageWriter writer(&method_call);
@@ -79,13 +80,13 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
     proxy_->CallMethod(
         &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::BindOnce(&PermissionBrokerClientImpl::OnResponse,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
   void RequestUdpPortAccess(uint16_t port,
                             const std::string& interface,
                             int lifeline_fd,
-                            const ResultCallback& callback) override {
+                            ResultCallback callback) override {
     dbus::MethodCall method_call(kPermissionBrokerInterface,
                                  kRequestUdpPortAccess);
     dbus::MessageWriter writer(&method_call);
@@ -95,12 +96,12 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
     proxy_->CallMethod(
         &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::BindOnce(&PermissionBrokerClientImpl::OnResponse,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
   void ReleaseTcpPort(uint16_t port,
                       const std::string& interface,
-                      const ResultCallback& callback) override {
+                      ResultCallback callback) override {
     dbus::MethodCall method_call(kPermissionBrokerInterface, kReleaseTcpPort);
     dbus::MessageWriter writer(&method_call);
     writer.AppendUint16(port);
@@ -108,12 +109,12 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
     proxy_->CallMethod(
         &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::BindOnce(&PermissionBrokerClientImpl::OnResponse,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
   void ReleaseUdpPort(uint16_t port,
                       const std::string& interface,
-                      const ResultCallback& callback) override {
+                      ResultCallback callback) override {
     dbus::MethodCall method_call(kPermissionBrokerInterface, kReleaseUdpPort);
     dbus::MessageWriter writer(&method_call);
     writer.AppendUint16(port);
@@ -121,7 +122,7 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
     proxy_->CallMethod(
         &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::BindOnce(&PermissionBrokerClientImpl::OnResponse,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
   void Init(dbus::Bus* bus) {
@@ -133,10 +134,10 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
  private:
   // Handle a DBus response from the permission broker, invoking the callback
   // that the method was originally called with with the success response.
-  void OnResponse(const ResultCallback& callback, dbus::Response* response) {
+  void OnResponse(ResultCallback callback, dbus::Response* response) {
     if (!response) {
       LOG(WARNING) << "Access request method call failed.";
-      callback.Run(false);
+      std::move(callback).Run(false);
       return;
     }
 
@@ -144,19 +145,18 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
     dbus::MessageReader reader(response);
     if (!reader.PopBool(&result))
       LOG(WARNING) << "Could not parse response: " << response->ToString();
-    callback.Run(result);
+    std::move(callback).Run(result);
   }
 
-  void OnOpenPathResponse(const OpenPathCallback& callback,
-                          dbus::Response* response) {
+  void OnOpenPathResponse(OpenPathCallback callback, dbus::Response* response) {
     base::ScopedFD fd;
     dbus::MessageReader reader(response);
     if (!reader.PopFileDescriptor(&fd))
       LOG(WARNING) << "Could not parse response: " << response->ToString();
-    callback.Run(std::move(fd));
+    std::move(callback).Run(std::move(fd));
   }
 
-  void OnError(const ErrorCallback& callback, dbus::ErrorResponse* response) {
+  void OnError(ErrorCallback callback, dbus::ErrorResponse* response) {
     std::string error_name;
     std::string error_message;
     if (response) {
@@ -166,7 +166,7 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
     } else {
       error_name = kNoResponseError;
     }
-    callback.Run(error_name, error_message);
+    std::move(callback).Run(error_name, error_message);
   }
 
   dbus::ObjectProxy* proxy_ = nullptr;

chromeos/dbus/permission_broker/permission_broker_client.h

@@ -32,16 +32,15 @@ class COMPONENT_EXPORT(PERMISSION_BROKER) PermissionBrokerClient {
   // The ResultCallback is used for both the RequestPathAccess and
   // RequestUsbAccess methods. Its boolean parameter represents the result of
   // the operation that it was submitted alongside.
-  typedef base::Callback<void(bool)> ResultCallback;
+  using ResultCallback = base::OnceCallback<void(bool)>;
 
   // An OpenPathCallback callback is run when an OpenPath request is completed.
-  typedef base::Callback<void(base::ScopedFD)> OpenPathCallback;
+  using OpenPathCallback = base::OnceCallback<void(base::ScopedFD)>;
 
   // An ErrorCallback callback is run when an error is returned by the
   // permission broker.
-  typedef base::Callback<void(const std::string& error_name,
-                              const std::string& message)>
-      ErrorCallback;
+  using ErrorCallback = base::OnceCallback<void(const std::string& error_name,
+                                                const std::string& message)>;
 
   // Creates and initializes the global instance. |bus| must not be null.
   static void Initialize(dbus::Bus* bus);
@@ -60,14 +59,14 @@ class COMPONENT_EXPORT(PERMISSION_BROKER) PermissionBrokerClient {
   // the |interface_id| value passed to RequestPathAccess will be
   // UsbDevicePermissionsData::ANY_INTERFACE).
   virtual void CheckPathAccess(const std::string& path,
-                               const ResultCallback& callback) = 0;
+                               ResultCallback callback) = 0;
 
   // OpenPath requests that the permission broker open the device node
   // identified by |path| and return the resulting file descriptor. One of
   // |callback| or |error_callback| is called.
   virtual void OpenPath(const std::string& path,
-                        const OpenPathCallback& callback,
-                        const ErrorCallback& error_callback) = 0;
+                        OpenPathCallback callback,
+                        ErrorCallback error_callback) = 0;
 
   // Requests the |port| be opened on the firewall for incoming TCP/IP
   // connections received on |interface| (an empty string indicates all
@@ -78,7 +77,7 @@ class COMPONENT_EXPORT(PERMISSION_BROKER) PermissionBrokerClient {
   virtual void RequestTcpPortAccess(uint16_t port,
                                     const std::string& interface,
                                     int lifeline_fd,
-                                    const ResultCallback& callback) = 0;
+                                    ResultCallback callback) = 0;
 
   // Requests the |port| be opened on the firewall for incoming UDP packets
   // received on |interface| (an empty string indicates all interfaces). One end
@@ -89,21 +88,21 @@ class COMPONENT_EXPORT(PERMISSION_BROKER) PermissionBrokerClient {
   virtual void RequestUdpPortAccess(uint16_t port,
                                     const std::string& interface,
                                     int lifeline_fd,
-                                    const ResultCallback& callback) = 0;
+                                    ResultCallback callback) = 0;
 
   // Releases a request for an open firewall port for TCP/IP connections. The
   // |port| and |interface| parameters must be the same as a previous call to
   // RequestTcpPortAccess.
   virtual void ReleaseTcpPort(uint16_t port,
                               const std::string& interface,
-                              const ResultCallback& callback) = 0;
+                              ResultCallback callback) = 0;
 
   // Releases a request for an open firewall port for UDP packets. The |port|
   // and |interface| parameters must be the same as a previous call to
   // RequestUdpPortAccess.
   virtual void ReleaseUdpPort(uint16_t port,
                               const std::string& interface,
-                              const ResultCallback& callback) = 0;
+                              ResultCallback callback) = 0;
 
  protected:
   // Initialize/Shutdown should be used instead.

chromeos/network/firewall_hole.cc

@@ -60,9 +60,9 @@ void FirewallHole::Open(PortType type,
   base::ScopedFD lifeline_local(lifeline[0]);
   base::ScopedFD lifeline_remote(lifeline[1]);
 
-  base::Callback<void(bool)> access_granted_closure =
-      base::Bind(&FirewallHole::PortAccessGranted, type, port, interface,
-                 base::Passed(&lifeline_local), callback);
+  base::OnceCallback<void(bool)> access_granted_closure =
+      base::BindOnce(&FirewallHole::PortAccessGranted, type, port, interface,
+                     std::move(lifeline_local), callback);
 
   PermissionBrokerClient* client = PermissionBrokerClient::Get();
   DCHECK(client) << "Could not get permission broker client.";
@@ -70,27 +70,29 @@ void FirewallHole::Open(PortType type,
   switch (type) {
     case PortType::TCP:
       client->RequestTcpPortAccess(port, interface, lifeline_remote.get(),
-                                   access_granted_closure);
+                                   std::move(access_granted_closure));
       return;
     case PortType::UDP:
       client->RequestUdpPortAccess(port, interface, lifeline_remote.get(),
-                                   access_granted_closure);
+                                   std::move(access_granted_closure));
       return;
   }
 }
 
 FirewallHole::~FirewallHole() {
-  base::Callback<void(bool)> port_released_closure = base::Bind(
-      &PortReleased, type_, port_, interface_, base::Passed(&lifeline_fd_));
+  base::OnceCallback<void(bool)> port_released_closure = base::BindOnce(
+      &PortReleased, type_, port_, interface_, std::move(lifeline_fd_));
 
   PermissionBrokerClient* client = PermissionBrokerClient::Get();
   DCHECK(client) << "Could not get permission broker client.";
   switch (type_) {
     case PortType::TCP:
-      client->ReleaseTcpPort(port_, interface_, port_released_closure);
+      client->ReleaseTcpPort(port_, interface_,
+                             std::move(port_released_closure));
       return;
     case PortType::UDP:
-      client->ReleaseUdpPort(port_, interface_, port_released_closure);
+      client->ReleaseUdpPort(port_, interface_,
+                             std::move(port_released_closure));
       return;
   }
 }

components/arc/usb/usb_host_bridge.cc

@@ -159,8 +159,9 @@ void ArcUsbHostBridge::OpenDevice(const std::string& guid,
   auto repeating_callback =
       base::AdaptCallbackForRepeating(std::move(callback));
   chromeos::PermissionBrokerClient::Get()->OpenPath(
-      device->device_path(), base::Bind(&OnDeviceOpened, repeating_callback),
-      base::Bind(&OnDeviceOpenError, repeating_callback));
+      device->device_path(),
+      base::BindOnce(&OnDeviceOpened, repeating_callback),
+      base::BindOnce(&OnDeviceOpenError, repeating_callback));
 }
 
 void ArcUsbHostBridge::OpenDeviceDeprecated(

device/usb/mojo/device_manager_impl.cc

@@ -97,10 +97,10 @@ void DeviceManagerImpl::OpenFileDescriptor(
         static_cast<device::UsbDeviceLinux*>(device.get())->device_path();
     chromeos::PermissionBrokerClient::Get()->OpenPath(
         devpath,
-        base::BindRepeating(&DeviceManagerImpl::OnOpenFileDescriptor,
-                            weak_factory_.GetWeakPtr(), copyable_callback),
-        base::BindRepeating(&DeviceManagerImpl::OnOpenFileDescriptorError,
-                            weak_factory_.GetWeakPtr(), copyable_callback));
+        base::BindOnce(&DeviceManagerImpl::OnOpenFileDescriptor,
+                       weak_factory_.GetWeakPtr(), copyable_callback),
+        base::BindOnce(&DeviceManagerImpl::OnOpenFileDescriptorError,
+                       weak_factory_.GetWeakPtr(), copyable_callback));
   }
 }
 

device/usb/usb_device_linux.cc

@@ -48,8 +48,8 @@ UsbDeviceLinux::~UsbDeviceLinux() = default;
 
 void UsbDeviceLinux::CheckUsbAccess(ResultCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  chromeos::PermissionBrokerClient::Get()->CheckPathAccess(
-      device_path_, base::AdaptCallbackForRepeating(std::move(callback)));
+  chromeos::PermissionBrokerClient::Get()->CheckPathAccess(device_path_,
+                                                           std::move(callback));
 }
 
 #endif  // defined(OS_CHROMEOS)
@@ -61,9 +61,10 @@ void UsbDeviceLinux::Open(OpenCallback callback) {
   auto copyable_callback = base::AdaptCallbackForRepeating(std::move(callback));
   chromeos::PermissionBrokerClient::Get()->OpenPath(
       device_path_,
-      base::Bind(&UsbDeviceLinux::OnOpenRequestComplete, this,
-                 copyable_callback),
-      base::Bind(&UsbDeviceLinux::OnOpenRequestError, this, copyable_callback));
+      base::BindOnce(&UsbDeviceLinux::OnOpenRequestComplete, this,
+                     copyable_callback),
+      base::BindOnce(&UsbDeviceLinux::OnOpenRequestError, this,
+                     copyable_callback));
 #else
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner =
       UsbService::CreateBlockingTaskRunner();

extensions/browser/api/device_permissions_prompt.cc

@@ -262,8 +262,8 @@ class HidDevicePermissionsPrompt : public DevicePermissionsPrompt::Prompt,
 #if defined(OS_CHROMEOS)
       chromeos::PermissionBrokerClient::Get()->CheckPathAccess(
           device_info.get()->device()->device_node,
-          base::Bind(&HidDevicePermissionsPrompt::AddCheckedDevice, this,
-                     base::Passed(&device_info)));
+          base::BindOnce(&HidDevicePermissionsPrompt::AddCheckedDevice, this,
+                         std::move(device_info)));
 #else
       AddCheckedDevice(std::move(device_info), true);
 #endif  // defined(OS_CHROMEOS)

services/device/hid/hid_service_linux.cc

@@ -225,12 +225,12 @@ void HidServiceLinux::Connect(const std::string& device_guid,
 
 #if defined(OS_CHROMEOS)
   chromeos::PermissionBrokerClient::ErrorCallback error_callback =
-      base::Bind(&HidServiceLinux::OnPathOpenError,
-                 params->device_info->device_node(), params->callback);
+      base::BindOnce(&HidServiceLinux::OnPathOpenError,
+                     params->device_info->device_node(), params->callback);
   chromeos::PermissionBrokerClient::Get()->OpenPath(
       device_info->device_node(),
-      base::Bind(&HidServiceLinux::OnPathOpenComplete, base::Passed(&params)),
-      error_callback);
+      base::BindOnce(&HidServiceLinux::OnPathOpenComplete, std::move(params)),
+      std::move(error_callback));
 #else
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner =
       params->blocking_task_runner;