Bug 1570549 - P1 - refactor into common SDP parser interface - r=bwc

Differential Revision: https://phabricator.services.mozilla.com/D52323

--HG--
rename : media/webrtc/signaling/src/sdp/SdpErrorHolder.h => media/webrtc/signaling/src/sdp/SdpParser.h
extra : moz-landing-system : lando

media/webrtc/signaling/gtest/sdp_unittests.cpp

@@ -1562,14 +1562,14 @@ class NewSdpTest
     : public ::testing::Test,
       public ::testing::WithParamInterface< ::testing::tuple<bool, bool> > {
  public:
-  NewSdpTest() : mSdpErrorHolder(nullptr) {}
+  NewSdpTest() : mSdpParser(nullptr) {}
 
   void ParseSdp(const std::string& sdp, bool expectSuccess = true) {
     if (::testing::get<1>(GetParam())) {
-      mSdpErrorHolder = &mSipccParser;
+      mSdpParser = &mSipccParser;
       mSdp = mSipccParser.Parse(sdp);
     } else {
-      mSdpErrorHolder = &mRustParser;
+      mSdpParser = &mRustParser;
       mSdp = mRustParser.Parse(sdp);
     }
 
@@ -1609,7 +1609,7 @@ class NewSdpTest
     if (expectSuccess) {
       ASSERT_TRUE(!!mSdp)
       << "Parse failed: " << GetParseErrors();
-      ASSERT_EQ(0U, mSdpErrorHolder->GetParseErrors().size())
+      ASSERT_EQ(0U, mSdpParser->GetParseErrors().size())
           << "Got unexpected parse errors/warnings: " << GetParseErrors();
     }
   }
@@ -1617,7 +1617,7 @@ class NewSdpTest
   // For streaming parse errors
   std::string GetParseErrors() const {
     std::stringstream output;
-    for (auto e : mSdpErrorHolder->GetParseErrors()) {
+    for (auto e : mSdpParser->GetParseErrors()) {
       output << e.first << ": " << e.second << std::endl;
     }
     return output.str();
@@ -1625,7 +1625,7 @@ class NewSdpTest
 
   std::string GetParseWarnings() const {
     std::stringstream output;
-    for (auto e : mSdpErrorHolder->GetParseWarnings()) {
+    for (auto e : mSdpParser->GetParseWarnings()) {
       output << e.first << ": " << e.second << std::endl;
     }
     return output.str();
@@ -1693,7 +1693,7 @@ class NewSdpTest
     return ::testing::get<1>(GetParam());
   }
 
-  SdpErrorHolder* mSdpErrorHolder;
+  SdpParser* mSdpParser;
   SipccSdpParser mSipccParser;
   RsdparsaSdpParser mRustParser;
   mozilla::UniquePtr<Sdp> mSdp;
@@ -1704,7 +1704,7 @@ TEST_P(NewSdpTest, CreateDestroy) {}
 TEST_P(NewSdpTest, ParseEmpty) {
   ParseSdp("", false);
   ASSERT_FALSE(mSdp);
-  ASSERT_NE(0U, mSdpErrorHolder->GetParseErrors().size())
+  ASSERT_NE(0U, mSdpParser->GetParseErrors().size())
       << "Expected at least one parse error.";
 }
 
@@ -1713,24 +1713,24 @@ const std::string kBadSdp = "This is SDPARTA!!!!";
 TEST_P(NewSdpTest, ParseGarbage) {
   ParseSdp(kBadSdp, false);
   ASSERT_FALSE(mSdp);
-  ASSERT_NE(0U, mSdpErrorHolder->GetParseErrors().size())
+  ASSERT_NE(0U, mSdpParser->GetParseErrors().size())
       << "Expected at least one parse error.";
 }
 
 TEST_P(NewSdpTest, ParseGarbageTwice) {
   ParseSdp(kBadSdp, false);
   ASSERT_FALSE(mSdp);
-  size_t errorCount = mSdpErrorHolder->GetParseErrors().size();
+  size_t errorCount = mSdpParser->GetParseErrors().size();
   ASSERT_NE(0U, errorCount) << "Expected at least one parse error.";
   ParseSdp(kBadSdp, false);
   ASSERT_FALSE(mSdp);
-  ASSERT_EQ(errorCount, mSdpErrorHolder->GetParseErrors().size())
+  ASSERT_EQ(errorCount, mSdpParser->GetParseErrors().size())
       << "Expected same error count for same SDP.";
 }
 
 TEST_P(NewSdpTest, ParseMinimal) {
   ParseSdp(kVideoSdp);
-  ASSERT_EQ(0U, mSdpErrorHolder->GetParseErrors().size())
+  ASSERT_EQ(0U, mSdpParser->GetParseErrors().size())
       << "Got parse errors: " << GetParseErrors();
 }
 
@@ -3093,7 +3093,7 @@ const std::string kBasicAudioVideoDataOffer =
 
 TEST_P(NewSdpTest, BasicAudioVideoDataSdpParse) {
   ParseSdp(kBasicAudioVideoDataOffer);
-  ASSERT_EQ(0U, mSdpErrorHolder->GetParseErrors().size())
+  ASSERT_EQ(0U, mSdpParser->GetParseErrors().size())
       << "Got parse errors: " << GetParseErrors();
 }
 
@@ -4014,7 +4014,7 @@ TEST(NewSdpTestNoFixture, CheckParsingResultComparer)
   auto check_comparison = [](const std::string sdp_string) {
     SipccSdpParser sipccParser;
     RsdparsaSdpParser rustParser;
-    auto print_errors = [](const SdpErrorHolder& holder, const char* name) {
+    auto print_errors = [](const SdpParser& holder, const char* name) {
       for (const auto& e : holder.GetParseErrors()) {
         std::cerr << name << " Line " << e.first << ": " << e.second;
       }

media/webrtc/signaling/src/sdp/ParsingResultComparer.cpp

@@ -1,3 +1,5 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */

media/webrtc/signaling/src/sdp/RsdparsaSdp.cpp

@@ -1,3 +1,5 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
@@ -10,7 +12,7 @@
 #include "nsError.h"
 #include <iostream>
 
-#include "signaling/src/sdp/SdpErrorHolder.h"
+#include "signaling/src/sdp/SdpParser.h"
 #include "signaling/src/sdp/RsdparsaSdpInc.h"
 #include "signaling/src/sdp/RsdparsaSdpMediaSection.h"
 

media/webrtc/signaling/src/sdp/RsdparsaSdp.h

@@ -20,7 +20,7 @@
 namespace mozilla {
 
 class RsdparsaSdpParser;
-class SdpErrorHolder;
+class SdpParser;
 
 class RsdparsaSdp final : public Sdp {
   friend class RsdparsaSdpParser;

media/webrtc/signaling/src/sdp/RsdparsaSdpAttributeList.h

@@ -15,7 +15,7 @@ namespace mozilla {
 
 class RsdparsaSdp;
 class RsdparsaSdpMediaSection;
-class SdpErrorHolder;
+class SdpParser;
 
 class RsdparsaSdpAttributeList : public SdpAttributeList {
   friend class RsdparsaSdpMediaSection;
@@ -141,8 +141,7 @@ class RsdparsaSdpAttributeList : public SdpAttributeList {
   void LoadCandidate(RustAttributeList* attributeList);
 
   void WarnAboutMisplacedAttribute(SdpAttribute::AttributeType type,
-                                   uint32_t lineNumber,
+                                   uint32_t lineNumber, SdpParser& errorHolder);
-                                   SdpErrorHolder& errorHolder);
 
   SdpAttribute* mAttributes[kNumAttributeTypes];
 

media/webrtc/signaling/src/sdp/RsdparsaSdpMediaSection.cpp

@@ -11,7 +11,7 @@
 #include "signaling/src/sdp/RsdparsaSdpInc.h"
 
 #include <ostream>
-#include "signaling/src/sdp/SdpErrorHolder.h"
+#include "signaling/src/sdp/SdpParser.h"
 
 #ifdef CRLF
 #  undef CRLF

media/webrtc/signaling/src/sdp/RsdparsaSdpMediaSection.h

@@ -19,7 +19,7 @@
 namespace mozilla {
 
 class RsdparsaSdp;
-class SdpErrorHolder;
+class SdpParser;
 
 class RsdparsaSdpMediaSection final : public SdpMediaSection {
   friend class RsdparsaSdp;

media/webrtc/signaling/src/sdp/RsdparsaSdpParser.cpp

@@ -17,25 +17,31 @@
 
 namespace mozilla {
 
-UniquePtr<Sdp> RsdparsaSdpParser::Parse(const std::string& sdpText) {
+static const std::string& WEBRTC_SDP_NAME = "WEBRTC_SDP_NAME";
-  ClearParseErrors();
+
+const std::string& RsdparsaSdpParser::Name() const { return WEBRTC_SDP_NAME; }
+
+UniquePtr<SdpParser::Results> RsdparsaSdpParser::Parse(
+    const std::string& aText) {
+  UniquePtr<SdpParser::InternalResults> results(
+      new SdpParser::InternalResults(Name()));
   RustSdpSession* result = nullptr;
   RustSdpError* err = nullptr;
-  StringView sdpTextView{sdpText.c_str(), sdpText.length()};
+  StringView sdpTextView{aText.c_str(), aText.length()};
   nsresult rv = parse_sdp(sdpTextView, false, &result, &err);
   if (rv != NS_OK) {
     size_t line = sdp_get_error_line_num(err);
     std::string errMsg = convertStringView(sdp_get_error_message(err));
     sdp_free_error(err);
-    AddParseError(line, errMsg);
+    results->AddParseError(line, errMsg);
-    return nullptr;
+    return results;
   }
 
   if (err) {
     size_t line = sdp_get_error_line_num(err);
     std::string warningMsg = convertStringView(sdp_get_error_message(err));
     sdp_free_error(err);
-    AddParseWarnings(line, warningMsg);
+    results->AddParseWarning(line, warningMsg);
   }
 
   RsdparsaSessionHandle uniqueResult(result);
@@ -44,7 +50,12 @@ UniquePtr<Sdp> RsdparsaSdpParser::Parse(const std::string& sdpText) {
   SdpOrigin origin(convertStringView(rustOrigin.username), rustOrigin.sessionId,
                    rustOrigin.sessionVersion, address.first, address.second);
 
-  return MakeUnique<RsdparsaSdp>(std::move(uniqueResult), origin);
+  results->SetSdp(MakeUnique<RsdparsaSdp>(std::move(uniqueResult), origin));
+  return results;
+}
+
+bool RsdparsaSdpParser::IsNamed(const std::string& aName) {
+  return aName == WEBRTC_SDP_NAME;
 }
 
 }  // namespace mozilla

media/webrtc/signaling/src/sdp/RsdparsaSdpParser.h

@@ -12,20 +12,20 @@
 #include "mozilla/UniquePtr.h"
 
 #include "signaling/src/sdp/Sdp.h"
-#include "signaling/src/sdp/SdpErrorHolder.h"
+#include "signaling/src/sdp/SdpParser.h"
 
 namespace mozilla {
 
-class RsdparsaSdpParser final : public SdpErrorHolder {
+class RsdparsaSdpParser final : public SdpParser {
  public:
-  RsdparsaSdpParser() {}
+  RsdparsaSdpParser() = default;
-  virtual ~RsdparsaSdpParser() {}
+  virtual ~RsdparsaSdpParser() = default;
 
-  /**
+  const std::string& Name() const override;
-   * This parses the provided text into an SDP object.
+
-   * This returns a nullptr-valued pointer if things go poorly.
+  UniquePtr<SdpParser::Results> Parse(const std::string& text) override;
-   */
+
-  UniquePtr<Sdp> Parse(const std::string& sdpText);
+  static bool IsNamed(const std::string& aName);
 };
 
 }  // namespace mozilla

media/webrtc/signaling/src/sdp/Sdp.h

@@ -83,8 +83,8 @@ class SdpMediaSection;
  */
 class Sdp {
  public:
-  Sdp(){};
+  Sdp() = default;
-  virtual ~Sdp(){};
+  virtual ~Sdp() = default;
 
   virtual const SdpOrigin& GetOrigin() const = 0;
   // Note: connection information is always retrieved from media sections

media/webrtc/signaling/src/sdp/SdpErrorHolder.h

@@ -1,51 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=2 et sw=2 tw=80: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this file,
- * You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#ifndef _SDPERRORHOLDER_H_
-#define _SDPERRORHOLDER_H_
-
-#include <vector>
-#include <string>
-
-namespace mozilla {
-
-class SdpErrorHolder {
- public:
-  SdpErrorHolder() {}
-  virtual ~SdpErrorHolder() {}
-
-  void AddParseError(size_t line, const std::string& message) {
-    mErrors.push_back(std::make_pair(line, message));
-  }
-
-  void AddParseWarnings(size_t line, const std::string& message) {
-    mWarnings.push_back(std::make_pair(line, message));
-  }
-
-  void ClearParseErrors() { mErrors.clear(); }
-
-  void ClearParseWarnings() { mWarnings.clear(); }
-
-  /**
-   * Returns a reference to the list of parse errors.
-   * This gets cleared out when you call Parse.
-   */
-  const std::vector<std::pair<size_t, std::string> >& GetParseErrors() const {
-    return mErrors;
-  }
-
-  const std::vector<std::pair<size_t, std::string> >& GetParseWarnings() const {
-    return mWarnings;
-  }
-
- private:
-  std::vector<std::pair<size_t, std::string> > mErrors;
-  std::vector<std::pair<size_t, std::string> > mWarnings;
-};
-
-}  // namespace mozilla
-
-#endif

media/webrtc/signaling/src/sdp/SdpHelper.cpp

@@ -1,3 +1,5 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */

media/webrtc/signaling/src/sdp/SdpHelper.h

@@ -1,3 +1,5 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */

media/webrtc/signaling/src/sdp/SdpParser.h

@@ -0,0 +1,72 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef _SDPPARSER_H_
+#define _SDPPARSER_H_
+
+#include <vector>
+#include <string>
+#include "signaling/src/sdp/Sdp.h"
+
+namespace mozilla {
+
+class SdpParser {
+ public:
+  SdpParser() = default;
+  virtual ~SdpParser() = default;
+
+  class Results {
+   public:
+    typedef std::pair<size_t, std::string> Anomaly;
+    typedef std::vector<Anomaly> AnomalyVec;
+    virtual ~Results() = default;
+    UniquePtr<mozilla::Sdp>& Sdp() { return mSdp; }
+    AnomalyVec& Errors() { return mErrors; }
+    AnomalyVec& Warnings() { return mWarnings; }
+    virtual const std::string& ParserName() const = 0;
+    bool Ok() const { return mErrors.empty(); }
+
+   protected:
+    UniquePtr<mozilla::Sdp> mSdp;
+    AnomalyVec mErrors;
+    AnomalyVec mWarnings;
+  };
+
+  // The name of the parser implementation
+  virtual const std::string& Name() const = 0;
+
+  /**
+   * This parses the provided text into an SDP object.
+   * This returns a nullptr-valued pointer if things go poorly.
+   */
+  virtual UniquePtr<SdpParser::Results> Parse(const std::string& aText) = 0;
+
+  class InternalResults : public Results {
+   public:
+    explicit InternalResults(const std::string& aParserName)
+        : mParserName(aParserName) {}
+    virtual ~InternalResults() = default;
+
+    void SetSdp(UniquePtr<mozilla::Sdp>&& aSdp) { mSdp = std::move(aSdp); }
+
+    void AddParseError(size_t line, const std::string& message) {
+      mErrors.push_back(std::make_pair(line, message));
+    }
+
+    void AddParseWarning(size_t line, const std::string& message) {
+      mWarnings.push_back(std::make_pair(line, message));
+    }
+
+    const std::string& ParserName() const override { return mParserName; }
+
+   private:
+    const std::string mParserName;
+  };
+};
+
+}  // namespace mozilla
+
+#endif

media/webrtc/signaling/src/sdp/SipccSdp.cpp

@@ -1,3 +1,5 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
@@ -7,7 +9,7 @@
 #include <cstdlib>
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Assertions.h"
-#include "signaling/src/sdp/SdpErrorHolder.h"
+#include "signaling/src/sdp/SdpParser.h"
 
 #ifdef CRLF
 #  undef CRLF
@@ -59,14 +61,14 @@ SdpMediaSection& SipccSdp::AddMediaSection(SdpMediaSection::MediaType mediaType,
   return *media;
 }
 
-bool SipccSdp::LoadOrigin(sdp_t* sdp, SdpErrorHolder& errorHolder) {
+bool SipccSdp::LoadOrigin(sdp_t* sdp, InternalResults& results) {
   std::string username = sdp_get_owner_username(sdp);
   uint64_t sessId = strtoull(sdp_get_owner_sessionid(sdp), nullptr, 10);
   uint64_t sessVer = strtoull(sdp_get_owner_version(sdp), nullptr, 10);
 
   sdp_nettype_e type = sdp_get_owner_network_type(sdp);
   if (type != SDP_NT_INTERNET) {
-    errorHolder.AddParseError(2, "Unsupported network type");
+    results.AddParseError(2, "Unsupported network type");
     return false;
   }
 
@@ -79,7 +81,7 @@ bool SipccSdp::LoadOrigin(sdp_t* sdp, SdpErrorHolder& errorHolder) {
       addrType = sdp::kIPv6;
       break;
     default:
-      errorHolder.AddParseError(2, "Unsupported address type");
+      results.AddParseError(2, "Unsupported address type");
       return false;
   }
 
@@ -88,17 +90,17 @@ bool SipccSdp::LoadOrigin(sdp_t* sdp, SdpErrorHolder& errorHolder) {
   return true;
 }
 
-bool SipccSdp::Load(sdp_t* sdp, SdpErrorHolder& errorHolder) {
+bool SipccSdp::Load(sdp_t* sdp, InternalResults& results) {
   // Believe it or not, SDP_SESSION_LEVEL is 0xFFFF
-  if (!mAttributeList.Load(sdp, SDP_SESSION_LEVEL, errorHolder)) {
+  if (!mAttributeList.Load(sdp, SDP_SESSION_LEVEL, results)) {
     return false;
   }
 
-  if (!LoadOrigin(sdp, errorHolder)) {
+  if (!LoadOrigin(sdp, results)) {
     return false;
   }
 
-  if (!mBandwidths.Load(sdp, SDP_SESSION_LEVEL, errorHolder)) {
+  if (!mBandwidths.Load(sdp, SDP_SESSION_LEVEL, results)) {
     return false;
   }
 
@@ -107,7 +109,7 @@ bool SipccSdp::Load(sdp_t* sdp, SdpErrorHolder& errorHolder) {
     // sipcc counts media sections from 1, using 0xFFFF as the "session"
     UniquePtr<SipccSdpMediaSection> section(
         new SipccSdpMediaSection(i, &mAttributeList));
-    if (!section->Load(sdp, i + 1, errorHolder)) {
+    if (!section->Load(sdp, i + 1, results)) {
       return false;
     }
     mMediaSections.push_back(std::move(section));
@@ -136,7 +138,7 @@ void SipccSdp::Serialize(std::ostream& os) const {
 }
 
 bool SipccSdpBandwidths::Load(sdp_t* sdp, uint16_t level,
-                              SdpErrorHolder& errorHolder) {
+                              InternalResults& results) {
   size_t count = sdp_get_num_bw_lines(sdp, level);
   for (size_t i = 1; i <= count; ++i) {
     sdp_bw_modifier_e bwtype = sdp_get_bw_modifier(sdp, level, i);

media/webrtc/signaling/src/sdp/SipccSdp.h

@@ -12,6 +12,7 @@
 #include "mozilla/Attributes.h"
 
 #include "signaling/src/sdp/Sdp.h"
+#include "signaling/src/sdp/SdpParser.h"
 #include "signaling/src/sdp/SipccSdpMediaSection.h"
 #include "signaling/src/sdp/SipccSdpAttributeList.h"
 extern "C" {
@@ -21,7 +22,6 @@ extern "C" {
 namespace mozilla {
 
 class SipccSdpParser;
-class SdpErrorHolder;
 
 class SipccSdp final : public Sdp {
   friend class SipccSdpParser;
@@ -61,10 +61,12 @@ class SipccSdp final : public Sdp {
   virtual void Serialize(std::ostream&) const override;
 
  private:
+  using InternalResults = SdpParser::InternalResults;
+
   SipccSdp() : mOrigin("", 0, 0, sdp::kIPv4, ""), mAttributeList(nullptr) {}
 
-  bool Load(sdp_t* sdp, SdpErrorHolder& errorHolder);
+  bool Load(sdp_t* sdp, InternalResults& results);
-  bool LoadOrigin(sdp_t* sdp, SdpErrorHolder& errorHolder);
+  bool LoadOrigin(sdp_t* sdp, InternalResults& results);
 
   SdpOrigin mOrigin;
   SipccSdpBandwidths mBandwidths;

media/webrtc/signaling/src/sdp/SipccSdpAttributeList.cpp

@@ -8,7 +8,6 @@
 
 #include <ostream>
 #include "mozilla/Assertions.h"
-#include "signaling/src/sdp/SdpErrorHolder.h"
 
 extern "C" {
 #include "signaling/src/sdp/sipcc/sdp_private.h"
@@ -16,6 +15,8 @@ extern "C" {
 
 namespace mozilla {
 
+using InternalResults = SdpParser::InternalResults;
+
 /* static */
 const std::string SipccSdpAttributeList::kEmptyString = "";
 
@@ -82,12 +83,12 @@ void SipccSdpAttributeList::SetAttribute(SdpAttribute* attr) {
 void SipccSdpAttributeList::LoadSimpleString(sdp_t* sdp, uint16_t level,
                                              sdp_attr_e attr,
                                              AttributeType targetType,
-                                             SdpErrorHolder& errorHolder) {
+                                             InternalResults& results) {
   const char* value = sdp_attr_get_simple_string(sdp, attr, level, 0, 1);
   if (value) {
     if (!IsAllowedHere(targetType)) {
       uint32_t lineNumber = sdp_attr_line_number(sdp, attr, level, 0, 1);
-      WarnAboutMisplacedAttribute(targetType, lineNumber, errorHolder);
+      WarnAboutMisplacedAttribute(targetType, lineNumber, results);
     } else {
       SetAttribute(new SdpStringAttribute(targetType, std::string(value)));
     }
@@ -95,21 +96,21 @@ void SipccSdpAttributeList::LoadSimpleString(sdp_t* sdp, uint16_t level,
 }
 
 void SipccSdpAttributeList::LoadSimpleStrings(sdp_t* sdp, uint16_t level,
-                                              SdpErrorHolder& errorHolder) {
+                                              InternalResults& results) {
   LoadSimpleString(sdp, level, SDP_ATTR_MID, SdpAttribute::kMidAttribute,
-                   errorHolder);
+                   results);
   LoadSimpleString(sdp, level, SDP_ATTR_LABEL, SdpAttribute::kLabelAttribute,
-                   errorHolder);
+                   results);
 }
 
 void SipccSdpAttributeList::LoadSimpleNumber(sdp_t* sdp, uint16_t level,
                                              sdp_attr_e attr,
                                              AttributeType targetType,
-                                             SdpErrorHolder& errorHolder) {
+                                             InternalResults& results) {
   if (sdp_attr_valid(sdp, attr, level, 0, 1)) {
     if (!IsAllowedHere(targetType)) {
       uint32_t lineNumber = sdp_attr_line_number(sdp, attr, level, 0, 1);
-      WarnAboutMisplacedAttribute(targetType, lineNumber, errorHolder);
+      WarnAboutMisplacedAttribute(targetType, lineNumber, results);
     } else {
       uint32_t value = sdp_attr_get_simple_u32(sdp, attr, level, 0, 1);
       SetAttribute(new SdpNumberAttribute(targetType, value));
@@ -118,15 +119,15 @@ void SipccSdpAttributeList::LoadSimpleNumber(sdp_t* sdp, uint16_t level,
 }
 
 void SipccSdpAttributeList::LoadSimpleNumbers(sdp_t* sdp, uint16_t level,
-                                              SdpErrorHolder& errorHolder) {
+                                              InternalResults& results) {
   LoadSimpleNumber(sdp, level, SDP_ATTR_PTIME, SdpAttribute::kPtimeAttribute,
-                   errorHolder);
+                   results);
   LoadSimpleNumber(sdp, level, SDP_ATTR_MAXPTIME,
-                   SdpAttribute::kMaxptimeAttribute, errorHolder);
+                   SdpAttribute::kMaxptimeAttribute, results);
   LoadSimpleNumber(sdp, level, SDP_ATTR_SCTPPORT,
-                   SdpAttribute::kSctpPortAttribute, errorHolder);
+                   SdpAttribute::kSctpPortAttribute, results);
   LoadSimpleNumber(sdp, level, SDP_ATTR_MAXMESSAGESIZE,
-                   SdpAttribute::kMaxMessageSizeAttribute, errorHolder);
+                   SdpAttribute::kMaxMessageSizeAttribute, results);
 }
 
 void SipccSdpAttributeList::LoadFlags(sdp_t* sdp, uint16_t level) {
@@ -173,7 +174,7 @@ static void ConvertDirection(sdp_direction_e sipcc_direction,
 }
 
 void SipccSdpAttributeList::LoadDirection(sdp_t* sdp, uint16_t level,
-                                          SdpErrorHolder& errorHolder) {
+                                          InternalResults& results) {
   SdpDirectionAttribute::Direction dir;
   ConvertDirection(sdp_get_media_direction(sdp, level, 0), &dir);
   SetAttribute(new SdpDirectionAttribute(dir));
@@ -205,7 +206,7 @@ void SipccSdpAttributeList::LoadIceAttributes(sdp_t* sdp, uint16_t level) {
 }
 
 bool SipccSdpAttributeList::LoadFingerprint(sdp_t* sdp, uint16_t level,
-                                            SdpErrorHolder& errorHolder) {
+                                            InternalResults& results) {
   char* value;
   UniquePtr<SdpFingerprintAttributeList> fingerprintAttrs;
 
@@ -224,14 +225,14 @@ bool SipccSdpAttributeList::LoadFingerprint(sdp_t* sdp, uint16_t level,
     // sipcc does not expose parse code for this
     size_t start = fingerprintAttr.find_first_not_of(" \t");
     if (start == std::string::npos) {
-      errorHolder.AddParseError(lineNumber, "Empty fingerprint attribute");
+      results.AddParseError(lineNumber, "Empty fingerprint attribute");
       return false;
     }
 
     size_t end = fingerprintAttr.find_first_of(" \t", start);
     if (end == std::string::npos) {
       // One token, no trailing ws
-      errorHolder.AddParseError(lineNumber,
+      results.AddParseError(lineNumber,
                             "Only one token in fingerprint attribute");
       return false;
     }
@@ -241,7 +242,7 @@ bool SipccSdpAttributeList::LoadFingerprint(sdp_t* sdp, uint16_t level,
     start = fingerprintAttr.find_first_not_of(" \t", end);
     if (start == std::string::npos) {
       // One token, trailing ws
-      errorHolder.AddParseError(lineNumber,
+      results.AddParseError(lineNumber,
                             "Only one token in fingerprint attribute");
       return false;
     }
@@ -251,7 +252,7 @@ bool SipccSdpAttributeList::LoadFingerprint(sdp_t* sdp, uint16_t level,
     std::vector<uint8_t> fingerprint =
         SdpFingerprintAttributeList::ParseFingerprint(fingerprintToken);
     if (fingerprint.empty()) {
-      errorHolder.AddParseError(lineNumber, "Malformed fingerprint token");
+      results.AddParseError(lineNumber, "Malformed fingerprint token");
       return false;
     }
 
@@ -292,7 +293,7 @@ void SipccSdpAttributeList::LoadCandidate(sdp_t* sdp, uint16_t level) {
 }
 
 bool SipccSdpAttributeList::LoadSctpmap(sdp_t* sdp, uint16_t level,
-                                        SdpErrorHolder& errorHolder) {
+                                        InternalResults& results) {
   auto sctpmap = MakeUnique<SdpSctpmapAttributeList>();
   for (uint16_t i = 0; i < UINT16_MAX; ++i) {
     sdp_attr_t* attr = sdp_find_attr(sdp, level, 0, SDP_ATTR_SCTPMAP, i + 1);
@@ -367,14 +368,14 @@ SdpRtpmapAttributeList::CodecType SipccSdpAttributeList::GetCodecType(
 }
 
 bool SipccSdpAttributeList::LoadRtpmap(sdp_t* sdp, uint16_t level,
-                                       SdpErrorHolder& errorHolder) {
+                                       InternalResults& results) {
   auto rtpmap = MakeUnique<SdpRtpmapAttributeList>();
   uint16_t count;
   sdp_result_e result =
       sdp_attr_num_instances(sdp, level, 0, SDP_ATTR_RTPMAP, &count);
   if (result != SDP_SUCCESS) {
     MOZ_ASSERT(false, "Unable to get rtpmap size");
-    errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+    results.AddParseError(sdp_get_media_line_number(sdp, level),
                           "Unable to get rtpmap size");
     return false;
   }
@@ -384,7 +385,7 @@ bool SipccSdpAttributeList::LoadRtpmap(sdp_t* sdp, uint16_t level,
 
     if (!ccName) {
       // Probably no rtpmap attribute for a pt in an m-line
-      errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+      results.AddParseError(sdp_get_media_line_number(sdp, level),
                             "No rtpmap attribute for payload type");
       continue;
     }
@@ -467,7 +468,7 @@ void SipccSdpAttributeList::LoadSsrc(sdp_t* sdp, uint16_t level) {
 }
 
 bool SipccSdpAttributeList::LoadImageattr(sdp_t* sdp, uint16_t level,
-                                          SdpErrorHolder& errorHolder) {
+                                          InternalResults& results) {
   UniquePtr<SdpImageattrAttributeList> imageattrs(
       new SdpImageattrAttributeList);
 
@@ -483,7 +484,7 @@ bool SipccSdpAttributeList::LoadImageattr(sdp_t* sdp, uint16_t level,
     if (!imageattrs->PushEntry(imageattrRaw, &error, &errorPos)) {
       std::ostringstream fullError;
       fullError << error << " at column " << errorPos;
-      errorHolder.AddParseError(
+      results.AddParseError(
           sdp_attr_line_number(sdp, SDP_ATTR_IMAGEATTR, level, 0, i),
           fullError.str());
       return false;
@@ -497,7 +498,7 @@ bool SipccSdpAttributeList::LoadImageattr(sdp_t* sdp, uint16_t level,
 }
 
 bool SipccSdpAttributeList::LoadSimulcast(sdp_t* sdp, uint16_t level,
-                                          SdpErrorHolder& errorHolder) {
+                                          InternalResults& results) {
   const char* simulcastRaw =
       sdp_attr_get_simple_string(sdp, SDP_ATTR_SIMULCAST, level, 0, 1);
   if (!simulcastRaw) {
@@ -511,7 +512,7 @@ bool SipccSdpAttributeList::LoadSimulcast(sdp_t* sdp, uint16_t level,
   if (!simulcast->Parse(is, &error)) {
     std::ostringstream fullError;
     fullError << error << " at column " << is.tellg();
-    errorHolder.AddParseError(
+    results.AddParseError(
         sdp_attr_line_number(sdp, SDP_ATTR_SIMULCAST, level, 0, 1),
         fullError.str());
     return false;
@@ -522,12 +523,12 @@ bool SipccSdpAttributeList::LoadSimulcast(sdp_t* sdp, uint16_t level,
 }
 
 bool SipccSdpAttributeList::LoadGroups(sdp_t* sdp, uint16_t level,
-                                       SdpErrorHolder& errorHolder) {
+                                       InternalResults& results) {
   uint16_t attrCount = 0;
   if (sdp_attr_num_instances(sdp, level, 0, SDP_ATTR_GROUP, &attrCount) !=
       SDP_SUCCESS) {
     MOZ_ASSERT(false, "Could not get count of group attributes");
-    errorHolder.AddParseError(0, "Could not get count of group attributes");
+    results.AddParseError(0, "Could not get count of group attributes");
     return false;
   }
 
@@ -559,7 +560,7 @@ bool SipccSdpAttributeList::LoadGroups(sdp_t* sdp, uint16_t level,
       if (!idStr) {
         std::ostringstream os;
         os << "bad a=group identifier at " << (attr - 1) << ", " << (id - 1);
-        errorHolder.AddParseError(0, os.str());
+        results.AddParseError(0, os.str());
         return false;
       }
       tags.push_back(std::string(idStr));
@@ -575,7 +576,7 @@ bool SipccSdpAttributeList::LoadGroups(sdp_t* sdp, uint16_t level,
 }
 
 bool SipccSdpAttributeList::LoadMsidSemantics(sdp_t* sdp, uint16_t level,
-                                              SdpErrorHolder& errorHolder) {
+                                              InternalResults& results) {
   auto msidSemantics = MakeUnique<SdpMsidSemanticAttributeList>();
 
   for (uint16_t i = 1; i < UINT16_MAX; ++i) {
@@ -728,12 +729,12 @@ void SipccSdpAttributeList::LoadFmtp(sdp_t* sdp, uint16_t level) {
 }
 
 void SipccSdpAttributeList::LoadMsids(sdp_t* sdp, uint16_t level,
-                                      SdpErrorHolder& errorHolder) {
+                                      InternalResults& results) {
   uint16_t attrCount = 0;
   if (sdp_attr_num_instances(sdp, level, 0, SDP_ATTR_MSID, &attrCount) !=
       SDP_SUCCESS) {
     MOZ_ASSERT(false, "Unable to get count of msid attributes");
-    errorHolder.AddParseError(0, "Unable to get count of msid attributes");
+    results.AddParseError(0, "Unable to get count of msid attributes");
     return;
   }
   auto msids = MakeUnique<SdpMsidAttributeList>();
@@ -742,13 +743,13 @@ void SipccSdpAttributeList::LoadMsids(sdp_t* sdp, uint16_t level,
 
     const char* identifier = sdp_attr_get_msid_identifier(sdp, level, 0, i);
     if (!identifier) {
-      errorHolder.AddParseError(lineNumber, "msid attribute with bad identity");
+      results.AddParseError(lineNumber, "msid attribute with bad identity");
       continue;
     }
 
     const char* appdata = sdp_attr_get_msid_appdata(sdp, level, 0, i);
     if (!appdata) {
-      errorHolder.AddParseError(lineNumber, "msid attribute with bad appdata");
+      results.AddParseError(lineNumber, "msid attribute with bad appdata");
       continue;
     }
 
@@ -761,7 +762,7 @@ void SipccSdpAttributeList::LoadMsids(sdp_t* sdp, uint16_t level,
 }
 
 bool SipccSdpAttributeList::LoadRid(sdp_t* sdp, uint16_t level,
-                                    SdpErrorHolder& errorHolder) {
+                                    InternalResults& results) {
   UniquePtr<SdpRidAttributeList> rids(new SdpRidAttributeList);
 
   for (uint16_t i = 1; i < UINT16_MAX; ++i) {
@@ -776,7 +777,7 @@ bool SipccSdpAttributeList::LoadRid(sdp_t* sdp, uint16_t level,
     if (!rids->PushEntry(ridRaw, &error, &errorPos)) {
       std::ostringstream fullError;
       fullError << error << " at column " << errorPos;
-      errorHolder.AddParseError(
+      results.AddParseError(
           sdp_attr_line_number(sdp, SDP_ATTR_RID, level, 0, i),
           fullError.str());
       return false;
@@ -790,7 +791,7 @@ bool SipccSdpAttributeList::LoadRid(sdp_t* sdp, uint16_t level,
 }
 
 void SipccSdpAttributeList::LoadExtmap(sdp_t* sdp, uint16_t level,
-                                       SdpErrorHolder& errorHolder) {
+                                       InternalResults& results) {
   auto extmaps = MakeUnique<SdpExtmapAttributeList>();
 
   for (uint16_t i = 1; i < UINT16_MAX; ++i) {
@@ -817,7 +818,7 @@ void SipccSdpAttributeList::LoadExtmap(sdp_t* sdp, uint16_t level,
         mSessionLevel->HasAttribute(SdpAttribute::kExtmapAttribute)) {
       uint32_t lineNumber =
           sdp_attr_line_number(sdp, SDP_ATTR_EXTMAP, level, 0, 1);
-      errorHolder.AddParseError(
+      results.AddParseError(
           lineNumber, "extmap attributes in both session and media level");
     }
     SetAttribute(extmaps.release());
@@ -825,7 +826,7 @@ void SipccSdpAttributeList::LoadExtmap(sdp_t* sdp, uint16_t level,
 }
 
 void SipccSdpAttributeList::LoadRtcpFb(sdp_t* sdp, uint16_t level,
-                                       SdpErrorHolder& errorHolder) {
+                                       InternalResults& results) {
   auto rtcpfbs = MakeUnique<SdpRtcpFbAttributeList>();
 
   for (uint16_t i = 1; i < UINT16_MAX; ++i) {
@@ -933,7 +934,7 @@ void SipccSdpAttributeList::LoadRtcpFb(sdp_t* sdp, uint16_t level,
 }
 
 void SipccSdpAttributeList::LoadRtcp(sdp_t* sdp, uint16_t level,
-                                     SdpErrorHolder& errorHolder) {
+                                     InternalResults& results) {
   sdp_attr_t* attr = sdp_find_attr(sdp, level, 0, SDP_ATTR_RTCP, 1);
 
   if (!attr) {
@@ -960,18 +961,18 @@ void SipccSdpAttributeList::LoadRtcp(sdp_t* sdp, uint16_t level,
 }
 
 bool SipccSdpAttributeList::Load(sdp_t* sdp, uint16_t level,
-                                 SdpErrorHolder& errorHolder) {
+                                 InternalResults& results) {
-  LoadSimpleStrings(sdp, level, errorHolder);
+  LoadSimpleStrings(sdp, level, results);
-  LoadSimpleNumbers(sdp, level, errorHolder);
+  LoadSimpleNumbers(sdp, level, results);
   LoadFlags(sdp, level);
-  LoadDirection(sdp, level, errorHolder);
+  LoadDirection(sdp, level, results);
 
   if (AtSessionLevel()) {
-    if (!LoadGroups(sdp, level, errorHolder)) {
+    if (!LoadGroups(sdp, level, results)) {
       return false;
     }
 
-    if (!LoadMsidSemantics(sdp, level, errorHolder)) {
+    if (!LoadMsidSemantics(sdp, level, results)) {
       return false;
     }
 
@@ -980,35 +981,35 @@ bool SipccSdpAttributeList::Load(sdp_t* sdp, uint16_t level,
   } else {
     sdp_media_e mtype = sdp_get_media_type(sdp, level);
     if (mtype == SDP_MEDIA_APPLICATION) {
-      LoadSctpmap(sdp, level, errorHolder);
+      LoadSctpmap(sdp, level, results);
     } else {
-      if (!LoadRtpmap(sdp, level, errorHolder)) {
+      if (!LoadRtpmap(sdp, level, results)) {
         return false;
       }
     }
     LoadCandidate(sdp, level);
     LoadFmtp(sdp, level);
-    LoadMsids(sdp, level, errorHolder);
+    LoadMsids(sdp, level, results);
-    LoadRtcpFb(sdp, level, errorHolder);
+    LoadRtcpFb(sdp, level, results);
-    LoadRtcp(sdp, level, errorHolder);
+    LoadRtcp(sdp, level, results);
     LoadSsrc(sdp, level);
-    if (!LoadImageattr(sdp, level, errorHolder)) {
+    if (!LoadImageattr(sdp, level, results)) {
       return false;
     }
-    if (!LoadSimulcast(sdp, level, errorHolder)) {
+    if (!LoadSimulcast(sdp, level, results)) {
       return false;
     }
-    if (!LoadRid(sdp, level, errorHolder)) {
+    if (!LoadRid(sdp, level, results)) {
       return false;
     }
   }
 
   LoadIceAttributes(sdp, level);
-  if (!LoadFingerprint(sdp, level, errorHolder)) {
+  if (!LoadFingerprint(sdp, level, results)) {
     return false;
   }
   LoadSetup(sdp, level);
-  LoadExtmap(sdp, level, errorHolder);
+  LoadExtmap(sdp, level, results);
 
   return true;
 }
@@ -1028,11 +1029,11 @@ bool SipccSdpAttributeList::IsAllowedHere(
 
 void SipccSdpAttributeList::WarnAboutMisplacedAttribute(
     SdpAttribute::AttributeType type, uint32_t lineNumber,
-    SdpErrorHolder& errorHolder) {
+    InternalResults& results) {
   std::string warning = SdpAttribute::GetAttributeTypeString(type) +
                         (AtSessionLevel() ? " at session level. Ignoring."
                                           : " at media level. Ignoring.");
-  errorHolder.AddParseError(lineNumber, warning);
+  results.AddParseError(lineNumber, warning);
 }
 
 const std::vector<std::string>& SipccSdpAttributeList::GetCandidate() const {

media/webrtc/signaling/src/sdp/SipccSdpAttributeList.h

@@ -7,6 +7,7 @@
 #ifndef _SIPCCSDPATTRIBUTELIST_H_
 #define _SIPCCSDPATTRIBUTELIST_H_
 
+#include "signaling/src/sdp/SdpParser.h"
 #include "signaling/src/sdp/SdpAttributeList.h"
 
 extern "C" {
@@ -17,7 +18,6 @@ namespace mozilla {
 
 class SipccSdp;
 class SipccSdpMediaSection;
-class SdpErrorHolder;
 
 class SipccSdpAttributeList : public SdpAttributeList {
   friend class SipccSdpMediaSection;
@@ -88,44 +88,43 @@ class SipccSdpAttributeList : public SdpAttributeList {
   // otherwise pass nullptr
   explicit SipccSdpAttributeList(const SipccSdpAttributeList* sessionLevel);
 
-  bool Load(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  using InternalResults = SdpParser::InternalResults;
-  void LoadSimpleStrings(sdp_t* sdp, uint16_t level,
+
-                         SdpErrorHolder& errorHolder);
+  bool Load(sdp_t* sdp, uint16_t level, InternalResults& results);
+  void LoadSimpleStrings(sdp_t* sdp, uint16_t level, InternalResults& results);
   void LoadSimpleString(sdp_t* sdp, uint16_t level, sdp_attr_e attr,
-                        AttributeType targetType, SdpErrorHolder& errorHolder);
+                        AttributeType targetType, InternalResults& results);
-  void LoadSimpleNumbers(sdp_t* sdp, uint16_t level,
+  void LoadSimpleNumbers(sdp_t* sdp, uint16_t level, InternalResults& results);
-                         SdpErrorHolder& errorHolder);
   void LoadSimpleNumber(sdp_t* sdp, uint16_t level, sdp_attr_e attr,
-                        AttributeType targetType, SdpErrorHolder& errorHolder);
+                        AttributeType targetType, InternalResults& results);
   void LoadFlags(sdp_t* sdp, uint16_t level);
-  void LoadDirection(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  void LoadDirection(sdp_t* sdp, uint16_t level, InternalResults& results);
-  bool LoadRtpmap(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool LoadRtpmap(sdp_t* sdp, uint16_t level, InternalResults& results);
-  bool LoadSctpmap(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool LoadSctpmap(sdp_t* sdp, uint16_t level, InternalResults& results);
   void LoadIceAttributes(sdp_t* sdp, uint16_t level);
-  bool LoadFingerprint(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool LoadFingerprint(sdp_t* sdp, uint16_t level, InternalResults& results);
   void LoadCandidate(sdp_t* sdp, uint16_t level);
   void LoadSetup(sdp_t* sdp, uint16_t level);
   void LoadSsrc(sdp_t* sdp, uint16_t level);
-  bool LoadImageattr(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool LoadImageattr(sdp_t* sdp, uint16_t level, InternalResults& results);
-  bool LoadSimulcast(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool LoadSimulcast(sdp_t* sdp, uint16_t level, InternalResults& results);
-  bool LoadGroups(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool LoadGroups(sdp_t* sdp, uint16_t level, InternalResults& results);
-  bool LoadMsidSemantics(sdp_t* sdp, uint16_t level,
+  bool LoadMsidSemantics(sdp_t* sdp, uint16_t level, InternalResults& results);
-                         SdpErrorHolder& errorHolder);
   void LoadIdentity(sdp_t* sdp, uint16_t level);
   void LoadDtlsMessage(sdp_t* sdp, uint16_t level);
   void LoadFmtp(sdp_t* sdp, uint16_t level);
-  void LoadMsids(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  void LoadMsids(sdp_t* sdp, uint16_t level, InternalResults& results);
-  bool LoadRid(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool LoadRid(sdp_t* sdp, uint16_t level, InternalResults& results);
-  void LoadExtmap(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  void LoadExtmap(sdp_t* sdp, uint16_t level, InternalResults& results);
-  void LoadRtcpFb(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  void LoadRtcpFb(sdp_t* sdp, uint16_t level, InternalResults& results);
-  void LoadRtcp(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  void LoadRtcp(sdp_t* sdp, uint16_t level, InternalResults& results);
   static SdpRtpmapAttributeList::CodecType GetCodecType(rtp_ptype type);
 
   bool AtSessionLevel() const { return !mSessionLevel; }
   bool IsAllowedHere(SdpAttribute::AttributeType type) const;
   void WarnAboutMisplacedAttribute(SdpAttribute::AttributeType type,
                                    uint32_t lineNumber,
-                                   SdpErrorHolder& errorHolder);
+                                   InternalResults& results);
 
   const SipccSdpAttributeList* mSessionLevel;
 

media/webrtc/signaling/src/sdp/SipccSdpMediaSection.cpp

@@ -7,7 +7,7 @@
 #include "signaling/src/sdp/SipccSdpMediaSection.h"
 
 #include <ostream>
-#include "signaling/src/sdp/SdpErrorHolder.h"
+#include "signaling/src/sdp/SdpParser.h"
 
 #ifdef CRLF
 #  undef CRLF
@@ -57,7 +57,7 @@ SdpDirectionAttribute SipccSdpMediaSection::GetDirectionAttribute() const {
 }
 
 bool SipccSdpMediaSection::Load(sdp_t* sdp, uint16_t level,
-                                SdpErrorHolder& errorHolder) {
+                                InternalResults& results) {
   switch (sdp_get_media_type(sdp, level)) {
     case SDP_MEDIA_AUDIO:
       mMediaType = kAudio;
@@ -73,7 +73,7 @@ bool SipccSdpMediaSection::Load(sdp_t* sdp, uint16_t level,
       break;
 
     default:
-      errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+      results.AddParseError(sdp_get_media_line_number(sdp, level),
                             "Unsupported media section type");
       return false;
   }
@@ -84,38 +84,38 @@ bool SipccSdpMediaSection::Load(sdp_t* sdp, uint16_t level,
     // SDP_INVALID_VALUE (ie; -2) is used when there is no port count. :(
     mPortCount = 0;
   } else if (pc > static_cast<int32_t>(UINT16_MAX) || pc < 0) {
-    errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+    results.AddParseError(sdp_get_media_line_number(sdp, level),
                           "Invalid port count");
     return false;
   } else {
     mPortCount = pc;
   }
 
-  if (!LoadProtocol(sdp, level, errorHolder)) {
+  if (!LoadProtocol(sdp, level, results)) {
     return false;
   }
 
-  if (!LoadFormats(sdp, level, errorHolder)) {
+  if (!LoadFormats(sdp, level, results)) {
     return false;
   }
 
-  if (!mAttributeList.Load(sdp, level, errorHolder)) {
+  if (!mAttributeList.Load(sdp, level, results)) {
     return false;
   }
 
-  if (!ValidateSimulcast(sdp, level, errorHolder)) {
+  if (!ValidateSimulcast(sdp, level, results)) {
     return false;
   }
 
-  if (!mBandwidths.Load(sdp, level, errorHolder)) {
+  if (!mBandwidths.Load(sdp, level, results)) {
     return false;
   }
 
-  return LoadConnection(sdp, level, errorHolder);
+  return LoadConnection(sdp, level, results);
 }
 
 bool SipccSdpMediaSection::LoadProtocol(sdp_t* sdp, uint16_t level,
-                                        SdpErrorHolder& errorHolder) {
+                                        InternalResults& results) {
   switch (sdp_get_media_transport(sdp, level)) {
     case SDP_TRANSPORT_RTPAVP:
       mProtocol = kRtpAvp;
@@ -152,7 +152,7 @@ bool SipccSdpMediaSection::LoadProtocol(sdp_t* sdp, uint16_t level,
       break;
 
     default:
-      errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+      results.AddParseError(sdp_get_media_line_number(sdp, level),
                             "Unsupported media transport type");
       return false;
   }
@@ -160,7 +160,7 @@ bool SipccSdpMediaSection::LoadProtocol(sdp_t* sdp, uint16_t level,
 }
 
 bool SipccSdpMediaSection::LoadFormats(sdp_t* sdp, uint16_t level,
-                                       SdpErrorHolder& errorHolder) {
+                                       InternalResults& results) {
   sdp_media_e mtype = sdp_get_media_type(sdp, level);
 
   if (mtype == SDP_MEDIA_APPLICATION) {
@@ -185,7 +185,7 @@ bool SipccSdpMediaSection::LoadFormats(sdp_t* sdp, uint16_t level,
           sdp_get_media_payload_type(sdp, level, i + 1, &indicator);
 
       if (GET_DYN_PAYLOAD_TYPE_VALUE(ptype) > UINT8_MAX) {
-        errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+        results.AddParseError(sdp_get_media_line_number(sdp, level),
                               "Format is too large");
         return false;
       }
@@ -204,19 +204,19 @@ bool SipccSdpMediaSection::LoadFormats(sdp_t* sdp, uint16_t level,
   return true;
 }
 
-bool SipccSdpMediaSection::ValidateSimulcast(
+bool SipccSdpMediaSection::ValidateSimulcast(sdp_t* sdp, uint16_t level,
-    sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder) const {
+                                             InternalResults& results) const {
   if (!GetAttributeList().HasAttribute(SdpAttribute::kSimulcastAttribute)) {
     return true;
   }
 
   const SdpSimulcastAttribute& simulcast(GetAttributeList().GetSimulcast());
   if (!ValidateSimulcastVersions(sdp, level, simulcast.sendVersions, sdp::kSend,
-                                 errorHolder)) {
+                                 results)) {
     return false;
   }
   if (!ValidateSimulcastVersions(sdp, level, simulcast.recvVersions, sdp::kRecv,
-                                 errorHolder)) {
+                                 results)) {
     return false;
   }
   return true;
@@ -224,9 +224,9 @@ bool SipccSdpMediaSection::ValidateSimulcast(
 
 bool SipccSdpMediaSection::ValidateSimulcastVersions(
     sdp_t* sdp, uint16_t level, const SdpSimulcastAttribute::Versions& versions,
-    sdp::Direction direction, SdpErrorHolder& errorHolder) const {
+    sdp::Direction direction, InternalResults& results) const {
   if (versions.IsSet() && !(direction & GetDirectionAttribute().mValue)) {
-    errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+    results.AddParseError(sdp_get_media_line_number(sdp, level),
                           "simulcast attribute has a direction that is "
                           "inconsistent with the direction of this media "
                           "section.");
@@ -239,8 +239,8 @@ bool SipccSdpMediaSection::ValidateSimulcastVersions(
       if (!ridAttr || (ridAttr->direction != direction)) {
         std::ostringstream os;
         os << "No rid attribute for \'" << encoding.rid << "\'";
-        errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+        results.AddParseError(sdp_get_media_line_number(sdp, level), os.str());
-                                  os.str());
+        results.AddParseError(sdp_get_media_line_number(sdp, level), os.str());
         return false;
       }
     }
@@ -249,11 +249,11 @@ bool SipccSdpMediaSection::ValidateSimulcastVersions(
 }
 
 bool SipccSdpMediaSection::LoadConnection(sdp_t* sdp, uint16_t level,
-                                          SdpErrorHolder& errorHolder) {
+                                          InternalResults& results) {
   if (!sdp_connection_valid(sdp, level)) {
     level = SDP_SESSION_LEVEL;
     if (!sdp_connection_valid(sdp, level)) {
-      errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+      results.AddParseError(sdp_get_media_line_number(sdp, level),
                             "Missing c= line");
       return false;
     }
@@ -261,7 +261,7 @@ bool SipccSdpMediaSection::LoadConnection(sdp_t* sdp, uint16_t level,
 
   sdp_nettype_e type = sdp_get_conn_nettype(sdp, level);
   if (type != SDP_NT_INTERNET) {
-    errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+    results.AddParseError(sdp_get_media_line_number(sdp, level),
                           "Unsupported network type");
     return false;
   }
@@ -275,7 +275,7 @@ bool SipccSdpMediaSection::LoadConnection(sdp_t* sdp, uint16_t level,
       addrType = sdp::kIPv6;
       break;
     default:
-      errorHolder.AddParseError(sdp_get_media_line_number(sdp, level),
+      results.AddParseError(sdp_get_media_line_number(sdp, level),
                             "Unsupported address type");
       return false;
   }

media/webrtc/signaling/src/sdp/SipccSdpMediaSection.h

@@ -21,11 +21,13 @@ extern "C" {
 namespace mozilla {
 
 class SipccSdp;
-class SdpErrorHolder;
+class SdpParser;
+
+using InternalResults = SdpParser::InternalResults;
 
 class SipccSdpBandwidths final : public std::map<std::string, uint32_t> {
  public:
-  bool Load(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool Load(sdp_t* sdp, uint16_t level, InternalResults& results);
   void Serialize(std::ostream& os) const;
 };
 
@@ -68,16 +70,16 @@ class SipccSdpMediaSection final : public SdpMediaSection {
         mProtocol(static_cast<Protocol>(0)),
         mAttributeList(sessionLevel) {}
 
-  bool Load(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool Load(sdp_t* sdp, uint16_t level, InternalResults& results);
-  bool LoadConnection(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool LoadConnection(sdp_t* sdp, uint16_t level, InternalResults& results);
-  bool LoadProtocol(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool LoadProtocol(sdp_t* sdp, uint16_t level, InternalResults& results);
-  bool LoadFormats(sdp_t* sdp, uint16_t level, SdpErrorHolder& errorHolder);
+  bool LoadFormats(sdp_t* sdp, uint16_t level, InternalResults& results);
   bool ValidateSimulcast(sdp_t* sdp, uint16_t level,
-                         SdpErrorHolder& errorHolder) const;
+                         InternalResults& results) const;
   bool ValidateSimulcastVersions(
       sdp_t* sdp, uint16_t level,
       const SdpSimulcastAttribute::Versions& versions, sdp::Direction direction,
-      SdpErrorHolder& errorHolder) const;
+      InternalResults& results) const;
 
   // the following values are cached on first get
   MediaType mMediaType;

media/webrtc/signaling/src/sdp/SipccSdpParser.cpp

@@ -14,23 +14,26 @@ extern "C" {
 
 namespace mozilla {
 
+static const std::string SIPCC_NAME = "SIPCC";
+
 extern "C" {
 
-void sipcc_sdp_parser_error_handler(void* context, uint32_t line,
+void sipcc_sdp_parser_results_handler(void* context, uint32_t line,
                                       const char* message) {
-  SdpErrorHolder* errorHolder = static_cast<SdpErrorHolder*>(context);
+  auto* results = static_cast<UniquePtr<InternalResults>*>(context);
   std::string err(message);
-  errorHolder->AddParseError(line, err);
+  (*results)->AddParseError(line, err);
 }
 
 }  // extern "C"
 
-UniquePtr<Sdp> SipccSdpParser::Parse(const std::string& sdpText) {
+const std::string& SipccSdpParser::Name() const { return SIPCC_NAME; }
-  ClearParseErrors();
 
+UniquePtr<SdpParser::Results> SipccSdpParser::Parse(const std::string& aText) {
+  UniquePtr<InternalResults> results(new InternalResults(Name()));
   sdp_conf_options_t* sipcc_config = sdp_init_config();
   if (!sipcc_config) {
-    return UniquePtr<Sdp>();
+    return UniquePtr<SdpParser::Results>();
   }
 
   sdp_nettype_supported(sipcc_config, SDP_NT_INTERNET, true);
@@ -49,32 +52,36 @@ UniquePtr<Sdp> SipccSdpParser::Parse(const std::string& sdpText) {
   sdp_transport_supported(sipcc_config, SDP_TRANSPORT_TCPDTLSSCTP, true);
   sdp_require_session_name(sipcc_config, false);
 
-  sdp_config_set_error_handler(sipcc_config, &sipcc_sdp_parser_error_handler,
+  sdp_config_set_error_handler(sipcc_config, &sipcc_sdp_parser_results_handler,
-                               this);
+                               &results);
 
   // Takes ownership of |sipcc_config| iff it succeeds
   sdp_t* sdp = sdp_init_description(sipcc_config);
   if (!sdp) {
     sdp_free_config(sipcc_config);
-    return UniquePtr<Sdp>();
+    return results;
   }
 
-  const char* rawString = sdpText.c_str();
+  const char* rawString = aText.c_str();
-  sdp_result_e sdpres = sdp_parse(sdp, rawString, sdpText.length());
+  sdp_result_e sdpres = sdp_parse(sdp, rawString, aText.length());
   if (sdpres != SDP_SUCCESS) {
     sdp_free_description(sdp);
-    return UniquePtr<Sdp>();
+    return results;
   }
 
   UniquePtr<SipccSdp> sipccSdp(new SipccSdp);
 
-  bool success = sipccSdp->Load(sdp, *this);
+  bool success = sipccSdp->Load(sdp, *results);
   sdp_free_description(sdp);
-  if (!success) {
+  if (success) {
-    return UniquePtr<Sdp>();
+    results->SetSdp(UniquePtr<mozilla::Sdp>(std::move(sipccSdp)));
   }
 
-  return UniquePtr<Sdp>(std::move(sipccSdp));
+  return results;
+}
+
+bool SipccSdpParser::IsNamed(const std::string& aName) {
+  return aName == SIPCC_NAME;
 }
 
 }  // namespace mozilla

media/webrtc/signaling/src/sdp/SipccSdpParser.h

@@ -12,20 +12,20 @@
 #include "mozilla/UniquePtr.h"
 
 #include "signaling/src/sdp/Sdp.h"
-#include "signaling/src/sdp/SdpErrorHolder.h"
+#include "signaling/src/sdp/SdpParser.h"
 
 namespace mozilla {
 
-class SipccSdpParser final : public SdpErrorHolder {
+class SipccSdpParser final : public SdpParser {
  public:
-  SipccSdpParser() {}
+  SipccSdpParser() = default;
-  virtual ~SipccSdpParser() {}
+  virtual ~SipccSdpParser() = default;
 
-  /**
+  const std::string& Name() const override;
-   * This parses the provided text into an SDP object.
+
-   * This returns a nullptr-valued pointer if things go poorly.
+  UniquePtr<SdpParser::Results> Parse(const std::string& aText) override;
-   */
+
-  UniquePtr<Sdp> Parse(const std::string& sdpText);
+  static bool IsNamed(const std::string& aName);
 };
 
 }  // namespace mozilla