Accept SDP with TRANSPORT attributes missing from bundled m= sections.

webrtc/api/webrtcsession.cc

Index: webrtc/api/webrtcsession.cc
diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc
index 8487d2cee11241c5a256334f89a4b674e0308509..a9662966e0b140d9c9fc70366dc6c0e30bb37d78 100644
--- a/webrtc/api/webrtcsession.cc
+++ b/webrtc/api/webrtcsession.cc
@@ -164,20 +164,28 @@ static bool VerifyMediaDescriptions(
 }
 
 // Checks that each non-rejected content has SDES crypto keys or a DTLS
-// fingerprint. Mismatches, such as replying with a DTLS fingerprint to SDES
-// keys, will be caught in Transport negotiation, and backstopped by Channel's
-// |srtp_required| check.
+// fingerprint, unless it's in a BUNDLE group, in which case only the
+// BUNDLE-tag section needs a ufrag and pwd. Mismatches, such as replying with

[pthatcher1, 2017/02/17 19:17:32]

I translation from spec speak to understandable text would be nice, perhaps
something like:

BUNDLE-tag section (the first media section of the BUNDLE group)

[Taylor Brandstetter, 2017/02/17 21:49:14]

Done.

+// a DTLS fingerprint to SDES keys, will be caught in JsepTransport
+// negotiation, and backstopped by Channel's |srtp_required| check.
 static bool VerifyCrypto(const SessionDescription* desc,
                          bool dtls_enabled,
                          std::string* error) {
+  const cricket::ContentGroup* bundle =
+      desc->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
   const ContentInfos& contents = desc->contents();
   for (size_t index = 0; index < contents.size(); ++index) {
     const ContentInfo* cinfo = &contents[index];
     if (cinfo->rejected) {
       continue;
     }
+    if (bundle && bundle->HasContentName(cinfo->name) &&
+        cinfo->name != *(bundle->FirstContentName())) {
+      continue;

[pthatcher1, 2017/02/17 19:17:32]

A comment here like "It's not the first, so it spec-compliant sections may not
have any attributes".

[Taylor Brandstetter, 2017/02/17 21:49:14]

Done.

+    }
 
-    // If the content isn't rejected, crypto must be present.
+    // If the content isn't rejected or bundled into another m= section, crypto
+    // must be present.
     const MediaContentDescription* media =
         static_cast<const MediaContentDescription*>(cinfo->description);
     const TransportInfo* tinfo = desc->GetTransportInfoByName(cinfo->name);
@@ -207,16 +215,25 @@ static bool VerifyCrypto(const SessionDescription* desc,
   return true;
 }
 
-// Checks that each non-rejected content has ice-ufrag and ice-pwd set.
+// Checks that each non-rejected content has ice-ufrag and ice-pwd set, unless
+// it's in a BUNDLE group, in which case only the BUNDLE-tag section needs a
+// ufrag and pwd.

[pthatcher1, 2017/02/17 19:17:32]

Like the comment above, please translate "BUNDLE-tag section" (that might not be
the worse name I've seen from a spec, but it's up there).

[Taylor Brandstetter, 2017/02/17 21:49:14]

Done.

 static bool VerifyIceUfragPwdPresent(const SessionDescription* desc) {
+  const cricket::ContentGroup* bundle =
+      desc->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
   const ContentInfos& contents = desc->contents();
   for (size_t index = 0; index < contents.size(); ++index) {
     const ContentInfo* cinfo = &contents[index];
     if (cinfo->rejected) {
       continue;
     }
+    if (bundle && bundle->HasContentName(cinfo->name) &&
+        cinfo->name != *(bundle->FirstContentName())) {
+      continue;
+    }
 
-    // If the content isn't rejected, ice-ufrag and ice-pwd must be present.
+    // If the content isn't rejected or bundled into another m= section,
+    // ice-ufrag and ice-pwd must be present.
     const TransportInfo* tinfo = desc->GetTransportInfoByName(cinfo->name);
     if (!tinfo) {
       // Something is not right.