Do not create a temporary transport channel when using max-bundle

webrtc/api/webrtcsession_unittest.cc

 /*
  *  Copyright 2012 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 145 matching lines @@
       : oncandidatesready_(false),
         ice_connection_state_(PeerConnectionInterface::kIceConnectionNew),
         ice_gathering_state_(PeerConnectionInterface::kIceGatheringNew) {
   }
 
   virtual ~MockIceObserver() = default;
 
   void OnIceConnectionChange(
       PeerConnectionInterface::IceConnectionState new_state) override {
     ice_connection_state_ = new_state;
+    ice_connection_state_history_.push_back(new_state);
   }
   void OnIceGatheringChange(
       PeerConnectionInterface::IceGatheringState new_state) override {
     // We can never transition back to "new".
     EXPECT_NE(PeerConnectionInterface::kIceGatheringNew, new_state);
     ice_gathering_state_ = new_state;
     oncandidatesready_ =
         new_state == PeerConnectionInterface::kIceGatheringComplete;
   }
 
@@ skipping 19 matching lines @@
   void OnIceCandidatesRemoved(
       const std::vector<cricket::Candidate>& candidates) override {
     num_candidates_removed_ += candidates.size();
   }
 
   bool oncandidatesready_;
   std::vector<cricket::Candidate> mline_0_candidates_;
   std::vector<cricket::Candidate> mline_1_candidates_;
   PeerConnectionInterface::IceConnectionState ice_connection_state_;
   PeerConnectionInterface::IceGatheringState ice_gathering_state_;
+  std::vector<PeerConnectionInterface::IceConnectionState>
+      ice_connection_state_history_;
   size_t num_candidates_removed_ = 0;
 };
 
 class WebRtcSessionForTest : public webrtc::WebRtcSession {
  public:
   WebRtcSessionForTest(webrtc::MediaControllerInterface* media_controller,
                        rtc::Thread* signaling_thread,
                        rtc::Thread* worker_thread,
                        cricket::PortAllocator* port_allocator,
                        webrtc::IceObserver* ice_observer)
@@ skipping 3039 matching lines @@
   PeerConnectionInterface::RTCOfferAnswerOptions options;
   options.use_rtp_mux = true;
 
   SessionDescriptionInterface* offer = CreateOffer(options);
   SetRemoteDescriptionWithoutError(offer);
 
   EXPECT_EQ(session_->voice_rtp_transport_channel(),
             session_->video_rtp_transport_channel());
 }
 
+// Adding a new channel to a BUNDLE which is already connected should directly
+// assign the bundle transport to the channel, without first setting a
+// disconnected non-bundle transport and then replacing it. The application
+// should not receive any changes in the ICE state.
+TEST_F(WebRtcSessionTest, TestAddChannelToConnectedBundle) {
+  LoopbackNetworkConfiguration config;
+  LoopbackNetworkManager loopback_network_manager(this, config);
+  // Both BUNDLE and RTCP-mux need to be enabled for the ICE state to remain
+  // connected. Disabling either of these two means that we need to wait for the
+  // answer to find out if more transports are needed.
+  configuration_.bundle_policy =
+      PeerConnectionInterface::kBundlePolicyMaxBundle;
+  configuration_.rtcp_mux_policy =
+      PeerConnectionInterface::kRtcpMuxPolicyRequire;
+  options_.disable_encryption = true;
+  Init();
+
+  // Negotiate an audio channel with MAX_BUNDLE enabled.
+  SendAudioOnlyStream2();
+  SessionDescriptionInterface* offer = CreateOffer();
+  SetLocalDescriptionWithoutError(offer);
+  EXPECT_EQ_WAIT(PeerConnectionInterface::kIceGatheringComplete,
+                 observer_.ice_gathering_state_, kIceCandidatesTimeout);
+  std::string sdp;
+  offer->ToString(&sdp);

[Taylor Brandstetter, 2016/05/16 19:59:47]

nit: I know the old tests don't do this, but it would be better to just make a
copy of the session description rather than converting it to SDP and back. No
need to change this CL, but in the future that's what we should do.

+  SessionDescriptionInterface* answer = webrtc::CreateSessionDescription(
+      JsepSessionDescription::kAnswer, sdp, nullptr);
+  ASSERT_TRUE(answer != NULL);
+  SetRemoteDescriptionWithoutError(answer);
+
+  // Wait for the ICE state to stabilize.
+  EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
+                 observer_.ice_connection_state_, kIceCandidatesTimeout);
+  observer_.ice_connection_state_history_.clear();
+
+  // Now add a video channel which should be using the same bundle transport.
+  SendAudioVideoStream2();
+  offer = CreateOffer();
+  offer->ToString(&sdp);
+  SetLocalDescriptionWithoutError(offer);
+  answer = webrtc::CreateSessionDescription(JsepSessionDescription::kAnswer,
+                                            sdp, nullptr);
+  ASSERT_TRUE(answer != NULL);
+  SetRemoteDescriptionWithoutError(answer);
+
+  // Wait for ICE state to stabilize
+  rtc::Thread::Current()->ProcessMessages(0);
+  EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
+                 observer_.ice_connection_state_, kIceCandidatesTimeout);
+
+  // No ICE state changes are expected to happen.
+  EXPECT_EQ(0, observer_.ice_connection_state_history_.size());
+}
+
 TEST_F(WebRtcSessionTest, TestRequireRtcpMux) {
   InitWithRtcpMuxPolicy(PeerConnectionInterface::kRtcpMuxPolicyRequire);
   SendAudioVideoStream1();
 
   PeerConnectionInterface::RTCOfferAnswerOptions options;
   SessionDescriptionInterface* offer = CreateOffer(options);
   SetLocalDescriptionWithoutError(offer);
 
   EXPECT_TRUE(session_->voice_rtcp_transport_channel() == NULL);
   EXPECT_TRUE(session_->video_rtcp_transport_channel() == NULL);
@@ skipping 1084 matching lines @@
 }
 
 // TODO(bemasc): Add a TestIceStatesBundle with BUNDLE enabled.  That test
 // currently fails because upon disconnection and reconnection OnIceComplete is
 // called more than once without returning to IceGatheringGathering.
 
 INSTANTIATE_TEST_CASE_P(WebRtcSessionTests,
                         WebRtcSessionTest,
                         testing::Values(ALREADY_GENERATED,
                                         DTLS_IDENTITY_STORE));