Use scoped_ptr<> for channel creation callbacks.

Changed callbacks in Session and ChannelAuthenticator interfaces for
cleaner ownership semantics.

Review URL: https://chromiumcodereview.appspot.com/9316074

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@120508 0039d316-1c4b-4281-b951-d872f2087c98

remoting/protocol/authenticator_test_base.cc

@@ -91,14 +91,14 @@ void AuthenticatorTestBase::RunChannelAuth(bool expected_fail) {
   client_fake_socket_->PairWith(host_fake_socket_.get());
 
   client_auth_->SecureAndAuthenticate(
-      client_fake_socket_.release(),
-      base::Bind(&MockChannelDoneCallback::OnDone,
-                 base::Unretained(&client_callback_)));
+      client_fake_socket_.PassAs<net::StreamSocket>(),
+      base::Bind(&AuthenticatorTestBase::OnClientConnected,
+                 base::Unretained(this)));
 
   host_auth_->SecureAndAuthenticate(
-      host_fake_socket_.release(),
-      base::Bind(&MockChannelDoneCallback::OnDone,
-                 base::Unretained(&host_callback_)));
+      host_fake_socket_.PassAs<net::StreamSocket>(),
+      base::Bind(&AuthenticatorTestBase::OnHostConnected,
+                 base::Unretained(this)));
 
   net::StreamSocket* client_socket = NULL;
   net::StreamSocket* host_socket = NULL;
@@ -121,5 +121,19 @@ void AuthenticatorTestBase::RunChannelAuth(bool expected_fail) {
   host_socket_.reset(host_socket);
 }
 
+void AuthenticatorTestBase::OnHostConnected(
+    net::Error error,
+    scoped_ptr<net::StreamSocket> socket) {
+  host_callback_.OnDone(error, socket.get());
+  host_socket_ = socket.Pass();
+}
+
+void AuthenticatorTestBase::OnClientConnected(
+    net::Error error,
+    scoped_ptr<net::StreamSocket> socket) {
+  client_callback_.OnDone(error, socket.get());
+  client_socket_ = socket.Pass();
+}
+
 }  // namespace protocol
 }  // namespace remoting

remoting/protocol/authenticator_test_base.h

@@ -44,6 +44,11 @@ class AuthenticatorTestBase : public testing::Test {
   void RunAuthExchange();
   void RunChannelAuth(bool expected_fail);
 
+  void OnHostConnected(net::Error error,
+                       scoped_ptr<net::StreamSocket> socket);
+  void OnClientConnected(net::Error error,
+                         scoped_ptr<net::StreamSocket> socket);
+
   MessageLoop message_loop_;
 
   scoped_ptr<crypto::RSAPrivateKey> private_key_;

remoting/protocol/channel_authenticator.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -23,18 +23,17 @@ namespace protocol {
 // should be used only once for one channel.
 class ChannelAuthenticator {
  public:
-  typedef base::Callback<void(net::Error error, net::StreamSocket*)>
+  typedef base::Callback<void(net::Error error, scoped_ptr<net::StreamSocket>)>
       DoneCallback;
 
   virtual ~ChannelAuthenticator() {}
 
-  // Start authentication of the given |socket|. Takes ownership of
-  // |socket|, and caller must not use |socket| after calling this
-  // method. |done_callback| is called when authentication is
-  // finished. Callback may be invoked before this method
-  // returns. Callback handler must take ownership of the result.
+  // Start authentication of the given |socket|. |done_callback| is
+  // called when authentication is finished. Callback may be invoked
+  // before this method returns.
   virtual void SecureAndAuthenticate(
-      net::StreamSocket* socket, const DoneCallback& done_callback) = 0;
+      scoped_ptr<net::StreamSocket> socket,
+      const DoneCallback& done_callback) = 0;
 };
 
 }  // namespace protocol

remoting/protocol/channel_dispatcher_base.cc

@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -31,13 +31,14 @@ void ChannelDispatcherBase::Init(Session* session,
       &ChannelDispatcherBase::OnChannelReady, base::Unretained(this)));
 }
 
-void ChannelDispatcherBase::OnChannelReady(net::StreamSocket* socket) {
-  if (!socket) {
+void ChannelDispatcherBase::OnChannelReady(
+    scoped_ptr<net::StreamSocket> socket) {
+  if (!socket.get()) {
     initialized_callback_.Run(false);
     return;
   }
 
-  channel_.reset(socket);
+  channel_ = socket.Pass();
 
   OnInitialized();
 

remoting/protocol/channel_dispatcher_base.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -49,7 +49,7 @@ class ChannelDispatcherBase {
   virtual void OnInitialized() = 0;
 
  private:
-  void OnChannelReady(net::StreamSocket* socket);
+  void OnChannelReady(scoped_ptr<net::StreamSocket> socket);
 
   std::string channel_name_;
   Session* session_;

remoting/protocol/fake_authenticator.cc

@@ -24,31 +24,31 @@ FakeChannelAuthenticator::~FakeChannelAuthenticator() {
 }
 
 void FakeChannelAuthenticator::SecureAndAuthenticate(
-    net::StreamSocket* socket, const DoneCallback& done_callback) {
+    scoped_ptr<net::StreamSocket> socket,
+    const DoneCallback& done_callback) {
   net::Error error;
 
   if (accept_) {
     error = net::OK;
   } else {
     error = net::ERR_FAILED;
-    delete socket;
-    socket = NULL;
+    socket.reset();
   }
 
   if (async_) {
     MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
         &FakeChannelAuthenticator::CallCallback, weak_factory_.GetWeakPtr(),
-        done_callback, error, socket));
+        done_callback, error, base::Passed(&socket)));
   } else {
-    done_callback.Run(error, socket);
+    done_callback.Run(error, socket.Pass());
   }
 }
 
 void FakeChannelAuthenticator::CallCallback(
     const DoneCallback& done_callback,
     net::Error error,
-    net::StreamSocket* socket) {
-  done_callback.Run(error, socket);
+    scoped_ptr<net::StreamSocket> socket) {
+  done_callback.Run(error, socket.Pass());
 }
 
 FakeAuthenticator::FakeAuthenticator(

remoting/protocol/fake_authenticator.h

@@ -19,13 +19,14 @@ class FakeChannelAuthenticator : public ChannelAuthenticator {
 
   // ChannelAuthenticator interface.
   virtual void SecureAndAuthenticate(
-      net::StreamSocket* socket, const DoneCallback& done_callback) OVERRIDE;
+      scoped_ptr<net::StreamSocket> socket,
+      const DoneCallback& done_callback) OVERRIDE;
 
  private:
   void CallCallback(
       const DoneCallback& done_callback,
       net::Error error,
-      net::StreamSocket* socket);
+      scoped_ptr<net::StreamSocket> socket);
 
   bool accept_;
   bool async_;

remoting/protocol/fake_session.cc

@@ -253,16 +253,16 @@ Session::Error FakeSession::error() {
 
 void FakeSession::CreateStreamChannel(
     const std::string& name, const StreamChannelCallback& callback) {
-  FakeSocket* channel = new FakeSocket();
-  stream_channels_[name] = channel;
-  callback.Run(channel);
+  scoped_ptr<FakeSocket> channel(new FakeSocket());
+  stream_channels_[name] = channel.get();
+  callback.Run(channel.PassAs<net::StreamSocket>());
 }
 
 void FakeSession::CreateDatagramChannel(
     const std::string& name, const DatagramChannelCallback& callback) {
-  FakeUdpSocket* channel = new FakeUdpSocket();
-  datagram_channels_[name] = channel;
-  callback.Run(channel);
+  scoped_ptr<FakeUdpSocket> channel(new FakeUdpSocket());
+  datagram_channels_[name] = channel.get();
+  callback.Run(channel.PassAs<net::Socket>());
 }
 
 void FakeSession::CancelChannelCreation(const std::string& name) {

remoting/protocol/jingle_datagram_connector.cc

@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -30,13 +30,13 @@ void JingleDatagramConnector::Connect(
 
   authenticator_ = authenticator.Pass();
 
-  net::Socket* socket =
-      new jingle_glue::TransportChannelSocketAdapter(raw_channel);
+  scoped_ptr<net::Socket> socket(
+      new jingle_glue::TransportChannelSocketAdapter(raw_channel));
 
   // TODO(sergeyu): Implement encryption for datagram channels.
 
   session_->OnChannelConnectorFinished(name_, this);
-  callback_.Run(socket);
+  callback_.Run(socket.Pass());
   delete this;
 }
 

remoting/protocol/jingle_session_unittest.cc

@@ -105,6 +105,16 @@ class JingleSessionTest : public testing::Test {
     session->set_config(SessionConfig::GetDefault());
   }
 
+  void OnClientChannelCreated(scoped_ptr<net::StreamSocket> socket) {
+    client_channel_callback_.OnDone(socket.get());
+    client_socket_ = socket.Pass();
+  }
+
+  void OnHostChannelCreated(scoped_ptr<net::StreamSocket> socket) {
+    host_channel_callback_.OnDone(socket.get());
+    host_socket_ = socket.Pass();
+  }
+
  protected:
   virtual void SetUp() {
   }
@@ -236,32 +246,17 @@ class JingleSessionTest : public testing::Test {
   }
 
   void CreateChannel() {
-    MockStreamChannelCallback client_callback;
-    MockStreamChannelCallback host_callback;
-
     client_session_->CreateStreamChannel(kChannelName, base::Bind(
-        &MockStreamChannelCallback::OnDone,
-        base::Unretained(&host_callback)));
+        &JingleSessionTest::OnClientChannelCreated, base::Unretained(this)));
     host_session_->CreateStreamChannel(kChannelName, base::Bind(
-        &MockStreamChannelCallback::OnDone,
-        base::Unretained(&client_callback)));
+        &JingleSessionTest::OnHostChannelCreated, base::Unretained(this)));
 
     int counter = 2;
-    net::StreamSocket* client_socket = NULL;
-    net::StreamSocket* host_socket = NULL;
-    EXPECT_CALL(client_callback, OnDone(_))
-        .WillOnce(DoAll(SaveArg<0>(&client_socket),
-                        QuitThreadOnCounter(&counter)));
-    EXPECT_CALL(host_callback, OnDone(_))
-        .WillOnce(DoAll(SaveArg<0>(&host_socket),
-                        QuitThreadOnCounter(&counter)));
+    EXPECT_CALL(client_channel_callback_, OnDone(_))
+        .WillOnce(QuitThreadOnCounter(&counter));
+    EXPECT_CALL(host_channel_callback_, OnDone(_))
+        .WillOnce(QuitThreadOnCounter(&counter));
     message_loop_.Run();
-
-    ASSERT_TRUE(client_socket != NULL);
-    ASSERT_TRUE(host_socket != NULL);
-
-    client_socket_.reset(client_socket);
-    host_socket_.reset(host_socket);
   }
 
   JingleThreadMessageLoop message_loop_;
@@ -279,6 +274,9 @@ class JingleSessionTest : public testing::Test {
   scoped_ptr<Session> client_session_;
   MockSessionCallback client_connection_callback_;
 
+  MockStreamChannelCallback client_channel_callback_;
+  MockStreamChannelCallback host_channel_callback_;
+
   scoped_ptr<net::StreamSocket> client_socket_;
   scoped_ptr<net::StreamSocket> host_socket_;
 };
@@ -347,20 +345,14 @@ TEST_F(JingleSessionTest, ConnectWithBadChannelAuth) {
   ASSERT_NO_FATAL_FAILURE(
       InitiateConnection(1, FakeAuthenticator::ACCEPT, false));
 
-  MockStreamChannelCallback client_callback;
-  MockStreamChannelCallback host_callback;
-
   client_session_->CreateStreamChannel(kChannelName, base::Bind(
-      &MockStreamChannelCallback::OnDone,
-      base::Unretained(&client_callback)));
+      &JingleSessionTest::OnClientChannelCreated, base::Unretained(this)));
   host_session_->CreateStreamChannel(kChannelName, base::Bind(
-      &MockStreamChannelCallback::OnDone,
-      base::Unretained(&host_callback)));
+      &JingleSessionTest::OnHostChannelCreated, base::Unretained(this)));
 
-  EXPECT_CALL(client_callback, OnDone(_))
-      .Times(AtMost(1))
-      .WillOnce(DeleteArg<0>());
-  EXPECT_CALL(host_callback, OnDone(NULL))
+  EXPECT_CALL(client_channel_callback_, OnDone(_))
+      .Times(AtMost(1));
+  EXPECT_CALL(host_channel_callback_, OnDone(NULL))
       .WillOnce(QuitThread());
 
   message_loop_.Run();
@@ -399,46 +391,5 @@ TEST_F(JingleSessionTest, TestMultistepAuthTcpChannel) {
   tester.CheckResults();
 }
 
-// Verify that data can be transmitted over the video RTP channel.
-TEST_F(JingleSessionTest, TestUdpChannel) {
-  CreateSessionManagers(1, FakeAuthenticator::ACCEPT);
-  ASSERT_NO_FATAL_FAILURE(
-      InitiateConnection(1, FakeAuthenticator::ACCEPT, false));
-
-  MockDatagramChannelCallback client_callback;
-  MockDatagramChannelCallback host_callback;
-
-  int counter = 2;
-  net::Socket* client_socket = NULL;
-  net::Socket* host_socket = NULL;
-  EXPECT_CALL(client_callback, OnDone(_))
-      .WillOnce(DoAll(SaveArg<0>(&client_socket),
-                      QuitThreadOnCounter(&counter)));
-  EXPECT_CALL(host_callback, OnDone(_))
-      .WillOnce(DoAll(SaveArg<0>(&host_socket),
-                      QuitThreadOnCounter(&counter)));
-
-  client_session_->CreateDatagramChannel(kChannelName, base::Bind(
-      &MockDatagramChannelCallback::OnDone,
-      base::Unretained(&host_callback)));
-  host_session_->CreateDatagramChannel(kChannelName, base::Bind(
-      &MockDatagramChannelCallback::OnDone,
-      base::Unretained(&client_callback)));
-
-  message_loop_.Run();
-
-  scoped_ptr<net::Socket> client_socket_ptr(client_socket);
-  scoped_ptr<net::Socket> host_socket_ptr(host_socket);
-
-  ASSERT_TRUE(client_socket != NULL);
-  ASSERT_TRUE(host_socket != NULL);
-
-  DatagramConnectionTester tester(
-      client_socket, host_socket, kMessageSize, kMessages, kUdpWriteDelayMs);
-  tester.Start();
-  message_loop_.Run();
-  tester.CheckResults();
-}
-
 }  // namespace protocol
 }  // namespace remoting