commit d9ebe015405d8b239c6855e579d4ff2d8616adbb
author Eldar Rello <elrello@microsoft.com> Wed Mar 18 18:41:45 2020
committer Commit Bot <commit-bot@chromium.org> Wed Mar 18 21:03:20 2020
tree 588ba6d31c71e992d0206d8877e05b1170cec26d
parent 560e87aa8193079d8b2900f1b7cbceec95c1f305

Improve rollback for rtp data channel

Bug: chromium:1057333
Change-Id: I4df21bc183a8df398033ebf29a8407bacf873fac
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/170621
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Eldar Rello <elrello@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#30824}

pc/peer_connection.cc

diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index c6af185..fd2b81f 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -6017,6 +6017,7 @@
     RTC_DCHECK(type == SdpType::kAnswer);
     ChangeSignalingState(PeerConnectionInterface::kStable);
     transceiver_stable_states_by_transceivers_.clear();
+    have_pending_rtp_data_channel_ = false;
   }
 
   // Update internal objects according to the session description's media
@@ -6688,6 +6689,7 @@
           this, &PeerConnection::OnSentPacket_w);
       data_channel_controller_.rtp_data_channel()->SetRtpTransport(
           rtp_transport);
+      have_pending_rtp_data_channel_ = true;
       return true;
   }
   return false;
@@ -7610,6 +7612,10 @@
     transceiver->internal()->set_mline_index(state.mline_index());
   }
   transport_controller_->RollbackTransports();
+  if (have_pending_rtp_data_channel_) {
+    DestroyDataChannelTransport();
+    have_pending_rtp_data_channel_ = false;
+  }
   transceiver_stable_states_by_transceivers_.clear();
   pending_local_description_.reset();
   pending_remote_description_.reset();

pc/peer_connection.h

diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index cd5c001..4b23842 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -1295,6 +1295,9 @@
   std::map<rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>,
            TransceiverStableState>
       transceiver_stable_states_by_transceivers_;
+  // Used when rolling back RTP data channels.
+  bool have_pending_rtp_data_channel_ RTC_GUARDED_BY(signaling_thread()) =
+      false;
   // Holds remote stream ids for transceivers from stable state.
   std::map<rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>,
            std::vector<std::string>>

pc/peer_connection_integrationtest.cc

diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index 0a6e5d1..054091e 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -592,6 +592,10 @@
     pc()->CreateOffer(observer, offer_answer_options_);
     return WaitForDescriptionFromObserver(observer);
   }
+  bool Rollback() {
+    return SetRemoteDescription(
+        webrtc::CreateSessionDescription(SdpType::kRollback, ""));
+  }
 
  private:
   explicit PeerConnectionWrapper(const std::string& debug_name)
@@ -3246,6 +3250,31 @@
                  kDefaultTimeout);
 }
 
+TEST_P(PeerConnectionIntegrationTest, RtpDataChannelWorksAfterRollback) {
+  PeerConnectionInterface::RTCConfiguration rtc_config;
+  rtc_config.enable_rtp_data_channel = true;
+  rtc_config.enable_dtls_srtp = false;
+  ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(rtc_config, rtc_config));
+  ConnectFakeSignaling();
+  auto data_channel = caller()->pc()->CreateDataChannel("label_1", nullptr);
+  ASSERT_TRUE(data_channel.get() != nullptr);
+  caller()->CreateAndSetAndSignalOffer();
+  ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+
+  caller()->CreateDataChannel("label_2", nullptr);
+  rtc::scoped_refptr<MockSetSessionDescriptionObserver> observer(
+      new rtc::RefCountedObject<MockSetSessionDescriptionObserver>());
+  caller()->pc()->SetLocalDescription(observer,
+                                      caller()->CreateOfferAndWait().release());
+  EXPECT_TRUE_WAIT(observer->called(), kDefaultTimeout);
+  caller()->Rollback();
+
+  std::string data = "hello world";
+  SendRtpDataWithRetries(data_channel, data, 5);
+  EXPECT_EQ_WAIT(data, callee()->data_observer()->last_message(),
+                 kDefaultTimeout);
+}
+
 // Ensure that an RTP data channel is signaled as closed for the caller when
 // the callee rejects it in a subsequent offer.
 TEST_P(PeerConnectionIntegrationTest,

pc/peer_connection_jsep_unittest.cc

diff --git a/pc/peer_connection_jsep_unittest.cc b/pc/peer_connection_jsep_unittest.cc
index 2a3c4c60..0b2f375 100644
--- a/pc/peer_connection_jsep_unittest.cc
+++ b/pc/peer_connection_jsep_unittest.cc
@@ -2198,4 +2198,16 @@
   EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
 }
 
+TEST_F(PeerConnectionJsepTest, RollbackRtpDataChannel) {
+  RTCConfiguration config;
+  config.sdp_semantics = SdpSemantics::kUnifiedPlan;
+  config.enable_rtp_data_channel = true;
+  auto pc = CreatePeerConnection(config);
+  pc->CreateDataChannel("dummy");
+  auto offer = pc->CreateOffer();
+  EXPECT_TRUE(pc->CreateOfferAndSetAsLocal());
+  EXPECT_TRUE(pc->SetRemoteDescription(pc->CreateRollback()));
+  EXPECT_TRUE(pc->SetLocalDescription(std::move(offer)));
+}
+
 }  // namespace webrtc