TCPConnection can never be deteted if they fail to connect

webrtc/p2p/base/tcpport.cc

Index: webrtc/p2p/base/tcpport.cc
diff --git a/webrtc/p2p/base/tcpport.cc b/webrtc/p2p/base/tcpport.cc
index 6dc66d26cf82d466b7a2cf362a70fc98e08a7850..2ae19e5811b281f66ca7baf740a87138f4fe9efb 100644
--- a/webrtc/p2p/base/tcpport.cc
+++ b/webrtc/p2p/base/tcpport.cc
@@ -378,6 +378,8 @@ void TCPConnection::OnConnect(rtc::AsyncPacketSocket* socket) {
                             << socket_ip.ToSensitiveString()
                             << ", different from the local candidate IP "
                             << port()->ip().ToSensitiveString();
+    // This doesn't trigger OnClose. We should either fix that behavior or call
+    // Destroy() to ensure this connection doesn't linger.

[pthatcher1, 2015/08/22 05:05:22]

It seems like we should call OnClose here.  Is there a reason not to?

[guoweis_webrtc, 2015/08/22 05:29:49]

Done.

     socket_->Close();
   }
 }
@@ -397,6 +399,11 @@ void TCPConnection::OnClose(rtc::AsyncPacketSocket* socket, int error) {
     // when the connection is used to Send() or Ping().
     port()->thread()->PostDelayed(reconnection_timeout(), this,
                                   MSG_TCPCONNECTION_DELAYED_ONCLOSE);
+  } else {
+    // OnClose could be called when the underneath socket times out during
+    // connect(). We have to manually destroy here as this connection, as never
+    // connected, will not be scheduled for ping to trigger destroy.
+    Destroy();

[pthatcher1, 2015/08/22 05:05:22]

I don't think this matches the state diagram at the top.  It says if we're not
connect and get OnClose, we should wait for a timeout before dying.  Is the
diagram wrong, the code wrong, or am I wrong?

[guoweis_webrtc, 2015/08/22 05:29:49]

the diagram above misses an arrow from state 1 to state 7. I'll update that.

   }
 }
 
@@ -407,7 +414,9 @@ void TCPConnection::OnMessage(rtc::Message* pmsg) {
       // seconds, it's time to tear this down. This is the case for the original
       // TCP connection on passive side during a reconnect.
       if (pretending_to_be_writable_) {
-        set_write_state(STATE_WRITE_TIMEOUT);
+        // Make sure this connection is destroyed. It will linger around if it
+        // has been READABLE.

[pthatcher1, 2015/08/22 05:05:22]

This part seems correct.  It matches the state diagram ("the connection is
dead"), so I don't think you need the comment.

[guoweis_webrtc, 2015/08/22 05:29:50]

Will remove.

+        Destroy();
       }
       break;
     default: