Adding an end-to-end connection time test.

webrtc/api/peerconnection_unittest.cc

Index: webrtc/api/peerconnection_unittest.cc
diff --git a/webrtc/api/peerconnection_unittest.cc b/webrtc/api/peerconnection_unittest.cc
index 4b06b90d01c6c1c9dd5ed7e9aaf181c9f1fddfb2..b9eb7e4db3f95d3892c30642292524833c8539fb 100644
--- a/webrtc/api/peerconnection_unittest.cc
+++ b/webrtc/api/peerconnection_unittest.cc
@@ -30,6 +30,7 @@
 #include "webrtc/api/test/fakertccertificategenerator.h"
 #include "webrtc/api/test/fakevideotrackrenderer.h"
 #include "webrtc/api/test/mockpeerconnectionobservers.h"
+#include "webrtc/base/fakenetwork.h"
 #include "webrtc/base/gunit.h"
 #include "webrtc/base/physicalsocketserver.h"
 #include "webrtc/base/ssladapter.h"
@@ -37,9 +38,10 @@
 #include "webrtc/base/thread.h"
 #include "webrtc/base/virtualsocketserver.h"
 #include "webrtc/media/engine/fakewebrtcvideoengine.h"
-#include "webrtc/p2p/base/fakeportallocator.h"
 #include "webrtc/p2p/base/p2pconstants.h"
 #include "webrtc/p2p/base/sessiondescription.h"
+#include "webrtc/p2p/base/testturnserver.h"
+#include "webrtc/p2p/client/basicportallocator.h"
 #include "webrtc/pc/mediasession.h"
 
 #define MAYBE_SKIP_TEST(feature)                    \
@@ -102,6 +104,9 @@ static const char kDataChannelLabel[] = "data_channel";
 static const int kDefaultSrtpCryptoSuite = rtc::SRTP_AES128_CM_SHA1_32;
 #endif
 
+// Used to simulate signaling ICE/SDP between two PeerConnections.
+enum { MSG_SDP_MESSAGE, MSG_ICE_MESSAGE };

[pthatcher1, 2016/08/08 22:08:51]

We can start using "enum class" instead of enum now.  This would look nice:

Message::SDP
Message::ICE


 

[Taylor Brandstetter, 2016/08/09 01:42:52]

One of the things "enum class" does is prevent implicit integer conversions. But
that's what this pattern relies on. In my opinion, using "enum class" but having
to use "static_cast<int>" and "static_cast<Message>" looks bad and defeats the
purpose.

+
 static void RemoveLinesFromSdp(const std::string& line_start,
                                std::string* sdp) {
   const char kSdpLineEnd[] = "\r\n";
@@ -167,18 +172,21 @@ class MockRtpReceiverObserver : public webrtc::RtpReceiverObserverInterface {
 
 class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
                                  public SignalingMessageReceiver,
-                                 public ObserverInterface {
+                                 public ObserverInterface,
+                                 public rtc::MessageHandler {
  public:
+  // If |config| is not provided, uses a default constructed RTCConfiguration.
   static PeerConnectionTestClient* CreateClientWithDtlsIdentityStore(
       const std::string& id,
       const MediaConstraintsInterface* constraints,
+      const webrtc::PeerConnectionInterface::RTCConfiguration* config,
       const PeerConnectionFactory::Options* options,
       std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
       bool prefer_constraint_apis,
       rtc::Thread* network_thread,
       rtc::Thread* worker_thread) {
     PeerConnectionTestClient* client(new PeerConnectionTestClient(id));
-    if (!client->Init(constraints, options, std::move(cert_generator),
+    if (!client->Init(constraints, config, options, std::move(cert_generator),
                       prefer_constraint_apis, network_thread, worker_thread)) {
       delete client;
       return nullptr;
@@ -189,6 +197,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
   static PeerConnectionTestClient* CreateClient(
       const std::string& id,
       const MediaConstraintsInterface* constraints,
+      const webrtc::PeerConnectionInterface::RTCConfiguration* config,
       const PeerConnectionFactory::Options* options,
       rtc::Thread* network_thread,
       rtc::Thread* worker_thread) {
@@ -196,9 +205,9 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
         rtc::SSLStreamAdapter::HaveDtlsSrtp() ?
             new FakeRTCCertificateGenerator() : nullptr);
 
-    return CreateClientWithDtlsIdentityStore(
-        id, constraints, options, std::move(cert_generator), true,
-        network_thread, worker_thread);
+    return CreateClientWithDtlsIdentityStore(id, constraints, config, options,
+                                             std::move(cert_generator), true,
+                                             network_thread, worker_thread);
   }
 
   static PeerConnectionTestClient* CreateClientPreferNoConstraints(
@@ -210,9 +219,9 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
         rtc::SSLStreamAdapter::HaveDtlsSrtp() ?
             new FakeRTCCertificateGenerator() : nullptr);
 
-    return CreateClientWithDtlsIdentityStore(
-        id, nullptr, options, std::move(cert_generator), false,
-        network_thread, worker_thread);
+    return CreateClientWithDtlsIdentityStore(id, nullptr, nullptr, options,
+                                             std::move(cert_generator), false,
+                                             network_thread, worker_thread);
   }
 
   ~PeerConnectionTestClient() {
@@ -234,8 +243,68 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
     std::string sdp;
     EXPECT_TRUE(offer->ToString(&sdp));
     EXPECT_TRUE(DoSetLocalDescription(offer.release()));
-    signaling_message_receiver_->ReceiveSdpMessage(
-        webrtc::SessionDescriptionInterface::kOffer, sdp);
+    SendSdpMessage(webrtc::SessionDescriptionInterface::kOffer, sdp);
+  }
+
+  typedef std::pair<std::string, std::string> SdpMessage;

[pthatcher1, 2016/08/08 22:08:51]

Why not just use a struct for the 3 things you have here?  It's pretty fast to
define a constructor, and "message.type" sure looks better than
"std::get<0>(message)"

[Taylor Brandstetter, 2016/08/09 01:42:52]

Done. Though no need for a constructor in my opinion.

+  void SendSdpMessage(const std::string& type, std::string& msg) {
+    if (signaling_delay_ms_ == 0) {
+      if (signaling_message_receiver_) {
+        signaling_message_receiver_->ReceiveSdpMessage(type, msg);
+      }
+    } else {
+      rtc::Thread::Current()->PostDelayed(
+          RTC_FROM_HERE, signaling_delay_ms_, this, MSG_SDP_MESSAGE,
+          new rtc::TypedMessageData<SdpMessage>(SdpMessage(type, msg)));
+    }
+  }
+
+  typedef std::tuple<std::string, int, std::string> IceMessage;
+  void SendIceMessage(const std::string& sdp_mid,
+                      int sdp_mline_index,
+                      const std::string& msg) {
+    if (signaling_delay_ms_ == 0) {
+      if (signaling_message_receiver_) {
+        signaling_message_receiver_->ReceiveIceMessage(sdp_mid, sdp_mline_index,
+                                                       msg);
+      }
+    } else {
+      rtc::Thread::Current()->PostDelayed(
+          RTC_FROM_HERE, signaling_delay_ms_, this, MSG_ICE_MESSAGE,
+          new rtc::TypedMessageData<IceMessage>(
+              IceMessage(sdp_mid, sdp_mline_index, msg)));
+    }
+  }
+
+  // MessageHandler callback.
+  void OnMessage(rtc::Message* msg) override {
+    switch (msg->message_id) {
+      case MSG_SDP_MESSAGE: {
+        auto sdp_message =
+            static_cast<rtc::TypedMessageData<SdpMessage>*>(msg->pdata);
+        if (signaling_message_receiver_) {
+          signaling_message_receiver_->ReceiveSdpMessage(
+              std::get<0>(sdp_message->data()),
+              std::get<1>(sdp_message->data()));
+        }
+        delete sdp_message;
+        break;
+      }
+      case MSG_ICE_MESSAGE: {
+        auto ice_message =
+            static_cast<rtc::TypedMessageData<IceMessage>*>(msg->pdata);
+        if (signaling_message_receiver_) {
+          signaling_message_receiver_->ReceiveIceMessage(
+              std::get<0>(ice_message->data()),
+              std::get<1>(ice_message->data()),
+              std::get<2>(ice_message->data()));
+        }
+        delete ice_message;
+        break;
+      }
+      default:
+        RTC_CHECK(false);
+    }
   }
 
   // SignalingMessageReceiver callback.
@@ -294,8 +363,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
       // Remote party may be deleted.
       return;
     }
-    signaling_message_receiver_->ReceiveIceMessage(
-        candidate->sdp_mid(), candidate->sdp_mline_index(), ice_sdp);
+    SendIceMessage(candidate->sdp_mid(), candidate->sdp_mline_index(), ice_sdp);
   }
 
   // MediaStreamInterface callback
@@ -370,6 +438,8 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
     signaling_message_receiver_ = signaling_message_receiver;
   }
 
+  void set_signaling_delay_ms(int delay_ms) { signaling_delay_ms_ = delay_ms; }
+
   void EnableVideoDecoderFactory() {
     video_decoder_factory_enabled_ = true;
     fake_video_decoder_factory_->AddSupportedVideoCodecType(
@@ -388,6 +458,9 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
 
   bool ExpectIceRestart() const { return expect_ice_restart_; }
 
+  // Below 3 methods assume streams will be offered.
+  // Thus they'll only set the "offer to receive" flag to true if it's
+  // currently false, not if it's just unset.
   void SetReceiveAudioVideo(bool audio, bool video) {
     SetReceiveAudio(audio);
     SetReceiveVideo(video);
@@ -396,15 +469,24 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
   }
 
   void SetReceiveAudio(bool audio) {
-    if (audio && can_receive_audio())
+    if (audio && can_receive_audio()) {
       return;
+    }
     offer_answer_constraints_.SetMandatoryReceiveAudio(audio);
     offer_answer_options_.offer_to_receive_audio = audio ? 1 : 0;
   }
 
   void SetReceiveVideo(bool video) {
-    if (video && can_receive_video())
+    if (video && can_receive_video()) {
       return;
+    }
+    offer_answer_constraints_.SetMandatoryReceiveVideo(video);
+    offer_answer_options_.offer_to_receive_video = video ? 1 : 0;
+  }
+
+  void SetOfferToReceiveAudioVideo(bool audio, bool video) {
+    offer_answer_constraints_.SetMandatoryReceiveAudio(audio);
+    offer_answer_options_.offer_to_receive_audio = audio ? 1 : 0;
     offer_answer_constraints_.SetMandatoryReceiveVideo(video);
     offer_answer_options_.offer_to_receive_video = video ? 1 : 0;
   }
@@ -843,6 +925,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
 
   bool Init(
       const MediaConstraintsInterface* constraints,
+      const webrtc::PeerConnectionInterface::RTCConfiguration* config,
       const PeerConnectionFactory::Options* options,
       std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
       bool prefer_constraint_apis,
@@ -855,8 +938,11 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
     }
     prefer_constraint_apis_ = prefer_constraint_apis;
 
+    fake_network_manager_.reset(new rtc::FakeNetworkManager());
+    fake_network_manager_->AddInterface(rtc::SocketAddress("192.168.1.1", 0));
+
     std::unique_ptr<cricket::PortAllocator> port_allocator(
-        new cricket::FakePortAllocator(network_thread, nullptr));
+        new cricket::BasicPortAllocator(fake_network_manager_.get()));
     fake_audio_capture_module_ = FakeAudioCaptureModule::Create();
 
     if (fake_audio_capture_module_ == nullptr) {
@@ -875,23 +961,26 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
     if (options) {
       peer_connection_factory_->SetOptions(*options);
     }
-    peer_connection_ = CreatePeerConnection(
-        std::move(port_allocator), constraints, std::move(cert_generator));
+    peer_connection_ =
+        CreatePeerConnection(std::move(port_allocator), constraints, config,
+                             std::move(cert_generator));
     return peer_connection_.get() != nullptr;
   }
 
   rtc::scoped_refptr<webrtc::PeerConnectionInterface> CreatePeerConnection(
       std::unique_ptr<cricket::PortAllocator> port_allocator,
       const MediaConstraintsInterface* constraints,
+      const webrtc::PeerConnectionInterface::RTCConfiguration* config,
       std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator) {
     // CreatePeerConnection with RTCConfiguration.
-    webrtc::PeerConnectionInterface::RTCConfiguration config;
-    webrtc::PeerConnectionInterface::IceServer ice_server;
-    ice_server.uri = "stun:stun.l.google.com:19302";
-    config.servers.push_back(ice_server);
+    webrtc::PeerConnectionInterface::RTCConfiguration default_config;
+
+    if (!config) {
+      config = &default_config;
+    }
 
     return peer_connection_factory_->CreatePeerConnection(
-        config, constraints, std::move(port_allocator),
+        *config, constraints, std::move(port_allocator),
         std::move(cert_generator), this);
   }
 
@@ -911,10 +1000,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
     std::string sdp;
     EXPECT_TRUE(answer->ToString(&sdp));
     EXPECT_TRUE(DoSetLocalDescription(answer.release()));
-    if (signaling_message_receiver_) {
-      signaling_message_receiver_->ReceiveSdpMessage(
-          webrtc::SessionDescriptionInterface::kAnswer, sdp);
-    }
+    SendSdpMessage(webrtc::SessionDescriptionInterface::kAnswer, sdp);
   }
 
   void HandleIncomingAnswer(const std::string& msg) {
@@ -1015,6 +1101,8 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
 
   std::string id_;
 
+  std::unique_ptr<rtc::FakeNetworkManager> fake_network_manager_;
+
   rtc::scoped_refptr<webrtc::PeerConnectionInterface> peer_connection_;
   rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface>
       peer_connection_factory_;
@@ -1043,6 +1131,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
 
   // For remote peer communication.
   SignalingMessageReceiver* signaling_message_receiver_ = nullptr;
+  int signaling_delay_ms_ = 0;
 
   // Store references to the video capturers we've created, so that we can stop
   // them, if required.
@@ -1179,8 +1268,8 @@ class P2PTestConductor : public testing::Test {
 
   bool CreateTestClients(MediaConstraintsInterface* init_constraints,
                          MediaConstraintsInterface* recv_constraints) {
-    return CreateTestClients(init_constraints, nullptr, recv_constraints,
-                             nullptr);
+    return CreateTestClients(init_constraints, nullptr, nullptr,
+                             recv_constraints, nullptr, nullptr);
   }
 
   bool CreateTestClientsThatPreferNoConstraints() {
@@ -1204,16 +1293,24 @@ class P2PTestConductor : public testing::Test {
     receiving_client_->set_signaling_message_receiver(initiating_client_.get());
   }
 
-  bool CreateTestClients(MediaConstraintsInterface* init_constraints,
-                         PeerConnectionFactory::Options* init_options,
-                         MediaConstraintsInterface* recv_constraints,
-                         PeerConnectionFactory::Options* recv_options) {
+  void SetSignalingDelayMs(int delay_ms) {
+    initiating_client_->set_signaling_delay_ms(delay_ms);
+    receiving_client_->set_signaling_delay_ms(delay_ms);
+  }
+
+  bool CreateTestClients(
+      MediaConstraintsInterface* init_constraints,
+      const webrtc::PeerConnectionInterface::RTCConfiguration* init_config,
+      PeerConnectionFactory::Options* init_options,
+      MediaConstraintsInterface* recv_constraints,
+      const webrtc::PeerConnectionInterface::RTCConfiguration* recv_config,
+      PeerConnectionFactory::Options* recv_options) {
     initiating_client_.reset(PeerConnectionTestClient::CreateClient(
-        "Caller: ", init_constraints, init_options, network_thread_.get(),
-        worker_thread_.get()));
+        "Caller: ", init_constraints, init_config, init_options,
+        network_thread_.get(), worker_thread_.get()));
     receiving_client_.reset(PeerConnectionTestClient::CreateClient(
-        "Callee: ", recv_constraints, recv_options, network_thread_.get(),
-        worker_thread_.get()));
+        "Callee: ", recv_constraints, recv_config, recv_options,
+        network_thread_.get(), worker_thread_.get()));
     if (!initiating_client_ || !receiving_client_) {
       return false;
     }
@@ -1313,7 +1410,7 @@ class P2PTestConductor : public testing::Test {
 
     // Make sure the new client is using a different certificate.
     return PeerConnectionTestClient::CreateClientWithDtlsIdentityStore(
-        "New Peer: ", &setup_constraints, nullptr,
+        "New Peer: ", &setup_constraints, nullptr, nullptr,
         std::move(cert_generator), prefer_constraint_apis_,
         network_thread_.get(), worker_thread_.get());
   }
@@ -1328,6 +1425,10 @@ class P2PTestConductor : public testing::Test {
     }
   }
 
+  rtc::Thread* network_thread() { return network_thread_.get(); }
+
+  rtc::VirtualSocketServer* virtual_socket_server() { return ss_.get(); }
+
   PeerConnectionTestClient* initializing_client() {
     return initiating_client_.get();
   }
@@ -1718,8 +1819,8 @@ TEST_F(P2PTestConductor, GetDtls12None) {
   init_options.ssl_max_version = rtc::SSL_PROTOCOL_DTLS_10;
   PeerConnectionFactory::Options recv_options;
   recv_options.ssl_max_version = rtc::SSL_PROTOCOL_DTLS_10;
-  ASSERT_TRUE(
-      CreateTestClients(nullptr, &init_options, nullptr, &recv_options));
+  ASSERT_TRUE(CreateTestClients(nullptr, nullptr, &init_options, nullptr,
+                                nullptr, &recv_options));
   rtc::scoped_refptr<webrtc::FakeMetricsObserver>
       init_observer = new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
   initializing_client()->pc()->RegisterUMAObserver(init_observer);
@@ -1743,8 +1844,8 @@ TEST_F(P2PTestConductor, GetDtls12Both) {
   init_options.ssl_max_version = rtc::SSL_PROTOCOL_DTLS_12;
   PeerConnectionFactory::Options recv_options;
   recv_options.ssl_max_version = rtc::SSL_PROTOCOL_DTLS_12;
-  ASSERT_TRUE(
-      CreateTestClients(nullptr, &init_options, nullptr, &recv_options));
+  ASSERT_TRUE(CreateTestClients(nullptr, nullptr, &init_options, nullptr,
+                                nullptr, &recv_options));
   rtc::scoped_refptr<webrtc::FakeMetricsObserver>
       init_observer = new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
   initializing_client()->pc()->RegisterUMAObserver(init_observer);
@@ -1769,8 +1870,8 @@ TEST_F(P2PTestConductor, GetDtls12Init) {
   init_options.ssl_max_version = rtc::SSL_PROTOCOL_DTLS_12;
   PeerConnectionFactory::Options recv_options;
   recv_options.ssl_max_version = rtc::SSL_PROTOCOL_DTLS_10;
-  ASSERT_TRUE(
-      CreateTestClients(nullptr, &init_options, nullptr, &recv_options));
+  ASSERT_TRUE(CreateTestClients(nullptr, nullptr, &init_options, nullptr,
+                                nullptr, &recv_options));
   rtc::scoped_refptr<webrtc::FakeMetricsObserver>
       init_observer = new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
   initializing_client()->pc()->RegisterUMAObserver(init_observer);
@@ -1795,8 +1896,8 @@ TEST_F(P2PTestConductor, GetDtls12Recv) {
   init_options.ssl_max_version = rtc::SSL_PROTOCOL_DTLS_10;
   PeerConnectionFactory::Options recv_options;
   recv_options.ssl_max_version = rtc::SSL_PROTOCOL_DTLS_12;
-  ASSERT_TRUE(
-      CreateTestClients(nullptr, &init_options, nullptr, &recv_options));
+  ASSERT_TRUE(CreateTestClients(nullptr, nullptr, &init_options, nullptr,
+                                nullptr, &recv_options));
   rtc::scoped_refptr<webrtc::FakeMetricsObserver>
       init_observer = new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
   initializing_client()->pc()->RegisterUMAObserver(init_observer);
@@ -2106,6 +2207,103 @@ TEST_F(P2PTestConductor, ForwardVideoOnlyStream) {
       kMaxWaitForFramesMs);
 }
 
+// Test that we achieve the expected end-to-end connection time, using a
+// fake clock and simulated latency on the media and signaling paths.
+// We use a TURN<->TURN connection because this is usually the quickest to
+// set up initially, especially when we're confident the connection will work
+// and can start sending media before we get a STUN response.
+//
+// With various optimizations enabled, here are the trips we expect to be
+// required:
+// 1. 2 signaling trip: Signaling offer and offerer's TURN candidate, then

[pthatcher1, 2016/08/08 22:08:51]

trip => trips

[Taylor Brandstetter, 2016/08/09 01:42:52]

Done.

+//                      signaling answer (with DTLS fingerprint).
+// 2. 9 media trips: Rest of the DTLS handshake. 3 hops in each direction when
+//                   using TURN<->TURN pair, and DTLS exchange is 4 packets,
+//                   the first of which should have arrived before the answer.

[pthatcher1, 2016/08/08 22:08:51]

2 signaling trips == 1 signaling round trip?
9 media trips == 4.5 media round trips?

[Taylor Brandstetter, 2016/08/09 01:42:52]

A round trip over TURN<->TURN is actually modeled as 6 hops here (going between
TURN servers is a separate hop). I think the issue here is I said "trips" when I
meant "hops".

+TEST_F(P2PTestConductor, EndToEndConnectionTimeWithTurnTurnPair) {
+  rtc::ScopedFakeClock fake_clock;
+  // Some things use a time of "0" as a special value, so we need to start out
+  // the fake clock at a nonzero time.
+  // TODO(deadbeef): Fix this.
+  fake_clock.AdvanceTime(rtc::TimeDelta::FromSeconds(1));
+
+  static constexpr int media_trip_delay_ms = 50;
+  static constexpr int signaling_trip_delay_ms = 500;
+  // For explanation of these values, see comment above.
+  static constexpr int required_media_trips = 9;
+  static constexpr int required_signaling_trips = 2;
+  // For internal delays (such as posting an event asychronously).
+  static constexpr int allowed_internal_delay_ms = 20;
+  static constexpr int total_connection_time_ms =
+      media_trip_delay_ms * required_media_trips +
+      signaling_trip_delay_ms * required_signaling_trips +
+      allowed_internal_delay_ms;
+
+  static const rtc::SocketAddress turn_server_1_internal_address{"88.88.88.0",
+                                                                 3478};
+  static const rtc::SocketAddress turn_server_1_external_address{"88.88.88.1",
+                                                                 0};
+  static const rtc::SocketAddress turn_server_2_internal_address{"99.99.99.0",
+                                                                 3478};
+  static const rtc::SocketAddress turn_server_2_external_address{"99.99.99.1",
+                                                                 0};
+  cricket::TestTurnServer turn_server_1(network_thread(),
+                                        turn_server_1_internal_address,
+                                        turn_server_1_external_address);
+  cricket::TestTurnServer turn_server_2(network_thread(),
+                                        turn_server_2_internal_address,
+                                        turn_server_2_external_address);
+  // Bypass permission check on received packets so media can be sent before
+  // the candidate is signaled.
+  turn_server_1.set_enable_permission_checks(false);
+  turn_server_2.set_enable_permission_checks(false);
+
+  webrtc::PeerConnectionInterface::RTCConfiguration client_1_config;
+  webrtc::PeerConnectionInterface::IceServer ice_server_1;
+  ice_server_1.urls.push_back("turn:88.88.88.0:3478");
+  ice_server_1.username = "test";
+  ice_server_1.password = "test";
+  client_1_config.servers.push_back(ice_server_1);
+  client_1_config.type = webrtc::PeerConnectionInterface::kRelay;
+  client_1_config.presume_writable_when_fully_relayed = true;
+
+  webrtc::PeerConnectionInterface::RTCConfiguration client_2_config;
+  webrtc::PeerConnectionInterface::IceServer ice_server_2;
+  ice_server_2.urls.push_back("turn:99.99.99.0:3478");
+  ice_server_2.username = "test";
+  ice_server_2.password = "test";
+  client_2_config.servers.push_back(ice_server_2);
+  client_2_config.type = webrtc::PeerConnectionInterface::kRelay;
+  client_2_config.presume_writable_when_fully_relayed = true;
+
+  ASSERT_TRUE(CreateTestClients(nullptr, &client_1_config, nullptr, nullptr,
+                                &client_2_config, nullptr));
+  // Set up the simulated delays.
+  SetSignalingDelayMs(signaling_trip_delay_ms);
+  virtual_socket_server()->set_delay_mean(media_trip_delay_ms);
+  virtual_socket_server()->UpdateDelayDistribution();
+
+  initializing_client()->SetOfferToReceiveAudioVideo(true, true);
+  initializing_client()->Negotiate();
+  // TODO(deadbeef): kIceConnectionConnected currently means both ICE and DTLS
+  // are connected. This is an important distinction. Once we have separate ICE
+  // and DTLS state, this check needs to use the DTLS state.
+  EXPECT_TRUE_SIMULATED_WAIT(
+      (receiving_client()->ice_connection_state() ==
+           webrtc::PeerConnectionInterface::kIceConnectionConnected ||
+       receiving_client()->ice_connection_state() ==
+           webrtc::PeerConnectionInterface::kIceConnectionCompleted) &&
+          (initializing_client()->ice_connection_state() ==
+               webrtc::PeerConnectionInterface::kIceConnectionConnected ||
+           initializing_client()->ice_connection_state() ==
+               webrtc::PeerConnectionInterface::kIceConnectionCompleted),
+      total_connection_time_ms, fake_clock);
+  // Need to free the clients here since they're using things we created on
+  // the stack.
+  delete set_initializing_client(nullptr);
+  delete set_receiving_client(nullptr);

[honghaiz3, 2016/08/05 22:45:05]

Shouldn't they be destroyed when the test is done as they are both held by a
uniqe_ptr?

[Taylor Brandstetter, 2016/08/05 23:02:49]

See the comment above. They need to be freed before the things we created on the
stack go out of scope.

[honghaiz3, 2016/08/05 23:37:11]

Is it because you used the address client_1_config and client_2_config? I don't
see those addresses of the configs are stored anywhere inside the client though.

[Taylor Brandstetter, 2016/08/08 17:12:06]

Actually, it may just be the fake clock. I think it used to be something more,
but then I refactored the test.

[honghaiz3, 2016/08/08 17:17:29]

Ah. That's possible. 

+}
+
 class IceServerParsingTest : public testing::Test {
  public:
   // Convenience for parsing a single URL.