Bug 822956. Part 2: Don't wrap getUserMedia's SourceMediaStream in a DOM object wrapper. Create an nsDOMUserMediaStream wrapper specifically to clean up the SourceMediaStream and the MediaInputPort. r=jesup

content/media/nsDOMMediaStream.cpp

@@ -78,9 +78,7 @@ already_AddRefed<nsDOMMediaStream>
 nsDOMMediaStream::CreateSourceStream(uint32_t aHintContents)
 {
   nsRefPtr<nsDOMMediaStream> stream = new nsDOMMediaStream();
-  stream->SetHintContents(aHintContents);
+  stream->InitSourceStream(aHintContents);
-  MediaStreamGraph* gm = MediaStreamGraph::GetInstance();
-  stream->mStream = gm->CreateSourceStream(stream);
   return stream.forget();
 }
 
@@ -88,9 +86,7 @@ already_AddRefed<nsDOMLocalMediaStream>
 nsDOMLocalMediaStream::CreateSourceStream(uint32_t aHintContents)
 {
   nsRefPtr<nsDOMLocalMediaStream> stream = new nsDOMLocalMediaStream();
-  stream->SetHintContents(aHintContents);
+  stream->InitSourceStream(aHintContents);
-  MediaStreamGraph* gm = MediaStreamGraph::GetInstance();
-  stream->mStream = gm->CreateSourceStream(stream);
   return stream.forget();
 }
 
@@ -98,9 +94,7 @@ already_AddRefed<nsDOMMediaStream>
 nsDOMMediaStream::CreateTrackUnionStream(uint32_t aHintContents)
 {
   nsRefPtr<nsDOMMediaStream> stream = new nsDOMMediaStream();
-  stream->SetHintContents(aHintContents);
+  stream->InitTrackUnionStream(aHintContents);
-  MediaStreamGraph* gm = MediaStreamGraph::GetInstance();
-  stream->mStream = gm->CreateTrackUnionStream(stream);
   return stream.forget();
 }
 
@@ -108,9 +102,7 @@ already_AddRefed<nsDOMLocalMediaStream>
 nsDOMLocalMediaStream::CreateTrackUnionStream(uint32_t aHintContents)
 {
   nsRefPtr<nsDOMLocalMediaStream> stream = new nsDOMLocalMediaStream();
-  stream->SetHintContents(aHintContents);
+  stream->InitTrackUnionStream(aHintContents);
-  MediaStreamGraph* gm = MediaStreamGraph::GetInstance();
-  stream->mStream = gm->CreateTrackUnionStream(stream);
   return stream.forget();
 }
 

content/media/nsDOMMediaStream.h

@@ -26,9 +26,11 @@ class nsXPCClassInfo;
 class nsDOMMediaStream : public nsIDOMMediaStream
 {
   friend class nsDOMLocalMediaStream;
-  typedef mozilla::MediaStream MediaStream;
 
 public:
+  typedef mozilla::MediaStream MediaStream;
+  typedef mozilla::MediaStreamGraph MediaStreamGraph;
+
   nsDOMMediaStream() : mStream(nullptr), mHintContents(0) {}
   virtual ~nsDOMMediaStream();
 
@@ -73,6 +75,19 @@ public:
   static already_AddRefed<nsDOMMediaStream> CreateTrackUnionStream(uint32_t aHintContents = 0);
 
 protected:
+  void InitSourceStream(uint32_t aHintContents)
+  {
+    SetHintContents(aHintContents);
+    MediaStreamGraph* gm = MediaStreamGraph::GetInstance();
+    mStream = gm->CreateSourceStream(this);
+  }
+  void InitTrackUnionStream(uint32_t aHintContents)
+  {
+    SetHintContents(aHintContents);
+    MediaStreamGraph* gm = MediaStreamGraph::GetInstance();
+    mStream = gm->CreateTrackUnionStream(this);
+  }
+
   // MediaStream is owned by the graph, but we tell it when to die, and it won't
   // die until we let it.
   MediaStream* mStream;

dom/media/MediaManager.cpp

@@ -221,6 +221,37 @@ MediaDevice::GetSource()
   return mSource;
 }
 
+/**
+ * A subclass that we only use to stash internal pointers to MediaStreamGraph objects
+ * that need to be cleaned up.
+ */
+class nsDOMUserMediaStream : public nsDOMLocalMediaStream
+{
+public:
+  static already_AddRefed<nsDOMUserMediaStream>
+  CreateTrackUnionStream(uint32_t aHintContents)
+  {
+    nsRefPtr<nsDOMUserMediaStream> stream = new nsDOMUserMediaStream();
+    stream->InitTrackUnionStream(aHintContents);
+    return stream.forget();
+  }
+
+  virtual ~nsDOMUserMediaStream()
+  {
+    if (mPort) {
+      mPort->Destroy();
+    }
+    if (mSourceStream) {
+      mSourceStream->Destroy();
+    }
+  }
+
+  // The actual MediaStream is a TrackUnionStream. But these resources need to be
+  // explicitly destroyed too.
+  nsRefPtr<SourceMediaStream> mSourceStream;
+  nsRefPtr<MediaInputPort> mPort;
+};
+
 /**
  * Creates a MediaStream, attaches a listener and fires off a success callback
  * to the DOM with the stream. We also pass in the error callback so it can
@@ -267,29 +298,31 @@ public:
     }
 
     // Create a media stream.
-    nsRefPtr<nsDOMLocalMediaStream> stream;
-    nsRefPtr<nsDOMLocalMediaStream> trackunion;
     uint32_t hints = (mAudioSource ? nsDOMMediaStream::HINT_CONTENTS_AUDIO : 0);
     hints |= (mVideoSource ? nsDOMMediaStream::HINT_CONTENTS_VIDEO : 0);
 
-    stream     = nsDOMLocalMediaStream::CreateSourceStream(hints);
+    nsRefPtr<nsDOMUserMediaStream> trackunion =
-    trackunion = nsDOMLocalMediaStream::CreateTrackUnionStream(hints);
+      nsDOMUserMediaStream::CreateTrackUnionStream(hints);
-    if (!stream || !trackunion) {
+    if (!trackunion) {
       nsCOMPtr<nsIDOMGetUserMediaErrorCallback> error(mError);
       LOG(("Returning error for getUserMedia() - no stream"));
       error->OnError(NS_LITERAL_STRING("NO_STREAM"));
       return NS_OK;
     }
+
+    MediaStreamGraph* gm = MediaStreamGraph::GetInstance();
+    nsRefPtr<SourceMediaStream> stream = gm->CreateSourceStream(nullptr);
+
     // connect the source stream to the track union stream to avoid us blocking
     trackunion->GetStream()->AsProcessedStream()->SetAutofinish(true);
     nsRefPtr<MediaInputPort> port = trackunion->GetStream()->AsProcessedStream()->
-      AllocateInputPort(stream->GetStream()->AsSourceStream(),
+      AllocateInputPort(stream, MediaInputPort::FLAG_BLOCK_OUTPUT);
-                        MediaInputPort::FLAG_BLOCK_OUTPUT);
+    trackunion->mSourceStream = stream;
+    trackunion->mPort = port;
 
     nsPIDOMWindow *window = static_cast<nsPIDOMWindow*>
       (nsGlobalWindow::GetInnerWindowWithId(mWindowID));
     if (window && window->GetExtantDoc()) {
-      stream->CombineWithPrincipal(window->GetExtantDoc()->NodePrincipal());
       trackunion->CombineWithPrincipal(window->GetExtantDoc()->NodePrincipal());
     }
 
@@ -300,11 +333,11 @@ public:
     // that the MediaStream has started consuming. The listener is freed
     // when the page is invalidated (on navigation or close).
     GetUserMediaCallbackMediaStreamListener* listener =
-      new GetUserMediaCallbackMediaStreamListener(mediaThread, stream,
+      new GetUserMediaCallbackMediaStreamListener(mediaThread, stream.forget(),
                                                   port.forget(),
                                                   mAudioSource,
                                                   mVideoSource);
-    stream->GetStream()->AddListener(listener);
+    listener->Stream()->AddListener(listener);
 
     // No need for locking because we always do this in the main thread.
     listeners->AppendElement(listener);
@@ -312,7 +345,7 @@ public:
     // Dispatch to the media thread to ask it to start the sources,
     // because that can take a while
     nsRefPtr<MediaOperationRunnable> runnable(
-      new MediaOperationRunnable(MEDIA_START, stream,
+      new MediaOperationRunnable(MEDIA_START, listener,
                                  mAudioSource, mVideoSource));
     mediaThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
 

dom/media/MediaManager.h

@@ -70,7 +70,7 @@ class GetUserMediaCallbackMediaStreamListener : public MediaStreamListener
 {
 public:
   GetUserMediaCallbackMediaStreamListener(nsIThread *aThread,
-    nsDOMMediaStream* aStream,
+    already_AddRefed<SourceMediaStream> aStream,
     already_AddRefed<MediaInputPort> aPort,
     MediaEngineSource* aAudioSource,
     MediaEngineSource* aVideoSource)
@@ -79,25 +79,22 @@ public:
     , mVideoSource(aVideoSource)
     , mStream(aStream)
     , mPort(aPort)
-    , mSourceStream(aStream->GetStream()->AsSourceStream())
     , mLastEndTimeAudio(0)
-    , mLastEndTimeVideo(0) { MOZ_ASSERT(mSourceStream); }
+    , mLastEndTimeVideo(0) {}
 
   ~GetUserMediaCallbackMediaStreamListener()
   {
-    // In theory this could be released from the MediaStreamGraph thread (RemoveListener)
+    // It's OK to release mStream and mPort on any thread; they have thread-safe
-    if (mStream) {
+    // refcounts.
-      nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
-      nsDOMMediaStream *stream;
-      mStream.forget(&stream);
-      // Releases directly if on MainThread already
-      NS_ProxyRelease(mainThread, stream, false);
-    }
   }
 
+  MediaStream *Stream()
+  {
+    return mStream;
+  }
   SourceMediaStream *GetSourceStream()
   {
-    return mStream->GetStream()->AsSourceStream();
+    return mStream->AsSourceStream();
   }
 
   void
@@ -108,7 +105,7 @@ public:
   {
     NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
     // Caller holds strong reference to us, so no death grip required
-    mStream->GetStream()->RemoveListener(this);
+    mStream->RemoveListener(this);
   }
 
   // Proxy NotifyPull() to sources
@@ -118,10 +115,10 @@ public:
     // Currently audio sources ignore NotifyPull, but they could
     // watch it especially for fake audio.
     if (mAudioSource) {
-      mAudioSource->NotifyPull(aGraph, mSourceStream, kAudioTrack, aDesiredTime, mLastEndTimeAudio);
+      mAudioSource->NotifyPull(aGraph, mStream, kAudioTrack, aDesiredTime, mLastEndTimeAudio);
     }
     if (mVideoSource) {
-      mVideoSource->NotifyPull(aGraph, mSourceStream, kVideoTrack, aDesiredTime, mLastEndTimeVideo);
+      mVideoSource->NotifyPull(aGraph, mStream, kVideoTrack, aDesiredTime, mLastEndTimeVideo);
     }
   }
 
@@ -136,9 +133,8 @@ private:
   nsCOMPtr<nsIThread> mMediaThread;
   nsRefPtr<MediaEngineSource> mAudioSource;
   nsRefPtr<MediaEngineSource> mVideoSource;
-  nsRefPtr<nsDOMMediaStream> mStream;
+  nsRefPtr<SourceMediaStream> mStream;
   nsRefPtr<MediaInputPort> mPort;
-  SourceMediaStream *mSourceStream; // mStream controls ownership
   TrackTicks mLastEndTimeAudio;
   TrackTicks mLastEndTimeVideo;
 };
@@ -154,16 +150,6 @@ typedef enum {
 class MediaOperationRunnable : public nsRunnable
 {
 public:
-  MediaOperationRunnable(MediaOperation aType,
-    nsDOMMediaStream* aStream,
-    MediaEngineSource* aAudioSource,
-    MediaEngineSource* aVideoSource)
-    : mType(aType)
-    , mAudioSource(aAudioSource)
-    , mVideoSource(aVideoSource)
-    , mStream(aStream)
-    {}
-
   // so we can send Stop without AddRef()ing from the MSG thread
   MediaOperationRunnable(MediaOperation aType,
     GetUserMediaCallbackMediaStreamListener* aListener,
@@ -172,33 +158,20 @@ public:
     : mType(aType)
     , mAudioSource(aAudioSource)
     , mVideoSource(aVideoSource)
-    , mStream(nullptr)
     , mListener(aListener)
     {}
 
   ~MediaOperationRunnable()
   {
-    // nsDOMMediaStreams are cycle-collected and thus main-thread-only for
+    // MediaStreams can be released on any thread.
-    // refcounting and releasing
-    if (mStream) {
-      nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
-      nsDOMMediaStream *stream;
-      mStream.forget(&stream);
-      NS_ProxyRelease(mainThread, stream, true);
-    }
   }
 
   NS_IMETHOD
   Run()
   {
-    SourceMediaStream *source;
+    SourceMediaStream *source = mListener->GetSourceStream();
     // No locking between these is required as all the callbacks for the
     // same MediaStream will occur on the same thread.
-    if (mStream) {
-      source = mStream->GetStream()->AsSourceStream();
-    } else {
-      source = mListener->GetSourceStream();
-    }
     MOZ_ASSERT(source);
     if (!source)  // paranoia
       return NS_ERROR_FAILURE;
@@ -266,7 +239,6 @@ private:
   MediaOperation mType;
   nsRefPtr<MediaEngineSource> mAudioSource; // threadsafe
   nsRefPtr<MediaEngineSource> mVideoSource; // threadsafe
-  nsRefPtr<nsDOMMediaStream> mStream;       // not threadsafe
   nsRefPtr<GetUserMediaCallbackMediaStreamListener> mListener; // threadsafe
 };