Properly handle different transports having different SSL roles.

This meant splitting "transport_options" into audio/video/data options,
for when creating the answer, and giving "GetSslRole" a "transport_name"
parameter so we can retrieve the current role on a per-transport basis.

BUG=webrtc:4525
R=pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1516993002 .

Cr-Commit-Position: refs/heads/master@{#11192}

11 files changed, 170 insertions, 55 deletions

diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc
index 5f1e3ebdc5..ccca18af67 100644
--- a/talk/app/webrtc/peerconnection.cc
+++ b/talk/app/webrtc/peerconnection.cc
@@ -461,7 +461,11 @@ bool ConvertRtcOptionsForOffer(
   }
 
   session_options->vad_enabled = rtc_options.voice_activity_detection;
-  session_options->transport_options.ice_restart = rtc_options.ice_restart;
+  session_options->audio_transport_options.ice_restart =
+      rtc_options.ice_restart;
+  session_options->video_transport_options.ice_restart =
+      rtc_options.ice_restart;
+  session_options->data_transport_options.ice_restart = rtc_options.ice_restart;
   session_options->bundle_enabled = rtc_options.use_rtp_mux;
 
   return true;
@@ -507,10 +511,14 @@ bool ParseConstraintsForAnswer(const MediaConstraintsInterface* constraints,
 
   if (FindConstraint(constraints, MediaConstraintsInterface::kIceRestart,
                      &value, &mandatory_constraints_satisfied)) {
-    session_options->transport_options.ice_restart = value;
+    session_options->audio_transport_options.ice_restart = value;
+    session_options->video_transport_options.ice_restart = value;
+    session_options->data_transport_options.ice_restart = value;
   } else {
     // kIceRestart defaults to false according to spec.
-    session_options->transport_options.ice_restart = false;
+    session_options->audio_transport_options.ice_restart = false;
+    session_options->video_transport_options.ice_restart = false;
+    session_options->data_transport_options.ice_restart = false;
   }
 
   if (!constraints) {
@@ -962,7 +970,7 @@ void PeerConnection::SetLocalDescription(
   // SCTP sids.
   rtc::SSLRole role;
   if (session_->data_channel_type() == cricket::DCT_SCTP &&
-      session_->GetSslRole(&role)) {
+      session_->GetSslRole(session_->data_channel(), &role)) {
     AllocateSctpSids(role);
   }
 
@@ -1040,7 +1048,7 @@ void PeerConnection::SetRemoteDescription(
   // SCTP sids.
   rtc::SSLRole role;
   if (session_->data_channel_type() == cricket::DCT_SCTP &&
-      session_->GetSslRole(&role)) {
+      session_->GetSslRole(session_->data_channel(), &role)) {
     AllocateSctpSids(role);
   }
 
@@ -1833,7 +1841,7 @@ rtc::scoped_refptr<DataChannel> PeerConnection::InternalCreateDataChannel(
   if (session_->data_channel_type() == cricket::DCT_SCTP) {
     if (new_config.id < 0) {
       rtc::SSLRole role;
-      if (session_->GetSslRole(&role) &&
+      if ((session_->GetSslRole(session_->data_channel(), &role)) &&
           !sid_allocator_.AllocateSid(role, &new_config.id)) {
         LOG(LS_ERROR) << "No id can be allocated for the SCTP data channel.";
         return nullptr;
diff --git a/talk/app/webrtc/peerconnectioninterface_unittest.cc b/talk/app/webrtc/peerconnectioninterface_unittest.cc
index b705da4bb3..c3789b7dd8 100644
--- a/talk/app/webrtc/peerconnectioninterface_unittest.cc
+++ b/talk/app/webrtc/peerconnectioninterface_unittest.cc
@@ -2323,7 +2323,9 @@ TEST(CreateSessionOptionsTest, GetDefaultMediaSessionOptionsForOffer) {
   EXPECT_FALSE(options.has_video());
   EXPECT_TRUE(options.bundle_enabled);
   EXPECT_TRUE(options.vad_enabled);
-  EXPECT_FALSE(options.transport_options.ice_restart);
+  EXPECT_FALSE(options.audio_transport_options.ice_restart);
+  EXPECT_FALSE(options.video_transport_options.ice_restart);
+  EXPECT_FALSE(options.data_transport_options.ice_restart);
 }
 
 // Test that a correct MediaSessionOptions is created for an offer if
@@ -2358,18 +2360,22 @@ TEST(CreateSessionOptionsTest,
 
 // Test that a correct MediaSessionOptions is created to restart ice if
 // IceRestart is set. It also tests that subsequent MediaSessionOptions don't
-// have |transport_options.ice_restart| set.
+// have |audio_transport_options.ice_restart| etc. set.
 TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithIceRestart) {
   RTCOfferAnswerOptions rtc_options;
   rtc_options.ice_restart = true;
 
   cricket::MediaSessionOptions options;
   EXPECT_TRUE(ConvertRtcOptionsForOffer(rtc_options, &options));
-  EXPECT_TRUE(options.transport_options.ice_restart);
+  EXPECT_TRUE(options.audio_transport_options.ice_restart);
+  EXPECT_TRUE(options.video_transport_options.ice_restart);
+  EXPECT_TRUE(options.data_transport_options.ice_restart);
 
   rtc_options = RTCOfferAnswerOptions();
   EXPECT_TRUE(ConvertRtcOptionsForOffer(rtc_options, &options));
-  EXPECT_FALSE(options.transport_options.ice_restart);
+  EXPECT_FALSE(options.audio_transport_options.ice_restart);
+  EXPECT_FALSE(options.video_transport_options.ice_restart);
+  EXPECT_FALSE(options.data_transport_options.ice_restart);
 }
 
 // Test that the MediaConstraints in an answer don't affect if audio and video
diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc
index 29a4f3394a..d8f76379c1 100644
--- a/talk/app/webrtc/webrtcsession.cc
+++ b/talk/app/webrtc/webrtcsession.cc
@@ -761,14 +761,20 @@ cricket::SecurePolicy WebRtcSession::SdesPolicy() const {
   return webrtc_session_desc_factory_->SdesPolicy();
 }
 
-bool WebRtcSession::GetSslRole(rtc::SSLRole* role) {
+bool WebRtcSession::GetSslRole(const std::string& transport_name,
+                               rtc::SSLRole* role) {
   if (!local_desc_ || !remote_desc_) {
     LOG(LS_INFO) << "Local and Remote descriptions must be applied to get "
                  << "SSL Role of the session.";
     return false;
   }
 
-  return transport_controller_->GetSslRole(role);
+  return transport_controller_->GetSslRole(transport_name, role);
+}
+
+bool WebRtcSession::GetSslRole(const cricket::BaseChannel* channel,
+                               rtc::SSLRole* role) {
+  return channel && GetSslRole(channel->transport_name(), role);
 }
 
 void WebRtcSession::CreateOffer(
@@ -970,15 +976,12 @@ bool WebRtcSession::UpdateSessionState(
       return BadPranswerSdp(source, GetSessionErrorMsg(), err_desc);
     }
   } else if (action == kAnswer) {
-    if (!PushdownTransportDescription(source, cricket::CA_ANSWER, &td_err)) {
-      return BadAnswerSdp(source, MakeTdErrorString(td_err), err_desc);
-    }
     const cricket::ContentGroup* local_bundle =
         local_desc_->description()->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
     const cricket::ContentGroup* remote_bundle =
         remote_desc_->description()->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
     if (local_bundle && remote_bundle) {
-      // The answerer decides the transport to bundle on
+      // The answerer decides the transport to bundle on.
       const cricket::ContentGroup* answer_bundle =
           (source == cricket::CS_LOCAL ? local_bundle : remote_bundle);
       if (!EnableBundle(*answer_bundle)) {
@@ -986,6 +989,11 @@ bool WebRtcSession::UpdateSessionState(
         return BadAnswerSdp(source, kEnableBundleFailed, err_desc);
       }
     }
+    // Only push down the transport description after enabling BUNDLE; we don't
+    // want to push down a description on a transport about to be destroyed.
+    if (!PushdownTransportDescription(source, cricket::CA_ANSWER, &td_err)) {
+      return BadAnswerSdp(source, MakeTdErrorString(td_err), err_desc);
+    }
     EnableChannels();
     SetState(STATE_INPROGRESS);
     if (!PushdownMediaDescription(cricket::CA_ANSWER, source, err_desc)) {
diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h
index dd5ca1811d..b79e0ec270 100644
--- a/talk/app/webrtc/webrtcsession.h
+++ b/talk/app/webrtc/webrtcsession.h
@@ -204,7 +204,11 @@ class WebRtcSession : public AudioProviderInterface,
   cricket::SecurePolicy SdesPolicy() const;
 
   // Get current ssl role from transport.
-  bool GetSslRole(rtc::SSLRole* role);
+  bool GetSslRole(const std::string& transport_name, rtc::SSLRole* role);
+
+  // Get current SSL role for this channel's transport.
+  // If |transport| is null, returns false.
+  bool GetSslRole(const cricket::BaseChannel* channel, rtc::SSLRole* role);
 
   void CreateOffer(
       CreateSessionDescriptionObserver* observer,
diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc
index bf7fc83d0b..e81b8b5b54 100644
--- a/talk/app/webrtc/webrtcsession_unittest.cc
+++ b/talk/app/webrtc/webrtcsession_unittest.cc
@@ -1957,6 +1957,67 @@ TEST_P(WebRtcSessionTest, TestCreateAnswerReceiveOfferWithoutEncryption) {
   SetLocalDescriptionWithoutError(answer);
 }
 
+// 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_P(WebRtcSessionTest, TestCreateAnswerWithDifferentSslRoles) {
+  SendAudioVideoStream1();
+  InitWithDtls(GetParam());
+  SetFactoryDtlsSrtp();
+
+  SessionDescriptionInterface* offer = CreateOffer();
+  SetLocalDescriptionWithoutError(offer);
+
+  cricket::MediaSessionOptions options;
+  options.recv_video = true;
+
+  // First, negotiate different SSL roles.
+  SessionDescriptionInterface* answer =
+      CreateRemoteAnswer(offer, options, cricket::SEC_DISABLED);
+  TransportInfo* audio_transport_info =
+      answer->description()->GetTransportInfoByName("audio");
+  audio_transport_info->description.connection_role =
+      cricket::CONNECTIONROLE_ACTIVE;
+  TransportInfo* video_transport_info =
+      answer->description()->GetTransportInfoByName("video");
+  video_transport_info->description.connection_role =
+      cricket::CONNECTIONROLE_PASSIVE;
+  SetRemoteDescriptionWithoutError(answer);
+
+  // Now create an offer in the reverse direction, and ensure the initial
+  // offerer responds with an answer with correct SSL roles.
+  offer = CreateRemoteOfferWithVersion(options, cricket::SEC_DISABLED,
+                                       kSessionVersion,
+                                       session_->remote_description());
+  SetRemoteDescriptionWithoutError(offer);
+
+  answer = CreateAnswer(nullptr);
+  audio_transport_info = answer->description()->GetTransportInfoByName("audio");
+  EXPECT_EQ(cricket::CONNECTIONROLE_PASSIVE,
+            audio_transport_info->description.connection_role);
+  video_transport_info = answer->description()->GetTransportInfoByName("video");
+  EXPECT_EQ(cricket::CONNECTIONROLE_ACTIVE,
+            video_transport_info->description.connection_role);
+  SetLocalDescriptionWithoutError(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.
+  options.bundle_enabled = true;
+  offer = CreateRemoteOfferWithVersion(options, cricket::SEC_DISABLED,
+                                       kSessionVersion,
+                                       session_->remote_description());
+  SetRemoteDescriptionWithoutError(offer);
+  answer = CreateAnswer(nullptr);
+  audio_transport_info = answer->description()->GetTransportInfoByName("audio");
+  EXPECT_EQ(cricket::CONNECTIONROLE_PASSIVE,
+            audio_transport_info->description.connection_role);
+  video_transport_info = answer->description()->GetTransportInfoByName("video");
+  EXPECT_EQ(cricket::CONNECTIONROLE_PASSIVE,
+            video_transport_info->description.connection_role);
+  SetLocalDescriptionWithoutError(answer);
+}
+
 TEST_F(WebRtcSessionTest, TestSetLocalOfferTwice) {
   Init();
   SendNothing();
@@ -3625,7 +3686,9 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerWithNewUfragAndPassword) {
   SetLocalDescriptionWithoutError(answer.release());
 
   // Receive an offer with new ufrag and password.
-  options.transport_options.ice_restart = true;
+  options.audio_transport_options.ice_restart = true;
+  options.video_transport_options.ice_restart = true;
+  options.data_transport_options.ice_restart = true;
   rtc::scoped_ptr<JsepSessionDescription> updated_offer1(
       CreateRemoteOffer(options, session_->remote_description()));
   SetRemoteDescriptionWithoutError(updated_offer1.release());
@@ -3656,7 +3719,9 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerWithOldUfragAndPassword) {
   SetLocalDescriptionWithoutError(answer.release());
 
   // Receive an offer without changed ufrag or password.
-  options.transport_options.ice_restart = false;
+  options.audio_transport_options.ice_restart = false;
+  options.video_transport_options.ice_restart = false;
+  options.data_transport_options.ice_restart = false;
   rtc::scoped_ptr<JsepSessionDescription> updated_offer2(
       CreateRemoteOffer(options, session_->remote_description()));
   SetRemoteDescriptionWithoutError(updated_offer2.release());
diff --git a/talk/app/webrtc/webrtcsessiondescriptionfactory.cc b/talk/app/webrtc/webrtcsessiondescriptionfactory.cc
index bfdbb1ac0a..f08b77eb40 100644
--- a/talk/app/webrtc/webrtcsessiondescriptionfactory.cc
+++ b/talk/app/webrtc/webrtcsessiondescriptionfactory.cc
@@ -392,7 +392,9 @@ void WebRtcSessionDescriptionFactory::InternalCreateOffer(
     return;
   }
   if (session_->local_description() &&
-      !request.options.transport_options.ice_restart) {
+      !request.options.audio_transport_options.ice_restart &&
+      !request.options.video_transport_options.ice_restart &&
+      !request.options.data_transport_options.ice_restart) {
     // Include all local ice candidates in the SessionDescription unless
     // the an ice restart has been requested.
     CopyCandidatesFromSessionDescription(session_->local_description(), offer);
@@ -405,12 +407,25 @@ void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
   // According to http://tools.ietf.org/html/rfc5245#section-9.2.1.1
   // an answer should also contain new ice ufrag and password if an offer has
   // been received with new ufrag and password.
-  request.options.transport_options.ice_restart = session_->IceRestartPending();
+  request.options.audio_transport_options.ice_restart =
+      session_->IceRestartPending();
+  request.options.video_transport_options.ice_restart =
+      session_->IceRestartPending();
+  request.options.data_transport_options.ice_restart =
+      session_->IceRestartPending();
   // We should pass current ssl role to the transport description factory, if
   // there is already an existing ongoing session.
   rtc::SSLRole ssl_role;
-  if (session_->GetSslRole(&ssl_role)) {
-    request.options.transport_options.prefer_passive_role =
+  if (session_->GetSslRole(session_->voice_channel(), &ssl_role)) {
+    request.options.audio_transport_options.prefer_passive_role =
+        (rtc::SSL_SERVER == ssl_role);
+  }
+  if (session_->GetSslRole(session_->video_channel(), &ssl_role)) {
+    request.options.video_transport_options.prefer_passive_role =
+        (rtc::SSL_SERVER == ssl_role);
+  }
+  if (session_->GetSslRole(session_->data_channel(), &ssl_role)) {
+    request.options.data_transport_options.prefer_passive_role =
         (rtc::SSL_SERVER == ssl_role);
   }
 
@@ -439,7 +454,9 @@ void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
     return;
   }
   if (session_->local_description() &&
-      !request.options.transport_options.ice_restart) {
+      !request.options.audio_transport_options.ice_restart &&
+      !request.options.video_transport_options.ice_restart &&
+      !request.options.data_transport_options.ice_restart) {
     // Include all local ice candidates in the SessionDescription unless
     // the remote peer has requested an ice restart.
     CopyCandidatesFromSessionDescription(session_->local_description(), answer);
diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc
index 2dfaa1e515..24f01b4463 100644
--- a/talk/session/media/mediasession.cc
+++ b/talk/session/media/mediasession.cc
@@ -549,8 +549,8 @@ static bool AddStreamParams(
 
 // Updates the transport infos of the |sdesc| according to the given
 // |bundle_group|. The transport infos of the content names within the
-// |bundle_group| should be updated to use the ufrag and pwd of the first
-// content within the |bundle_group|.
+// |bundle_group| should be updated to use the ufrag, pwd and DTLS role of the
+// first content within the |bundle_group|.
 static bool UpdateTransportInfoForBundle(const ContentGroup& bundle_group,
                                          SessionDescription* sdesc) {
   // The bundle should not be empty.
@@ -571,6 +571,8 @@ static bool UpdateTransportInfoForBundle(const ContentGroup& bundle_group,
       selected_transport_info->description.ice_ufrag;
   const std::string& selected_pwd =
       selected_transport_info->description.ice_pwd;
+  ConnectionRole selected_connection_role =
+      selected_transport_info->description.connection_role;
   for (TransportInfos::iterator it =
            sdesc->transport_infos().begin();
        it != sdesc->transport_infos().end(); ++it) {
@@ -578,6 +580,7 @@ static bool UpdateTransportInfoForBundle(const ContentGroup& bundle_group,
         it->content_name != selected_content_name) {
       it->description.ice_ufrag = selected_ufrag;
       it->description.ice_pwd = selected_pwd;
+      it->description.connection_role = selected_connection_role;
     }
   }
   return true;
@@ -1600,7 +1603,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer(
   }
 
   desc->AddContent(content_name, NS_JINGLE_RTP, audio.release());
-  if (!AddTransportOffer(content_name, options.transport_options,
+  if (!AddTransportOffer(content_name, options.audio_transport_options,
                          current_description, desc)) {
     return false;
   }
@@ -1660,7 +1663,7 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer(
   }
 
   desc->AddContent(content_name, NS_JINGLE_RTP, video.release());
-  if (!AddTransportOffer(content_name, options.transport_options,
+  if (!AddTransportOffer(content_name, options.video_transport_options,
                          current_description, desc)) {
     return false;
   }
@@ -1724,7 +1727,7 @@ bool MediaSessionDescriptionFactory::AddDataContentForOffer(
     SetMediaProtocol(secure_transport, data.get());
     desc->AddContent(content_name, NS_JINGLE_RTP, data.release());
   }
-  if (!AddTransportOffer(content_name, options.transport_options,
+  if (!AddTransportOffer(content_name, options.data_transport_options,
                          current_description, desc)) {
     return false;
   }
@@ -1739,10 +1742,9 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer(
     SessionDescription* answer) const {
   const ContentInfo* audio_content = GetFirstAudioContent(offer);
 
-  scoped_ptr<TransportDescription> audio_transport(
-      CreateTransportAnswer(audio_content->name, offer,
-                            options.transport_options,
-                            current_description));
+  scoped_ptr<TransportDescription> audio_transport(CreateTransportAnswer(
+      audio_content->name, offer, options.audio_transport_options,
+      current_description));
   if (!audio_transport) {
     return false;
   }
@@ -1798,10 +1800,9 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer(
     StreamParamsVec* current_streams,
     SessionDescription* answer) const {
   const ContentInfo* video_content = GetFirstVideoContent(offer);
-  scoped_ptr<TransportDescription> video_transport(
-      CreateTransportAnswer(video_content->name, offer,
-                            options.transport_options,
-                            current_description));
+  scoped_ptr<TransportDescription> video_transport(CreateTransportAnswer(
+      video_content->name, offer, options.video_transport_options,
+      current_description));
   if (!video_transport) {
     return false;
   }
@@ -1854,10 +1855,9 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer(
     StreamParamsVec* current_streams,
     SessionDescription* answer) const {
   const ContentInfo* data_content = GetFirstDataContent(offer);
-  scoped_ptr<TransportDescription> data_transport(
-      CreateTransportAnswer(data_content->name, offer,
-                            options.transport_options,
-                            current_description));
+  scoped_ptr<TransportDescription> data_transport(CreateTransportAnswer(
+      data_content->name, offer, options.data_transport_options,
+      current_description));
   if (!data_transport) {
     return false;
   }
diff --git a/talk/session/media/mediasession.h b/talk/session/media/mediasession.h
index 8a7f4ccf06..1540274665 100644
--- a/talk/session/media/mediasession.h
+++ b/talk/session/media/mediasession.h
@@ -134,6 +134,10 @@ struct MediaSessionOptions {
 
   bool HasSendMediaStream(MediaType type) const;
 
+  // TODO(deadbeef): Put all the audio/video/data-specific options into a map
+  // structure (content name -> options).
+  // MediaSessionDescriptionFactory assumes there will never be more than one
+  // audio/video/data content, but this will change with unified plan.
   bool recv_audio;
   bool recv_video;
   DataChannelType data_channel_type;
@@ -144,7 +148,9 @@ struct MediaSessionOptions {
   // bps. -1 == auto.
   int video_bandwidth;
   int data_bandwidth;
-  TransportOptions transport_options;
+  TransportOptions audio_transport_options;
+  TransportOptions video_transport_options;
+  TransportOptions data_transport_options;
 
   struct Stream {
     Stream(MediaType type,
diff --git a/webrtc/p2p/base/transportcontroller.cc b/webrtc/p2p/base/transportcontroller.cc
index 22b827a1a5..053388eeb8 100644
--- a/webrtc/p2p/base/transportcontroller.cc
+++ b/webrtc/p2p/base/transportcontroller.cc
@@ -66,9 +66,10 @@ void TransportController::SetIceRole(IceRole ice_role) {
       rtc::Bind(&TransportController::SetIceRole_w, this, ice_role));
 }
 
-bool TransportController::GetSslRole(rtc::SSLRole* role) {
-  return worker_thread_->Invoke<bool>(
-      rtc::Bind(&TransportController::GetSslRole_w, this, role));
+bool TransportController::GetSslRole(const std::string& transport_name,
+                                     rtc::SSLRole* role) {
+  return worker_thread_->Invoke<bool>(rtc::Bind(
+      &TransportController::GetSslRole_w, this, transport_name, role));
 }
 
 bool TransportController::SetLocalCertificate(
@@ -343,13 +344,16 @@ void TransportController::SetIceRole_w(IceRole ice_role) {
   }
 }
 
-bool TransportController::GetSslRole_w(rtc::SSLRole* role) {
+bool TransportController::GetSslRole_w(const std::string& transport_name,
+                                       rtc::SSLRole* role) {
   RTC_DCHECK(worker_thread()->IsCurrent());
 
-  if (transports_.empty()) {
+  Transport* t = GetTransport_w(transport_name);
+  if (!t) {
     return false;
   }
-  return transports_.begin()->second->GetSslRole(role);
+
+  return t->GetSslRole(role);
 }
 
 bool TransportController::SetLocalCertificate_w(
diff --git a/webrtc/p2p/base/transportcontroller.h b/webrtc/p2p/base/transportcontroller.h
index 8d57b460e8..e26f3b5f17 100644
--- a/webrtc/p2p/base/transportcontroller.h
+++ b/webrtc/p2p/base/transportcontroller.h
@@ -48,11 +48,7 @@ class TransportController : public sigslot::has_slots<>,
   void SetIceConfig(const IceConfig& config);
   void SetIceRole(IceRole ice_role);
 
-  // TODO(deadbeef) - Return role of each transport, as role may differ from
-  // one another.
-  // In current implementaion we just return the role of the first transport
-  // alphabetically.
-  bool GetSslRole(rtc::SSLRole* role);
+  bool GetSslRole(const std::string& transport_name, rtc::SSLRole* role);
 
   // Specifies the identity to use in this session.
   // Can only be called once.
@@ -160,7 +156,7 @@ class TransportController : public sigslot::has_slots<>,
   bool SetSslMaxProtocolVersion_w(rtc::SSLProtocolVersion version);
   void SetIceConfig_w(const IceConfig& config);
   void SetIceRole_w(IceRole ice_role);
-  bool GetSslRole_w(rtc::SSLRole* role);
+  bool GetSslRole_w(const std::string& transport_name, rtc::SSLRole* role);
   bool SetLocalCertificate_w(
       const rtc::scoped_refptr<rtc::RTCCertificate>& certificate);
   bool GetLocalCertificate_w(
diff --git a/webrtc/p2p/base/transportcontroller_unittest.cc b/webrtc/p2p/base/transportcontroller_unittest.cc
index 140852984a..6ff158e8fc 100644
--- a/webrtc/p2p/base/transportcontroller_unittest.cc
+++ b/webrtc/p2p/base/transportcontroller_unittest.cc
@@ -262,7 +262,8 @@ TEST_F(TransportControllerTest, TestGetSslRole) {
   ASSERT_NE(nullptr, channel);
   ASSERT_TRUE(channel->SetSslRole(rtc::SSL_CLIENT));
   rtc::SSLRole role;
-  EXPECT_TRUE(transport_controller_->GetSslRole(&role));
+  EXPECT_FALSE(transport_controller_->GetSslRole("video", &role));
+  EXPECT_TRUE(transport_controller_->GetSslRole("audio", &role));
   EXPECT_EQ(rtc::SSL_CLIENT, role);
 }