Fix crash condition if caller deletes PseudoTcpAdaptor from within a callback.

BUG=82171
TEST=


Review URL: http://codereview.chromium.org/7040021

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

jingle/glue/pseudotcp_adapter.h

@@ -17,12 +17,15 @@
 
 namespace jingle_glue {
 
-class PseudoTcpAdapter : public net::StreamSocket,
-                         public cricket::IPseudoTcpNotify,
-                         public base::NonThreadSafe {
+// PseudoTcpAdapter adapts a connectionless net::Socket to a connection-
+// oriented net::StreamSocket using PseudoTcp.  Because net::StreamSockets
+// can be deleted during callbacks, while PseudoTcp cannot, the core of the
+// PseudoTcpAdapter is reference counted, with a reference held by the
+// adapter, and an additional reference held on the stack during callbacks.
+class PseudoTcpAdapter : public net::StreamSocket, base::NonThreadSafe {
  public:
-  // Creates adapter for the specified |socket|. |socket| is assumed
-  // to be already connected. Takes ownership of |socket|.
+  // Creates an adapter for the supplied Socket.  |socket| should already
+  // be ready for use, and ownership of it will be assumed by the adapter.
   PseudoTcpAdapter(net::Socket* socket);
   virtual ~PseudoTcpAdapter();
 
@@ -47,43 +50,10 @@ class PseudoTcpAdapter : public net::StreamSocket,
   virtual bool WasEverUsed() const OVERRIDE;
   virtual bool UsingTCPFastOpen() const OVERRIDE;
 
-  // cricket::IPseudoTcpNotify implementation.
-  virtual void OnTcpOpen(cricket::PseudoTcp* tcp) OVERRIDE;
-  virtual void OnTcpReadable(cricket::PseudoTcp* tcp) OVERRIDE;
-  virtual void OnTcpWriteable(cricket::PseudoTcp* tcp) OVERRIDE;
-  virtual void OnTcpClosed(cricket::PseudoTcp* tcp, uint32 error) OVERRIDE;
-  virtual WriteResult TcpWritePacket(cricket::PseudoTcp* tcp,
-                                     const char* buffer, size_t len) OVERRIDE;
-
  private:
-  void DoReadFromSocket();
-  void HandleReadResults(int result);
-
-  // Callback functions for Read() and Write() in |socket_|
-  void OnRead(int result);
-  void OnWritten(int result);
-
-  void AdjustClock();
-  void HandleTcpClock();
-
-  scoped_ptr<net::Socket> socket_;
-  cricket::PseudoTcp pseudotcp_;
-
-  net::CompletionCallback* connect_callback_;
-  scoped_refptr<net::IOBuffer> read_buffer_;
-  int read_buffer_size_;
-  net::CompletionCallback* read_callback_;
-  scoped_refptr<net::IOBuffer> write_buffer_;
-  int write_buffer_size_;
-  net::CompletionCallback* write_callback_;
-
-  bool socket_write_pending_;
-  scoped_refptr<net::IOBuffer> socket_read_buffer_;
-
-  net::CompletionCallbackImpl<PseudoTcpAdapter> socket_read_callback_;
-  net::CompletionCallbackImpl<PseudoTcpAdapter> socket_write_callback_;
+  class Core;
 
-  base::OneShotTimer<PseudoTcpAdapter> timer_;
+  scoped_refptr<Core> core_;
 
   net::BoundNetLog net_log_;
 

jingle/glue/pseudotcp_adapter_unittest.cc

@@ -307,6 +307,35 @@ TEST_F(P2PTransportImplTest, TestLossyChannel) {
   tester->CheckResults();
 }
 
+class DeleteOnConnected {
+ public:
+  DeleteOnConnected(MessageLoop* message_loop,
+                    scoped_ptr<PseudoTcpAdapter>* adapter)
+      : message_loop_(message_loop), adapter_(adapter) {}
+  void OnConnected(int error) {
+    adapter_->reset();
+    message_loop_->PostTask(FROM_HERE, new MessageLoop::QuitTask());
+  }
+  MessageLoop* message_loop_;
+  scoped_ptr<PseudoTcpAdapter>* adapter_;
+};
+
+TEST_F(P2PTransportImplTest, TestDeleteOnConnected) {
+  // This test verifies that deleting the adapter mid-callback doesn't lead
+  // to deleted structures being touched as the stack unrolls, so the failure
+  // mode is a crash rather than a normal test failure.
+  TestCompletionCallback client_connect_cb;
+  DeleteOnConnected host_delete(&message_loop_, &host_pseudotcp_);
+  net::CompletionCallbackImpl<DeleteOnConnected>
+      host_connect_cb(&host_delete, &DeleteOnConnected::OnConnected);
+
+  host_pseudotcp_->Connect(&host_connect_cb);
+  client_pseudotcp_->Connect(&client_connect_cb);
+  message_loop_.Run();
+
+  ASSERT_EQ(NULL, host_pseudotcp_.get());
+}
+
 }  // namespace
 
 }  // namespace jingle_glue

remoting/protocol/jingle_session.cc

@@ -381,6 +381,8 @@ bool JingleSession::EstablishSSLConnection(
       new jingle_glue::PseudoTcpAdapter(channel);
   pseudotcp->Connect(&connect_callback_);
 
+  // TODO(wez): We shouldn't try to start SSL until the socket we're
+  // starting it on is connected.
   if (cricket_session_->initiator()) {
     // Create client SSL socket.
     net::SSLClientSocket* socket = CreateSSLClientSocket(