Bug 1208371 - Remove ref counting from DOMMediaStream::TrackListener. r?roc draft
authorAndreas Pehrson <pehrsons@gmail.com>
Tue, 05 Jan 2016 10:16:28 +0800
changeset 342136 0192824ef8e7eee8a84a96f7e774307bf81436ff
parent 342135 30970d7ea3df4930afaab72b2390293472c23727
child 342137 e34b4e58f56faffee71a006f547f1d5d7deb4c8b
push id13352
push userpehrsons@gmail.com
push dateFri, 18 Mar 2016 13:49:47 +0000
reviewersroc
bugs1208371
milestone47.0a1
Bug 1208371 - Remove ref counting from DOMMediaStream::TrackListener. r?roc This makes it consistent with PrincipalChangeObserver. MozReview-Commit-ID: 91PtqFZRcW6
dom/html/HTMLMediaElement.cpp
dom/html/HTMLMediaElement.h
dom/media/DOMMediaStream.cpp
dom/media/DOMMediaStream.h
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -2235,17 +2235,16 @@ HTMLMediaElement::HTMLMediaElement(alrea
     mMediaSecurityVerified(false),
     mCORSMode(CORS_NONE),
     mIsEncrypted(false),
     mDownloadSuspendedByCache(false, "HTMLMediaElement::mDownloadSuspendedByCache"),
     mAudioChannelVolume(1.0),
     mPlayingThroughTheAudioChannel(false),
     mDisableVideo(false),
     mPlayBlockedBecauseHidden(false),
-    mMediaStreamTrackListener(nullptr),
     mElementInTreeState(ELEMENT_NOT_INTREE),
     mHasUserInteraction(false),
     mFirstFrameLoaded(false),
     mDefaultPlaybackStartPosition(0.0),
     mIsAudioTrackAudible(false)
 {
   mAudioChannel = AudioChannelService::GetDefaultAudioChannel();
 
@@ -3266,18 +3265,16 @@ public:
   }
 
   void NotifyTrackRemoved(const RefPtr<MediaStreamTrack>& aTrack) override
   {
     mElement->NotifyMediaStreamTrackRemoved(aTrack);
   }
 
 protected:
-  ~MediaStreamTrackListener() {}
-
   HTMLMediaElement* const mElement;
 };
 
 void HTMLMediaElement::UpdateSrcMediaStreamPlaying(uint32_t aFlags)
 {
   if (!mSrcStream) {
     return;
   }
--- a/dom/html/HTMLMediaElement.h
+++ b/dom/html/HTMLMediaElement.h
@@ -1497,17 +1497,17 @@ protected:
   nsCOMPtr<nsIAudioChannelAgent> mAudioChannelAgent;
 
   RefPtr<TextTrackManager> mTextTrackManager;
 
   RefPtr<AudioTrackList> mAudioTrackList;
 
   RefPtr<VideoTrackList> mVideoTrackList;
 
-  RefPtr<MediaStreamTrackListener> mMediaStreamTrackListener;
+  nsAutoPtr<MediaStreamTrackListener> mMediaStreamTrackListener;
 
   // The principal guarding mVideoFrameContainer access when playing a
   // MediaStream.
   nsCOMPtr<nsIPrincipal> mSrcStreamVideoPrincipal;
 
   enum ElementInTreeState {
     // The MediaElement is not in the DOM tree now.
     ELEMENT_NOT_INTREE,
--- a/dom/media/DOMMediaStream.cpp
+++ b/dom/media/DOMMediaStream.cpp
@@ -1144,31 +1144,29 @@ DOMMediaStream::NotifyTrackAdded(const R
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   RecomputePrincipal();
 
   aTrack->AddPrincipalChangeObserver(this);
 
   for (int32_t i = mTrackListeners.Length() - 1; i >= 0; --i) {
-    const RefPtr<TrackListener>& listener = mTrackListeners[i];
-    listener->NotifyTrackAdded(aTrack);
+    mTrackListeners[i]->NotifyTrackAdded(aTrack);
   }
 }
 
 void
 DOMMediaStream::NotifyTrackRemoved(const RefPtr<MediaStreamTrack>& aTrack)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   aTrack->RemovePrincipalChangeObserver(this);
 
   for (int32_t i = mTrackListeners.Length() - 1; i >= 0; --i) {
-    const RefPtr<TrackListener>& listener = mTrackListeners[i];
-    listener->NotifyTrackRemoved(aTrack);
+    mTrackListeners[i]->NotifyTrackRemoved(aTrack);
   }
 
   RecomputePrincipal();
 }
 
 void
 DOMMediaStream::CreateAndAddPlaybackStreamListener(MediaStream* aStream)
 {
--- a/dom/media/DOMMediaStream.h
+++ b/dom/media/DOMMediaStream.h
@@ -217,35 +217,32 @@ class DOMMediaStream : public DOMEventTa
   typedef dom::VideoTrack VideoTrack;
   typedef dom::AudioTrackList AudioTrackList;
   typedef dom::VideoTrackList VideoTrackList;
 
 public:
   typedef dom::MediaTrackConstraints MediaTrackConstraints;
 
   class TrackListener {
-    NS_INLINE_DECL_REFCOUNTING(TrackListener)
+  public:
+    virtual ~TrackListener() {}
 
-  public:
     /**
      * Called when the DOMMediaStream has a new track added, either by
      * JS (addTrack()) or the source creating one.
      */
     virtual void
     NotifyTrackAdded(const RefPtr<MediaStreamTrack>& aTrack) {};
 
     /**
      * Called when the DOMMediaStream removes a track, either by
      * JS (removeTrack()) or the source ending it.
      */
     virtual void
     NotifyTrackRemoved(const RefPtr<MediaStreamTrack>& aTrack) {};
-
-  protected:
-    virtual ~TrackListener() {}
   };
 
   /**
    * TrackPort is a representation of a MediaStreamTrack-MediaInputPort pair
    * that make up a link between the Owned stream and the Playback stream.
    *
    * Semantically, the track is the identifier/key and the port the value of this
    * connection.
@@ -561,17 +558,24 @@ public:
    */
   void AddConsumerToKeepAlive(nsISupports* aConsumer)
   {
     if (!IsFinished() && !mNotifiedOfMediaStreamGraphShutdown) {
       mConsumersToKeepAlive.AppendElement(aConsumer);
     }
   }
 
+  // Registers a track listener to this MediaStream, for listening to changes
+  // to our track set. The caller must call UnregisterTrackListener before
+  // being destroyed, so we don't hold on to a dead pointer. Main thread only.
   void RegisterTrackListener(TrackListener* aListener);
+
+  // Unregisters a track listener from this MediaStream. The caller must call
+  // UnregisterTrackListener before being destroyed, so we don't hold on to
+  // a dead pointer. Main thread only.
   void UnregisterTrackListener(TrackListener* aListener);
 
 protected:
   virtual ~DOMMediaStream();
 
   void Destroy();
   void InitSourceStream(MediaStreamGraph* aGraph);
   void InitTrackUnionStream(MediaStreamGraph* aGraph);
@@ -670,17 +674,17 @@ protected:
   nsString mID;
 
   // Keep these alive until the stream finishes
   nsTArray<nsCOMPtr<nsISupports> > mConsumersToKeepAlive;
 
   bool mNotifiedOfMediaStreamGraphShutdown;
 
   // The track listeners subscribe to changes in this stream's track set.
-  nsTArray<RefPtr<TrackListener>> mTrackListeners;
+  nsTArray<TrackListener*> mTrackListeners;
 
 private:
   void NotifyPrincipalChanged();
   // Principal identifying who may access the collected contents of this stream.
   // If null, this stream can be used by anyone because it has no content yet.
   nsCOMPtr<nsIPrincipal> mPrincipal;
   // Video principal is used by video element as access is requested to its
   // image data.