[Code health] Convert callbacks in AdbClientSocket to Once/Repeating

Bug: 1152274
Change-Id: I81358a8df4b853654ba031bce0a251ed8445a8ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2586156
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836715}

chrome/browser/devtools/device/adb/adb_client_socket.cc

@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/callback_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
@@ -47,23 +48,24 @@ class AdbTransportSocket : public AdbClientSocket {
   AdbTransportSocket(int port,
                      const std::string& serial,
                      const std::string& socket_name,
-                     const SocketCallback& callback)
+                     SocketCallback callback)
       : AdbClientSocket(port),
         serial_(serial),
         socket_name_(socket_name),
-      callback_(callback) {
+        callback_(std::move(callback)) {
     Connect(base::BindOnce(&AdbTransportSocket::OnConnected,
                            base::Unretained(this)));
   }
 
  private:
-  ~AdbTransportSocket() {}
+  ~AdbTransportSocket() { DCHECK(callback_.is_null()); }
 
   void OnConnected(int result) {
     if (!CheckNetResultOrDie(result))
       return;
     SendCommand(base::StringPrintf(kHostTransportCommand, serial_.c_str()),
-        true, base::Bind(&AdbTransportSocket::SendLocalAbstract,
+                true,
+                base::BindOnce(&AdbTransportSocket::SendLocalAbstract,
                                base::Unretained(this)));
   }
 
@@ -71,21 +73,22 @@ class AdbTransportSocket : public AdbClientSocket {
     if (!CheckNetResultOrDie(result))
       return;
     SendCommand(socket_name_, true,
-                base::Bind(&AdbTransportSocket::OnSocketAvailable,
+                base::BindOnce(&AdbTransportSocket::OnSocketAvailable,
                                base::Unretained(this)));
   }
 
   void OnSocketAvailable(int result, const std::string& response) {
     if (!CheckNetResultOrDie(result))
       return;
-    callback_.Run(net::OK, std::move(socket_));
+    std::move(callback_).Run(net::OK, std::move(socket_));
     delete this;
   }
 
   bool CheckNetResultOrDie(int result) {
     if (result >= 0)
       return true;
-    callback_.Run(result, base::WrapUnique<net::StreamSocket>(NULL));
+    std::move(callback_).Run(result,
+                             base::WrapUnique<net::StreamSocket>(nullptr));
     delete this;
     return false;
   }
@@ -97,12 +100,10 @@ class AdbTransportSocket : public AdbClientSocket {
 
 class AdbQuerySocket : AdbClientSocket {
  public:
-  AdbQuerySocket(int port,
-                 const std::string& query,
-                 const CommandCallback& callback)
+  AdbQuerySocket(int port, const std::string& query, CommandCallback callback)
       : AdbClientSocket(port),
         current_query_(0),
-        callback_(callback) {
+        callback_(std::move(callback)) {
     queries_ = base::SplitString(query, "|", base::KEEP_WHITESPACE,
                                  base::SPLIT_WANT_NONEMPTY);
     if (queries_.empty()) {
@@ -114,8 +115,7 @@ class AdbQuerySocket : AdbClientSocket {
   }
 
  private:
-  ~AdbQuerySocket() {
-  }
+  ~AdbQuerySocket() { DCHECK(callback_.is_null()); }
 
   void SendNextQuery(int result) {
     if (!CheckNetResultOrDie(result))
@@ -134,7 +134,7 @@ class AdbQuerySocket : AdbClientSocket {
     if (++current_query_ < queries_.size()) {
       SendNextQuery(net::OK);
     } else {
-      callback_.Run(result, response);
+      std::move(callback_).Run(result, response);
       delete this;
     }
   }
@@ -142,7 +142,7 @@ class AdbQuerySocket : AdbClientSocket {
   bool CheckNetResultOrDie(int result) {
     if (result >= 0)
       return true;
-    callback_.Run(result, std::string());
+    std::move(callback_).Run(result, std::string());
     delete this;
     return false;
   }
@@ -157,16 +157,16 @@ class AdbQuerySocket : AdbClientSocket {
 // static
 void AdbClientSocket::AdbQuery(int port,
                                const std::string& query,
-                               const CommandCallback& callback) {
-  new AdbQuerySocket(port, query, callback);
+                               CommandCallback callback) {
+  new AdbQuerySocket(port, query, std::move(callback));
 }
 
 // static
 void AdbClientSocket::TransportQuery(int port,
                                      const std::string& serial,
                                      const std::string& socket_name,
-                                     const SocketCallback& callback) {
-  new AdbTransportSocket(port, serial, socket_name, callback);
+                                     SocketCallback callback) {
+  new AdbTransportSocket(port, serial, socket_name, std::move(callback));
 }
 
 AdbClientSocket::AdbClientSocket(int port)
@@ -200,7 +200,7 @@ void AdbClientSocket::Connect(net::CompletionOnceCallback callback) {
 
 void AdbClientSocket::SendCommand(const std::string& command,
                                   bool is_void,
-                                  const CommandCallback& callback) {
+                                  CommandCallback callback) {
   scoped_refptr<net::StringIOBuffer> request_buffer =
       base::MakeRefCounted<net::StringIOBuffer>(EncodeMessage(command));
   net::NetworkTrafficAnnotationTag traffic_annotation =
@@ -226,71 +226,75 @@ void AdbClientSocket::SendCommand(const std::string& command,
             "This is not a network request and is only used for remote "
             "debugging."
         })");
-  int result =
-      socket_->Write(request_buffer.get(), request_buffer->size(),
-                     base::BindOnce(&AdbClientSocket::ReadResponse,
-                                    base::Unretained(this), callback, is_void),
-                     traffic_annotation);
+
+  base::RepeatingCallback<void(int)> on_response =
+      base::AdaptCallbackForRepeating(
+          base::BindOnce(&AdbClientSocket::ReadResponse, base::Unretained(this),
+                         std::move(callback), is_void));
+  int result = socket_->Write(request_buffer.get(), request_buffer->size(),
+                              on_response, traffic_annotation);
   if (result != net::ERR_IO_PENDING)
-    ReadResponse(callback, is_void, result);
+    on_response.Run(result);
 }
 
-void AdbClientSocket::ReadResponse(const CommandCallback& callback,
+void AdbClientSocket::ReadResponse(CommandCallback callback,
                                    bool is_void,
                                    int result) {
   if (result < 0) {
-    callback.Run(result, "IO error");
+    std::move(callback).Run(result, "IO error");
     return;
   }
   scoped_refptr<net::IOBuffer> response_buffer =
       base::MakeRefCounted<net::IOBuffer>(kBufferSize);
-  result = socket_->Read(
-      response_buffer.get(), kBufferSize,
-      base::BindOnce(&AdbClientSocket::OnResponseHeader, base::Unretained(this),
-                     callback, is_void, response_buffer));
+  base::RepeatingCallback<void(int)> on_response_header =
+      base::AdaptCallbackForRepeating(base::BindOnce(
+          &AdbClientSocket::OnResponseHeader, base::Unretained(this),
+          std::move(callback), is_void, response_buffer));
+  result =
+      socket_->Read(response_buffer.get(), kBufferSize, on_response_header);
   if (result != net::ERR_IO_PENDING)
-    OnResponseHeader(callback, is_void, response_buffer, result);
+    on_response_header.Run(result);
 }
 
 void AdbClientSocket::OnResponseHeader(
-    const CommandCallback& callback,
+    CommandCallback callback,
     bool is_void,
     scoped_refptr<net::IOBuffer> response_buffer,
     int result) {
   if (result <= 0) {
-    callback.Run(result == 0 ? net::ERR_CONNECTION_CLOSED : result,
+    std::move(callback).Run(result == 0 ? net::ERR_CONNECTION_CLOSED : result,
                             "IO error");
     return;
   }
 
   std::string data = std::string(response_buffer->data(), result);
   if (result < 4) {
-    callback.Run(net::ERR_FAILED, "Response is too short: " + data);
+    std::move(callback).Run(net::ERR_FAILED, "Response is too short: " + data);
     return;
   }
 
   std::string status = data.substr(0, 4);
   if (status != kOkayResponse) {
-    callback.Run(net::ERR_FAILED, data);
+    std::move(callback).Run(net::ERR_FAILED, data);
     return;
   }
 
   // Trim OKAY.
   data = data.substr(4);
   if (!is_void)
-    OnResponseData(callback, data, response_buffer, -1, 0);
+    OnResponseData(std::move(callback), data, response_buffer, -1, 0);
   else
-    callback.Run(net::OK, data);
+    std::move(callback).Run(net::OK, data);
 }
 
 void AdbClientSocket::OnResponseData(
-    const CommandCallback& callback,
+    CommandCallback callback,
     const std::string& response,
     scoped_refptr<net::IOBuffer> response_buffer,
     int bytes_left,
     int result) {
   if (result < 0) {
-    callback.Run(result, "IO error");
+    std::move(callback).Run(result, "IO error");
     return;
   }
 
@@ -310,17 +314,21 @@ void AdbClientSocket::OnResponseData(
   }
 
   if (bytes_left == 0) {
-    callback.Run(net::OK, new_response);
+    std::move(callback).Run(net::OK, new_response);
     return;
   }
 
   // Read tail
+  base::RepeatingCallback<void(int, const std::string&)> on_finished =
+      base::AdaptCallbackForRepeating(std::move(callback));
   result = socket_->Read(
       response_buffer.get(), kBufferSize,
       base::BindOnce(&AdbClientSocket::OnResponseData, base::Unretained(this),
-                     callback, new_response, response_buffer, bytes_left));
-  if (result > 0)
-    OnResponseData(callback, new_response, response_buffer, bytes_left, result);
-  else if (result != net::ERR_IO_PENDING)
-    callback.Run(net::OK, new_response);
+                     on_finished, new_response, response_buffer, bytes_left));
+  if (result > 0) {
+    OnResponseData(on_finished, new_response, response_buffer, bytes_left,
+                   result);
+  } else if (result != net::ERR_IO_PENDING) {
+    on_finished.Run(net::OK, new_response);
+  }
 }

chrome/browser/devtools/device/adb/adb_client_socket.h

@@ -14,19 +14,19 @@
 
 class AdbClientSocket {
  public:
-  typedef base::Callback<void(int, const std::string&)> CommandCallback;
-  typedef base::Callback<void(int result, std::unique_ptr<net::StreamSocket>)>
+  typedef base::OnceCallback<void(int, const std::string&)> CommandCallback;
+  typedef base::OnceCallback<void(int result,
+                                  std::unique_ptr<net::StreamSocket>)>
       SocketCallback;
 
   static void AdbQuery(int port,
                        const std::string& query,
-                       const CommandCallback& callback);
-
+                       CommandCallback callback);
 
   static void TransportQuery(int port,
                              const std::string& serial,
                              const std::string& socket_name,
-                             const SocketCallback& callback);
+                             SocketCallback callback);
 
  protected:
   explicit AdbClientSocket(int port);
@@ -36,19 +36,19 @@ class AdbClientSocket {
 
   void SendCommand(const std::string& command,
                    bool is_void,
-                   const CommandCallback& callback);
+                   CommandCallback callback);
 
   std::unique_ptr<net::StreamSocket> socket_;
 
  private:
-  void ReadResponse(const CommandCallback& callback, bool is_void, int result);
+  void ReadResponse(CommandCallback callback, bool is_void, int result);
 
-  void OnResponseHeader(const CommandCallback& callback,
+  void OnResponseHeader(CommandCallback callback,
                         bool is_void,
                         scoped_refptr<net::IOBuffer> response_buffer,
                         int result);
 
-  void OnResponseData(const CommandCallback& callback,
+  void OnResponseData(CommandCallback callback,
                       const std::string& response,
                       scoped_refptr<net::IOBuffer> response_buffer,
                       int bytes_left,

chrome/browser/devtools/device/tcp_device_provider.cc

@@ -14,7 +14,6 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/devtools/device/adb/adb_client_socket.h"
 #include "chrome/browser/net/system_network_context_manager.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -44,7 +43,7 @@ static void RunSocketCallback(
 class ResolveHostAndOpenSocket final : public network::ResolveHostClientBase {
  public:
   ResolveHostAndOpenSocket(const net::HostPortPair& address,
-                           const AdbClientSocket::SocketCallback& callback)
+                           const AndroidDeviceManager::SocketCallback& callback)
       : callback_(callback) {
     mojo::Remote<network::mojom::HostResolver> resolver;
     content::GetUIThreadTaskRunner({})->PostTask(
@@ -92,7 +91,7 @@ class ResolveHostAndOpenSocket final : public network::ResolveHostClientBase {
     delete this;
   }
 
-  AdbClientSocket::SocketCallback callback_;
+  AndroidDeviceManager::SocketCallback callback_;
   mojo::Receiver<network::mojom::ResolveHostClient> receiver_{this};
 };