Revert "Use CompletionOnceCallback in ServerSocket."

This reverts commit 920b13e9.

Reason for revert: developer reports Chrome crashes on every launch.

Original change's description:
> Use CompletionOnceCallback in ServerSocket.
> 
> Bug: 807724
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: Idb037fd78ab3f092af08bef665c4f65c786cdd53
> Reviewed-on: https://chromium-review.googlesource.com/1100900
> Reviewed-by: Eric Roman <eroman@chromium.org>
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
> Reviewed-by: Helen Li <xunjieli@chromium.org>
> Commit-Queue: Bence Béky <bnc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575237}

TBR=eroman@chromium.org,caseq@chromium.org,sergeyu@chromium.org,bnc@chromium.org,xunjieli@chromium.org

Change-Id: I4f9d3038a48070d2e78b8a2db319f0830abc31d6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 807724
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1138353Reviewed-by: Bence Béky <bnc@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575252}

content/browser/devtools/devtools_http_handler_unittest.cc

@@ -24,7 +24,6 @@
 #include "content/public/browser/devtools_socket_factory.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_utils.h"
-#include "net/base/completion_once_callback.h"
 #include "net/base/ip_address.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_errors.h"
@@ -53,7 +52,7 @@ class DummyServerSocket : public net::ServerSocket {
   }
 
   int Accept(std::unique_ptr<net::StreamSocket>* socket,
-             net::CompletionOnceCallback callback) override {
+             const net::CompletionCallback& callback) override {
     return net::ERR_IO_PENDING;
   }
 };

content/browser/renderer_host/p2p/socket_host_tcp_server.cc

@@ -4,8 +4,6 @@
 
 #include "content/browser/renderer_host/p2p/socket_host_tcp_server.h"
 
-#include <utility>
-
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "content/browser/renderer_host/p2p/socket_host_tcp.h"
@@ -27,8 +25,9 @@ P2PSocketHostTcpServer::P2PSocketHostTcpServer(IPC::Sender* message_sender,
     : P2PSocketHost(message_sender, socket_id, P2PSocketHost::TCP),
       client_type_(client_type),
       socket_(new net::TCPServerSocket(nullptr, net::NetLogSource())),
-      accept_callback_(base::BindRepeating(&P2PSocketHostTcpServer::OnAccepted,
-                                           base::Unretained(this))) {}
+      accept_callback_(base::Bind(&P2PSocketHostTcpServer::OnAccepted,
+                                  base::Unretained(this))) {
+}
 
 P2PSocketHostTcpServer::~P2PSocketHostTcpServer() {
   if (state_ == STATE_OPEN) {

content/browser/renderer_host/p2p/socket_host_tcp_server.h

@@ -17,7 +17,7 @@
 #include "content/common/content_export.h"
 #include "content/common/p2p_socket_type.h"
 #include "ipc/ipc_sender.h"
-#include "net/base/completion_repeating_callback.h"
+#include "net/base/completion_callback.h"
 #include "net/socket/tcp_server_socket.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 
@@ -68,7 +68,7 @@ class CONTENT_EXPORT P2PSocketHostTcpServer : public P2PSocketHost {
   std::map<net::IPEndPoint, std::unique_ptr<net::StreamSocket>>
       accepted_sockets_;
 
-  const net::CompletionRepeatingCallback accept_callback_;
+  net::CompletionCallback accept_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(P2PSocketHostTcpServer);
 };

content/browser/renderer_host/p2p/socket_host_tcp_server_unittest.cc

@@ -7,11 +7,10 @@
 #include <stdint.h>
 
 #include <list>
-#include <utility>
 
 #include "content/browser/renderer_host/p2p/socket_host_tcp.h"
 #include "content/browser/renderer_host/p2p/socket_host_test_utils.h"
-#include "net/base/completion_once_callback.h"
+#include "net/base/completion_callback.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -36,7 +35,12 @@ class FakeServerSocket : public net::ServerSocket {
       accept_socket_->reset(socket);
       accept_socket_ = nullptr;
 
-      std::move(accept_callback_).Run(net::OK);
+      // This copy is necessary because this implementation of ServerSocket
+      // bases logic on the null-ness of |accept_callback_| in the bound
+      // callback.
+      net::CompletionCallback cb = accept_callback_;
+      accept_callback_.Reset();
+      std::move(cb).Run(net::OK);
     } else {
       incoming_sockets_.push_back(socket);
     }
@@ -54,7 +58,7 @@ class FakeServerSocket : public net::ServerSocket {
   }
 
   int Accept(std::unique_ptr<net::StreamSocket>* socket,
-             net::CompletionOnceCallback callback) override {
+             const net::CompletionCallback& callback) override {
     DCHECK(socket);
     if (!incoming_sockets_.empty()) {
       socket->reset(incoming_sockets_.front());
@@ -62,7 +66,7 @@ class FakeServerSocket : public net::ServerSocket {
       return net::OK;
     } else {
       accept_socket_ = socket;
-      accept_callback_ = std::move(callback);
+      accept_callback_ = callback;
       return net::ERR_IO_PENDING;
     }
   }
@@ -73,7 +77,7 @@ class FakeServerSocket : public net::ServerSocket {
   net::IPEndPoint local_address_;
 
   std::unique_ptr<net::StreamSocket>* accept_socket_;
-  net::CompletionOnceCallback accept_callback_;
+  net::CompletionCallback accept_callback_;
 
   std::list<net::StreamSocket*> incoming_sockets_;
 };

net/socket/fuzzed_server_socket.cc

@@ -4,8 +4,6 @@
 
 #include "net/socket/fuzzed_server_socket.h"
 
-#include <utility>
-
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -36,12 +34,11 @@ int FuzzedServerSocket::GetLocalAddress(IPEndPoint* address) const {
 }
 
 int FuzzedServerSocket::Accept(std::unique_ptr<StreamSocket>* socket,
-                               CompletionOnceCallback callback) {
+                               const CompletionCallback& callback) {
   if (first_accept_) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(&FuzzedServerSocket::DispatchAccept,
-                                  weak_factory_.GetWeakPtr(), socket,
-                                  std::move(callback)));
+        FROM_HERE, base::Bind(&FuzzedServerSocket::DispatchAccept,
+                              weak_factory_.GetWeakPtr(), socket, callback));
   }
   first_accept_ = false;
 
@@ -49,15 +46,15 @@ int FuzzedServerSocket::Accept(std::unique_ptr<StreamSocket>* socket,
 }
 
 void FuzzedServerSocket::DispatchAccept(std::unique_ptr<StreamSocket>* socket,
-                                        CompletionOnceCallback callback) {
+                                        const CompletionCallback& callback) {
   std::unique_ptr<FuzzedSocket> connected_socket(
       std::make_unique<FuzzedSocket>(data_provider_, net_log_));
   // The Connect call should always succeed synchronously, without using the
   // callback, since connected_socket->set_fuzz_connect_result(true) has not
   // been called.
-  CHECK_EQ(net::OK, connected_socket->Connect(CompletionOnceCallback()));
+  CHECK_EQ(net::OK, connected_socket->Connect(callback));
   *socket = std::move(connected_socket);
-  std::move(callback).Run(OK);
+  callback.Run(OK);
 }
 
 }  // namespace net

net/socket/fuzzed_server_socket.h

@@ -12,7 +12,6 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "net/base/completion_once_callback.h"
 #include "net/base/ip_endpoint.h"
 #include "net/socket/server_socket.h"
 
@@ -42,11 +41,11 @@ class FuzzedServerSocket : public ServerSocket {
   int GetLocalAddress(IPEndPoint* address) const override;
 
   int Accept(std::unique_ptr<StreamSocket>* socket,
-             CompletionOnceCallback callback) override;
+             const CompletionCallback& callback) override;
 
  private:
   void DispatchAccept(std::unique_ptr<StreamSocket>* socket,
-                      CompletionOnceCallback callback);
+                      const CompletionCallback& callback);
 
   base::FuzzedDataProvider* data_provider_;
   net::NetLog* net_log_;

net/socket/server_socket.h

@@ -11,7 +11,7 @@
 #include <string>
 
 #include "base/macros.h"
-#include "net/base/completion_once_callback.h"
+#include "net/base/completion_callback.h"
 #include "net/base/net_export.h"
 
 namespace net {
@@ -40,7 +40,7 @@ class NET_EXPORT ServerSocket {
   // Accepts connection. Callback is called when new connection is
   // accepted.
   virtual int Accept(std::unique_ptr<StreamSocket>* socket,
-                     CompletionOnceCallback callback) = 0;
+                     const CompletionCallback& callback) = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ServerSocket);

net/socket/tcp_server_socket.cc

@@ -55,7 +55,7 @@ int TCPServerSocket::GetLocalAddress(IPEndPoint* address) const {
 }
 
 int TCPServerSocket::Accept(std::unique_ptr<StreamSocket>* socket,
-                            CompletionOnceCallback callback) {
+                            const CompletionCallback& callback) {
   DCHECK(socket);
   DCHECK(!callback.is_null());
 
@@ -66,11 +66,11 @@ int TCPServerSocket::Accept(std::unique_ptr<StreamSocket>* socket,
 
   // It is safe to use base::Unretained(this). |socket_| is owned by this class,
   // and the callback won't be run after |socket_| is destroyed.
-  CompletionOnceCallback accept_callback =
-      base::BindOnce(&TCPServerSocket::OnAcceptCompleted,
-                     base::Unretained(this), socket, std::move(callback));
+  CompletionCallback accept_callback =
+      base::Bind(&TCPServerSocket::OnAcceptCompleted, base::Unretained(this),
+                 socket, callback);
   int result = socket_.Accept(&accepted_socket_, &accepted_address_,
-                              std::move(accept_callback));
+                              accept_callback);
   if (result != ERR_IO_PENDING) {
     // |accept_callback| won't be called so we need to run
     // ConvertAcceptedSocket() ourselves in order to do the conversion from
@@ -103,11 +103,11 @@ int TCPServerSocket::ConvertAcceptedSocket(
 
 void TCPServerSocket::OnAcceptCompleted(
     std::unique_ptr<StreamSocket>* output_accepted_socket,
-    CompletionOnceCallback forward_callback,
+    const CompletionCallback& forward_callback,
     int result) {
   result = ConvertAcceptedSocket(result, output_accepted_socket);
   pending_accept_ = false;
-  std::move(forward_callback).Run(result);
+  forward_callback.Run(result);
 }
 
 }  // namespace net

net/socket/tcp_server_socket.h

@@ -8,7 +8,6 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "net/base/completion_once_callback.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_export.h"
 #include "net/socket/server_socket.h"
@@ -36,7 +35,7 @@ class NET_EXPORT TCPServerSocket : public ServerSocket {
   int Listen(const IPEndPoint& address, int backlog) override;
   int GetLocalAddress(IPEndPoint* address) const override;
   int Accept(std::unique_ptr<StreamSocket>* socket,
-             CompletionOnceCallback callback) override;
+             const CompletionCallback& callback) override;
 
   // Detachs from the current thread, to allow the socket to be transferred to
   // a new thread. Should only be called when the object is no longer used by
@@ -53,7 +52,7 @@ class NET_EXPORT TCPServerSocket : public ServerSocket {
       std::unique_ptr<StreamSocket>* output_accepted_socket);
   // Completion callback for calling TCPSocket::Accept().
   void OnAcceptCompleted(std::unique_ptr<StreamSocket>* output_accepted_socket,
-                         CompletionOnceCallback forward_callback,
+                         const CompletionCallback& forward_callback,
                          int result);
 
   TCPSocket socket_;

net/socket/tcp_socket_win.cc

@@ -384,7 +384,7 @@ int TCPSocketWin::Listen(int backlog) {
 
 int TCPSocketWin::Accept(std::unique_ptr<TCPSocketWin>* socket,
                          IPEndPoint* address,
-                         CompletionOnceCallback callback) {
+                         const CompletionCallback& callback) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   DCHECK(socket);
   DCHECK(address);
@@ -402,7 +402,7 @@ int TCPSocketWin::Accept(std::unique_ptr<TCPSocketWin>* socket,
 
     accept_socket_ = socket;
     accept_address_ = address;
-    accept_callback_ = std::move(callback);
+    accept_callback_ = callback;
   }
 
   return result;

net/socket/tcp_socket_win.h

@@ -16,6 +16,7 @@
 #include "base/threading/thread_checker.h"
 #include "base/win/object_watcher.h"
 #include "net/base/address_family.h"
+#include "net/base/completion_callback.h"
 #include "net/base/completion_once_callback.h"
 #include "net/base/net_export.h"
 #include "net/log/net_log_with_source.h"
@@ -58,7 +59,7 @@ class NET_EXPORT TCPSocketWin : public base::win::ObjectWatcher::Delegate {
   int Listen(int backlog);
   int Accept(std::unique_ptr<TCPSocketWin>* socket,
              IPEndPoint* address,
-             CompletionOnceCallback callback);
+             const CompletionCallback& callback);
 
   int Connect(const IPEndPoint& address, CompletionOnceCallback callback);
   bool IsConnected() const;
@@ -161,7 +162,7 @@ class NET_EXPORT TCPSocketWin : public base::win::ObjectWatcher::Delegate {
 
   std::unique_ptr<TCPSocketWin>* accept_socket_;
   IPEndPoint* accept_address_;
-  CompletionOnceCallback accept_callback_;
+  CompletionCallback accept_callback_;
 
   // The various states that the socket could be in.
   bool waiting_connect_;

net/socket/unix_domain_server_socket_posix.cc

@@ -119,39 +119,36 @@ int UnixDomainServerSocket::GetLocalAddress(IPEndPoint* address) const {
 }
 
 int UnixDomainServerSocket::Accept(std::unique_ptr<StreamSocket>* socket,
-                                   CompletionOnceCallback callback) {
+                                   const CompletionCallback& callback) {
   DCHECK(socket);
-  DCHECK(callback);
-  DCHECK(!callback_);
 
-  callback_ = std::move(callback);
   SetterCallback setter_callback = base::Bind(&SetStreamSocket, socket);
-  return DoAccept(setter_callback);
+  return DoAccept(setter_callback, callback);
 }
 
 int UnixDomainServerSocket::AcceptSocketDescriptor(
     SocketDescriptor* socket,
-    CompletionOnceCallback callback) {
+    const CompletionCallback& callback) {
   DCHECK(socket);
-  DCHECK(callback);
-  DCHECK(!callback_);
 
-  callback_ = std::move(callback);
   SetterCallback setter_callback = base::Bind(&SetSocketDescriptor, socket);
-  return DoAccept(setter_callback);
+  return DoAccept(setter_callback, callback);
 }
 
-int UnixDomainServerSocket::DoAccept(const SetterCallback& setter_callback) {
+int UnixDomainServerSocket::DoAccept(const SetterCallback& setter_callback,
+                                     const CompletionCallback& callback) {
   DCHECK(!setter_callback.is_null());
-  DCHECK(!callback_.is_null());
+  DCHECK(!callback.is_null());
   DCHECK(listen_socket_);
   DCHECK(!accept_socket_);
 
   while (true) {
     int rv = listen_socket_->Accept(
         &accept_socket_,
-        base::BindOnce(&UnixDomainServerSocket::AcceptCompleted,
-                       base::Unretained(this), setter_callback));
+        base::Bind(&UnixDomainServerSocket::AcceptCompleted,
+                   base::Unretained(this),
+                   setter_callback,
+                   callback));
     if (rv != OK)
       return rv;
     if (AuthenticateAndGetStreamSocket(setter_callback))
@@ -163,22 +160,23 @@ int UnixDomainServerSocket::DoAccept(const SetterCallback& setter_callback) {
 
 void UnixDomainServerSocket::AcceptCompleted(
     const SetterCallback& setter_callback,
+    const CompletionCallback& callback,
     int rv) {
   if (rv != OK) {
-    std::move(callback_).Run(rv);
+    callback.Run(rv);
     return;
   }
 
   if (AuthenticateAndGetStreamSocket(setter_callback)) {
-    std::move(callback_).Run(OK);
+    callback.Run(OK);
     return;
   }
 
   // Accept another socket because authentication error should be transparent
   // to the caller.
-  rv = DoAccept(setter_callback);
+  rv = DoAccept(setter_callback, callback);
   if (rv != ERR_IO_PENDING)
-    std::move(callback_).Run(rv);
+    callback.Run(rv);
 }
 
 bool UnixDomainServerSocket::AuthenticateAndGetStreamSocket(

net/socket/unix_domain_server_socket_posix.h

@@ -14,7 +14,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "build/build_config.h"
-#include "net/base/completion_once_callback.h"
+
 #include "net/base/net_export.h"
 #include "net/socket/server_socket.h"
 #include "net/socket/socket_descriptor.h"
@@ -59,7 +59,7 @@ class NET_EXPORT UnixDomainServerSocket : public ServerSocket {
                                int backlog) override;
   int GetLocalAddress(IPEndPoint* address) const override;
   int Accept(std::unique_ptr<StreamSocket>* socket,
-             CompletionOnceCallback callback) override;
+             const CompletionCallback& callback) override;
 
   // Creates a server socket, binds it to the specified |socket_path| and
   // starts listening for incoming connections with the specified |backlog|.
@@ -68,7 +68,7 @@ class NET_EXPORT UnixDomainServerSocket : public ServerSocket {
   // Accepts an incoming connection on |listen_socket_|, but passes back
   // a raw SocketDescriptor instead of a StreamSocket.
   int AcceptSocketDescriptor(SocketDescriptor* socket_descriptor,
-                             CompletionOnceCallback callback);
+                             const CompletionCallback& callback);
 
  private:
   // A callback to wrap the setting of the out-parameter to Accept().
@@ -76,14 +76,15 @@ class NET_EXPORT UnixDomainServerSocket : public ServerSocket {
   // a manner that's agnostic to the caller's desired output.
   typedef base::Callback<void(std::unique_ptr<SocketPosix>)> SetterCallback;
 
-  int DoAccept(const SetterCallback& setter_callback);
+  int DoAccept(const SetterCallback& setter_callback,
+               const CompletionCallback& callback);
   void AcceptCompleted(const SetterCallback& setter_callback,
+                       const CompletionCallback& callback,
                        int rv);
   bool AuthenticateAndGetStreamSocket(const SetterCallback& setter_callback);
 
   std::unique_ptr<SocketPosix> listen_socket_;
   const AuthCallback auth_callback_;
-  CompletionOnceCallback callback_;
   const bool use_abstract_namespace_;
 
   std::unique_ptr<SocketPosix> accept_socket_;

services/network/tcp_socket_unittest.cc

@@ -17,6 +17,7 @@
 #include "base/threading/thread.h"
 #include "mojo/public/cpp/system/data_pipe_utils.h"
 #include "mojo/public/cpp/system/simple_watcher.h"
+#include "net/base/completion_callback.h"
 #include "net/base/completion_once_callback.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
@@ -56,13 +57,13 @@ class MockServerSocket : public net::ServerSocket {
   }
 
   int Accept(std::unique_ptr<net::StreamSocket>* socket,
-             net::CompletionOnceCallback callback) override {
+             const net::CompletionCallback& callback) override {
     DCHECK(accept_callback_.is_null());
     if (accept_result_ == net::OK && mode_ == net::SYNCHRONOUS)
       *socket = CreateMockAcceptSocket();
     if (mode_ == net::ASYNC || accept_result_ == net::ERR_IO_PENDING) {
       accept_socket_ = socket;
-      accept_callback_ = std::move(callback);
+      accept_callback_ = callback;
     }
     run_loop_.Quit();
 
@@ -107,7 +108,7 @@ class MockServerSocket : public net::ServerSocket {
 
   net::IoMode mode_ = net::SYNCHRONOUS;
   int accept_result_ = net::OK;
-  net::CompletionOnceCallback accept_callback_;
+  net::CompletionCallback accept_callback_;
   std::unique_ptr<net::StreamSocket>* accept_socket_;
   base::RunLoop run_loop_;
   std::vector<std::unique_ptr<net::StaticSocketDataProvider>> data_providers_;