Properly handle different transports having different SSL roles.

talk/app/webrtc/webrtcsession.cc

Index: talk/app/webrtc/webrtcsession.cc
diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc
index 29a4f3394a32c8674d272aa7b007bc0053066c52..12c89fce02e232f50ddadb3ef0089af98abd7ddf 100644
--- a/talk/app/webrtc/webrtcsession.cc
+++ b/talk/app/webrtc/webrtcsession.cc
@@ -761,14 +761,15 @@ 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);
 }
 
 void WebRtcSession::CreateOffer(
@@ -970,15 +971,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 +984,9 @@ bool WebRtcSession::UpdateSessionState(
         return BadAnswerSdp(source, kEnableBundleFailed, err_desc);
       }
     }
+    if (!PushdownTransportDescription(source, cricket::CA_ANSWER, &td_err)) {
+      return BadAnswerSdp(source, MakeTdErrorString(td_err), err_desc);
+    }

[pthatcher1, 2016/01/07 21:12:03]

What effect does moving this down have?  Can you leave a comment explaining why
it needs to be down here?

[Taylor Brandstetter, 2016/01/07 22:25:55]

If somehow the DTLS roles were "active" for audio and "passive" for video, then
you start BUNDLEing on video, the answer will have "active" for both. But if you
try to push down the transport description before enabling BUNDLE (and
destroying the video transport), it will complain about a DTLS role conflict.

Bottom line is: we shouldn't be pushing down a transport description for a
transport that's about to be destroyed, especially since that description is now
referring to a DIFFERENT transport.

     EnableChannels();
     SetState(STATE_INPROGRESS);
     if (!PushdownMediaDescription(cricket::CA_ANSWER, source, err_desc)) {