Do not delete a connection in the turn port with permission error or refresh error.

webrtc/p2p/base/turnport_unittest.cc

 /*
  *  Copyright 2012 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 #if defined(WEBRTC_POSIX)
@@ skipping 177 matching lines @@
                         const rtc::PacketTime& packet_time) {
     turn_packets_.push_back(rtc::Buffer(data, size));
   }
   void OnUdpPortComplete(Port* port) {
     udp_ready_ = true;
   }
   void OnUdpReadPacket(Connection* conn, const char* data, size_t size,
                        const rtc::PacketTime& packet_time) {
     udp_packets_.push_back(rtc::Buffer(data, size));
   }
-  void OnConnectionDestroyed(Connection* conn) { connection_destroyed_ = true; }
   void OnSocketReadPacket(rtc::AsyncPacketSocket* socket,
                           const char* data, size_t size,
                           const rtc::SocketAddress& remote_addr,
                           const rtc::PacketTime& packet_time) {
     turn_port_->HandleIncomingPacket(socket, data, size, remote_addr,
                                      packet_time);
   }
   rtc::AsyncSocket* CreateServerSocket(const SocketAddress addr) {
     rtc::AsyncSocket* socket = ss_->CreateAsyncSocket(SOCK_STREAM);
     EXPECT_GE(socket->Bind(addr), 0);
@@ skipping 66 matching lines @@
         &TurnPortTest::OnTurnPortComplete);
     turn_port_->SignalPortError.connect(this,
         &TurnPortTest::OnTurnPortError);
     turn_port_->SignalUnknownAddress.connect(this,
         &TurnPortTest::OnTurnUnknownAddress);
     turn_port_->SignalCreatePermissionResult.connect(this,
         &TurnPortTest::OnTurnCreatePermissionResult);
     turn_port_->SignalTurnRefreshResult.connect(
         this, &TurnPortTest::OnTurnRefreshResult);
   }
-  void ConnectConnectionDestroyedSignal(Connection* conn) {
-    conn->SignalDestroyed.connect(this, &TurnPortTest::OnConnectionDestroyed);
-  }
 
   void CreateUdpPort() { CreateUdpPort(kLocalAddr2); }
 
   void CreateUdpPort(const SocketAddress& address) {
     udp_port_.reset(UDPPort::Create(main_, &socket_factory_, &network_,
                                     address.ipaddr(), 0, 0, kIceUfrag2,
                                     kIcePwd2, std::string(), false));
     // UDP port will be controlled.
     udp_port_->SetIceRole(cricket::ICEROLE_CONTROLLED);
     udp_port_->SignalPortComplete.connect(
         this, &TurnPortTest::OnUdpPortComplete);
   }
 
   void PrepareTurnAndUdpPorts() {
     // turn_port_ should have been created.
     ASSERT_TRUE(turn_port_ != nullptr);
     turn_port_->PrepareAddress();
     ASSERT_TRUE_WAIT(turn_ready_, kTimeout);
 
     CreateUdpPort();
     udp_port_->PrepareAddress();
     ASSERT_TRUE_WAIT(udp_ready_, kTimeout);
   }
 
-  bool CheckConnectionDestroyed() {
-    turn_port_->FlushRequests(cricket::kAllRequests);
-    rtc::Thread::Current()->ProcessMessages(50);
-    return connection_destroyed_;
+  bool CheckConnectionFailedAndPruned(Connection* conn) {
+    return conn && !conn->active() && conn->state() == Connection::STATE_FAILED;
+  }
+
+  // Checks that |turn_port_| has a nonempty set of connections and they are all
+  // failed and pruned.
+  bool CheckAllConnectionsFailedAndPruned() {
+    auto& connections = turn_port_->connections();
+    if (connections.empty()) {
+      return false;
+    }
+    for (auto kv : connections) {
+      if (!CheckConnectionFailedAndPruned(kv.second)) {
+        return false;
+      }
+    }
+    return true;
   }
 
   void TestTurnAlternateServer(cricket::ProtocolType protocol_type) {
     std::vector<rtc::SocketAddress> redirect_addresses;
     redirect_addresses.push_back(kTurnAlternateIntAddr);
 
     cricket::TestTurnRedirector redirector(redirect_addresses);
 
     turn_server_.AddInternalSocket(kTurnIntAddr, protocol_type);
     turn_server_.AddInternalSocket(kTurnAlternateIntAddr, protocol_type);
@@ skipping 191 matching lines @@
   cricket::TestTurnServer turn_server_;
   std::unique_ptr<TurnPort> turn_port_;
   std::unique_ptr<UDPPort> udp_port_;
   bool turn_ready_;
   bool turn_error_;
   bool turn_unknown_address_;
   bool turn_create_permission_success_;
   bool udp_ready_;
   bool test_finish_;
   bool turn_refresh_success_ = false;
-  bool connection_destroyed_ = false;
   std::vector<rtc::Buffer> turn_packets_;
   std::vector<rtc::Buffer> udp_packets_;
   rtc::PacketOptions options;
 };
 
 // Do a normal TURN allocation.
 TEST_F(TurnPortTest, TestTurnAllocate) {
   CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
   EXPECT_EQ(0, turn_port_->SetOption(rtc::Socket::OPT_SNDBUF, 10*1024));
   turn_port_->PrepareAddress();
@@ skipping 188 matching lines @@
   // Verifies that the new port has the same address.
   EXPECT_EQ(first_addr, turn_port_->socket()->GetLocalAddress());
 
   EXPECT_TRUE_WAIT(turn_ready_, kTimeout);
 
   // Verifies that the new port has a different address now.
   EXPECT_NE(first_addr, turn_port_->socket()->GetLocalAddress());
 }
 
 TEST_F(TurnPortTest, TestRefreshRequestGetsErrorResponse) {
+  rtc::ScopedFakeClock clock;
   CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
   PrepareTurnAndUdpPorts();
   turn_port_->CreateConnection(udp_port_->Candidates()[0],
                                Port::ORIGIN_MESSAGE);
   // Set bad credentials.
   cricket::RelayCredentials bad_credentials("bad_user", "bad_pwd");
   turn_port_->set_credentials(bad_credentials);
   turn_refresh_success_ = false;
   // This sends out the first RefreshRequest with correct credentials.
   // When this succeeds, it will schedule a new RefreshRequest with the bad
   // credential.
   turn_port_->FlushRequests(cricket::TURN_REFRESH_REQUEST);
-  EXPECT_TRUE_WAIT(turn_refresh_success_, kTimeout);
+  EXPECT_TRUE_SIMULATED_WAIT(turn_refresh_success_, kTimeout, clock);
   // Flush it again, it will receive a bad response.
   turn_port_->FlushRequests(cricket::TURN_REFRESH_REQUEST);
-  EXPECT_TRUE_WAIT(!turn_refresh_success_, kTimeout);
-  EXPECT_TRUE_WAIT(!turn_port_->connected(), kTimeout);
-  EXPECT_TRUE_WAIT(turn_port_->connections().empty(), kTimeout);
-  EXPECT_FALSE(turn_port_->HasRequests());
+  EXPECT_TRUE_SIMULATED_WAIT(!turn_refresh_success_, kTimeout, clock);
+  EXPECT_TRUE_SIMULATED_WAIT(!turn_port_->connected(), kTimeout, clock);
+  EXPECT_TRUE_SIMULATED_WAIT(CheckAllConnectionsFailedAndPruned(), kTimeout,
+                             clock);
+  EXPECT_TRUE_SIMULATED_WAIT(!turn_port_->HasRequests(), kTimeout, clock);
 }
 
 // Test that TurnPort will not handle any incoming packets once it has been
 // closed.
 TEST_F(TurnPortTest, TestStopProcessingPacketsAfterClosed) {
   CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
   PrepareTurnAndUdpPorts();
   Connection* conn1 = turn_port_->CreateConnection(udp_port_->Candidates()[0],
                                                    Port::ORIGIN_MESSAGE);
   Connection* conn2 = udp_port_->CreateConnection(turn_port_->Candidates()[0],
@@ skipping 122 matching lines @@
 TEST_F(TurnPortTest, TestTurnConnectionUsingOTUNonce) {
   turn_server_.set_enable_otu_nonce(true);
   CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
   TestTurnConnection();
 }
 
 // Test that CreatePermissionRequest will be scheduled after the success
 // of the first create permission request and the request will get an
 // ErrorResponse if the ufrag and pwd are incorrect.
 TEST_F(TurnPortTest, TestRefreshCreatePermissionRequest) {
+  rtc::ScopedFakeClock clock;

[Taylor Brandstetter, 2016/06/21 18:48:08]

Why is the fake clock needed here and in the test below?

[honghaiz3, 2016/06/22 01:50:41]

Removed using FakeClock here and below. 

   CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
   PrepareTurnAndUdpPorts();
 
   Connection* conn = turn_port_->CreateConnection(udp_port_->Candidates()[0],
                                                   Port::ORIGIN_MESSAGE);
-  ConnectConnectionDestroyedSignal(conn);
   ASSERT_TRUE(conn != NULL);
-  ASSERT_TRUE_WAIT(turn_create_permission_success_, kTimeout);
+  EXPECT_TRUE_SIMULATED_WAIT(turn_create_permission_success_, kTimeout, clock);
   turn_create_permission_success_ = false;
   // A create-permission-request should be pending.
   // After the next create-permission-response is received, it will schedule
   // another request with bad_ufrag and bad_pwd.
   cricket::RelayCredentials bad_credentials("bad_user", "bad_pwd");
   turn_port_->set_credentials(bad_credentials);
   turn_port_->FlushRequests(cricket::kAllRequests);
-  ASSERT_TRUE_WAIT(turn_create_permission_success_, kTimeout);
+  EXPECT_TRUE_SIMULATED_WAIT(turn_create_permission_success_, kTimeout, clock);
   // Flush the requests again; the create-permission-request will fail.
   turn_port_->FlushRequests(cricket::kAllRequests);
-  EXPECT_TRUE_WAIT(!turn_create_permission_success_, kTimeout);
-  EXPECT_TRUE_WAIT(connection_destroyed_, kTimeout);
+  EXPECT_TRUE_SIMULATED_WAIT(!turn_create_permission_success_, kTimeout, clock);
+  EXPECT_TRUE_SIMULATED_WAIT(CheckConnectionFailedAndPruned(conn), kTimeout,
+                             clock);
 }
 
 TEST_F(TurnPortTest, TestChannelBindGetErrorResponse) {
+  rtc::ScopedFakeClock clock;
   CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
   PrepareTurnAndUdpPorts();
   Connection* conn1 = turn_port_->CreateConnection(udp_port_->Candidates()[0],
                                                    Port::ORIGIN_MESSAGE);
   ASSERT_TRUE(conn1 != nullptr);
   Connection* conn2 = udp_port_->CreateConnection(turn_port_->Candidates()[0],
                                                   Port::ORIGIN_MESSAGE);
   ASSERT_TRUE(conn2 != nullptr);
-  ConnectConnectionDestroyedSignal(conn1);
   conn1->Ping(0);
-  ASSERT_TRUE_WAIT(conn1->writable(), kTimeout);
-
-  std::string data = "ABC";
-  conn1->Send(data.data(), data.length(), options);
+  EXPECT_TRUE_SIMULATED_WAIT(conn1->writable(), kTimeout, clock);
   bool success =
       turn_port_->SetEntryChannelId(udp_port_->Candidates()[0].address(), -1);
   ASSERT_TRUE(success);
-  // Next time when the binding request is sent, it will get an ErrorResponse.
-  EXPECT_TRUE_WAIT(CheckConnectionDestroyed(), kTimeout);
+  std::string data = "ABC";
+  conn1->Send(data.data(), data.length(), options);

[Taylor Brandstetter, 2016/06/21 18:48:08]

Are there any EXPECTS that should be added for the data being sent?

[honghaiz3, 2016/06/22 01:50:41]

Added a test that future packets cannot be sent on the connection. 
Note: the first data on the connection may still be sent via the data
indication. 

+
+  EXPECT_TRUE_SIMULATED_WAIT(CheckConnectionFailedAndPruned(conn1), kTimeout,
+                             clock);
 }
 
 // Do a TURN allocation, establish a UDP connection, and send some data.
 TEST_F(TurnPortTest, TestTurnSendDataTurnUdpToUdp) {
   // Create ports and prepare addresses.
   CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
   TestTurnSendData();
   EXPECT_EQ(cricket::UDP_PROTOCOL_NAME,
             turn_port_->Candidates()[0].relay_protocol());
 }
@@ skipping 39 matching lines @@
                            kTurnUdpProtoAddr, kTestOrigin);
   turn_port_->PrepareAddress();
   EXPECT_TRUE_WAIT(turn_ready_, kTimeout);
   ASSERT_GT(turn_server_.server()->allocations().size(), 0U);
   SocketAddress local_address = turn_port_->GetLocalAddress();
   ASSERT_TRUE(turn_server_.FindAllocation(local_address) != NULL);
   EXPECT_EQ(kTestOrigin, turn_server_.FindAllocation(local_address)->origin());
 }
 
 // Test that a CreatePermission failure will result in the connection being
-// destroyed.
-TEST_F(TurnPortTest, TestConnectionDestroyedOnCreatePermissionFailure) {
+// pruned and failed.
+// TODO(honghaiz): Investigate why the test fails with FakeClock.
+TEST_F(TurnPortTest, TestConnectionFaildAndPredOnCreatePermissionFailure) {

[Taylor Brandstetter, 2016/06/21 18:48:08]

"AndPred" should be "AndPruned"?

[honghaiz3, 2016/06/22 01:50:41]

Done.

   turn_server_.AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP);
   turn_server_.server()->set_reject_private_addresses(true);
   CreateTurnPort(kTurnUsername, kTurnPassword, kTurnTcpProtoAddr);
   turn_port_->PrepareAddress();
   ASSERT_TRUE_WAIT(turn_ready_, kTimeout);
 
   CreateUdpPort(SocketAddress("10.0.0.10", 0));
   udp_port_->PrepareAddress();
   ASSERT_TRUE_WAIT(udp_ready_, kTimeout);
   // Create a connection.
   TestConnectionWrapper conn(turn_port_->CreateConnection(
       udp_port_->Candidates()[0], Port::ORIGIN_MESSAGE));
   ASSERT_TRUE(conn.connection() != nullptr);
 
-  // Asynchronously, CreatePermission request should be sent and fail, closing
-  // the connection.
-  EXPECT_TRUE_WAIT(conn.connection() == nullptr, kTimeout);
-  EXPECT_FALSE(turn_create_permission_success_);
+  // Asynchronously, CreatePermission request should be sent and fail, which
+  // will make the connection pruned and failed.
+  EXPECT_TRUE_WAIT(CheckConnectionFailedAndPruned(conn.connection()), kTimeout);
+  EXPECT_TRUE_WAIT(!turn_create_permission_success_, kTimeout);
+  rtc::Thread::Current()->ProcessMessages(500);

[Taylor Brandstetter, 2016/06/21 18:48:08]

Is the purpose of ProcessMessages(500) to make sure the connection isn't deleted
asynchronously? In that case, please try to get this test working with the fake
clock.

[honghaiz3, 2016/06/22 01:50:41]

Done. Fixing an issue in virtualsocketserver when virtualsocketserver is created
before the fakeclock is set. 

+  // Check that the connection is not deleted.
+  EXPECT_TRUE(conn.connection() != nullptr);
 }
 
 // Test that a TURN allocation is released when the port is closed.
 TEST_F(TurnPortTest, TestTurnReleaseAllocation) {
   CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
   turn_port_->PrepareAddress();
   EXPECT_TRUE_WAIT(turn_ready_, kTimeout);
 
   ASSERT_GT(turn_server_.server()->allocations().size(), 0U);
   turn_port_.reset();
@@ skipping 29 matching lines @@
   turn_port_->PrepareAddress();
   ASSERT_TRUE_WAIT(turn_error_, kResolverTimeout);
   EXPECT_TRUE(turn_port_->Candidates().empty());
   turn_port_.reset();
   rtc::Thread::Current()->Post(RTC_FROM_HERE, this, MSG_TESTFINISH);
   // Waiting for above message to be processed.
   ASSERT_TRUE_WAIT(test_finish_, kTimeout);
   EXPECT_EQ(last_fd_count, GetFDCount());
 }
 #endif