Bug 1507922 - make ownership of MetadataTags more clear in the ogg code; r=gerald,jya

Use UniquePtr for return types, so it's obvious who has ownership.

dom/media/MediaDecoder.cpp

@@ -595,7 +595,7 @@ void MediaDecoder::OnMetadataUpdate(TimedMetadata&& aMetadata) {
   AbstractThread::AutoEnter context(AbstractMainThread());
   GetOwner()->RemoveMediaTracks();
   MetadataLoaded(MakeUnique<MediaInfo>(*aMetadata.mInfo),
-                 UniquePtr<MetadataTags>(aMetadata.mTags.forget()),
+                 UniquePtr<MetadataTags>(std::move(aMetadata.mTags)),
                  MediaDecoderEventVisibility::Observable);
   FirstFrameLoaded(std::move(aMetadata.mInfo),
                    MediaDecoderEventVisibility::Observable);

dom/media/MediaMetadataManager.h

@@ -26,10 +26,11 @@ typedef MediaEventSourceExc<TimedMetadata> TimedMetadataEventSource;
 class TimedMetadata : public LinkedListElement<TimedMetadata> {
  public:
   TimedMetadata(const media::TimeUnit& aPublishTime,
-                nsAutoPtr<MetadataTags>&& aTags, nsAutoPtr<MediaInfo>&& aInfo)
-      : mPublishTime(aPublishTime),
-        mTags(std::move(aTags)),
-        mInfo(std::move(aInfo)) {}
+                UniquePtr<MetadataTags>&& aTags,
+                nsAutoPtr<MediaInfo>&& aInfo)
+    : mPublishTime(aPublishTime)
+    , mTags(std::move(aTags))
+    , mInfo(std::move(aInfo)) {}
 
   // Define our move constructor because we don't want to move the members of
   // LinkedListElement to change the list.
@@ -42,7 +43,7 @@ class TimedMetadata : public LinkedListElement<TimedMetadata> {
   media::TimeUnit mPublishTime;
   // The metadata. The ownership is transfered to the element when dispatching
   // to the main threads.
-  nsAutoPtr<MetadataTags> mTags;
+  UniquePtr<MetadataTags> mTags;
   // The media info, including the info of audio tracks and video tracks.
   // The ownership is transfered to MediaDecoder when dispatching to the
   // main thread.

dom/media/flac/FlacDemuxer.cpp

@@ -427,7 +427,7 @@ class FrameParser {
   AudioInfo Info() const { return mParser.mInfo; }
 
   // Return a hash table with tag metadata.
-  MetadataTags* GetTags() const { return mParser.GetTags(); }
+  UniquePtr<MetadataTags> GetTags() const { return mParser.GetTags(); }
 
  private:
   bool GetNextFrame(MediaResourceIndex& aResource) {
@@ -644,7 +644,7 @@ UniquePtr<TrackInfo> FlacTrackDemuxer::GetInfo() const {
   if (mParser->Info().IsValid()) {
     // We have a proper metadata header.
     UniquePtr<TrackInfo> info = mParser->Info().Clone();
-    nsAutoPtr<MetadataTags> tags(mParser->GetTags());
+    UniquePtr<MetadataTags> tags(mParser->GetTags());
     if (tags) {
       for (auto iter = tags->Iter(); !iter.Done(); iter.Next()) {
         info->mTags.AppendElement(MetadataTag(iter.Key(), iter.Data()));

dom/media/flac/FlacFrameParser.cpp

@@ -223,14 +223,12 @@ Result<bool, nsresult> FlacFrameParser::IsHeaderBlock(const uint8_t* aPacket,
   return type >= 1 && type <= 6;
 }
 
-MetadataTags* FlacFrameParser::GetTags() const {
+UniquePtr<MetadataTags> FlacFrameParser::GetTags() const {
   if (!mParser) {
     return nullptr;
   }
 
-  MetadataTags* tags;
-
-  tags = new MetadataTags;
+  auto tags = MakeUnique<MetadataTags>();
   for (uint32_t i = 0; i < mParser->mTags.Length(); i++) {
     OggCodecState::AddVorbisComment(tags, mParser->mTags[i].Data(),
                                     mParser->mTags[i].Length());

dom/media/flac/FlacFrameParser.h

@@ -49,7 +49,7 @@ class FlacFrameParser {
   int64_t BlockDuration(const uint8_t* aPacket, size_t aLength) const;
 
   // Return a hash table with tag metadata.
-  MetadataTags* GetTags() const;
+  UniquePtr<MetadataTags> GetTags() const;
 
   AudioInfo mInfo;
 

dom/media/ogg/OggCodecState.cpp

@@ -95,7 +95,8 @@ bool OggCodecState::IsValidVorbisTagName(nsCString& aName) {
   return true;
 }
 
-bool OggCodecState::AddVorbisComment(MetadataTags* aTags, const char* aComment,
+bool OggCodecState::AddVorbisComment(UniquePtr<MetadataTags>& aTags,
+                                     const char* aComment,
                                      uint32_t aLength) {
   const char* div = (const char*)memchr(aComment, '=', aLength);
   if (!div) {
@@ -712,11 +713,10 @@ bool VorbisState::IsHeader(ogg_packet* aPacket) {
   return aPacket->bytes > 0 ? (aPacket->packet[0] & 0x1) : false;
 }
 
-MetadataTags* VorbisState::GetTags() {
-  MetadataTags* tags;
+UniquePtr<MetadataTags> VorbisState::GetTags() {
   NS_ASSERTION(mComment.user_comments, "no vorbis comment strings!");
   NS_ASSERTION(mComment.comment_lengths, "no vorbis comment lengths!");
-  tags = new MetadataTags;
+  auto tags = MakeUnique<MetadataTags>();
   for (int i = 0; i < mComment.comments; i++) {
     AddVorbisComment(tags, mComment.user_comments[i],
                      mComment.comment_lengths[i]);
@@ -967,10 +967,8 @@ bool OpusState::DecodeHeader(OggPacketPtr aPacket) {
 }
 
 /* Construct and return a tags hashmap from our internal array */
-MetadataTags* OpusState::GetTags() {
-  MetadataTags* tags;
-
-  tags = new MetadataTags;
+UniquePtr<MetadataTags> OpusState::GetTags() {
+  auto tags = MakeUnique<MetadataTags>();
   for (uint32_t i = 0; i < mParser->mTags.Length(); i++) {
     AddVorbisComment(tags, mParser->mTags[i].Data(),
                      mParser->mTags[i].Length());
@@ -1228,7 +1226,9 @@ nsresult FlacState::PageIn(ogg_page* aPage) {
 }
 
 // Return a hash table with tag metadata.
-MetadataTags* FlacState::GetTags() { return mParser.GetTags(); }
+UniquePtr<MetadataTags> FlacState::GetTags() {
+  return mParser.GetTags();
+}
 
 const TrackInfo* FlacState::GetInfo() const { return &mParser.mInfo; }
 

dom/media/ogg/OggCodecState.h

@@ -118,7 +118,7 @@ class OggCodecState {
   }
 
   // Build a hash table with tag metadata parsed from the stream.
-  virtual MetadataTags* GetTags() { return nullptr; }
+  virtual UniquePtr<MetadataTags> GetTags() { return nullptr; }
 
   // Returns the end time that a granulepos represents.
   virtual int64_t Time(int64_t granulepos) { return -1; }
@@ -241,10 +241,11 @@ class OggCodecState {
   // Utility method to parse and add a vorbis-style comment
   // to a metadata hash table. Most Ogg-encapsulated codecs
   // use the vorbis comment format for metadata.
-  static bool AddVorbisComment(MetadataTags* aTags, const char* aComment,
+  static bool AddVorbisComment(UniquePtr<MetadataTags>& aTags,
+                               const char* aComment,
                                uint32_t aLength);
 
- protected:
+protected:
   // Constructs a new OggCodecState. aActive denotes whether the stream is
   // active. For streams of unsupported or unknown types, aActive should be
   // false.
@@ -289,7 +290,7 @@ class VorbisState : public OggCodecState {
   const TrackInfo* GetInfo() const override { return &mInfo; }
 
   // Return a hash table with tag metadata.
-  MetadataTags* GetTags() override;
+  UniquePtr<MetadataTags> GetTags() override;
 
  private:
   AudioInfo mInfo;
@@ -408,7 +409,7 @@ class OpusState : public OggCodecState {
   static int64_t Time(int aPreSkip, int64_t aGranulepos);
 
   // Construct and return a table of tags from the metadata header.
-  MetadataTags* GetTags() override;
+  UniquePtr<MetadataTags> GetTags() override;
 
  private:
   nsAutoPtr<OpusParser> mParser;
@@ -582,7 +583,7 @@ class FlacState : public OggCodecState {
   nsresult PageIn(ogg_page* aPage) override;
 
   // Return a hash table with tag metadata.
-  MetadataTags* GetTags() override;
+  UniquePtr<MetadataTags> GetTags() override;
 
   const TrackInfo* GetInfo() const override;
 

dom/media/ogg/OggDemuxer.cpp

@@ -392,12 +392,12 @@ void OggDemuxer::SetupMediaTracksInfo(const nsTArray<uint32_t>& aSerials) {
   }
 }
 
-void OggDemuxer::FillTags(TrackInfo* aInfo, MetadataTags* aTags) {
+void OggDemuxer::FillTags(TrackInfo* aInfo, UniquePtr<MetadataTags>&& aTags) {
   if (!aTags) {
     return;
   }
-  nsAutoPtr<MetadataTags> tags(aTags);
-  for (auto iter = aTags->Iter(); !iter.Done(); iter.Next()) {
+  UniquePtr<MetadataTags> tags(std::move(aTags));
+  for (auto iter = tags->Iter(); !iter.Done(); iter.Next()) {
     aInfo->mTags.AppendElement(MetadataTag(iter.Key(), iter.Data()));
   }
 }
@@ -567,7 +567,7 @@ bool OggDemuxer::ReadOggChain(const media::TimeUnit& aLastEndTime) {
   OpusState* newOpusState = nullptr;
   VorbisState* newVorbisState = nullptr;
   FlacState* newFlacState = nullptr;
-  nsAutoPtr<MetadataTags> tags;
+  UniquePtr<MetadataTags> tags;
 
   if (HasVideo() || HasSkeleton() || !HasAudio()) {
     return false;

dom/media/ogg/OggDemuxer.h

@@ -194,7 +194,7 @@ class OggDemuxer : public MediaDataDemuxer,
   void SetupTarget(OggCodecState** aSavedState, OggCodecState* aNewState);
   void SetupTargetSkeleton();
   void SetupMediaTracksInfo(const nsTArray<uint32_t>& aSerials);
-  void FillTags(TrackInfo* aInfo, MetadataTags* aTags);
+  void FillTags(TrackInfo* aInfo, UniquePtr<MetadataTags>&& aTags);
 
   // Compute an ogg page's checksum
   ogg_uint32_t GetPageChecksum(ogg_page* aPage);