Start ICE connectivity checks as soon as the first pair is pingable.

webrtc/p2p/base/p2ptransportchannel.cc

 /*
  *  Copyright 2004 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.
  */
 
@@ skipping 136 matching lines @@
   connection->SignalReadPacket.connect(
       this, &P2PTransportChannel::OnReadPacket);
   connection->SignalReadyToSend.connect(
       this, &P2PTransportChannel::OnReadyToSend);
   connection->SignalStateChange.connect(
       this, &P2PTransportChannel::OnConnectionStateChange);
   connection->SignalDestroyed.connect(
       this, &P2PTransportChannel::OnConnectionDestroyed);
   connection->SignalNominated.connect(this, &P2PTransportChannel::OnNominated);
   had_connection_ = true;
+  if (!started_pinging_) {
+    // Start checking and pinging as soon as we have the first connection.
+    thread()->Post(RTC_FROM_HERE, this, MSG_CHECK_AND_PING);

[honghaiz3, 2016/06/27 16:24:20]

Should we worry about that the first connection may not have ice_pwd?
It may be created from a stun ping from unknown address. 
If that happened and the signaling arrived shortly after, it may effectively
delay the first stun ping request by up to 50ms. 

[pthatcher1, 2016/06/27 18:32:20]

It's true that we need to make sure we have the remote pwd.   Perhaps a simple
(if (!connection->remote_candidate().pwd().empty())) check would do. 

But then we'll need to maybe start pinging inside of SetRemoteIceCredentials if
connections_ is not empty.  

By the way, we should probably add a test for this case: that we ping when we
get the remote pwd and a connection, but not until both.

[Taylor Brandstetter, 2016/06/27 22:19:34]

Great observation, Honghai. I was only thinking about the callee pinging as soon
as possible (which generally has ICE credentials before it has candidates) and
not the caller.

I fixed this and added a test. With the fake clock, a test for this is pretty
simple to write.

+    started_pinging_ = true;

[honghaiz3, 2016/06/27 16:24:20]

The two variables had_connection_ and started_pinging_  have identical values. 
Should we just keep one of them?

[pthatcher1, 2016/06/27 18:32:20]

I won't any more if we check that we have the remote pwd.

[Taylor Brandstetter, 2016/06/27 22:19:34]

Peter is correct.

+  }
 }
 
 // Determines whether we should switch the selected connection to
 // |new_connection| based the writable/receiving state, the nomination state,
 // and the last data received time. This prevents the controlled side from
 // switching the selected connection too frequently when the controlling side
 // is doing aggressive nominations. The precedence of the connection switching
 // criteria is as follows:
 // i) write/receiving/connected states
 // ii) For controlled side,
@@ skipping 178 matching lines @@
       LOG(LS_INFO) << "Set presume writable when fully relayed to "
                    << config_.presume_writable_when_fully_relayed;
     }
   }
 }
 
 const IceConfig& P2PTransportChannel::config() const {
   return config_;
 }
 
-// Go into the state of processing candidates, and running in general
-void P2PTransportChannel::Connect() {
-  ASSERT(worker_thread_ == rtc::Thread::Current());
+void P2PTransportChannel::MaybeStartGathering() {
   if (ice_ufrag_.empty() || ice_pwd_.empty()) {
-    ASSERT(false);
-    LOG(LS_ERROR) << "P2PTransportChannel::Connect: The ice_ufrag_ and the "
-                  << "ice_pwd_ are not set.";
     return;
   }
-
-  // Start checking and pinging as the ports come in.
-  thread()->Post(RTC_FROM_HERE, this, MSG_CHECK_AND_PING);
-}
-
-void P2PTransportChannel::MaybeStartGathering() {
   // Start gathering if we never started before, or if an ICE restart occurred.
   if (allocator_sessions_.empty() ||
       IceCredentialsChanged(allocator_sessions_.back()->ice_ufrag(),
                             allocator_sessions_.back()->ice_pwd(), ice_ufrag_,
                             ice_pwd_)) {
     if (gathering_state_ != kIceGatheringGathering) {
       gathering_state_ = kIceGatheringGathering;
       SignalGatheringState(this);
     }
     // Time for a new allocator.
@@ skipping 1419 matching lines @@
 
   // During the initial state when nothing has been pinged yet, return the first
   // one in the ordered |connections_|.
   return *(std::find_if(connections_.begin(), connections_.end(),
                         [conn1, conn2](Connection* conn) {
                           return conn == conn1 || conn == conn2;
                         }));
 }
 
 }  // namespace cricket