Fixing bug where Connection drops packets when presumed writable.

webrtc/p2p/base/port_unittest.cc

Index: webrtc/p2p/base/port_unittest.cc
diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc
index a345e82b6133fc6c5eb7dfacecedb801e238a932..743cddacdb86da47a011c5347010e757c42d9ece 100644
--- a/webrtc/p2p/base/port_unittest.cc
+++ b/webrtc/p2p/base/port_unittest.cc
@@ -2424,11 +2424,11 @@ TEST_F(PortTest, TestWritableState) {
   ch1.Ping();
   WAIT(!ch2.remote_address().IsNil(), kTimeout);
 
-  // Data should be unsendable until the connection is accepted.
+  // Data should be sendable before the connection is accepted.
   char data[] = "abcd";
   int data_size = arraysize(data);
   rtc::PacketOptions options;
-  EXPECT_EQ(SOCKET_ERROR, ch1.conn()->Send(data, data_size, options));
+  EXPECT_EQ(data_size, ch1.conn()->Send(data, data_size, options));
 
   // Accept the connection to return the binding response, transition to
   // writable, and allow data to be sent.
@@ -2464,8 +2464,9 @@ TEST_F(PortTest, TestWritableState) {
                           500u);
   EXPECT_EQ(Connection::STATE_WRITE_TIMEOUT, ch1.conn()->write_state());
 
-  // Now that the connection has completely timed out, data send should fail.
-  EXPECT_EQ(SOCKET_ERROR, ch1.conn()->Send(data, data_size, options));
+  // Even if the connection has timed out, the Connection shouldn't block
+  // the sending of data.
+  EXPECT_EQ(data_size, ch1.conn()->Send(data, data_size, options));

[honghaiz3, 2016/06/27 16:12:54]

I am really not sure about the decision we have made here that the connection
will just send down anything regardless its states. It will send even if it has
timed out, or even failed. This essentially makes Connection not an independent
class. It can only be used by the p2ptransportchannel. If any other class want
to use it, it has to repeat the same logic we have done in the
p2ptransportchannel. 

I am fine if you want to fix this issue quickly although I suggest we should
revisit the whole decision later. 

[Taylor Brandstetter, 2016/06/27 17:35:42]

I guess I have a different opinion. I feel that making it less restrictive makes
it more generally usable. If another class calls Send while it's in the timeout
or init state, it seems like we should assume that class knows what it's doing
and attempt to send. Maybe some optimization in P2PTransportChannel will do that
in the future.  We may even want to push the decision of whether or not to send
further up the chain in the future; maybe the audio/video engines would like to
decide whether or not to send when the state is "timeout".

 
   ch1.Stop();
   ch2.Stop();