[Blimp Net] Makes BlimpTransport.Connect async-only

BUG=558643

Review URL: https://codereview.chromium.org/1459923004

Cr-Commit-Position: refs/heads/master@{#360919}

6 files changed, 36 insertions, 26 deletions

diff --git a/blimp/net/blimp_transport.h b/blimp/net/blimp_transport.h
index 56d5e34..dcc68c6 100644
--- a/blimp/net/blimp_transport.h
+++ b/blimp/net/blimp_transport.h
@@ -22,14 +22,12 @@ class BlimpTransport {
 
   // Initiate or listen for a connection.
   //
-  // Returns net::OK if a connection is established synchronously.
-  // Returns an error code if a synchronous error occurred.
-  // Returns net::ERR_IO_PENDING if the connection is being established
-  // asynchronously. |callback| is later invoked with the connection outcome.
-  //
-  // If the connection is successful, the BlimpConnection can be taken by
-  // calling TakeConnectedSocket().
-  virtual int Connect(const net::CompletionCallback& callback) = 0;
+  // |callback| will be invoked with the connection outcome:
+  //   * net::OK if a connection is established successful, the BlimpConnection
+  //     can be taken by calling TakeConnection().
+  //   * net::ERR_IO_PENDING will never be the outcome
+  //   * Other error code indicates no connection was established.
+  virtual void Connect(const net::CompletionCallback& callback) = 0;
 
   // Returns the connection object after a successful Connect().
   virtual scoped_ptr<BlimpConnection> TakeConnection() = 0;
diff --git a/blimp/net/tcp_client_transport.cc b/blimp/net/tcp_client_transport.cc
index 6bdd18f..2bdf91d 100644
--- a/blimp/net/tcp_client_transport.cc
+++ b/blimp/net/tcp_client_transport.cc
@@ -7,6 +7,7 @@
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "blimp/net/stream_socket_connection.h"
 #include "net/socket/stream_socket.h"
 #include "net/socket/tcp_client_socket.h"
@@ -19,7 +20,7 @@ TCPClientTransport::TCPClientTransport(const net::AddressList& addresses,
 
 TCPClientTransport::~TCPClientTransport() {}
 
-int TCPClientTransport::Connect(const net::CompletionCallback& callback) {
+void TCPClientTransport::Connect(const net::CompletionCallback& callback) {
   DCHECK(!socket_);
   DCHECK(!callback.is_null());
 
@@ -31,11 +32,15 @@ int TCPClientTransport::Connect(const net::CompletionCallback& callback) {
   int result = socket_->Connect(completion_callback);
   if (result == net::ERR_IO_PENDING) {
     connect_callback_ = callback;
-  } else if (result != net::OK) {
+    return;
+  }
+
+  if (result != net::OK) {
     socket_ = nullptr;
   }
 
-  return result;
+  base::MessageLoop::current()->PostTask(FROM_HERE,
+                                         base::Bind(callback, result));
 }
 
 scoped_ptr<BlimpConnection> TCPClientTransport::TakeConnection() {
@@ -45,6 +50,7 @@ scoped_ptr<BlimpConnection> TCPClientTransport::TakeConnection() {
 }
 
 void TCPClientTransport::OnTCPConnectComplete(int result) {
+  DCHECK_NE(net::ERR_IO_PENDING, result);
   DCHECK(socket_);
   if (result != net::OK) {
     socket_ = nullptr;
diff --git a/blimp/net/tcp_client_transport.h b/blimp/net/tcp_client_transport.h
index d5255d3..bbac1ba 100644
--- a/blimp/net/tcp_client_transport.h
+++ b/blimp/net/tcp_client_transport.h
@@ -30,7 +30,7 @@ class BLIMP_NET_EXPORT TCPClientTransport : public BlimpTransport {
   ~TCPClientTransport() override;
 
   // BlimpTransport implementation.
-  int Connect(const net::CompletionCallback& callback) override;
+  void Connect(const net::CompletionCallback& callback) override;
   scoped_ptr<BlimpConnection> TakeConnection() override;
 
  private:
diff --git a/blimp/net/tcp_engine_transport.cc b/blimp/net/tcp_engine_transport.cc
index c2c96f9..112e350 100644
--- a/blimp/net/tcp_engine_transport.cc
+++ b/blimp/net/tcp_engine_transport.cc
@@ -7,6 +7,7 @@
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "blimp/net/stream_socket_connection.h"
 #include "net/socket/stream_socket.h"
 #include "net/socket/tcp_server_socket.h"
@@ -19,7 +20,7 @@ TCPEngineTransport::TCPEngineTransport(const net::IPEndPoint& address,
 
 TCPEngineTransport::~TCPEngineTransport() {}
 
-int TCPEngineTransport::Connect(const net::CompletionCallback& callback) {
+void TCPEngineTransport::Connect(const net::CompletionCallback& callback) {
   DCHECK(!accepted_socket_);
   DCHECK(!callback.is_null());
 
@@ -29,7 +30,9 @@ int TCPEngineTransport::Connect(const net::CompletionCallback& callback) {
     int result = server_socket_->Listen(address_, 5);
     if (result != net::OK) {
       server_socket_.reset();
-      return result;
+      base::MessageLoop::current()->PostTask(FROM_HERE,
+                                             base::Bind(callback, result));
+      return;
     }
   }
 
@@ -37,16 +40,18 @@ int TCPEngineTransport::Connect(const net::CompletionCallback& callback) {
       &TCPEngineTransport::OnTCPConnectAccepted, base::Unretained(this));
 
   int result = server_socket_->Accept(&accepted_socket_, accept_callback);
-  if (result == net::OK) {
-    callback.Run(result);
-  } else if (result == net::ERR_IO_PENDING) {
+  if (result == net::ERR_IO_PENDING) {
     connect_callback_ = callback;
-  } else {
+    return;
+  }
+
+  if (result != net::OK) {
     // TODO(haibinlu): investigate when we can keep using this server socket.
     server_socket_.reset();
   }
 
-  return result;
+  base::MessageLoop::current()->PostTask(FROM_HERE,
+                                         base::Bind(callback, result));
 }
 
 scoped_ptr<BlimpConnection> TCPEngineTransport::TakeConnection() {
@@ -62,6 +67,7 @@ int TCPEngineTransport::GetLocalAddressForTesting(
 }
 
 void TCPEngineTransport::OnTCPConnectAccepted(int result) {
+  DCHECK_NE(net::ERR_IO_PENDING, result);
   DCHECK(accepted_socket_);
   if (result != net::OK) {
     accepted_socket_.reset();
diff --git a/blimp/net/tcp_engine_transport.h b/blimp/net/tcp_engine_transport.h
index 88176e9..5bb92dd 100644
--- a/blimp/net/tcp_engine_transport.h
+++ b/blimp/net/tcp_engine_transport.h
@@ -31,7 +31,7 @@ class BLIMP_NET_EXPORT TCPEngineTransport : public BlimpTransport {
   ~TCPEngineTransport() override;
 
   // BlimpTransport implementation.
-  int Connect(const net::CompletionCallback& callback) override;
+  void Connect(const net::CompletionCallback& callback) override;
   scoped_ptr<BlimpConnection> TakeConnection() override;
 
   int GetLocalAddressForTesting(net::IPEndPoint* address) const;
diff --git a/blimp/net/tcp_transport_unittest.cc b/blimp/net/tcp_transport_unittest.cc
index 4d31326..2df4328 100644
--- a/blimp/net/tcp_transport_unittest.cc
+++ b/blimp/net/tcp_transport_unittest.cc
@@ -50,11 +50,11 @@ class TCPTransportTest : public testing::Test {
 
 TEST_F(TCPTransportTest, Connect) {
   net::TestCompletionCallback accept_callback;
-  ASSERT_EQ(net::ERR_IO_PENDING, server_->Connect(accept_callback.callback()));
+  server_->Connect(accept_callback.callback());
 
   net::TestCompletionCallback connect_callback;
   TCPClientTransport client(GetLocalAddressList(), nullptr);
-  ASSERT_EQ(net::ERR_IO_PENDING, client.Connect(connect_callback.callback()));
+  client.Connect(connect_callback.callback());
 
   EXPECT_EQ(net::OK, connect_callback.WaitForResult());
   EXPECT_EQ(net::OK, accept_callback.WaitForResult());
@@ -63,22 +63,22 @@ TEST_F(TCPTransportTest, Connect) {
 
 TEST_F(TCPTransportTest, TwoClientConnections) {
   net::TestCompletionCallback accept_callback1;
-  ASSERT_EQ(net::ERR_IO_PENDING, server_->Connect(accept_callback1.callback()));
+  server_->Connect(accept_callback1.callback());
 
   net::TestCompletionCallback connect_callback1;
   TCPClientTransport client1(GetLocalAddressList(), nullptr);
-  ASSERT_EQ(net::ERR_IO_PENDING, client1.Connect(connect_callback1.callback()));
+  client1.Connect(connect_callback1.callback());
 
   net::TestCompletionCallback connect_callback2;
   TCPClientTransport client2(GetLocalAddressList(), nullptr);
-  ASSERT_EQ(net::ERR_IO_PENDING, client2.Connect(connect_callback2.callback()));
+  client2.Connect(connect_callback2.callback());
 
   EXPECT_EQ(net::OK, connect_callback1.WaitForResult());
   EXPECT_EQ(net::OK, accept_callback1.WaitForResult());
   EXPECT_TRUE(server_->TakeConnection() != nullptr);
 
   net::TestCompletionCallback accept_callback2;
-  ASSERT_EQ(net::OK, server_->Connect(accept_callback2.callback()));
+  server_->Connect(accept_callback2.callback());
   EXPECT_EQ(net::OK, connect_callback2.WaitForResult());
   EXPECT_EQ(net::OK, accept_callback2.WaitForResult());
   EXPECT_TRUE(server_->TakeConnection() != nullptr);