Fixing Bug 1444354: spd group parsing fails intermittently with rust parser draft
authorJohannes Willbold <j.willbold@mozilla.com>
Tue, 03 Jul 2018 09:29:18 -0700
changeset 813851 5db18844e389027071b4e4090216815f431c0449
parent 813581 a0e47ebc4c06e652b919dabee711fdbd6bfd31b5
push id115022
push userbmo:johannes.willbold@rub.de
push dateTue, 03 Jul 2018 23:22:58 +0000
bugs1444354
milestone63.0a1
Fixing Bug 1444354: spd group parsing fails intermittently with rust parser Ignore this commit, this is only here to place it on 'try' MozReview-Commit-ID: IKHKEyFPk5C
media/webrtc/signaling/gtest/sdp_unittests.cpp
media/webrtc/signaling/src/sdp/RsdparsaSdpAttributeList.cpp
media/webrtc/signaling/src/sdp/RsdparsaSdpInc.h
--- a/media/webrtc/signaling/gtest/sdp_unittests.cpp
+++ b/media/webrtc/signaling/gtest/sdp_unittests.cpp
@@ -2960,20 +2960,32 @@ TEST_P(NewSdpTest, CheckMediaLevelIcePwd
             mSdp->GetMediaSection(0).GetAttributeList().GetIcePwd());
 
   ASSERT_TRUE(mSdp->GetMediaSection(1).GetAttributeList().HasAttribute(
         SdpAttribute::kIcePwdAttribute));
   ASSERT_EQ("e4cc12a910f106a0a744719425510e17",
             mSdp->GetMediaSection(1).GetAttributeList().GetIcePwd());
 }
 
+// const std::string kMinimalGroupTest =
+// "v=0" CRLF
+// "o=Mozilla-SIPUA-35.0a1 5184 0 IN IP4 0.0.0.0" CRLF
+// "s=SIP Call" CRLF
+// "c=IN IP4 224.0.0.1/100/12" CRLF
+// "t=0 0" CRLF
+// "a=group:BUNDLE first second" CRLF
+// "a=group:BUNDLE third" CRLF
+// "a=group:LS first third" CRLF
+// "m=audio 9 RTP/SAVPF 0" CRLF
+// "a=rtpmap:0 PCMU/8000" CRLF
+// "a=bundle-only" CRLF;
+
 TEST_P(NewSdpTest, CheckGroups) {
-  SKIP_TEST_WITH_RUST_PARSER; // See Bug 1444354
-
   ParseSdp(kBasicAudioVideoOffer);
+  // ParseSdp(kMinimalGroupTest);
   const SdpGroupAttributeList& group = mSdp->GetAttributeList().GetGroup();
   const SdpGroupAttributeList::Group& group1 = group.mGroups[0];
   ASSERT_EQ(SdpGroupAttributeList::kBundle, group1.semantics);
   ASSERT_EQ(2U, group1.tags.size());
   ASSERT_EQ("first", group1.tags[0]);
   ASSERT_EQ("second", group1.tags[1]);
 
   const SdpGroupAttributeList::Group& group2 = group.mGroups[1];
--- a/media/webrtc/signaling/src/sdp/RsdparsaSdpAttributeList.cpp
+++ b/media/webrtc/signaling/src/sdp/RsdparsaSdpAttributeList.cpp
@@ -6,16 +6,17 @@
 
 #include "nsCRT.h"
 
 #include "signaling/src/sdp/RsdparsaSdpAttributeList.h"
 #include "signaling/src/sdp/RsdparsaSdpInc.h"
 #include "signaling/src/sdp/RsdparsaSdpGlue.h"
 
 #include <ostream>
+#include <iostream>
 #include "mozilla/Assertions.h"
 
 #include <limits>
 
 namespace mozilla
 {
 
 const std::string RsdparsaSdpAttributeList::kEmptyString = "";
@@ -36,21 +37,24 @@ RsdparsaSdpAttributeList::HasAttribute(A
 
 const SdpAttribute*
 RsdparsaSdpAttributeList::GetAttribute(AttributeType type,
                                        bool sessionFallback) const
 {
   const SdpAttribute* value = mAttributes[static_cast<size_t>(type)];
   // Only do fallback when the attribute can appear at both the media and
   // session level
+  std::cout << "Got pointer: " << value << std::endl;
   if (!value && !AtSessionLevel() && sessionFallback &&
       SdpAttribute::IsAllowedAtSessionLevel(type) &&
       SdpAttribute::IsAllowedAtMediaLevel(type)) {
+    std::cout << "taking session level branch [expected]" << std::endl;
     return mSessionAttributes->GetAttribute(type, false);
   }
+  std::cout << "taking media level branch [(un)expected (2nd run)]" << std::endl;
   return value;
 }
 
 void
 RsdparsaSdpAttributeList::RemoveAttribute(AttributeType type)
 {
   delete mAttributes[static_cast<size_t>(type)];
   mAttributes[static_cast<size_t>(type)] = nullptr;
@@ -161,16 +165,17 @@ RsdparsaSdpAttributeList::GetFmtp() cons
 
   return *static_cast<const SdpFmtpAttributeList*>(
              GetAttribute(SdpAttribute::kFmtpAttribute));
 }
 
 const SdpGroupAttributeList&
 RsdparsaSdpAttributeList::GetGroup() const
 {
+  std::cout << "RsdparsaSdpAttributeList::GetGroup was called [expected]" << std::endl;
   if (!HasAttribute(SdpAttribute::kGroupAttribute)) {
     MOZ_CRASH();
   }
 
   return *static_cast<const SdpGroupAttributeList*>(
              GetAttribute(SdpAttribute::kGroupAttribute));
 }
 
@@ -857,24 +862,22 @@ RsdparsaSdpAttributeList::LoadMsidSemant
   }
   SetAttribute(msidSemantics.release());
 }
 
 void
 RsdparsaSdpAttributeList::LoadGroup(RustAttributeList* attributeList)
 {
   size_t numGroup = sdp_get_group_count(attributeList);
+  std::cout << "RsdparsaSdpAttributeList::LoadGroup was called, numGroup: " << numGroup << " [expected]" << std::endl;
   if (numGroup == 0) {
     return;
   }
   auto rustGroups = MakeUnique<RustSdpAttributeGroup[]>(numGroup);
-  nsresult nr = sdp_get_groups(attributeList, numGroup, rustGroups.get());
-  if (NS_FAILED(nr)) {
-    return;
-  }
+  sdp_get_groups(attributeList, numGroup, rustGroups.get());
   auto groups = MakeUnique<SdpGroupAttributeList>();
   for(size_t i = 0; i < numGroup; i++) {
     RustSdpAttributeGroup& group = rustGroups[i];
     SdpGroupAttributeList::Semantics semantic;
     switch(group.semantic) {
       case RustSdpAttributeGroupSemantic ::kRustLipSynchronization:
         semantic = SdpGroupAttributeList::kLs;
         break;
@@ -895,16 +898,20 @@ RsdparsaSdpAttributeList::LoadGroup(Rust
         break;
       case RustSdpAttributeGroupSemantic ::kRustBundle:
         semantic = SdpGroupAttributeList::kBundle;
         break;
     }
     std::vector<std::string> tags = convertStringVec(group.tags);
     groups->PushEntry(semantic, tags);
   }
+
+  std::cout << "Loaded " << groups->mGroups.size() << " groups [expected]" << std::endl;
+  std::cout << "Groups, Type: " << groups->GetType() << "should be: " << SdpAttribute::kGroupAttribute << std::endl;
+  std::cout << "Before: " << mAttributes[groups->GetType()] << std::endl;
   SetAttribute(groups.release());
 }
 
 void
 RsdparsaSdpAttributeList::LoadRtcp(RustAttributeList* attributeList)
 {
   RustSdpAttributeRtcp rtcp;
   if (NS_SUCCEEDED(sdp_get_rtcp(attributeList, &rtcp))) {
--- a/media/webrtc/signaling/src/sdp/RsdparsaSdpInc.h
+++ b/media/webrtc/signaling/src/sdp/RsdparsaSdpInc.h
@@ -344,18 +344,18 @@ size_t sdp_get_msid_count(const RustAttr
 void sdp_get_msids(const RustAttributeList* aList, size_t listSize,
                    RustSdpAttributeMsid* ret);
 
 size_t sdp_get_msid_semantic_count(RustAttributeList* aList);
 void sdp_get_msid_semantics(const RustAttributeList* aList, size_t listSize,
                             RustSdpAttributeMsidSemantic* ret);
 
 size_t sdp_get_group_count(const RustAttributeList* aList);
-nsresult sdp_get_groups(const RustAttributeList* aList, size_t listSize,
-                        RustSdpAttributeGroup* ret);
+void sdp_get_groups(const RustAttributeList* aList, size_t listSize,
+                    RustSdpAttributeGroup* ret);
 
 nsresult sdp_get_rtcp(const RustAttributeList* aList,
                       RustSdpAttributeRtcp* ret);
 
 
 size_t sdp_get_rtcpfb_count(const RustAttributeList* aList);
 void sdp_get_rtcpfbs(const RustAttributeList* aList, size_t listSize,
                      RustSdpAttributeRtcpFb* ret);