Rewrite the remaining few WebRtcSession tests.

Bug: webrtc:8222
Change-Id: I18e2a449b77cee2ecb8c0c2ae94c105247116458
Reviewed-on: https://webrtc-review.googlesource.com/8740
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20399}

p2p/base/fakeicetransport.h

@@ -190,8 +190,19 @@ class FakeIceTransport : public IceTransportInternal {
     SignalSentPacket(this, sent_packet);
     return static_cast<int>(len);
   }
-  int SetOption(rtc::Socket::Option opt, int value) override { return true; }
-  bool GetOption(rtc::Socket::Option opt, int* value) override { return true; }
+  int SetOption(rtc::Socket::Option opt, int value) override {
+    socket_options_[opt] = value;
+    return true;
+  }
+  bool GetOption(rtc::Socket::Option opt, int* value) override {
+    auto it = socket_options_.find(opt);
+    if (it != socket_options_.end()) {
+      *value = it->second;
+      return true;
+    } else {
+      return false;
+    }
+  }
   int GetError() override { return 0; }
 
  private:
@@ -244,6 +255,7 @@ class FakeIceTransport : public IceTransportInternal {
   bool receiving_ = false;
   bool combine_outgoing_packets_ = false;
   rtc::CopyOnWriteBuffer send_packet_;
+  std::map<rtc::Socket::Option, int> socket_options_;
 };
 
 }  // namespace cricket

pc/BUILD.gn

@@ -417,7 +417,6 @@ if (rtc_include_tests) {
       "videocapturertracksource_unittest.cc",
       "videotrack_unittest.cc",
       "webrtcsdp_unittest.cc",
-      "webrtcsession_unittest.cc",
     ]
 
     if (rtc_enable_sctp) {

pc/channel_unittest.cc

@@ -2051,6 +2051,35 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
     EXPECT_EQ(-1, media_channel1_->max_bps());
   }
 
+  // Test that when a channel gets new transports with a call to
+  // |SetTransports|, the socket options from the old transports are merged with
+  // the options on the new transport.
+  // For example, audio and video may use separate socket options, but initially
+  // be unbundled, then later become bundled. When this happens, their preferred
+  // socket options should be merged to the underlying transport they share.
+  void SocketOptionsMergedOnSetTransport() {
+    constexpr int kSndBufSize = 4000;
+    constexpr int kRcvBufSize = 8000;
+
+    CreateChannels(0, 0);
+
+    channel1_->SetOption(cricket::BaseChannel::ST_RTP,
+                         rtc::Socket::Option::OPT_SNDBUF, kSndBufSize);
+    channel2_->SetOption(cricket::BaseChannel::ST_RTP,
+                         rtc::Socket::Option::OPT_RCVBUF, kRcvBufSize);
+
+    channel1_->SetTransports(channel2_->rtp_dtls_transport(),
+                             channel2_->rtcp_dtls_transport());
+
+    int option_val;
+    ASSERT_TRUE(channel1_->rtp_dtls_transport()->GetOption(
+        rtc::Socket::Option::OPT_SNDBUF, &option_val));
+    EXPECT_EQ(kSndBufSize, option_val);
+    ASSERT_TRUE(channel1_->rtp_dtls_transport()->GetOption(
+        rtc::Socket::Option::OPT_RCVBUF, &option_val));
+    EXPECT_EQ(kRcvBufSize, option_val);
+  }
+
  protected:
   void WaitForThreads() { WaitForThreads(rtc::ArrayView<rtc::Thread*>()); }
   static void ProcessThreadQueue(rtc::Thread* thread) {
@@ -2617,6 +2646,10 @@ TEST_F(VoiceChannelSingleThreadTest, CanChangeMaxBitrate) {
   Base::CanChangeMaxBitrate();
 }
 
+TEST_F(VoiceChannelSingleThreadTest, SocketOptionsMergedOnSetTransport) {
+  Base::SocketOptionsMergedOnSetTransport();
+}
+
 // VoiceChannelDoubleThreadTest
 TEST_F(VoiceChannelDoubleThreadTest, TestInit) {
   Base::TestInit();
@@ -2976,6 +3009,10 @@ TEST_F(VoiceChannelDoubleThreadTest, CanChangeMaxBitrate) {
   Base::CanChangeMaxBitrate();
 }
 
+TEST_F(VoiceChannelDoubleThreadTest, SocketOptionsMergedOnSetTransport) {
+  Base::SocketOptionsMergedOnSetTransport();
+}
+
 // VideoChannelSingleThreadTest
 TEST_F(VideoChannelSingleThreadTest, TestInit) {
   Base::TestInit();
@@ -3207,6 +3244,10 @@ TEST_F(VideoChannelSingleThreadTest, CanChangeMaxBitrate) {
   Base::CanChangeMaxBitrate();
 }
 
+TEST_F(VideoChannelSingleThreadTest, SocketOptionsMergedOnSetTransport) {
+  Base::SocketOptionsMergedOnSetTransport();
+}
+
 // VideoChannelDoubleThreadTest
 TEST_F(VideoChannelDoubleThreadTest, TestInit) {
   Base::TestInit();
@@ -3438,6 +3479,10 @@ TEST_F(VideoChannelDoubleThreadTest, CanChangeMaxBitrate) {
   Base::CanChangeMaxBitrate();
 }
 
+TEST_F(VideoChannelDoubleThreadTest, SocketOptionsMergedOnSetTransport) {
+  Base::SocketOptionsMergedOnSetTransport();
+}
+
 // RtpDataChannelSingleThreadTest
 class RtpDataChannelSingleThreadTest : public ChannelTest<DataTraits> {
  public:
@@ -3634,6 +3679,10 @@ TEST_F(RtpDataChannelSingleThreadTest, TestMediaMonitor) {
   Base::TestMediaMonitor();
 }
 
+TEST_F(RtpDataChannelSingleThreadTest, SocketOptionsMergedOnSetTransport) {
+  Base::SocketOptionsMergedOnSetTransport();
+}
+
 TEST_F(RtpDataChannelSingleThreadTest, TestSendData) {
   CreateChannels(0, 0);
   EXPECT_TRUE(SendInitiate());
@@ -3766,6 +3815,10 @@ TEST_F(RtpDataChannelDoubleThreadTest, TestMediaMonitor) {
   Base::TestMediaMonitor();
 }
 
+TEST_F(RtpDataChannelDoubleThreadTest, SocketOptionsMergedOnSetTransport) {
+  Base::SocketOptionsMergedOnSetTransport();
+}
+
 TEST_F(RtpDataChannelDoubleThreadTest, TestSendData) {
   CreateChannels(0, 0);
   EXPECT_TRUE(SendInitiate());

pc/peerconnection.h

@@ -33,12 +33,6 @@ class MediaStreamObserver;
 class VideoRtpReceiver;
 class RtcEventLog;
 
-// TODO(zhihuang): Remove this declaration when the WebRtcSession tests don't
-// need it.
-void ExtractSharedMediaSessionOptions(
-    const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options,
-    cricket::MediaSessionOptions* session_options);
-
 // PeerConnection implements the PeerConnectionInterface interface.
 // It uses WebRtcSession to implement the PeerConnection functionality.
 class PeerConnection : public PeerConnectionInterface,

pc/peerconnection_crypto_unittest.cc

@@ -26,6 +26,7 @@
 namespace webrtc {
 
 using RTCConfiguration = PeerConnectionInterface::RTCConfiguration;
+using RTCOfferAnswerOptions = PeerConnectionInterface::RTCOfferAnswerOptions;
 using ::testing::Values;
 using ::testing::Combine;
 
@@ -46,6 +47,10 @@ class PeerConnectionCryptoUnitTest : public ::testing::Test {
         CreateBuiltinAudioDecoderFactory(), nullptr, nullptr);
   }
 
+  WrapperPtr CreatePeerConnection() {
+    return CreatePeerConnection(RTCConfiguration());
+  }
+
   WrapperPtr CreatePeerConnection(const RTCConfiguration& config) {
     return CreatePeerConnection(config, nullptr);
   }
@@ -80,6 +85,25 @@ class PeerConnectionCryptoUnitTest : public ::testing::Test {
     return wrapper;
   }
 
+  cricket::ConnectionRole& AudioConnectionRole(
+      cricket::SessionDescription* desc) {
+    return ConnectionRoleFromContent(desc, cricket::GetFirstAudioContent(desc));
+  }
+
+  cricket::ConnectionRole& VideoConnectionRole(
+      cricket::SessionDescription* desc) {
+    return ConnectionRoleFromContent(desc, cricket::GetFirstVideoContent(desc));
+  }
+
+  cricket::ConnectionRole& ConnectionRoleFromContent(
+      cricket::SessionDescription* desc,
+      cricket::ContentInfo* content) {
+    RTC_DCHECK(content);
+    auto* transport_info = desc->GetTransportInfoByName(content->name);
+    RTC_DCHECK(transport_info);
+    return transport_info->description.connection_role;
+  }
+
   std::unique_ptr<rtc::VirtualSocketServer> vss_;
   rtc::AutoSocketServerThread main_;
   rtc::scoped_refptr<PeerConnectionFactoryInterface> pc_factory_;
@@ -606,4 +630,58 @@ INSTANTIATE_TEST_CASE_P(
             Values(CertGenResult::kSucceed, CertGenResult::kFail),
             Values(1, 3)));
 
+// Test that we can create and set an answer correctly when different
+// SSL roles have been negotiated for different transports.
+// See: https://bugs.chromium.org/p/webrtc/issues/detail?id=4525
+TEST_F(PeerConnectionCryptoUnitTest, CreateAnswerWithDifferentSslRoles) {
+  auto caller = CreatePeerConnectionWithAudioVideo();
+  auto callee = CreatePeerConnectionWithAudioVideo();
+
+  RTCOfferAnswerOptions options_no_bundle;
+  options_no_bundle.use_rtp_mux = false;
+
+  // First, negotiate different SSL roles for audio and video.
+  ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
+  auto answer = callee->CreateAnswer(options_no_bundle);
+
+  AudioConnectionRole(answer->description()) = cricket::CONNECTIONROLE_ACTIVE;
+  VideoConnectionRole(answer->description()) = cricket::CONNECTIONROLE_PASSIVE;
+
+  ASSERT_TRUE(
+      callee->SetLocalDescription(CloneSessionDescription(answer.get())));
+  ASSERT_TRUE(caller->SetRemoteDescription(std::move(answer)));
+
+  // Now create an offer in the reverse direction, and ensure the initial
+  // offerer responds with an answer with the correct SSL roles.
+  ASSERT_TRUE(caller->SetRemoteDescription(callee->CreateOfferAndSetAsLocal()));
+  answer = caller->CreateAnswer(options_no_bundle);
+
+  EXPECT_EQ(cricket::CONNECTIONROLE_PASSIVE,
+            AudioConnectionRole(answer->description()));
+  EXPECT_EQ(cricket::CONNECTIONROLE_ACTIVE,
+            VideoConnectionRole(answer->description()));
+
+  ASSERT_TRUE(
+      caller->SetLocalDescription(CloneSessionDescription(answer.get())));
+  ASSERT_TRUE(callee->SetRemoteDescription(std::move(answer)));
+
+  // Lastly, start BUNDLE-ing on "audio", expecting that the "passive" role of
+  // audio is transferred over to video in the answer that completes the BUNDLE
+  // negotiation.
+  RTCOfferAnswerOptions options_bundle;
+  options_bundle.use_rtp_mux = true;
+
+  ASSERT_TRUE(caller->SetRemoteDescription(callee->CreateOfferAndSetAsLocal()));
+  answer = caller->CreateAnswer(options_bundle);
+
+  EXPECT_EQ(cricket::CONNECTIONROLE_PASSIVE,
+            AudioConnectionRole(answer->description()));
+  EXPECT_EQ(cricket::CONNECTIONROLE_PASSIVE,
+            VideoConnectionRole(answer->description()));
+
+  ASSERT_TRUE(
+      caller->SetLocalDescription(CloneSessionDescription(answer.get())));
+  ASSERT_TRUE(callee->SetRemoteDescription(std::move(answer)));
+}
+
 }  // namespace webrtc