Removing dependency on JsepTransport from DtlsTransport tests.

The DtlsTransport tests worked by relying on JsepTransport (a helper
class used by higher layers to set everything up in response to SDP).
dtlstransport_unittest has been switched to just calling SetSslRole and
SetRemoteFingerprint directly instead, which were really the only parts
that were necessary.

Some refactoring was also done, and some test coverage was moved to
jseptransport_unittest. jseptransport_unittests has more coverage to
ensure that negotiated parameters are propagated to the DtlsTransport
underneath, which were previously covered by the tests in
dtlstransport_unittest. It also has a test that covers RTP/RTCP not
being multiplexed, which dtlstransport_unittests really doesn't need
to be concerned about.

BUG=NONE

Change-Id: I1d67e9a06486ade39a255555af4052d76191d238
Reviewed-on: https://webrtc-review.googlesource.com/32941
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Peter Thatcher <pthatcher@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21309}
diff --git a/p2p/base/jseptransport_unittest.cc b/p2p/base/jseptransport_unittest.cc
index ff8484d..bcd12c5 100644
--- a/p2p/base/jseptransport_unittest.cc
+++ b/p2p/base/jseptransport_unittest.cc
@@ -16,11 +16,8 @@
 #include "rtc_base/gunit.h"
 #include "rtc_base/network.h"
 
-using cricket::JsepTransport;
-using cricket::FakeDtlsTransport;
-using cricket::FakeIceTransport;
-using cricket::IceRole;
-using cricket::TransportDescription;
+namespace cricket {
+
 using rtc::SocketAddress;
 using webrtc::SdpType;
 
@@ -30,63 +27,188 @@
 static const char kIceUfrag2[] = "TESTICEUFRAG0002";
 static const char kIcePwd2[] = "TESTICEPWD00000000000002";
 
+TransportDescription MakeTransportDescription(
+    const char* ufrag,
+    const char* pwd,
+    const rtc::scoped_refptr<rtc::RTCCertificate>& cert,
+    ConnectionRole role = CONNECTIONROLE_NONE) {
+  std::unique_ptr<rtc::SSLFingerprint> fingerprint;
+  if (cert) {
+    fingerprint.reset(rtc::SSLFingerprint::CreateFromCertificate(cert));
+  }
+  return TransportDescription(std::vector<std::string>(), ufrag, pwd,
+                              ICEMODE_FULL, role, fingerprint.get());
+}
+
 class JsepTransportTest : public testing::Test, public sigslot::has_slots<> {
  public:
   JsepTransportTest() { RecreateTransport(); }
 
-  bool SetupChannel() {
-    fake_ice_transport_.reset(new FakeIceTransport(transport_->mid(), 1));
-    fake_dtls_transport_.reset(
-        new FakeDtlsTransport(fake_ice_transport_.get()));
-    return transport_->AddChannel(fake_dtls_transport_.get(), 1);
+  bool SetupFakeTransports(int component) {
+    fake_ice_transports_.emplace_back(
+        new FakeIceTransport(transport_->mid(), component));
+    fake_dtls_transports_.emplace_back(
+        new FakeDtlsTransport(fake_ice_transports_.back().get()));
+    return transport_->AddChannel(fake_dtls_transports_.back().get(),
+                                  component);
   }
 
-  void DestroyChannel() { transport_->RemoveChannel(1); }
+  void DestroyChannel(int component) { transport_->RemoveChannel(component); }
 
   void RecreateTransport() {
     transport_.reset(new JsepTransport("test content name", nullptr));
   }
 
  protected:
-  std::unique_ptr<FakeDtlsTransport> fake_dtls_transport_;
-  std::unique_ptr<FakeIceTransport> fake_ice_transport_;
+  std::vector<std::unique_ptr<FakeDtlsTransport>> fake_dtls_transports_;
+  std::vector<std::unique_ptr<FakeIceTransport>> fake_ice_transports_;
   std::unique_ptr<JsepTransport> transport_;
 };
 
-// This test verifies channels are created with proper ICE
-// ufrag/password after a transport description is applied.
-TEST_F(JsepTransportTest, TestChannelIceParameters) {
-  cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1);
+// This test verifies transports are created with proper ICE ufrag/password
+// after a transport description is applied.
+TEST_F(JsepTransportTest, TestIceTransportParameters) {
+  EXPECT_TRUE(SetupFakeTransports(1));
+  TransportDescription local_desc(kIceUfrag1, kIcePwd1);
   ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc,
                                                        SdpType::kOffer, NULL));
-  EXPECT_TRUE(SetupChannel());
-  EXPECT_EQ(cricket::ICEMODE_FULL, fake_ice_transport_->remote_ice_mode());
-  EXPECT_EQ(kIceUfrag1, fake_ice_transport_->ice_ufrag());
-  EXPECT_EQ(kIcePwd1, fake_ice_transport_->ice_pwd());
+  EXPECT_EQ(ICEMODE_FULL, fake_ice_transports_[0]->remote_ice_mode());
+  EXPECT_EQ(kIceUfrag1, fake_ice_transports_[0]->ice_ufrag());
+  EXPECT_EQ(kIcePwd1, fake_ice_transports_[0]->ice_pwd());
 
-  cricket::TransportDescription remote_desc(kIceUfrag1, kIcePwd1);
+  TransportDescription remote_desc(kIceUfrag2, kIcePwd2);
   ASSERT_TRUE(transport_->SetRemoteTransportDescription(
       remote_desc, SdpType::kAnswer, NULL));
-  EXPECT_EQ(cricket::ICEMODE_FULL, fake_ice_transport_->remote_ice_mode());
-  EXPECT_EQ(kIceUfrag1, fake_ice_transport_->remote_ice_ufrag());
-  EXPECT_EQ(kIcePwd1, fake_ice_transport_->remote_ice_pwd());
+  EXPECT_EQ(ICEMODE_FULL, fake_ice_transports_[0]->remote_ice_mode());
+  EXPECT_EQ(kIceUfrag2, fake_ice_transports_[0]->remote_ice_ufrag());
+  EXPECT_EQ(kIcePwd2, fake_ice_transports_[0]->remote_ice_pwd());
+}
+
+// Similarly, test that DTLS parameters are applied after a transport
+// description is applied.
+TEST_F(JsepTransportTest, TestDtlsTransportParameters) {
+  EXPECT_TRUE(SetupFakeTransports(1));
+
+  // Create certificates.
+  rtc::scoped_refptr<rtc::RTCCertificate> local_cert =
+      rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
+          rtc::SSLIdentity::Generate("local", rtc::KT_DEFAULT)));
+  rtc::scoped_refptr<rtc::RTCCertificate> remote_cert =
+      rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
+          rtc::SSLIdentity::Generate("remote", rtc::KT_DEFAULT)));
+  transport_->SetLocalCertificate(local_cert);
+
+  // Apply offer/answer.
+  TransportDescription local_desc = MakeTransportDescription(
+      kIceUfrag1, kIcePwd1, local_cert, CONNECTIONROLE_ACTPASS);
+  ASSERT_TRUE(transport_->SetLocalTransportDescription(
+      local_desc, SdpType::kOffer, nullptr));
+  TransportDescription remote_desc = MakeTransportDescription(
+      kIceUfrag2, kIcePwd2, remote_cert, CONNECTIONROLE_ACTIVE);
+  ASSERT_TRUE(transport_->SetRemoteTransportDescription(
+      remote_desc, SdpType::kAnswer, nullptr));
+
+  // Verify that SSL role and remote fingerprint were set correctly based on
+  // transport descriptions.
+  rtc::SSLRole role;
+  EXPECT_TRUE(fake_dtls_transports_[0]->GetSslRole(&role));
+  EXPECT_EQ(rtc::SSL_SERVER, role);  // Because remote description was "active".
+  EXPECT_EQ(remote_desc.identity_fingerprint->ToString(),
+            fake_dtls_transports_[0]->dtls_fingerprint().ToString());
+}
+
+// Same as above test, but with remote transport description using
+// CONNECTIONROLE_PASSIVE, expecting SSL_CLIENT role.
+TEST_F(JsepTransportTest, TestDtlsTransportParametersWithPassiveAnswer) {
+  EXPECT_TRUE(SetupFakeTransports(1));
+
+  // Create certificates.
+  rtc::scoped_refptr<rtc::RTCCertificate> local_cert =
+      rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
+          rtc::SSLIdentity::Generate("local", rtc::KT_DEFAULT)));
+  rtc::scoped_refptr<rtc::RTCCertificate> remote_cert =
+      rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
+          rtc::SSLIdentity::Generate("remote", rtc::KT_DEFAULT)));
+  transport_->SetLocalCertificate(local_cert);
+
+  // Apply offer/answer.
+  TransportDescription local_desc = MakeTransportDescription(
+      kIceUfrag1, kIcePwd1, local_cert, CONNECTIONROLE_ACTPASS);
+  ASSERT_TRUE(transport_->SetLocalTransportDescription(
+      local_desc, SdpType::kOffer, nullptr));
+  TransportDescription remote_desc = MakeTransportDescription(
+      kIceUfrag2, kIcePwd2, remote_cert, CONNECTIONROLE_PASSIVE);
+  ASSERT_TRUE(transport_->SetRemoteTransportDescription(
+      remote_desc, SdpType::kAnswer, nullptr));
+
+  // Verify that SSL role and remote fingerprint were set correctly based on
+  // transport descriptions.
+  rtc::SSLRole role;
+  EXPECT_TRUE(fake_dtls_transports_[0]->GetSslRole(&role));
+  EXPECT_EQ(rtc::SSL_CLIENT,
+            role);  // Because remote description was "passive".
+  EXPECT_EQ(remote_desc.identity_fingerprint->ToString(),
+            fake_dtls_transports_[0]->dtls_fingerprint().ToString());
+}
+
+// Add two DtlsTransports/IceTransports and make sure parameters are applied to
+// both of them. Applicable when RTP/RTCP are not multiplexed, so they share
+// the same parameters but different connections.
+TEST_F(JsepTransportTest, TestTransportParametersAppliedToTwoComponents) {
+  EXPECT_TRUE(SetupFakeTransports(1));
+  EXPECT_TRUE(SetupFakeTransports(2));
+
+  // Create certificates.
+  rtc::scoped_refptr<rtc::RTCCertificate> local_cert =
+      rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
+          rtc::SSLIdentity::Generate("local", rtc::KT_DEFAULT)));
+  rtc::scoped_refptr<rtc::RTCCertificate> remote_cert =
+      rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
+          rtc::SSLIdentity::Generate("remote", rtc::KT_DEFAULT)));
+  transport_->SetLocalCertificate(local_cert);
+
+  // Apply offer/answer.
+  TransportDescription local_desc = MakeTransportDescription(
+      kIceUfrag1, kIcePwd1, local_cert, CONNECTIONROLE_ACTPASS);
+  ASSERT_TRUE(transport_->SetLocalTransportDescription(
+      local_desc, SdpType::kOffer, nullptr));
+  TransportDescription remote_desc = MakeTransportDescription(
+      kIceUfrag2, kIcePwd2, remote_cert, CONNECTIONROLE_ACTIVE);
+  ASSERT_TRUE(transport_->SetRemoteTransportDescription(
+      remote_desc, SdpType::kAnswer, nullptr));
+
+  for (int i = 0; i < 1; ++i) {
+    // Verify parameters of ICE transports.
+    EXPECT_EQ(ICEMODE_FULL, fake_ice_transports_[i]->remote_ice_mode());
+    EXPECT_EQ(kIceUfrag1, fake_ice_transports_[i]->ice_ufrag());
+    EXPECT_EQ(kIcePwd1, fake_ice_transports_[i]->ice_pwd());
+    EXPECT_EQ(kIceUfrag2, fake_ice_transports_[i]->remote_ice_ufrag());
+    EXPECT_EQ(kIcePwd2, fake_ice_transports_[i]->remote_ice_pwd());
+    // Verify parameters of DTLS transports.
+    rtc::SSLRole role;
+    EXPECT_TRUE(fake_dtls_transports_[i]->GetSslRole(&role));
+    EXPECT_EQ(rtc::SSL_SERVER,
+              role);  // Because remote description was "active".
+    EXPECT_EQ(remote_desc.identity_fingerprint->ToString(),
+              fake_dtls_transports_[i]->dtls_fingerprint().ToString());
+  }
 }
 
 // Verifies that IceCredentialsChanged returns true when either ufrag or pwd
 // changed, and false in other cases.
 TEST_F(JsepTransportTest, TestIceCredentialsChanged) {
-  EXPECT_TRUE(cricket::IceCredentialsChanged("u1", "p1", "u2", "p2"));
-  EXPECT_TRUE(cricket::IceCredentialsChanged("u1", "p1", "u2", "p1"));
-  EXPECT_TRUE(cricket::IceCredentialsChanged("u1", "p1", "u1", "p2"));
-  EXPECT_FALSE(cricket::IceCredentialsChanged("u1", "p1", "u1", "p1"));
+  EXPECT_TRUE(IceCredentialsChanged("u1", "p1", "u2", "p2"));
+  EXPECT_TRUE(IceCredentialsChanged("u1", "p1", "u2", "p1"));
+  EXPECT_TRUE(IceCredentialsChanged("u1", "p1", "u1", "p2"));
+  EXPECT_FALSE(IceCredentialsChanged("u1", "p1", "u1", "p1"));
 }
 
 // Tests SetNeedsIceRestartFlag and NeedsIceRestart, ensuring NeedsIceRestart
 // only starts returning "false" once an ICE restart has been initiated.
 TEST_F(JsepTransportTest, NeedsIceRestart) {
   // Do initial offer/answer so there's something to restart.
-  cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1);
-  cricket::TransportDescription remote_desc(kIceUfrag1, kIcePwd1);
+  TransportDescription local_desc(kIceUfrag1, kIcePwd1);
+  TransportDescription remote_desc(kIceUfrag1, kIcePwd1);
   ASSERT_TRUE(transport_->SetLocalTransportDescription(
       local_desc, SdpType::kOffer, nullptr));
   ASSERT_TRUE(transport_->SetRemoteTransportDescription(
@@ -107,8 +229,8 @@
   EXPECT_TRUE(transport_->NeedsIceRestart());
 
   // Doing an offer/answer that restarts ICE should clear the flag.
-  cricket::TransportDescription ice_restart_local_desc(kIceUfrag2, kIcePwd2);
-  cricket::TransportDescription ice_restart_remote_desc(kIceUfrag2, kIcePwd2);
+  TransportDescription ice_restart_local_desc(kIceUfrag2, kIcePwd2);
+  TransportDescription ice_restart_remote_desc(kIceUfrag2, kIcePwd2);
   ASSERT_TRUE(transport_->SetLocalTransportDescription(
       ice_restart_local_desc, SdpType::kOffer, nullptr));
   ASSERT_TRUE(transport_->SetRemoteTransportDescription(
@@ -117,18 +239,17 @@
 }
 
 TEST_F(JsepTransportTest, TestGetStats) {
-  EXPECT_TRUE(SetupChannel());
-  cricket::TransportStats stats;
+  EXPECT_TRUE(SetupFakeTransports(1));
+  TransportStats stats;
   EXPECT_TRUE(transport_->GetStats(&stats));
   // Note that this tests the behavior of a FakeIceTransport.
   ASSERT_EQ(1U, stats.channel_stats.size());
   EXPECT_EQ(1, stats.channel_stats[0].component);
   // Set local transport description for FakeTransport before connecting.
   TransportDescription faketransport_desc(
-      std::vector<std::string>(),
-      rtc::CreateRandomString(cricket::ICE_UFRAG_LENGTH),
-      rtc::CreateRandomString(cricket::ICE_PWD_LENGTH), cricket::ICEMODE_FULL,
-      cricket::CONNECTIONROLE_NONE, nullptr);
+      std::vector<std::string>(), rtc::CreateRandomString(ICE_UFRAG_LENGTH),
+      rtc::CreateRandomString(ICE_PWD_LENGTH), ICEMODE_FULL,
+      CONNECTIONROLE_NONE, nullptr);
   transport_->SetLocalTransportDescription(faketransport_desc, SdpType::kOffer,
                                            nullptr);
   EXPECT_TRUE(transport_->GetStats(&stats));
@@ -174,24 +295,20 @@
 
 // Tests the logic of DTLS role negotiation for an initial offer/answer.
 TEST_F(JsepTransportTest, DtlsRoleNegotiation) {
+  // Just use the same certificate for both sides; doesn't really matter in a
+  // non end-to-end test.
   rtc::scoped_refptr<rtc::RTCCertificate> certificate =
       rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
           rtc::SSLIdentity::Generate("testing", rtc::KT_ECDSA)));
-  std::unique_ptr<rtc::SSLFingerprint> fingerprint(
-      rtc::SSLFingerprint::CreateFromCertificate(certificate));
 
-  TransportDescription local_desc(kIceUfrag1, kIcePwd1);
-  TransportDescription remote_desc(kIceUfrag2, kIcePwd2);
-  // Just use the same fingerprint in both descriptions; the remote fingerprint
-  // doesn't matter in a non end-to-end test.
-  local_desc.identity_fingerprint.reset(
-      TransportDescription::CopyFingerprint(fingerprint.get()));
-  remote_desc.identity_fingerprint.reset(
-      TransportDescription::CopyFingerprint(fingerprint.get()));
+  TransportDescription local_desc =
+      MakeTransportDescription(kIceUfrag1, kIcePwd1, certificate);
+  TransportDescription remote_desc =
+      MakeTransportDescription(kIceUfrag2, kIcePwd2, certificate);
 
   struct NegotiateRoleParams {
-    cricket::ConnectionRole local_role;
-    cricket::ConnectionRole remote_role;
+    ConnectionRole local_role;
+    ConnectionRole remote_role;
     SdpType local_type;
     SdpType remote_type;
   };
@@ -200,14 +317,14 @@
 
   // Parameters which set the SSL role to SSL_CLIENT.
   NegotiateRoleParams valid_client_params[] = {
-      {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTPASS,
-       SdpType::kAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTPASS,
-       SdpType::kPrAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kOffer, SdpType::kAnswer},
-      {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kOffer, SdpType::kPrAnswer}};
+      {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTPASS, SdpType::kAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTPASS, SdpType::kPrAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_PASSIVE, SdpType::kOffer,
+       SdpType::kAnswer},
+      {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_PASSIVE, SdpType::kOffer,
+       SdpType::kPrAnswer}};
 
   for (auto& param : valid_client_params) {
     RecreateTransport();
@@ -233,14 +350,14 @@
 
   // Parameters which set the SSL role to SSL_SERVER.
   NegotiateRoleParams valid_server_params[] = {
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS,
-       SdpType::kAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS,
-       SdpType::kPrAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE,
-       SdpType::kOffer, SdpType::kAnswer},
-      {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE,
-       SdpType::kOffer, SdpType::kPrAnswer}};
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTPASS, SdpType::kAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTPASS, SdpType::kPrAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTIVE, SdpType::kOffer,
+       SdpType::kAnswer},
+      {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTIVE, SdpType::kOffer,
+       SdpType::kPrAnswer}};
 
   for (auto& param : valid_server_params) {
     RecreateTransport();
@@ -266,30 +383,30 @@
 
   // Invalid parameters due to both peers having a duplicate role.
   NegotiateRoleParams duplicate_params[] = {
-      {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE,
-       SdpType::kAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS,
-       SdpType::kAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE,
-       SdpType::kPrAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS,
-       SdpType::kPrAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kPrAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE,
-       SdpType::kOffer, SdpType::kAnswer},
-      {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS,
-       SdpType::kOffer, SdpType::kAnswer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kOffer, SdpType::kAnswer},
-      {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE,
-       SdpType::kOffer, SdpType::kPrAnswer},
-      {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS,
-       SdpType::kOffer, SdpType::kPrAnswer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kOffer, SdpType::kPrAnswer}};
+      {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTIVE, SdpType::kAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTPASS, SdpType::kAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_PASSIVE, SdpType::kAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTIVE, SdpType::kPrAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTPASS, SdpType::kPrAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_PASSIVE, SdpType::kPrAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTIVE, SdpType::kOffer,
+       SdpType::kAnswer},
+      {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTPASS, SdpType::kOffer,
+       SdpType::kAnswer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_PASSIVE, SdpType::kOffer,
+       SdpType::kAnswer},
+      {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTIVE, SdpType::kOffer,
+       SdpType::kPrAnswer},
+      {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTPASS, SdpType::kOffer,
+       SdpType::kPrAnswer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_PASSIVE, SdpType::kOffer,
+       SdpType::kPrAnswer}};
 
   for (auto& param : duplicate_params) {
     RecreateTransport();
@@ -314,30 +431,30 @@
 
   // Invalid parameters due to the offerer not using ACTPASS.
   NegotiateRoleParams offerer_without_actpass_params[] = {
-      {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE,
-       SdpType::kAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kPrAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE,
-       SdpType::kPrAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kPrAnswer, SdpType::kOffer},
-      {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kOffer, SdpType::kAnswer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE,
-       SdpType::kOffer, SdpType::kAnswer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS,
-       SdpType::kOffer, SdpType::kAnswer},
-      {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE,
-       SdpType::kOffer, SdpType::kPrAnswer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE,
-       SdpType::kOffer, SdpType::kPrAnswer},
-      {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS,
-       SdpType::kOffer, SdpType::kPrAnswer}};
+      {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_PASSIVE, SdpType::kAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTIVE, SdpType::kAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_PASSIVE, SdpType::kAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_PASSIVE, SdpType::kPrAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTIVE, SdpType::kPrAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_PASSIVE, SdpType::kPrAnswer,
+       SdpType::kOffer},
+      {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_PASSIVE, SdpType::kOffer,
+       SdpType::kAnswer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTIVE, SdpType::kOffer,
+       SdpType::kAnswer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTPASS, SdpType::kOffer,
+       SdpType::kAnswer},
+      {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_PASSIVE, SdpType::kOffer,
+       SdpType::kPrAnswer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTIVE, SdpType::kOffer,
+       SdpType::kPrAnswer},
+      {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTPASS, SdpType::kOffer,
+       SdpType::kPrAnswer}};
 
   for (auto& param : offerer_without_actpass_params) {
     RecreateTransport();
@@ -363,28 +480,88 @@
   }
 }
 
+// Test that a reoffer in the opposite direction is successful as long as the
+// role isn't changing. Doesn't test every possible combination like the test
+// above.
+TEST_F(JsepTransportTest, ValidDtlsReofferFromAnswerer) {
+  // Just use the same certificate for both sides; doesn't really matter in a
+  // non end-to-end test.
+  rtc::scoped_refptr<rtc::RTCCertificate> certificate =
+      rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
+          rtc::SSLIdentity::Generate("testing", rtc::KT_ECDSA)));
+  transport_->SetLocalCertificate(certificate);
+
+  TransportDescription local_offer = MakeTransportDescription(
+      kIceUfrag1, kIcePwd1, certificate, CONNECTIONROLE_ACTPASS);
+  TransportDescription remote_answer = MakeTransportDescription(
+      kIceUfrag2, kIcePwd2, certificate, CONNECTIONROLE_ACTIVE);
+
+  EXPECT_TRUE(transport_->SetLocalTransportDescription(
+      local_offer, SdpType::kOffer, nullptr));
+  EXPECT_TRUE(transport_->SetRemoteTransportDescription(
+      remote_answer, SdpType::kAnswer, nullptr));
+
+  // We were actpass->active previously, now in the other direction it's
+  // actpass->passive.
+  TransportDescription remote_offer = MakeTransportDescription(
+      kIceUfrag2, kIcePwd2, certificate, CONNECTIONROLE_ACTPASS);
+  TransportDescription local_answer = MakeTransportDescription(
+      kIceUfrag1, kIcePwd1, certificate, CONNECTIONROLE_PASSIVE);
+
+  EXPECT_TRUE(transport_->SetRemoteTransportDescription(
+      remote_offer, SdpType::kOffer, nullptr));
+  EXPECT_TRUE(transport_->SetLocalTransportDescription(
+      local_answer, SdpType::kAnswer, nullptr));
+}
+
+// Test that a reoffer in the opposite direction fails if the role changes.
+// Inverse of test above.
+TEST_F(JsepTransportTest, InvalidDtlsReofferFromAnswerer) {
+  // Just use the same certificate for both sides; doesn't really matter in a
+  // non end-to-end test.
+  rtc::scoped_refptr<rtc::RTCCertificate> certificate =
+      rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
+          rtc::SSLIdentity::Generate("testing", rtc::KT_ECDSA)));
+  transport_->SetLocalCertificate(certificate);
+
+  TransportDescription local_offer = MakeTransportDescription(
+      kIceUfrag1, kIcePwd1, certificate, CONNECTIONROLE_ACTPASS);
+  TransportDescription remote_answer = MakeTransportDescription(
+      kIceUfrag2, kIcePwd2, certificate, CONNECTIONROLE_ACTIVE);
+
+  EXPECT_TRUE(transport_->SetLocalTransportDescription(
+      local_offer, SdpType::kOffer, nullptr));
+  EXPECT_TRUE(transport_->SetRemoteTransportDescription(
+      remote_answer, SdpType::kAnswer, nullptr));
+
+  // Changing role to passive here isn't allowed. Though for some reason this
+  // only fails in SetLocalTransportDescription.
+  TransportDescription remote_offer = MakeTransportDescription(
+      kIceUfrag2, kIcePwd2, certificate, CONNECTIONROLE_PASSIVE);
+  TransportDescription local_answer = MakeTransportDescription(
+      kIceUfrag1, kIcePwd1, certificate, CONNECTIONROLE_ACTIVE);
+
+  EXPECT_TRUE(transport_->SetRemoteTransportDescription(
+      remote_offer, SdpType::kOffer, nullptr));
+  EXPECT_FALSE(transport_->SetLocalTransportDescription(
+      local_answer, SdpType::kAnswer, nullptr));
+}
+
 // Test that a remote offer with the current negotiated role can be accepted.
 // This is allowed by dtls-sdp, though we'll never generate such an offer,
 // since JSEP requires generating "actpass".
 TEST_F(JsepTransportTest, RemoteOfferWithCurrentNegotiatedDtlsRole) {
+  // Just use the same certificate in both descriptions; the remote fingerprint
+  // doesn't matter in a non end-to-end test.
   rtc::scoped_refptr<rtc::RTCCertificate> certificate =
       rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
           rtc::SSLIdentity::Generate("testing", rtc::KT_ECDSA)));
-  std::unique_ptr<rtc::SSLFingerprint> fingerprint(
-      rtc::SSLFingerprint::CreateFromCertificate(certificate));
   transport_->SetLocalCertificate(certificate);
 
-  TransportDescription local_desc(kIceUfrag1, kIcePwd1);
-  TransportDescription remote_desc(kIceUfrag2, kIcePwd2);
-  // Just use the same fingerprint in both descriptions; the remote fingerprint
-  // doesn't matter in a non end-to-end test.
-  local_desc.identity_fingerprint.reset(
-      TransportDescription::CopyFingerprint(fingerprint.get()));
-  remote_desc.identity_fingerprint.reset(
-      TransportDescription::CopyFingerprint(fingerprint.get()));
-
-  remote_desc.connection_role = cricket::CONNECTIONROLE_ACTPASS;
-  local_desc.connection_role = cricket::CONNECTIONROLE_ACTIVE;
+  TransportDescription remote_desc = MakeTransportDescription(
+      kIceUfrag1, kIcePwd1, certificate, CONNECTIONROLE_ACTPASS);
+  TransportDescription local_desc = MakeTransportDescription(
+      kIceUfrag2, kIcePwd2, certificate, CONNECTIONROLE_ACTIVE);
 
   // Normal initial offer/answer with "actpass" in the offer and "active" in
   // the answer.
@@ -399,7 +576,7 @@
   EXPECT_EQ(rtc::SSL_CLIENT, *role);
 
   // Subsequent offer with current negotiated role of "passive".
-  remote_desc.connection_role = cricket::CONNECTIONROLE_PASSIVE;
+  remote_desc.connection_role = CONNECTIONROLE_PASSIVE;
   EXPECT_TRUE(transport_->SetRemoteTransportDescription(
       remote_desc, SdpType::kOffer, nullptr));
   EXPECT_TRUE(transport_->SetLocalTransportDescription(
@@ -425,8 +602,8 @@
   remote_desc.identity_fingerprint.reset(
       TransportDescription::CopyFingerprint(fingerprint.get()));
 
-  remote_desc.connection_role = cricket::CONNECTIONROLE_ACTPASS;
-  local_desc.connection_role = cricket::CONNECTIONROLE_ACTIVE;
+  remote_desc.connection_role = CONNECTIONROLE_ACTPASS;
+  local_desc.connection_role = CONNECTIONROLE_ACTIVE;
 
   // Normal initial offer/answer with "actpass" in the offer and "active" in
   // the answer.
@@ -444,9 +621,45 @@
   // endpoint's negotiated role.
   // TODO(deadbeef): Really this should fail as soon as the offer is
   // attempted to be applied, and not when the answer is applied.
-  remote_desc.connection_role = cricket::CONNECTIONROLE_ACTIVE;
+  remote_desc.connection_role = CONNECTIONROLE_ACTIVE;
   EXPECT_TRUE(transport_->SetRemoteTransportDescription(
       remote_desc, SdpType::kOffer, nullptr));
   EXPECT_FALSE(transport_->SetLocalTransportDescription(
       local_desc, SdpType::kAnswer, nullptr));
 }
+
+// Testing that a legacy client that doesn't use the setup attribute will be
+// interpreted as having an active role.
+TEST_F(JsepTransportTest, TestDtlsSetupWithLegacyAsAnswerer) {
+  rtc::scoped_refptr<rtc::RTCCertificate> certificate =
+      rtc::RTCCertificate::Create(std::unique_ptr<rtc::SSLIdentity>(
+          rtc::SSLIdentity::Generate("testing", rtc::KT_ECDSA)));
+  std::unique_ptr<rtc::SSLFingerprint> fingerprint(
+      rtc::SSLFingerprint::CreateFromCertificate(certificate));
+  transport_->SetLocalCertificate(certificate);
+
+  TransportDescription local_desc(kIceUfrag1, kIcePwd1);
+  TransportDescription remote_desc(kIceUfrag2, kIcePwd2);
+  // Just use the same fingerprint in both descriptions; the remote fingerprint
+  // doesn't matter in a non end-to-end test.
+  local_desc.identity_fingerprint.reset(
+      TransportDescription::CopyFingerprint(fingerprint.get()));
+  remote_desc.identity_fingerprint.reset(
+      TransportDescription::CopyFingerprint(fingerprint.get()));
+
+  local_desc.connection_role = CONNECTIONROLE_ACTPASS;
+  ASSERT_TRUE(transport_->SetLocalTransportDescription(
+      local_desc, SdpType::kOffer, nullptr));
+  // Use CONNECTIONROLE_NONE to simulate legacy endpoint.
+  remote_desc.connection_role = CONNECTIONROLE_NONE;
+  ASSERT_TRUE(transport_->SetRemoteTransportDescription(
+      remote_desc, SdpType::kAnswer, nullptr));
+
+  rtc::Optional<rtc::SSLRole> role = transport_->GetSslRole();
+  ASSERT_TRUE(role);
+  // Since legacy answer ommitted setup atribute, and we offered actpass, we
+  // should act as passive (server).
+  EXPECT_EQ(rtc::SSL_SERVER, *role);
+}
+
+}  // namespace cricket