Delay destroying a port if new connections are created and destroyed.

If all connections on a port is destroyed, it will schedule an event
to check if it is dead after a timeout. Previously if a new connection
is created but destroyed before the event is fired, it will destroy the
port. With this change, we will not destoy it until it times out again
after the last created connection is destroyed.

BUG=
R=pthatcher@webrtc.org, zhihuang@webrtc.org

Review URL: https://codereview.webrtc.org/2184013003 .

Cr-Commit-Position: refs/heads/master@{#13563}

webrtc/p2p/base/port.cc

@@ -645,7 +645,7 @@ void Port::SendBindingErrorResponse(StunMessage* request,
 }
 
 void Port::OnMessage(rtc::Message *pmsg) {
-  ASSERT(pmsg->message_id == MSG_DEAD);
+  ASSERT(pmsg->message_id == MSG_CHECK_DEAD);
   if (dead()) {
     Destroy();
   }
@@ -702,12 +702,19 @@ void Port::OnConnectionDestroyed(Connection* conn) {
   // On the controlled side, ports time out after all connections fail.
   // Note: If a new connection is added after this message is posted, but it
   // fails and is removed before kPortTimeoutDelay, then this message will
-  // still cause the Port to be destroyed.
-  if (dead()) {
-    thread_->PostDelayed(RTC_FROM_HERE, timeout_delay_, this, MSG_DEAD);
+  // not cause the Port to be destroyed.
+  if (ice_role_ == ICEROLE_CONTROLLED && connections_.empty()) {
+    last_time_all_connections_removed_ = rtc::TimeMillis();
+    thread_->PostDelayed(RTC_FROM_HERE, timeout_delay_, this, MSG_CHECK_DEAD);
   }
 }
 
+bool Port::dead() const {
+  return ice_role_ == ICEROLE_CONTROLLED && connections_.empty() &&
+         rtc::TimeMillis() - last_time_all_connections_removed_ >=
+             timeout_delay_;
+}
+
 void Port::Destroy() {
   ASSERT(connections_.empty());
   LOG_J(LS_INFO, this) << "Port deleted";

webrtc/p2p/base/port.h

@@ -297,10 +297,7 @@ class Port : public PortInterface, public rtc::MessageHandler,
   int16_t network_cost() const { return network_cost_; }
 
  protected:
-  enum {
-    MSG_DEAD = 0,
-    MSG_FIRST_AVAILABLE
-  };
+  enum { MSG_CHECK_DEAD = 0, MSG_FIRST_AVAILABLE };
 
   virtual void UpdateNetworkCost();
 
@@ -359,9 +356,7 @@ class Port : public PortInterface, public rtc::MessageHandler,
 
   // Whether this port is dead, and hence, should be destroyed on the controlled
   // side.
-  bool dead() const {
-    return ice_role_ == ICEROLE_CONTROLLED && connections_.empty();
-  }
+  bool dead() const;
 
   void OnNetworkTypeChanged(const rtc::Network* network);
 
@@ -401,6 +396,7 @@ class Port : public PortInterface, public rtc::MessageHandler,
   // (WiFi. vs. Cellular). It takes precedence over the priority when
   // comparing two connections.
   uint16_t network_cost_;
+  int64_t last_time_all_connections_removed_ = 0;
 
   friend class Connection;
 };

webrtc/p2p/base/port_unittest.cc

@@ -2592,15 +2592,17 @@ TEST_F(PortTest, TestControllingNoTimeout) {
 }
 
 // This test case verifies that the CONTROLLED port does time out, but only
-// after connectivity is lost.
+// after connectivity is lost and no connection was created during the timeout
+// period.
 TEST_F(PortTest, TestControlledTimeout) {
+  rtc::ScopedFakeClock clock;
   UDPPort* port1 = CreateUdpPort(kLocalAddr1);
   port1->SetIceRole(cricket::ICEROLE_CONTROLLING);
   port1->SetIceTiebreaker(kTiebreaker1);
 
   UDPPort* port2 = CreateUdpPort(kLocalAddr2);
   ConnectToSignalDestroyed(port2);
-  port2->set_timeout_delay(10);  // milliseconds
+  port2->set_timeout_delay(100);  // milliseconds
   port2->SetIceRole(cricket::ICEROLE_CONTROLLED);
   port2->SetIceTiebreaker(kTiebreaker2);
 
@@ -2617,8 +2619,21 @@ TEST_F(PortTest, TestControlledTimeout) {
   // Simulate a connection that succeeds, and then is destroyed.
   StartConnectAndStopChannels(&ch1, &ch2);
 
-  // The controlled port should be destroyed after 10 milliseconds.
-  EXPECT_TRUE_WAIT(destroyed(), kTimeout);
+  SIMULATED_WAIT(false, 80, clock);
+  ch2.CreateConnection(GetCandidate(ch1.port()));
+
+  // ch2 creates a connection so it will not be destroyed.
+  SIMULATED_WAIT(destroyed(), 80, clock);
+  EXPECT_FALSE(destroyed());
+
+  // Even if ch2 stops now, it won't be destroyed until 100ms after the
+  // connection is destroyed.
+  ch2.Stop();
+  SIMULATED_WAIT(destroyed(), 80, clock);
+  EXPECT_FALSE(destroyed());
+
+  // The controlled port should be destroyed after timeout.
+  EXPECT_TRUE_SIMULATED_WAIT(destroyed(), 30, clock);
 }
 
 // This test case verifies that if the role of a port changes from controlled