Bug 1146529 - Part 2: Rework part 1 to not rely on std::vector<UniquePtr>, since that is not supported by our buildconfig yet. r=jesup

media/webrtc/signaling/signaling.gyp

@@ -83,6 +83,7 @@
         './src/common/CommonTypes.h',
         './src/common/csf_common.h',
         './src/common/NullDeleter.h',
+        './src/common/PtrVector.h',
         './src/common/Wrapper.h',
         './src/common/NullTransport.h',
         './src/common/YuvStamper.cpp',

media/webrtc/signaling/src/common/PtrVector.h

@@ -0,0 +1,30 @@
+/* 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 PtrVector_h
+#define PtrVector_h
+
+#include <vector>
+
+namespace mozilla
+{
+
+// Trivial wrapper class around a vector of ptrs.
+// TODO: Remove this once our buildconfig allows us to put unique_ptr in stl
+// containers, and just use std::vector<unique_ptr<T>> instead.
+template <class T> class PtrVector
+{
+public:
+  ~PtrVector()
+  {
+    for (T* value : values) { delete value; }
+  }
+
+  std::vector<T*> values;
+};
+
+} // namespace mozilla
+
+#endif // PtrVector_h
+

media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp

@@ -37,13 +37,6 @@ MOZ_MTLOG_MODULE("jsep")
     MOZ_MTLOG(ML_ERROR, mLastError);                                           \
   } while (0);
 
-JsepSessionImpl::~JsepSessionImpl()
-{
-  for (auto i = mCodecs.begin(); i != mCodecs.end(); ++i) {
-    delete *i;
-  }
-}
-
 nsresult
 JsepSessionImpl::Init()
 {
@@ -816,8 +809,8 @@ void
 JsepSessionImpl::AddCodecs(SdpMediaSection* msection) const
 {
   msection->ClearCodecs();
-  for (auto c = mCodecs.begin(); c != mCodecs.end(); ++c) {
-    (*c)->AddToMediaSection(*msection);
+  for (const JsepCodecDescription* codec : mCodecs.values) {
+    codec->AddToMediaSection(*msection);
   }
 }
 
@@ -881,8 +874,7 @@ JsepCodecDescription*
 JsepSessionImpl::FindMatchingCodec(const std::string& fmt,
                                    const SdpMediaSection& msection) const
 {
-  for (auto c = mCodecs.begin(); c != mCodecs.end(); ++c) {
-    auto codec = *c;
+  for (JsepCodecDescription* codec : mCodecs.values) {
     if (codec->mEnabled && codec->Matches(fmt, msection)) {
       return codec;
     }
@@ -905,25 +897,26 @@ JsepSessionImpl::GetRtpExtensions(SdpMediaSection::MediaType type) const
 }
 
 static bool
-CompareCodec(const UniquePtr<JsepCodecDescription>& lhs,
-             const UniquePtr<JsepCodecDescription>& rhs)
+CompareCodec(const JsepCodecDescription* lhs, const JsepCodecDescription* rhs)
 {
   return lhs->mStronglyPreferred && !rhs->mStronglyPreferred;
 }
 
-std::vector<UniquePtr<JsepCodecDescription>>
+PtrVector<JsepCodecDescription>
 JsepSessionImpl::GetCommonCodecs(const SdpMediaSection& remoteMsection)
 {
-  std::vector<UniquePtr<JsepCodecDescription>> matchingCodecs;
+  PtrVector<JsepCodecDescription> matchingCodecs;
   for (const std::string& fmt : remoteMsection.GetFormats()) {
     JsepCodecDescription* codec = FindMatchingCodec(fmt, remoteMsection);
     if (codec) {
       codec->mDefaultPt = fmt; // Remember the other side's PT
-      matchingCodecs.push_back(UniquePtr<JsepCodecDescription>(codec->Clone()));
+      matchingCodecs.values.push_back(codec->Clone());
     }
   }
 
-  std::stable_sort(matchingCodecs.begin(), matchingCodecs.end(), CompareCodec);
+  std::stable_sort(matchingCodecs.values.begin(),
+                   matchingCodecs.values.end(),
+                   CompareCodec);
 
   return matchingCodecs;
 }
@@ -1179,10 +1172,10 @@ JsepSessionImpl::CreateAnswerMSection(const JsepAnswerOptions& options,
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Now add the codecs.
-  std::vector<UniquePtr<JsepCodecDescription>> matchingCodecs(
+  PtrVector<JsepCodecDescription> matchingCodecs(
       GetCommonCodecs(remoteMsection));
 
-  for (const UniquePtr<JsepCodecDescription>& codec : matchingCodecs) {
+  for (const JsepCodecDescription* codec : matchingCodecs.values) {
     UniquePtr<JsepCodecDescription> negotiated(
         codec->MakeNegotiatedCodec(remoteMsection));
     if (negotiated) {
@@ -1682,10 +1675,10 @@ JsepSessionImpl::NegotiateTrack(const SdpMediaSection& remoteMsection,
   negotiatedDetails->mProtocol = remoteMsection.GetProtocol();
 
   // Insert all the codecs we jointly support.
-  std::vector<UniquePtr<JsepCodecDescription>> commonCodecs(
+  PtrVector<JsepCodecDescription> commonCodecs(
       GetCommonCodecs(remoteMsection));
 
-  for (const UniquePtr<JsepCodecDescription>& codec : commonCodecs) {
+  for (const JsepCodecDescription* codec : commonCodecs.values) {
     bool sending = (direction == JsepTrack::kJsepTrackSending);
 
     // Everywhere else in JsepSessionImpl, a JsepCodecDescription describes
@@ -1720,11 +1713,11 @@ JsepSessionImpl::NegotiateTrack(const SdpMediaSection& remoteMsection,
       }
     }
 
-    negotiatedDetails->mCodecs.push_back(sendOrReceiveCodec.release());
+    negotiatedDetails->mCodecs.values.push_back(sendOrReceiveCodec.release());
     break;
   }
 
-  if (negotiatedDetails->mCodecs.empty()) {
+  if (negotiatedDetails->mCodecs.values.empty()) {
     JSEP_SET_ERROR("Failed to negotiate codec details for all codecs");
     return NS_ERROR_INVALID_ARG;
   }
@@ -2359,7 +2352,7 @@ void
 JsepSessionImpl::SetupDefaultCodecs()
 {
   // Supported audio codecs.
-  mCodecs.push_back(new JsepAudioCodecDescription(
+  mCodecs.values.push_back(new JsepAudioCodecDescription(
       "109",
       "opus",
       48000,
@@ -2367,7 +2360,7 @@ JsepSessionImpl::SetupDefaultCodecs()
       960,
       16000));
 
-  mCodecs.push_back(new JsepAudioCodecDescription(
+  mCodecs.values.push_back(new JsepAudioCodecDescription(
       "9",
       "G722",
       8000,
@@ -2377,7 +2370,7 @@ JsepSessionImpl::SetupDefaultCodecs()
 
   // packet size and bitrate values below copied from sipcc.
   // May need reevaluation from a media expert.
-  mCodecs.push_back(
+  mCodecs.values.push_back(
       new JsepAudioCodecDescription("0",
                                     "PCMU",
                                     8000,
@@ -2386,7 +2379,7 @@ JsepSessionImpl::SetupDefaultCodecs()
                                     8 * 8000 * 1 // 8 * frequency * channels
                                     ));
 
-  mCodecs.push_back(
+  mCodecs.values.push_back(
       new JsepAudioCodecDescription("8",
                                     "PCMA",
                                     8000,
@@ -2404,7 +2397,7 @@ JsepSessionImpl::SetupDefaultCodecs()
   // Defaults for mandatory params
   vp8->mMaxFs = 12288;
   vp8->mMaxFr = 60;
-  mCodecs.push_back(vp8);
+  mCodecs.values.push_back(vp8);
 
   JsepVideoCodecDescription* vp9 = new JsepVideoCodecDescription(
       "121",
@@ -2414,7 +2407,7 @@ JsepSessionImpl::SetupDefaultCodecs()
   // Defaults for mandatory params
   vp9->mMaxFs = 12288;
   vp9->mMaxFr = 60;
-  mCodecs.push_back(vp9);
+  mCodecs.values.push_back(vp9);
 
   JsepVideoCodecDescription* h264_1 = new JsepVideoCodecDescription(
       "126",
@@ -2424,7 +2417,7 @@ JsepSessionImpl::SetupDefaultCodecs()
   h264_1->mPacketizationMode = 1;
   // Defaults for mandatory params
   h264_1->mProfileLevelId = 0x42E00D;
-  mCodecs.push_back(h264_1);
+  mCodecs.values.push_back(h264_1);
 
   JsepVideoCodecDescription* h264_0 = new JsepVideoCodecDescription(
       "97",
@@ -2434,9 +2427,9 @@ JsepSessionImpl::SetupDefaultCodecs()
   h264_0->mPacketizationMode = 0;
   // Defaults for mandatory params
   h264_0->mProfileLevelId = 0x42E00D;
-  mCodecs.push_back(h264_0);
+  mCodecs.values.push_back(h264_0);
 
-  mCodecs.push_back(new JsepApplicationCodecDescription(
+  mCodecs.values.push_back(new JsepApplicationCodecDescription(
       "5000",
       "webrtc-datachannel",
       16

media/webrtc/signaling/src/jsep/JsepSessionImpl.h

@@ -15,6 +15,7 @@
 #include "signaling/src/jsep/JsepTrack.h"
 #include "signaling/src/jsep/JsepTrackImpl.h"
 #include "signaling/src/sdp/SipccSdpParser.h"
+#include "signaling/src/common/PtrVector.h"
 
 namespace mozilla {
 
@@ -39,8 +40,6 @@ public:
   {
   }
 
-  virtual ~JsepSessionImpl();
-
   // Implement JsepSession methods.
   virtual nsresult Init() override;
 
@@ -76,7 +75,7 @@ public:
   virtual std::vector<JsepCodecDescription*>&
   Codecs() override
   {
-    return mCodecs;
+    return mCodecs.values;
   }
 
   virtual nsresult ReplaceTrack(const std::string& oldStreamId,
@@ -183,7 +182,8 @@ private:
       const SdpMediaSection& msection) const;
   const std::vector<SdpExtmapAttributeList::Extmap>* GetRtpExtensions(
       SdpMediaSection::MediaType type) const;
-  std::vector<UniquePtr<JsepCodecDescription>> GetCommonCodecs(
+
+  PtrVector<JsepCodecDescription> GetCommonCodecs(
       const SdpMediaSection& remoteMsection);
   void AddCommonExtmaps(const SdpMediaSection& remoteMsection,
                         SdpMediaSection* msection);
@@ -346,7 +346,7 @@ private:
   UniquePtr<Sdp> mCurrentRemoteDescription;
   UniquePtr<Sdp> mPendingLocalDescription;
   UniquePtr<Sdp> mPendingRemoteDescription;
-  std::vector<JsepCodecDescription*> mCodecs;
+  PtrVector<JsepCodecDescription> mCodecs;
   std::string mLastError;
   SipccSdpParser mParser;
 };

media/webrtc/signaling/src/jsep/JsepTrackImpl.h

@@ -14,6 +14,7 @@
 #include "signaling/src/jsep/JsepTrack.h"
 #include "signaling/src/sdp/Sdp.h"
 #include "signaling/src/sdp/SdpMediaSection.h"
+#include "signaling/src/common/PtrVector.h"
 
 namespace mozilla {
 
@@ -21,11 +22,7 @@ class JsepTrackNegotiatedDetailsImpl : public JsepTrackNegotiatedDetails
 {
 public:
   virtual ~JsepTrackNegotiatedDetailsImpl()
-  {
-    for (auto c = mCodecs.begin(); c != mCodecs.end(); ++c) {
-      delete *c;
-    }
-  }
+  {}
 
   // Implement JsepTrackNegotiatedDetails.
   virtual mozilla::SdpMediaSection::Protocol
@@ -41,15 +38,15 @@ public:
   virtual size_t
   GetCodecCount() const override
   {
-    return mCodecs.size();
+    return mCodecs.values.size();
   }
   virtual nsresult
   GetCodec(size_t index, const JsepCodecDescription** config) const override
   {
-    if (index >= mCodecs.size()) {
+    if (index >= mCodecs.values.size()) {
       return NS_ERROR_INVALID_ARG;
     }
-    *config = mCodecs[index];
+    *config = mCodecs.values[index];
     return NS_OK;
   }
 
@@ -85,7 +82,7 @@ private:
 
   mozilla::SdpMediaSection::Protocol mProtocol;
   Maybe<std::string> mBandwidth;
-  std::vector<JsepCodecDescription*> mCodecs;
+  PtrVector<JsepCodecDescription> mCodecs;
   std::map<std::string, SdpExtmapAttributeList::Extmap> mExtmap;
   std::vector<uint8_t> mUniquePayloadTypes;
 };

media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp

@@ -14,6 +14,7 @@
 
 #include "signaling/src/jsep/JsepTrack.h"
 #include "signaling/src/jsep/JsepTransport.h"
+#include "signaling/src/common/PtrVector.h"
 
 #ifdef MOZILLA_INTERNAL_API
 #include "MediaStreamTrack.h"
@@ -35,20 +36,6 @@ namespace mozilla {
 
 MOZ_MTLOG_MODULE("MediaPipelineFactory")
 
-// Trivial wrapper class around a vector of ptrs.
-template <class T> class PtrVector
-{
-public:
-  ~PtrVector()
-  {
-    for (auto it = values.begin(); it != values.end(); ++it) {
-      delete *it;
-    }
-  }
-
-  std::vector<T*> values;
-};
-
 static nsresult
 JsepCodecDescToCodecConfig(const JsepCodecDescription& aCodec,
                            AudioCodecConfig** aConfig)

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

@@ -18,6 +18,7 @@ namespace mozilla
 class SdpAttributeList
 {
 public:
+  virtual ~SdpAttributeList() {}
   typedef SdpAttribute::AttributeType AttributeType;
 
   // Avoid default params on virtual functions

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

@@ -5,6 +5,7 @@
 #include "signaling/src/sdp/SipccSdp.h"
 
 #include <cstdlib>
+#include "mozilla/UniquePtr.h"
 #include "mozilla/Assertions.h"
 #include "signaling/src/sdp/SdpErrorHolder.h"
 
@@ -16,13 +17,6 @@
 namespace mozilla
 {
 
-SipccSdp::~SipccSdp()
-{
-  for (auto i = mMediaSections.begin(); i != mMediaSections.end(); ++i) {
-    delete *i;
-  }
-}
-
 const SdpOrigin&
 SipccSdp::GetOrigin() const
 {
@@ -42,19 +36,19 @@ SipccSdp::GetBandwidth(const std::string& type) const
 const SdpMediaSection&
 SipccSdp::GetMediaSection(size_t level) const
 {
-  if (level > mMediaSections.size()) {
+  if (level > mMediaSections.values.size()) {
     MOZ_CRASH();
   }
-  return *mMediaSections[level];
+  return *mMediaSections.values[level];
 }
 
 SdpMediaSection&
 SipccSdp::GetMediaSection(size_t level)
 {
-  if (level > mMediaSections.size()) {
+  if (level > mMediaSections.values.size()) {
     MOZ_CRASH();
   }
-  return *mMediaSections[level];
+  return *mMediaSections.values[level];
 }
 
 SdpMediaSection&
@@ -63,7 +57,7 @@ SipccSdp::AddMediaSection(SdpMediaSection::MediaType mediaType,
                           SdpMediaSection::Protocol protocol,
                           sdp::AddrType addrType, const std::string& addr)
 {
-  size_t level = mMediaSections.size();
+  size_t level = mMediaSections.values.size();
   SipccSdpMediaSection* media =
       new SipccSdpMediaSection(level, &mAttributeList);
   media->mMediaType = mediaType;
@@ -72,7 +66,7 @@ SipccSdp::AddMediaSection(SdpMediaSection::MediaType mediaType,
   media->mProtocol = protocol;
   media->mConnection = MakeUnique<SdpConnection>(addrType, addr);
   media->GetAttributeList().SetAttribute(new SdpDirectionAttribute(dir));
-  mMediaSections.push_back(media);
+  mMediaSections.values.push_back(media);
   return *media;
 }
 
@@ -131,7 +125,7 @@ SipccSdp::Load(sdp_t* sdp, SdpErrorHolder& errorHolder)
     if (!section->Load(sdp, i + 1, errorHolder)) {
       return false;
     }
-    mMediaSections.push_back(section.release());
+    mMediaSections.values.push_back(section.release());
   }
   return true;
 }
@@ -153,8 +147,8 @@ SipccSdp::Serialize(std::ostream& os) const
   os << mAttributeList;
 
   // media sections
-  for (auto i = mMediaSections.begin(); i != mMediaSections.end(); ++i) {
-    os << (**i);
+  for (const SdpMediaSection* msection : mMediaSections.values) {
+    os << *msection;
   }
 }
 

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

@@ -10,7 +10,6 @@
 #include <map>
 #include <vector>
 #include "mozilla/Attributes.h"
-#include "mozilla/UniquePtr.h"
 
 #include "signaling/src/sdp/Sdp.h"
 #include "signaling/src/sdp/SipccSdpMediaSection.h"
@@ -19,6 +18,8 @@ extern "C" {
 #include "signaling/src/sdp/sipcc/sdp.h"
 }
 
+#include "signaling/src/common/PtrVector.h"
+
 namespace mozilla
 {
 
@@ -34,7 +35,6 @@ public:
       : mOrigin(origin), mAttributeList(nullptr)
   {
   }
-  ~SipccSdp();
 
   virtual const SdpOrigin& GetOrigin() const override;
 
@@ -44,7 +44,7 @@ public:
   virtual size_t
   GetMediaSectionCount() const override
   {
-    return mMediaSections.size();
+    return mMediaSections.values.size();
   }
 
   virtual const SdpAttributeList&
@@ -80,7 +80,7 @@ private:
   SdpOrigin mOrigin;
   SipccSdpBandwidths mBandwidths;
   SipccSdpAttributeList mAttributeList;
-  std::vector<SipccSdpMediaSection*> mMediaSections;
+  PtrVector<SipccSdpMediaSection> mMediaSections;
 };
 
 } // namespace mozilla