Update talk to 59039880.

R=mallinath@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/6569004

git-svn-id: http://webrtc.googlecode.com/svn/trunk/talk@5339 4adac7df-926f-26a2-2b94-8c16560cd09d

4 files changed, 119 insertions, 28 deletions

diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index 24ef427..2fc2cb2 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -176,7 +176,7 @@ Port::Port(talk_base::Thread* thread, talk_base::PacketSocketFactory* factory,
       generation_(0),
       ice_username_fragment_(username_fragment),
       password_(password),
-      lifetime_(LT_PRESTART),
+      timeout_delay_(kPortTimeoutDelay),
       enable_port_packets_(false),
       ice_protocol_(ICEPROTO_GOOGLE),
       ice_role_(ICEROLE_UNKNOWN),
@@ -203,7 +203,7 @@ Port::Port(talk_base::Thread* thread, const std::string& type,
       generation_(0),
       ice_username_fragment_(username_fragment),
       password_(password),
-      lifetime_(LT_PRESTART),
+      timeout_delay_(kPortTimeoutDelay),
       enable_port_packets_(false),
       ice_protocol_(ICEPROTO_GOOGLE),
       ice_role_(ICEROLE_UNKNOWN),
@@ -669,8 +669,6 @@ void Port::SendBindingErrorResponse(StunMessage* request,
 
 void Port::OnMessage(talk_base::Message *pmsg) {
   ASSERT(pmsg->message_id == MSG_CHECKTIMEOUT);
-  ASSERT(lifetime_ == LT_PRETIMEOUT);
-  lifetime_ = LT_POSTTIMEOUT;
   CheckTimeout();
 }
 
@@ -686,24 +684,18 @@ void Port::EnablePortPackets() {
   enable_port_packets_ = true;
 }
 
-void Port::Start() {
-  // The port sticks around for a minimum lifetime, after which
-  // we destroy it when it drops to zero connections.
-  if (lifetime_ == LT_PRESTART) {
-    lifetime_ = LT_PRETIMEOUT;
-    thread_->PostDelayed(kPortTimeoutDelay, this, MSG_CHECKTIMEOUT);
-  } else {
-    LOG_J(LS_WARNING, this) << "Port restart attempted";
-  }
-}
-
 void Port::OnConnectionDestroyed(Connection* conn) {
   AddressMap::iterator iter =
       connections_.find(conn->remote_candidate().address());
   ASSERT(iter != connections_.end());
   connections_.erase(iter);
 
-  CheckTimeout();
+  // On the controlled side, ports time out, but only 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 (ice_role_ == ICEROLE_CONTROLLED)
+    thread_->PostDelayed(timeout_delay_, this, MSG_CHECKTIMEOUT);
 }
 
 void Port::Destroy() {
@@ -714,13 +706,13 @@ void Port::Destroy() {
 }
 
 void Port::CheckTimeout() {
+  ASSERT(ice_role_ == ICEROLE_CONTROLLED);
   // If this port has no connections, then there's no reason to keep it around.
   // When the connections time out (both read and write), they will delete
   // themselves, so if we have any connections, they are either readable or
   // writable (or still connecting).
-  if ((lifetime_ == LT_POSTTIMEOUT) && connections_.empty()) {
+  if (connections_.empty())
     Destroy();
-  }
 }
 
 const std::string Port::username_fragment() const {
diff --git a/p2p/base/port.h b/p2p/base/port.h
index 9ea3f0c..21f3d61 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -262,10 +262,6 @@ class Port : public PortInterface, public talk_base::MessageHandler,
 
   virtual void EnablePortPackets();
 
-  // Indicates to the port that its official use has now begun.  This will
-  // start the timer that checks to see if the port is being used.
-  void Start();
-
   // Called if the port has no connections and is no longer useful.
   void Destroy();
 
@@ -277,6 +273,9 @@ class Port : public PortInterface, public talk_base::MessageHandler,
   int min_port() { return min_port_; }
   int max_port() { return max_port_; }
 
+  // Timeout shortening function to speed up unit tests.
+  void set_timeout_delay(int delay) { timeout_delay_ = delay; }
+
   // This method will return local and remote username fragements from the
   // stun username attribute if present.
   bool ParseStunUsername(const StunMessage* stun_msg,
@@ -379,7 +378,7 @@ class Port : public PortInterface, public talk_base::MessageHandler,
   std::string password_;
   std::vector<Candidate> candidates_;
   AddressMap connections_;
-  enum Lifetime { LT_PRESTART, LT_PRETIMEOUT, LT_POSTTIMEOUT } lifetime_;
+  int timeout_delay_;
   bool enable_port_packets_;
   IceProtocolType ice_protocol_;
   IceRole ice_role_;
diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc
index 1122d8a..3ea6375 100644
--- a/p2p/base/port_unittest.cc
+++ b/p2p/base/port_unittest.cc
@@ -213,12 +213,14 @@ class TestPort : public Port {
 
 class TestChannel : public sigslot::has_slots<> {
  public:
+  // Takes ownership of |p1| (but not |p2|).
   TestChannel(Port* p1, Port* p2)
       : ice_mode_(ICEMODE_FULL), src_(p1), dst_(p2), complete_count_(0),
 	conn_(NULL), remote_request_(), nominated_(false) {
     src_->SignalPortComplete.connect(
         this, &TestChannel::OnPortComplete);
     src_->SignalUnknownAddress.connect(this, &TestChannel::OnUnknownAddress);
+    src_->SignalDestroyed.connect(this, &TestChannel::OnSrcPortDestroyed);
   }
 
   int complete_count() { return complete_count_; }
@@ -305,6 +307,11 @@ class TestChannel : public sigslot::has_slots<> {
     conn_ = NULL;
   }
 
+  void OnSrcPortDestroyed(PortInterface* port) {
+    Port* destroyed_src = src_.release();
+    ASSERT_EQ(destroyed_src, port);
+  }
+
   bool nominated() const { return nominated_; }
 
  private:
@@ -341,7 +348,8 @@ class PortTest : public testing::Test, public sigslot::has_slots<> {
         username_(talk_base::CreateRandomString(ICE_UFRAG_LENGTH)),
         password_(talk_base::CreateRandomString(ICE_PWD_LENGTH)),
         ice_protocol_(cricket::ICEPROTO_GOOGLE),
-        role_conflict_(false) {
+        role_conflict_(false),
+        destroyed_(false) {
     network_.AddIP(talk_base::IPAddress(INADDR_ANY));
   }
 
@@ -513,12 +521,17 @@ class PortTest : public testing::Test, public sigslot::has_slots<> {
 
   void TestCrossFamilyPorts(int type);
 
-  // this does all the work
+  // This does all the work and then deletes |port1| and |port2|.
   void TestConnectivity(const char* name1, Port* port1,
                         const char* name2, Port* port2,
                         bool accept, bool same_addr1,
                         bool same_addr2, bool possible);
 
+  // This connects and disconnects the provided channels in the same sequence as
+  // TestConnectivity with all options set to |true|.  It does not delete either
+  // channel.
+  void ConnectAndDisconnectChannels(TestChannel* ch1, TestChannel* ch2);
+
   void SetIceProtocolType(cricket::IceProtocolType protocol) {
     ice_protocol_ = protocol;
   }
@@ -562,6 +575,15 @@ class PortTest : public testing::Test, public sigslot::has_slots<> {
   }
   bool role_conflict() const { return role_conflict_; }
 
+  void ConnectToSignalDestroyed(PortInterface* port) {
+    port->SignalDestroyed.connect(this, &PortTest::OnDestroyed);
+  }
+
+  void OnDestroyed(PortInterface* port) {
+    destroyed_ = true;
+  }
+  bool destroyed() const { return destroyed_; }
+
   talk_base::BasicPacketSocketFactory* nat_socket_factory1() {
     return &nat_socket_factory1_;
   }
@@ -586,6 +608,7 @@ class PortTest : public testing::Test, public sigslot::has_slots<> {
   std::string password_;
   cricket::IceProtocolType ice_protocol_;
   bool role_conflict_;
+  bool destroyed_;
 };
 
 void PortTest::TestConnectivity(const char* name1, Port* port1,
@@ -596,7 +619,7 @@ void PortTest::TestConnectivity(const char* name1, Port* port1,
   port1->set_component(cricket::ICE_CANDIDATE_COMPONENT_DEFAULT);
   port2->set_component(cricket::ICE_CANDIDATE_COMPONENT_DEFAULT);
 
-  // Set up channels.
+  // Set up channels and ensure both ports will be deleted.
   TestChannel ch1(port1, port2);
   TestChannel ch2(port2, port1);
   EXPECT_EQ(0, ch1.complete_count());
@@ -719,6 +742,29 @@ void PortTest::TestConnectivity(const char* name1, Port* port1,
   EXPECT_TRUE_WAIT(ch2.conn() == NULL, kTimeout);
 }
 
+void PortTest::ConnectAndDisconnectChannels(TestChannel* ch1,
+                                            TestChannel* ch2) {
+  // Acquire addresses.
+  ch1->Start();
+  ch2->Start();
+
+  // Send a ping from src to dst.
+  ch1->CreateConnection();
+  EXPECT_TRUE_WAIT(ch1->conn()->connected(), kTimeout);  // for TCP connect
+  ch1->Ping();
+  WAIT(!ch2->remote_address().IsNil(), kTimeout);
+
+  // Send a ping from dst to src.
+  ch2->AcceptConnection();
+  ch2->Ping();
+  EXPECT_EQ_WAIT(Connection::STATE_WRITABLE, ch2->conn()->write_state(),
+                 kTimeout);
+
+  // Destroy the connections.
+  ch1->Stop();
+  ch2->Stop();
+}
+
 class FakePacketSocketFactory : public talk_base::PacketSocketFactory {
  public:
   FakePacketSocketFactory()
@@ -2292,3 +2338,59 @@ TEST_F(PortTest, TestIceLiteConnectivity) {
   EXPECT_TRUE(msg->GetByteString(STUN_ATTR_USE_CANDIDATE) != NULL);
   ch1.Stop();
 }
+
+// This test case verifies that the CONTROLLING port does not time out.
+TEST_F(PortTest, TestControllingNoTimeout) {
+  SetIceProtocolType(cricket::ICEPROTO_RFC5245);
+  UDPPort* port1 = CreateUdpPort(kLocalAddr1);
+  ConnectToSignalDestroyed(port1);
+  port1->set_timeout_delay(10);  // milliseconds
+  port1->SetIceRole(cricket::ICEROLE_CONTROLLING);
+  port1->SetIceTiebreaker(kTiebreaker1);
+
+  UDPPort* port2 = CreateUdpPort(kLocalAddr2);
+  port2->SetIceRole(cricket::ICEROLE_CONTROLLED);
+  port2->SetIceTiebreaker(kTiebreaker2);
+
+  // Set up channels and ensure both ports will be deleted.
+  TestChannel ch1(port1, port2);
+  TestChannel ch2(port2, port1);
+
+  // Simulate a connection that succeeds, and then is destroyed.
+  ConnectAndDisconnectChannels(&ch1, &ch2);
+
+  // After the connection is destroyed, the port should not be destroyed.
+  talk_base::Thread::Current()->ProcessMessages(kTimeout);
+  EXPECT_FALSE(destroyed());
+}
+
+// This test case verifies that the CONTROLLED port does time out, but only
+// after connectivity is lost.
+TEST_F(PortTest, TestControlledTimeout) {
+  SetIceProtocolType(cricket::ICEPROTO_RFC5245);
+  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->SetIceRole(cricket::ICEROLE_CONTROLLED);
+  port2->SetIceTiebreaker(kTiebreaker2);
+
+  // The connection must not be destroyed before a connection is attempted.
+  EXPECT_FALSE(destroyed());
+
+  port1->set_component(cricket::ICE_CANDIDATE_COMPONENT_DEFAULT);
+  port2->set_component(cricket::ICE_CANDIDATE_COMPONENT_DEFAULT);
+
+  // Set up channels and ensure both ports will be deleted.
+  TestChannel ch1(port1, port2);
+  TestChannel ch2(port2, port1);
+
+  // Simulate a connection that succeeds, and then is destroyed.
+  ConnectAndDisconnectChannels(&ch1, &ch2);
+
+  // The controlled port should be destroyed after 10 milliseconds.
+  EXPECT_TRUE_WAIT(destroyed(), kTimeout);
+}
diff --git a/p2p/client/basicportallocator.cc b/p2p/client/basicportallocator.cc
index dbc2e33..e568e75 100644
--- a/p2p/client/basicportallocator.cc
+++ b/p2p/client/basicportallocator.cc
@@ -515,8 +515,6 @@ void BasicPortAllocatorSession::AddAllocatedPort(Port* port,
 
   if (prepare_address)
     port->PrepareAddress();
-  if (running_)
-    port->Start();
 }
 
 void BasicPortAllocatorSession::OnAllocationSequenceObjectsCreated() {