Bug 1161946 - MainThreadMediaStreamListener should be notified just when the stream is finished - patch 1, r=padenot

dom/media/DOMMediaStream.cpp

@@ -470,11 +470,10 @@ DOMMediaStream::NotifyMediaStreamGraphShutdown()
 }
 
 void
-DOMMediaStream::NotifyStreamStateChanged()
+DOMMediaStream::NotifyStreamFinished()
 {
-  if (IsFinished()) {
-    mConsumersToKeepAlive.Clear();
-  }
+  MOZ_ASSERT(IsFinished());
+  mConsumersToKeepAlive.Clear();
 }
 
 void

dom/media/DOMMediaStream.h

@@ -168,9 +168,9 @@ public:
    */
   void NotifyMediaStreamGraphShutdown();
   /**
-   * Called when the main-thread state of the MediaStream changed.
+   * Called when the main-thread state of the MediaStream goes to finished.
    */
-  void NotifyStreamStateChanged();
+  void NotifyStreamFinished();
 
   // Webrtc allows the remote side to name a stream whatever it wants, and we
   // need to surface this to content.

dom/media/MediaStreamGraph.cpp

@@ -1543,11 +1543,14 @@ MediaStreamGraphImpl::ApplyStreamUpdate(StreamUpdate* aUpdate)
   stream->mMainThreadCurrentTime = aUpdate->mNextMainThreadCurrentTime;
   stream->mMainThreadFinished = aUpdate->mNextMainThreadFinished;
 
-  if (stream->mWrapper) {
-    stream->mWrapper->NotifyStreamStateChanged();
-  }
-  for (int32_t i = stream->mMainThreadListeners.Length() - 1; i >= 0; --i) {
-    stream->mMainThreadListeners[i]->NotifyMainThreadStateChanged();
+  if (stream->ShouldNotifyStreamFinished()) {
+    if (stream->mWrapper) {
+      stream->mWrapper->NotifyStreamFinished();
+    }
+
+    for (int32_t i = stream->mMainThreadListeners.Length() - 1; i >= 0; --i) {
+      stream->mMainThreadListeners[i]->NotifyMainThreadStreamFinished();
+    }
   }
 }
 
@@ -1918,6 +1921,7 @@ MediaStream::MediaStream(DOMMediaStream* aWrapper)
   , mWrapper(aWrapper)
   , mMainThreadCurrentTime(0)
   , mMainThreadFinished(false)
+  , mFinishedNotificationSent(false)
   , mMainThreadDestroyed(false)
   , mGraph(nullptr)
   , mAudioChannelType(dom::AudioChannel::Normal)

dom/media/MediaStreamGraph.h

@@ -226,7 +226,7 @@ public:
  */
 class MainThreadMediaStreamListener {
 public:
-  virtual void NotifyMainThreadStateChanged() = 0;
+  virtual void NotifyMainThreadStreamFinished() = 0;
 };
 
 /**
@@ -417,6 +417,7 @@ public:
     NS_ASSERTION(NS_IsMainThread(), "Call only on main thread");
     return mMainThreadFinished;
   }
+
   bool IsDestroyed()
   {
     NS_ASSERTION(NS_IsMainThread(), "Call only on main thread");
@@ -596,6 +597,17 @@ protected:
     mBuffer.ForgetUpTo(aCurrentTime - mBufferStartTime);
   }
 
+  bool ShouldNotifyStreamFinished()
+  {
+    NS_ASSERTION(NS_IsMainThread(), "Call only on main thread");
+    if (!mMainThreadFinished || mFinishedNotificationSent) {
+      return false;
+    }
+
+    mFinishedNotificationSent = true;
+    return true;
+  }
+
   // This state is all initialized on the main thread but
   // otherwise modified only on the media graph thread.
 
@@ -696,6 +708,7 @@ protected:
   // Main-thread views of state
   StreamTime mMainThreadCurrentTime;
   bool mMainThreadFinished;
+  bool mFinishedNotificationSent;
   bool mMainThreadDestroyed;
 
   // Our media stream graph.  null if destroyed on the graph thread.

dom/media/webaudio/AudioBufferSourceNode.cpp

@@ -562,7 +562,6 @@ AudioBufferSourceNode::AudioBufferSourceNode(AudioContext* aContext)
   , mDetune(new AudioParam(this, SendDetuneToStream, 0.0f, "detune"))
   , mLoop(false)
   , mStartCalled(false)
-  , mStopped(false)
 {
   AudioBufferSourceNodeEngine* engine = new AudioBufferSourceNodeEngine(this, aContext->Destination());
   mStream = aContext->Graph()->CreateAudioNodeStream(engine, MediaStreamGraph::SOURCE_STREAM);
@@ -710,38 +709,35 @@ AudioBufferSourceNode::Stop(double aWhen, ErrorResult& aRv)
 }
 
 void
-AudioBufferSourceNode::NotifyMainThreadStateChanged()
+AudioBufferSourceNode::NotifyMainThreadStreamFinished()
 {
-  if (mStream->IsFinished()) {
-    class EndedEventDispatcher final : public nsRunnable
-    {
-    public:
-      explicit EndedEventDispatcher(AudioBufferSourceNode* aNode)
-        : mNode(aNode) {}
-      NS_IMETHODIMP Run() override
-      {
-        // If it's not safe to run scripts right now, schedule this to run later
-        if (!nsContentUtils::IsSafeToRunScript()) {
-          nsContentUtils::AddScriptRunner(this);
-          return NS_OK;
-        }
+  MOZ_ASSERT(mStream->IsFinished());
 
-        mNode->DispatchTrustedEvent(NS_LITERAL_STRING("ended"));
+  class EndedEventDispatcher final : public nsRunnable
+  {
+  public:
+    explicit EndedEventDispatcher(AudioBufferSourceNode* aNode)
+      : mNode(aNode) {}
+    NS_IMETHODIMP Run() override
+    {
+      // If it's not safe to run scripts right now, schedule this to run later
+      if (!nsContentUtils::IsSafeToRunScript()) {
+        nsContentUtils::AddScriptRunner(this);
         return NS_OK;
       }
-    private:
-      nsRefPtr<AudioBufferSourceNode> mNode;
-    };
-    if (!mStopped) {
-      // Only dispatch the ended event once
-      NS_DispatchToMainThread(new EndedEventDispatcher(this));
-      mStopped = true;
-    }
 
-    // Drop the playing reference
-    // Warning: The below line might delete this.
-    MarkInactive();
-  }
+      mNode->DispatchTrustedEvent(NS_LITERAL_STRING("ended"));
+      return NS_OK;
+    }
+  private:
+    nsRefPtr<AudioBufferSourceNode> mNode;
+  };
+
+  NS_DispatchToMainThread(new EndedEventDispatcher(this));
+
+  // Drop the playing reference
+  // Warning: The below line might delete this.
+  MarkInactive();
 }
 
 void

dom/media/webaudio/AudioBufferSourceNode.h

@@ -94,7 +94,7 @@ public:
 
   IMPL_EVENT_HANDLER(ended)
 
-  virtual void NotifyMainThreadStateChanged() override;
+  virtual void NotifyMainThreadStreamFinished() override;
 
   virtual const char* NodeType() const override
   {
@@ -147,7 +147,6 @@ private:
   nsRefPtr<AudioParam> mDetune;
   bool mLoop;
   bool mStartCalled;
-  bool mStopped;
 };
 
 }

dom/media/webaudio/AudioDestinationNode.cpp

@@ -351,7 +351,6 @@ AudioDestinationNode::AudioDestinationNode(AudioContext* aContext,
   , mFramesToProduce(aLength)
   , mAudioChannel(AudioChannel::Normal)
   , mIsOffline(aIsOffline)
-  , mHasFinished(false)
   , mAudioChannelAgentPlaying(false)
   , mExtraCurrentTime(0)
   , mExtraCurrentTimeSinceLastStartedBlocking(0)
@@ -426,15 +425,14 @@ AudioDestinationNode::DestroyMediaStream()
 }
 
 void
-AudioDestinationNode::NotifyMainThreadStateChanged()
+AudioDestinationNode::NotifyMainThreadStreamFinished()
 {
-  if (mStream->IsFinished() && !mHasFinished) {
-    mHasFinished = true;
-    if (mIsOffline) {
-      nsCOMPtr<nsIRunnable> runnable =
-        NS_NewRunnableMethod(this, &AudioDestinationNode::FireOfflineCompletionEvent);
-      NS_DispatchToCurrentThread(runnable);
-    }
+  MOZ_ASSERT(mStream->IsFinished());
+
+  if (mIsOffline) {
+    nsCOMPtr<nsIRunnable> runnable =
+      NS_NewRunnableMethod(this, &AudioDestinationNode::FireOfflineCompletionEvent);
+    NS_DispatchToCurrentThread(runnable);
   }
 }
 

dom/media/webaudio/AudioDestinationNode.h

@@ -64,7 +64,7 @@ public:
   AudioChannel MozAudioChannelType() const;
   void SetMozAudioChannelType(AudioChannel aValue, ErrorResult& aRv);
 
-  virtual void NotifyMainThreadStateChanged() override;
+  virtual void NotifyMainThreadStreamFinished() override;
   void FireOfflineCompletionEvent();
 
   // An amount that should be added to the MediaStream's current time to
@@ -109,7 +109,6 @@ private:
   // Audio Channel Type.
   AudioChannel mAudioChannel;
   bool mIsOffline;
-  bool mHasFinished;
   bool mAudioChannelAgentPlaying;
 
   TimeStamp mStartedBlockingDueToBeingOnlyNode;

dom/media/webaudio/OscillatorNode.cpp

@@ -384,7 +384,6 @@ OscillatorNode::OscillatorNode(AudioContext* aContext)
   , mFrequency(new AudioParam(this, SendFrequencyToStream, 440.0f, "frequency"))
   , mDetune(new AudioParam(this, SendDetuneToStream, 0.0f, "detune"))
   , mStartCalled(false)
-  , mStopped(false)
 {
   OscillatorNodeEngine* engine = new OscillatorNodeEngine(this, aContext->Destination());
   mStream = aContext->Graph()->CreateAudioNodeStream(engine, MediaStreamGraph::SOURCE_STREAM);
@@ -512,38 +511,35 @@ OscillatorNode::Stop(double aWhen, ErrorResult& aRv)
 }
 
 void
-OscillatorNode::NotifyMainThreadStateChanged()
+OscillatorNode::NotifyMainThreadStreamFinished()
 {
-  if (mStream->IsFinished()) {
-    class EndedEventDispatcher final : public nsRunnable
-    {
-    public:
-      explicit EndedEventDispatcher(OscillatorNode* aNode)
-        : mNode(aNode) {}
-      NS_IMETHOD Run() override
-      {
-        // If it's not safe to run scripts right now, schedule this to run later
-        if (!nsContentUtils::IsSafeToRunScript()) {
-          nsContentUtils::AddScriptRunner(this);
-          return NS_OK;
-        }
+  MOZ_ASSERT(mStream->IsFinished());
 
-        mNode->DispatchTrustedEvent(NS_LITERAL_STRING("ended"));
+  class EndedEventDispatcher final : public nsRunnable
+  {
+  public:
+    explicit EndedEventDispatcher(OscillatorNode* aNode)
+      : mNode(aNode) {}
+    NS_IMETHOD Run() override
+    {
+      // If it's not safe to run scripts right now, schedule this to run later
+      if (!nsContentUtils::IsSafeToRunScript()) {
+        nsContentUtils::AddScriptRunner(this);
         return NS_OK;
       }
-    private:
-      nsRefPtr<OscillatorNode> mNode;
-    };
-    if (!mStopped) {
-      // Only dispatch the ended event once
-      NS_DispatchToMainThread(new EndedEventDispatcher(this));
-      mStopped = true;
+
+      mNode->DispatchTrustedEvent(NS_LITERAL_STRING("ended"));
+      return NS_OK;
     }
+  private:
+    nsRefPtr<OscillatorNode> mNode;
+  };
 
-    // Drop the playing reference
-    // Warning: The below line might delete this.
-    MarkInactive();
-  }
+  NS_DispatchToMainThread(new EndedEventDispatcher(this));
+
+  // Drop the playing reference
+  // Warning: The below line might delete this.
+  MarkInactive();
 }
 
 }

dom/media/webaudio/OscillatorNode.h

@@ -78,7 +78,7 @@ public:
 
   IMPL_EVENT_HANDLER(ended)
 
-  virtual void NotifyMainThreadStateChanged() override;
+  virtual void NotifyMainThreadStreamFinished() override;
 
   virtual const char* NodeType() const override
   {
@@ -103,7 +103,6 @@ private:
   nsRefPtr<AudioParam> mFrequency;
   nsRefPtr<AudioParam> mDetune;
   bool mStartCalled;
-  bool mStopped;
 };
 
 }