Merging BaseSession code into WebRtcSession.

talk/app/webrtc/webrtcsession_unittest.cc

Index: talk/app/webrtc/webrtcsession_unittest.cc
diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc
index 2853ca43a7301b5e28b202f60041f35784342bb2..068aa84daeeef38affbb398b96bde5891bd67097 100644
--- a/talk/app/webrtc/webrtcsession_unittest.cc
+++ b/talk/app/webrtc/webrtcsession_unittest.cc
@@ -68,7 +68,6 @@
     return;                                         \
   }
 
-using cricket::BaseSession;
 using cricket::DF_PLAY;
 using cricket::DF_SEND;
 using cricket::FakeVoiceMediaChannel;
@@ -88,6 +87,7 @@ using webrtc::JsepSessionDescription;
 using webrtc::PeerConnectionFactoryInterface;
 using webrtc::PeerConnectionInterface;
 using webrtc::SessionDescriptionInterface;
+using webrtc::SessionStats;
 using webrtc::StreamCollection;
 using webrtc::WebRtcSession;
 using webrtc::kBundleWithoutRtcpMux;
@@ -758,7 +758,7 @@ class WebRtcSessionTest
     session_->MaybeStartGathering();
   }
   void SetLocalDescriptionExpectState(SessionDescriptionInterface* desc,
-                                      BaseSession::State expected_state) {
+                                      WebRtcSession::State expected_state) {
     SetLocalDescriptionWithoutError(desc);
     EXPECT_EQ(expected_state, session_->state());
   }
@@ -786,7 +786,7 @@ class WebRtcSessionTest
     EXPECT_TRUE(session_->SetRemoteDescription(desc, NULL));
   }
   void SetRemoteDescriptionExpectState(SessionDescriptionInterface* desc,
-                                       BaseSession::State expected_state) {
+                                       WebRtcSession::State expected_state) {
     SetRemoteDescriptionWithoutError(desc);
     EXPECT_EQ(expected_state, session_->state());
   }
@@ -1852,37 +1852,37 @@ TEST_F(WebRtcSessionTest, TestSetLocalPrAnswer) {
   Init();
   mediastream_signaling_.SendNothing();
   SessionDescriptionInterface* offer = CreateRemoteOffer();
-  SetRemoteDescriptionExpectState(offer, BaseSession::STATE_RECEIVEDINITIATE);
+  SetRemoteDescriptionExpectState(offer, WebRtcSession::STATE_RECEIVEDINITIATE);
 
   JsepSessionDescription* pranswer = static_cast<JsepSessionDescription*>(
       CreateAnswer(NULL));
   pranswer->set_type(SessionDescriptionInterface::kPrAnswer);
-  SetLocalDescriptionExpectState(pranswer, BaseSession::STATE_SENTPRACCEPT);
+  SetLocalDescriptionExpectState(pranswer, WebRtcSession::STATE_SENTPRACCEPT);
 
   mediastream_signaling_.SendAudioVideoStream1();
   JsepSessionDescription* pranswer2 = static_cast<JsepSessionDescription*>(
       CreateAnswer(NULL));
   pranswer2->set_type(SessionDescriptionInterface::kPrAnswer);
 
-  SetLocalDescriptionExpectState(pranswer2, BaseSession::STATE_SENTPRACCEPT);
+  SetLocalDescriptionExpectState(pranswer2, WebRtcSession::STATE_SENTPRACCEPT);
 
   mediastream_signaling_.SendAudioVideoStream2();
   SessionDescriptionInterface* answer = CreateAnswer(NULL);
-  SetLocalDescriptionExpectState(answer, BaseSession::STATE_SENTACCEPT);
+  SetLocalDescriptionExpectState(answer, WebRtcSession::STATE_SENTACCEPT);
 }
 
 TEST_F(WebRtcSessionTest, TestSetRemotePrAnswer) {
   Init();
   mediastream_signaling_.SendNothing();
   SessionDescriptionInterface* offer = CreateOffer();
-  SetLocalDescriptionExpectState(offer, BaseSession::STATE_SENTINITIATE);
+  SetLocalDescriptionExpectState(offer, WebRtcSession::STATE_SENTINITIATE);
 
   JsepSessionDescription* pranswer =
       CreateRemoteAnswer(session_->local_description());
   pranswer->set_type(SessionDescriptionInterface::kPrAnswer);
 
   SetRemoteDescriptionExpectState(pranswer,
-                                  BaseSession::STATE_RECEIVEDPRACCEPT);
+                                  WebRtcSession::STATE_RECEIVEDPRACCEPT);
 
   mediastream_signaling_.SendAudioVideoStream1();
   JsepSessionDescription* pranswer2 =
@@ -1890,12 +1890,12 @@ TEST_F(WebRtcSessionTest, TestSetRemotePrAnswer) {
   pranswer2->set_type(SessionDescriptionInterface::kPrAnswer);
 
   SetRemoteDescriptionExpectState(pranswer2,
-                                  BaseSession::STATE_RECEIVEDPRACCEPT);
+                                  WebRtcSession::STATE_RECEIVEDPRACCEPT);
 
   mediastream_signaling_.SendAudioVideoStream2();
   SessionDescriptionInterface* answer =
       CreateRemoteAnswer(session_->local_description());
-  SetRemoteDescriptionExpectState(answer, BaseSession::STATE_RECEIVEDACCEPT);
+  SetRemoteDescriptionExpectState(answer, WebRtcSession::STATE_RECEIVEDACCEPT);
 }
 
 TEST_F(WebRtcSessionTest, TestSetLocalAnswerWithoutOffer) {
@@ -2119,8 +2119,9 @@ TEST_F(WebRtcSessionTest, TestChannelCreationsWithContentNames) {
   rtc::scoped_ptr<SessionDescriptionInterface> offer(CreateOffer());
 
   // CreateOffer creates session description with the content names "audio" and
-  // "video". Goal is to modify these content names and verify transport channel
-  // proxy in the BaseSession, as proxies are created with the content names
+  // "video". Goal is to modify these content names and verify transport
+  // channels
+  // in the WebRtcSession, as channels are created with the content names
   // present in SDP.
   std::string sdp;
   EXPECT_TRUE(offer->ToString(&sdp));
@@ -2700,7 +2701,7 @@ TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionWithIceRestart) {
 }
 
 // Test that candidates sent to the "video" transport do not get pushed down to
-// the "audio" transport channel when bundling using TransportProxy.
+// the "audio" transport channel when bundling.
 TEST_F(WebRtcSessionTest, TestIgnoreCandidatesForUnusedTransportWhenBundling) {
   AddInterface(rtc::SocketAddress(kClientAddrHost1, kClientAddrPort));
 
@@ -2725,7 +2726,7 @@ TEST_F(WebRtcSessionTest, TestIgnoreCandidatesForUnusedTransportWhenBundling) {
   // Checks if one of the transport channels contains a connection using a given
   // port.
   auto connection_with_remote_port = [this, voice_channel](int port) {
-    cricket::SessionStats stats;
+    SessionStats stats;
     session_->GetChannelTransportStats(voice_channel, &stats);
     for (auto& kv : stats.transport_stats) {
       for (auto& chan_stat : kv.second.channel_stats) {
@@ -2785,6 +2786,54 @@ TEST_F(WebRtcSessionTest, TestIgnoreCandidatesForUnusedTransportWhenBundling) {
   EXPECT_FALSE(connection_with_remote_port(6000));
 }
 
+// Verifies that if a local offer is set, then remote candidates are received

[Taylor Brandstetter, 2015/10/09 02:48:50]

This is a bug I found while working on the CL. In SetRemoteDescription,
CopySavedCandidates was being called in the wrong place, so it was doing
nothing.

+// before the remote description is received, the candidates are used and the
+// connection completes.
+// This only works if the local description has been set; we don't accept any
+// candidates before we have an offer description.
+TEST_F(WebRtcSessionTest, ReceiveRemoteCandidateBeforeRemoteAnswer) {
+  AddInterface(rtc::SocketAddress(kClientAddrHost1, kClientAddrPort));
+
+  Init();
+  mediastream_signaling_.SendAudioVideoStream1();
+
+  PeerConnectionInterface::RTCOfferAnswerOptions options;
+  options.use_rtp_mux = true;
+
+  SessionDescriptionInterface* offer = CreateOffer();
+  SetLocalDescriptionWithoutError(offer);
+
+  cricket::Candidate candidate;
+  candidate.set_address(rtc::SocketAddress("1.1.1.1", 6000));
+  candidate.set_component(1);
+  candidate.set_protocol("udp");
+  JsepIceCandidate ice_candidate(kMediaContentName0, kMediaContentIndex0,
+                                 candidate);
+  EXPECT_TRUE(session_->ProcessIceMessage(&ice_candidate));

[pthatcher1, 2015/10/09 03:28:57]

I don't think that was what the copy of the candidates was for.  I don't think
we're supposed to process remote candidates until after we get the remote
description.  

I think the copying was so that if we got a new remote description without
candidates, it wouldn't blow away the candidates in the remote description.  I'm
not entirely sure why we care what candidates are in the remote description
(since we don't signal them and by then they are already passed down to the
transport), but maybe there was a good reason at some point.

[Taylor Brandstetter, 2015/10/09 23:02:07]

We discussed this and got rid of saved_candidates_ altogether.

+
+  cricket::BaseChannel* voice_channel = session_->voice_channel();
+  ASSERT(voice_channel != nullptr);
+
+  // Checks if a connection (pair of local/remote candidates) has been created.
+  auto has_connection = [this, voice_channel]() {
+    SessionStats stats;
+    session_->GetChannelTransportStats(voice_channel, &stats);
+    for (auto& kv : stats.transport_stats) {
+      for (auto& chan_stat : kv.second.channel_stats) {
+        if (!chan_stat.connection_infos.empty()) {
+          return true;
+        }
+      }
+    }
+    return false;
+  };
+
+  SessionDescriptionInterface* answer = CreateRemoteAnswer(offer);
+  SetRemoteDescriptionWithoutError(answer);
+
+  EXPECT_TRUE_WAIT(has_connection(), 1000);
+}
+
 // kBundlePolicyBalanced BUNDLE policy and answer contains BUNDLE.
 TEST_F(WebRtcSessionTest, TestBalancedBundleInAnswer) {
   InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyBalanced);
@@ -3479,10 +3528,17 @@ TEST_F(WebRtcSessionTest, TestSessionContentError) {
   video_channel_ = media_engine_->GetVideoChannel(0);
   video_channel_->set_fail_set_send_codecs(true);
 
-  mediastream_signaling_.SendAudioVideoStream2();
   SessionDescriptionInterface* answer =
       CreateRemoteAnswer(session_->local_description());
   SetRemoteDescriptionAnswerExpectError("ERROR_CONTENT", answer);
+
+  // Test that after a content error, setting any description will
+  // result in an error.
+  video_channel_->set_fail_set_send_codecs(false);
+  answer = CreateRemoteAnswer(session_->local_description());
+  SetRemoteDescriptionExpectError("", "ERROR_CONTENT", answer);
+  offer = CreateRemoteOffer();
+  SetLocalDescriptionExpectError("", "ERROR_CONTENT", offer);
 }
 
 // Runs the loopback call test with BUNDLE and STUN disabled.
@@ -3516,28 +3572,6 @@ TEST_F(WebRtcSessionTest, TestIceStatesBundle) {
   TestLoopbackCall();
 }
 
-TEST_F(WebRtcSessionTest, SetSdpFailedOnSessionError) {

[Taylor Brandstetter, 2015/10/09 02:48:50]

I moved this test into the end of TestSessionContentError, since now SetError is
private (as it should be).

-  Init();
-  cricket::MediaSessionOptions options;
-  options.recv_video = true;
-
-  cricket::BaseSession::Error error_code = cricket::BaseSession::ERROR_CONTENT;
-  std::string error_code_str = "ERROR_CONTENT";
-  std::string error_desc = "Fake session error description.";
-  session_->SetError(error_code, error_desc);
-
-  SessionDescriptionInterface* offer = CreateRemoteOffer(options);
-  SessionDescriptionInterface* answer =
-      CreateRemoteAnswer(offer, options);
-
-  std::string action;
-  std::ostringstream session_error_msg;
-  session_error_msg << kSessionError << error_code_str << ". ";
-  session_error_msg << kSessionErrorDesc << error_desc << ".";
-  SetRemoteDescriptionExpectError(action, session_error_msg.str(), offer);
-  SetLocalDescriptionExpectError(action, session_error_msg.str(), answer);
-}
-
 TEST_F(WebRtcSessionTest, TestRtpDataChannel) {
   constraints_.reset(new FakeConstraints());
   constraints_->AddOptional(