From c539f3d84bd5bf809cc0744ae2fb356e707ee05a Mon Sep 17 00:00:00 2001 From: haibinlu Date: Fri, 20 Nov 2015 13:35:38 -0800 Subject: [Blimp Net] Makes BlimpTransport.Connect async-only BUG=558643 Review URL: https://codereview.chromium.org/1459923004 Cr-Commit-Position: refs/heads/master@{#360919} --- blimp/net/blimp_transport.h | 14 ++++++-------- blimp/net/tcp_client_transport.cc | 12 +++++++++--- blimp/net/tcp_client_transport.h | 2 +- blimp/net/tcp_engine_transport.cc | 20 +++++++++++++------- blimp/net/tcp_engine_transport.h | 2 +- blimp/net/tcp_transport_unittest.cc | 12 ++++++------ 6 files changed, 36 insertions(+), 26 deletions(-) (limited to 'blimp/net') 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 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 TCPClientTransport::TakeConnection() { @@ -45,6 +50,7 @@ scoped_ptr 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 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 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 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); -- cgit v1.1