Bug 1299515 - Signal SetPullEnabled with a message. r?padenot draft
authorAndreas Pehrson <pehrsons@mozilla.com>
Wed, 03 Jan 2018 11:59:41 +0100
changeset 749434 8fdf9981cd49b1c4ba065cba9de9ca9a12b34fc2
parent 749433 12d1c497dcf870584518779cdd36e100af6e1294
child 749435 76f368a2b510b849d3e20ff6bd3eefc3ccb3b4ac
push id97396
push userbmo:apehrson@mozilla.com
push dateWed, 31 Jan 2018 13:27:39 +0000
reviewerspadenot
bugs1299515
milestone60.0a1
Bug 1299515 - Signal SetPullEnabled with a message. r?padenot With the added invariant that NotifyPull() needs a MediaStreamListener present to not underrun, we need SetPullEnabled() and AddListener() to stay in sync by using the same signaling mechanism. MozReview-Commit-ID: 49KWdiTOG98
dom/media/MediaManager.cpp
dom/media/MediaStreamGraph.cpp
dom/media/MediaStreamGraph.h
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@@ -1284,17 +1284,18 @@ public:
 
     // Dispatch to the media thread to ask it to start the sources,
     // because that can take a while.
     // Pass ownership of domStream through the lambda to the nested chrome
     // notification lambda to ensure it's kept alive until that lambda runs or is discarded.
     RefPtr<GetUserMediaStreamRunnable> self = this;
     MediaManager::PostTask(NewTaskFrom([self, domStream, callback]() mutable {
       MOZ_ASSERT(MediaManager::IsInMediaThread());
-      SourceMediaStream* source = self->mSourceListener->GetSourceStream();
+      RefPtr<SourceMediaStream> source =
+        self->mSourceListener->GetSourceStream();
 
       RefPtr<MediaMgrError> error = nullptr;
       if (self->mAudioDevice) {
         nsresult rv = self->mAudioDevice->SetTrack(source,
                                                    kAudioTrack,
                                                    self->mSourceListener->GetPrincipalHandle());
         if (NS_SUCCEEDED(rv)) {
           rv = self->mAudioDevice->Start();
@@ -1329,25 +1330,26 @@ public:
         NS_DispatchToMainThread(MakeAndAddRef<ErrorCallbackRunnable>(
           self->mOnFailure, *error, self->mWindowID));
         return NS_OK;
       }
 
       // Start() queued the tracks to be added synchronously to avoid races
       source->FinishAddTracks();
 
-      source->SetPullEnabled(true);
       source->AdvanceKnownTracksTime(STREAM_TIME_MAX);
 
       LOG(("started all sources"));
 
       // onTracksAvailableCallback must be added to domStream on the main thread.
       uint64_t windowID = self->mWindowID;
       NS_DispatchToMainThread(NS_NewRunnableFunction("MediaManager::NotifyChromeOfStart",
-                                                     [domStream, callback, windowID]() mutable {
+                                                     [source, domStream, callback, windowID]() mutable {
+        source->SetPullEnabled(true);
+
         MediaManager* manager = MediaManager::GetIfExists();
         if (!manager) {
           return;
         }
 
         nsGlobalWindowInner* window =
           nsGlobalWindowInner::GetInnerWindowWithId(windowID);
         if (!window) {
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -2725,21 +2725,32 @@ SourceMediaStream::DestroyImpl()
   // can null-check know that the graph will not destroyed.
   MutexAutoLock lock(mMutex);
   MediaStream::DestroyImpl();
 }
 
 void
 SourceMediaStream::SetPullEnabled(bool aEnabled)
 {
-  MutexAutoLock lock(mMutex);
-  mPullEnabled = aEnabled;
-  if (mPullEnabled && GraphImpl()) {
-    GraphImpl()->EnsureNextIteration();
-  }
+  class Message : public ControlMessage {
+  public:
+    Message(SourceMediaStream* aStream, bool aEnabled)
+      : ControlMessage(nullptr)
+      , mStream(aStream)
+      , mEnabled(aEnabled)
+    {}
+    void Run() override
+    {
+      MutexAutoLock lock(mStream->mMutex);
+      mStream->mPullEnabled = mEnabled;
+    }
+    SourceMediaStream* mStream;
+    bool mEnabled;
+  };
+  GraphImpl()->AppendMessage(MakeUnique<Message>(this, aEnabled));
 }
 
 bool
 SourceMediaStream::PullNewData(
   StreamTime aDesiredUpToTime,
   nsTArray<RefPtr<SourceMediaStream::NotifyPullPromise>>& aPromises)
 {
   MutexAutoLock lock(mMutex);
--- a/dom/media/MediaStreamGraph.h
+++ b/dom/media/MediaStreamGraph.h
@@ -671,40 +671,41 @@ protected:
  */
 class SourceMediaStream : public MediaStream
 {
 public:
   explicit SourceMediaStream();
 
   SourceMediaStream* AsSourceStream() override { return this; }
 
-  // Media graph thread only
+  // Main thread only
+
+  /**
+   * Enable or disable pulling. When pulling is enabled, NotifyPull
+   * gets called on MediaStreamListeners for this stream during the
+   * MediaStreamGraph control loop. Pulling is initially disabled.
+   * Due to unavoidable race conditions, after a call to SetPullEnabled(false)
+   * it is still possible for a NotifyPull to occur.
+   */
+  void SetPullEnabled(bool aEnabled);
 
   // Users of audio inputs go through the stream so it can track when the
   // last stream referencing an input goes away, so it can close the cubeb
   // input.  Also note: callable on any thread (though it bounces through
   // MainThread to set the command if needed).
   nsresult OpenAudioInput(int aID,
                           AudioDataListener *aListener);
   // Note: also implied when Destroy() happens
   void CloseAudioInput();
 
+  // MediaStreamGraph thread only
   void DestroyImpl() override;
 
   // Call these on any thread.
   /**
-   * Enable or disable pulling. When pulling is enabled, NotifyPull
-   * gets called on MediaStreamListeners for this stream during the
-   * MediaStreamGraph control loop. Pulling is initially disabled.
-   * Due to unavoidable race conditions, after a call to SetPullEnabled(false)
-   * it is still possible for a NotifyPull to occur.
-   */
-  void SetPullEnabled(bool aEnabled);
-
-  /**
    * Call all MediaStreamListeners to request new data via the NotifyPull API
    * (if enabled).
    * aDesiredUpToTime (in): end time of new data requested.
    * aPromises (out): NotifyPullPromises if async API is enabled.
    *
    * Returns true if new data is about to be added.
    */
   typedef MozPromise<bool, bool, true /* is exclusive */ > NotifyPullPromise;