Bug 890248. Avoid situations where adding a new input to an AudioNode can race with a message telling the AudioNode to release its mPlayingRef. r=ehsan

--HG--
extra : rebase_source : 913683cc16a717bf73f9976292af965aba6b7758

content/media/MediaStreamGraph.cpp

@@ -1539,6 +1539,12 @@ MediaStream::FinishOnGraphThread()
   GraphImpl()->FinishStream(this);
 }
 
+int64_t
+MediaStream::GetProcessingGraphUpdateIndex()
+{
+  return GraphImpl()->GetProcessingGraphUpdateIndex();
+}
+
 StreamBuffer::Track*
 MediaStream::EnsureTrack(TrackID aTrackId, TrackRate aSampleRate)
 {

content/media/MediaStreamGraph.h

@@ -430,6 +430,10 @@ public:
   GraphTime StreamTimeToGraphTime(StreamTime aTime);
   bool IsFinishedOnGraphThread() { return mFinished; }
   void FinishOnGraphThread();
+  /**
+   * Identify which graph update index we are currently processing.
+   */
+  int64_t GetProcessingGraphUpdateIndex();
 
   bool HasCurrentData() { return mHasCurrentData; }
 

content/media/MediaStreamGraphImpl.h

@@ -338,7 +338,7 @@ public:
    */
   bool IsEmpty() { return mStreams.IsEmpty() && mPortCount == 0; }
 
-  // For use by control messages
+  // For use by control messages, on graph thread only.
   /**
    * Identify which graph update index we are currently processing.
    */

content/media/webaudio/AudioNode.cpp

@@ -174,6 +174,7 @@ AudioNode::Connect(AudioNode& aDestination, uint32_t aOutput,
       ps->AllocateInputPort(mStream, MediaInputPort::FLAG_BLOCK_INPUT,
                             static_cast<uint16_t>(aInput),
                             static_cast<uint16_t>(aOutput));
+    aDestination.NotifyInputConnected();
   }
 
   // This connection may have connected a panner and a source.

content/media/webaudio/AudioNode.h

@@ -216,6 +216,8 @@ public:
 
   void RemoveOutputParam(AudioParam* aParam);
 
+  virtual void NotifyInputConnected() {}
+
 private:
   friend class AudioBufferSourceNode;
   // This could possibly delete 'this'.

content/media/webaudio/ConvolverNode.cpp

@@ -127,6 +127,10 @@ public:
 
       mLeftOverData -= WEBAUDIO_BLOCK_SIZE;
       if (mLeftOverData <= 0) {
+        // Note: this keeps spamming the main thread with messages as long
+        // as there is nothing to play. This isn't great, but it avoids
+        // problems with some messages being ignored when they're rejected by
+        // ConvolverNode::AcceptPlayingRefRelease.
         mLeftOverData = 0;
         nsRefPtr<PlayingRefChanged> refchanged =
           new PlayingRefChanged(aStream, PlayingRefChanged::RELEASE);
@@ -173,6 +177,7 @@ ConvolverNode::ConvolverNode(AudioContext* aContext)
               2,
               ChannelCountMode::Clamped_max,
               ChannelInterpretation::Speakers)
+  , mMediaStreamGraphUpdateIndexAtLastInputConnection(0)
   , mNormalize(true)
 {
   ConvolverNodeEngine* engine = new ConvolverNodeEngine(this, mNormalize);

content/media/webaudio/ConvolverNode.h

@@ -39,9 +39,23 @@ public:
 
   void SetNormalize(bool aNormal);
 
+  virtual void NotifyInputConnected() MOZ_OVERRIDE
+  {
+    mMediaStreamGraphUpdateIndexAtLastInputConnection =
+      mStream->Graph()->GetCurrentGraphUpdateIndex();
+  }
+  bool AcceptPlayingRefRelease(int64_t aLastGraphUpdateIndexProcessed) const
+  {
+    // Reject any requests to release mPlayingRef if the request was issued
+    // before the MediaStreamGraph was aware of the most-recently-added input
+    // connection.
+    return aLastGraphUpdateIndexProcessed >= mMediaStreamGraphUpdateIndexAtLastInputConnection;
+  }
+
 private:
   friend class PlayingRefChangeHandler<ConvolverNode>;
 
+  int64_t mMediaStreamGraphUpdateIndexAtLastInputConnection;
   nsRefPtr<AudioBuffer> mBuffer;
   SelfReference<ConvolverNode> mPlayingRef;
   bool mNormalize;

content/media/webaudio/DelayNode.cpp

@@ -121,7 +121,10 @@ public:
     } else if (mLeftOverData != INT32_MIN) {
       mLeftOverData -= WEBAUDIO_BLOCK_SIZE;
       if (mLeftOverData <= 0) {
-        mLeftOverData = INT32_MIN;
+        // Continue spamming the main thread with messages until we are destroyed.
+        // This isn't great, but it ensures a message will get through even if
+        // some are ignored by DelayNode::AcceptPlayingRefRelease
+        mLeftOverData = 0;
         playedBackAllLeftOvers = true;
 
         nsRefPtr<PlayingRefChanged> refchanged =
@@ -244,6 +247,7 @@ DelayNode::DelayNode(AudioContext* aContext, double aMaxDelay)
               2,
               ChannelCountMode::Max,
               ChannelInterpretation::Speakers)
+  , mMediaStreamGraphUpdateIndexAtLastInputConnection(0)
   , mDelay(new AudioParam(MOZ_THIS_IN_INITIALIZER_LIST(),
                           SendDelayToStream, 0.0f))
 {

content/media/webaudio/DelayNode.h

@@ -32,12 +32,26 @@ public:
     return mDelay;
   }
 
+  virtual void NotifyInputConnected() MOZ_OVERRIDE
+  {
+    mMediaStreamGraphUpdateIndexAtLastInputConnection =
+      mStream->Graph()->GetCurrentGraphUpdateIndex();
+  }
+  bool AcceptPlayingRefRelease(int64_t aLastGraphUpdateIndexProcessed) const
+  {
+    // Reject any requests to release mPlayingRef if the request was issued
+    // before the MediaStreamGraph was aware of the most-recently-added input
+    // connection.
+    return aLastGraphUpdateIndexProcessed >= mMediaStreamGraphUpdateIndexAtLastInputConnection;
+  }
+
 private:
   static void SendDelayToStream(AudioNode* aNode);
   friend class DelayNodeEngine;
   friend class PlayingRefChangeHandler<DelayNode>;
 
 private:
+  int64_t mMediaStreamGraphUpdateIndexAtLastInputConnection;
   nsRefPtr<AudioParam> mDelay;
   SelfReference<DelayNode> mPlayingRef;
 };

content/media/webaudio/PlayingRefChangeHandler.h

@@ -19,7 +19,8 @@ class PlayingRefChangeHandler : public nsRunnable
 public:
   enum ChangeType { ADDREF, RELEASE };
   PlayingRefChangeHandler(AudioNodeStream* aStream, ChangeType aChange)
-    : mStream(aStream)
+    : mLastProcessedGraphUpdateIndex(aStream->GetProcessingGraphUpdateIndex())
+    , mStream(aStream)
     , mChange(aChange)
   {
   }
@@ -38,7 +39,8 @@ public:
     if (node) {
       if (mChange == ADDREF) {
         node->mPlayingRef.Take(node);
-      } else if (mChange == RELEASE) {
+      } else if (mChange == RELEASE &&
+                 node->AcceptPlayingRefRelease(mLastProcessedGraphUpdateIndex)) {
         node->mPlayingRef.Drop(node);
       }
     }
@@ -46,6 +48,7 @@ public:
   }
 
 private:
+  int64_t mLastProcessedGraphUpdateIndex;
   nsRefPtr<AudioNodeStream> mStream;
   ChangeType mChange;
 };