Bug 834513 - Part 2: Add an AudioNode weak pointer to the AudioNodeEngine class; r=roc

content/media/AudioNodeEngine.h

@@ -12,6 +12,7 @@
 namespace mozilla {
 
 namespace dom {
+class AudioNode;
 struct ThreeDPoint;
 }
 
@@ -146,12 +147,15 @@ AudioBlockPanStereoToStereo(const float aInputL[WEBAUDIO_BLOCK_SIZE],
  */
 class AudioNodeEngine {
 public:
-  AudioNodeEngine()
+  explicit AudioNodeEngine(dom::AudioNode* aNode)
+    : mNode(aNode)
   {
+    MOZ_ASSERT(mNode, "The engine is constructed with a null node");
     MOZ_COUNT_CTOR(AudioNodeEngine);
   }
   virtual ~AudioNodeEngine()
   {
+    MOZ_ASSERT(!mNode, "The node reference must be already cleared");
     MOZ_COUNT_DTOR(AudioNodeEngine);
   }
 
@@ -199,6 +203,16 @@ public:
   {
     *aOutput = aInput;
   }
+
+  dom::AudioNode* Node() const
+  {
+    MOZ_ASSERT(NS_IsMainThread());
+    return mNode;
+  }
+
+protected:
+  friend class dom::AudioNode;
+  dom::AudioNode* mNode;
 };
 
 }

content/media/webaudio/AnalyserNode.cpp

@@ -21,38 +21,34 @@ class AnalyserNodeEngine : public AudioNodeEngine
   class TransferBuffer : public nsRunnable
   {
   public:
-    TransferBuffer(AnalyserNode* aNode,
+    TransferBuffer(AudioNodeStream* aStream,
                    const AudioChunk& aChunk)
-      : mNode(aNode)
+      : mStream(aStream)
       , mChunk(aChunk)
     {
     }
 
     NS_IMETHOD Run()
     {
-      mNode->AppendChunk(mChunk);
+      nsRefPtr<AnalyserNode> node = static_cast<AnalyserNode*>(mStream->Engine()->Node());
+      if (node) {
+        node->AppendChunk(mChunk);
+      }
       return NS_OK;
     }
 
   private:
-    AnalyserNode* mNode;
+    nsRefPtr<AudioNodeStream> mStream;
     AudioChunk mChunk;
   };
 
 public:
-  explicit AnalyserNodeEngine(AnalyserNode& aNode)
-    : mMutex("AnalyserNodeEngine")
-    , mNode(&aNode)
+  explicit AnalyserNodeEngine(AnalyserNode* aNode)
+    : AudioNodeEngine(aNode)
   {
     MOZ_ASSERT(NS_IsMainThread());
   }
 
-  void DisconnectFromNode()
-  {
-    MutexAutoLock lock(mMutex);
-    mNode = nullptr;
-  }
-
   virtual void ProduceAudioBlock(AudioNodeStream* aStream,
                                  const AudioChunk& aInput,
                                  AudioChunk* aOutput,
@@ -60,17 +56,12 @@ public:
   {
     *aOutput = aInput;
 
-    MutexAutoLock lock(mMutex);
     if (mNode &&
         aInput.mChannelData.Length() > 0) {
-      nsRefPtr<TransferBuffer> transfer = new TransferBuffer(mNode, aInput);
+      nsRefPtr<TransferBuffer> transfer = new TransferBuffer(aStream, aInput);
       NS_DispatchToMainThread(transfer);
     }
   }
-
-private:
-  Mutex mMutex;
-  AnalyserNode* mNode; // weak pointer, cleared by AnalyserNode::DestroyMediaStream
 };
 
 AnalyserNode::AnalyserNode(AudioContext* aContext)
@@ -81,7 +72,7 @@ AnalyserNode::AnalyserNode(AudioContext* aContext)
   , mSmoothingTimeConstant(.8)
   , mWriteIndex(0)
 {
-  mStream = aContext->Graph()->CreateAudioNodeStream(new AnalyserNodeEngine(*this),
+  mStream = aContext->Graph()->CreateAudioNodeStream(new AnalyserNodeEngine(this),
                                                      MediaStreamGraph::INTERNAL_STREAM);
   AllocateBuffer();
 }
@@ -255,11 +246,6 @@ AnalyserNode::ApplyBlackmanWindow(float* aBuffer, uint32_t aSize)
 void
 AnalyserNode::DestroyMediaStream()
 {
-  if (mStream) {
-    AudioNodeStream* ns = static_cast<AudioNodeStream*>(mStream.get());
-    AnalyserNodeEngine* engine = static_cast<AnalyserNodeEngine*>(ns->Engine());
-    engine->DisconnectFromNode();
-  }
   AudioNode::DestroyMediaStream();
 }
 

content/media/webaudio/AudioBufferSourceNode.cpp

@@ -38,7 +38,9 @@ NS_IMPL_RELEASE_INHERITED(AudioBufferSourceNode, AudioNode)
 class AudioBufferSourceNodeEngine : public AudioNodeEngine
 {
 public:
-  explicit AudioBufferSourceNodeEngine(AudioDestinationNode* aDestination) :
+  explicit AudioBufferSourceNodeEngine(AudioNode* aNode,
+                                       AudioDestinationNode* aDestination) :
+    AudioNodeEngine(aNode),
     mStart(0), mStop(TRACK_TICKS_MAX),
     mResampler(nullptr),
     mOffset(0), mDuration(0),
@@ -419,7 +421,7 @@ AudioBufferSourceNode::AudioBufferSourceNode(AudioContext* aContext)
   , mStartCalled(false)
 {
   mStream = aContext->Graph()->CreateAudioNodeStream(
-      new AudioBufferSourceNodeEngine(aContext->Destination()),
+      new AudioBufferSourceNodeEngine(this, aContext->Destination()),
       MediaStreamGraph::INTERNAL_STREAM);
   mStream->AddMainThreadListener(this);
 }

content/media/webaudio/AudioDestinationNode.cpp

@@ -18,7 +18,7 @@ NS_IMPL_ISUPPORTS_INHERITED0(AudioDestinationNode, AudioNode)
 AudioDestinationNode::AudioDestinationNode(AudioContext* aContext, MediaStreamGraph* aGraph)
   : AudioNode(aContext)
 {
-  mStream = aGraph->CreateAudioNodeStream(new AudioNodeEngine(),
+  mStream = aGraph->CreateAudioNodeStream(new AudioNodeEngine(this),
                                           MediaStreamGraph::EXTERNAL_STREAM);
 }
 

content/media/webaudio/AudioNode.cpp

@@ -201,5 +201,14 @@ AudioNode::Disconnect(uint32_t aOutput, ErrorResult& aRv)
   Context()->UpdatePannerSource();
 }
 
+void
+AudioNode::UnbindFromEngine()
+{
+  AudioNodeStream* ns = static_cast<AudioNodeStream*>(mStream.get());
+  MOZ_ASSERT(ns, "How come we don't have a stream here?");
+  MOZ_ASSERT(ns->Engine()->mNode == this, "Invalid node reference");
+  ns->Engine()->mNode = nullptr;
+}
+
 }
 }

content/media/webaudio/AudioNode.h

@@ -83,6 +83,7 @@ public:
   virtual void DestroyMediaStream()
   {
     if (mStream) {
+      UnbindFromEngine();
       mStream->Destroy();
       mStream = nullptr;
     }
@@ -152,6 +153,8 @@ private:
   // This could possibly delete 'this'.
   void DisconnectFromGraph();
 
+  void UnbindFromEngine();
+
 protected:
   static void Callback(AudioNode* aNode) { /* not implemented */ }
 

content/media/webaudio/BiquadFilterNode.cpp

@@ -26,8 +26,9 @@ NS_IMPL_RELEASE_INHERITED(BiquadFilterNode, AudioNode)
 class BiquadFilterNodeEngine : public AudioNodeEngine
 {
 public:
-  explicit BiquadFilterNodeEngine(AudioDestinationNode* aDestination)
-    : mSource(nullptr)
+  BiquadFilterNodeEngine(AudioNode* aNode, AudioDestinationNode* aDestination)
+    : AudioNodeEngine(aNode)
+    , mSource(nullptr)
     , mDestination(static_cast<AudioNodeStream*> (aDestination->Stream()))
     // Keep the default values in sync with the default values in
     // BiquadFilterNode::BiquadFilterNode
@@ -103,7 +104,7 @@ BiquadFilterNode::BiquadFilterNode(AudioContext* aContext)
   , mQ(new AudioParam(this, SendQToStream, 1.f))
   , mGain(new AudioParam(this, SendGainToStream, 0.f))
 {
-  BiquadFilterNodeEngine* engine = new BiquadFilterNodeEngine(aContext->Destination());
+  BiquadFilterNodeEngine* engine = new BiquadFilterNodeEngine(this, aContext->Destination());
   mStream = aContext->Graph()->CreateAudioNodeStream(engine, MediaStreamGraph::INTERNAL_STREAM);
   engine->SetSourceStream(static_cast<AudioNodeStream*> (mStream.get()));
 }

content/media/webaudio/DelayNode.cpp

@@ -29,32 +29,35 @@ class DelayNodeEngine : public AudioNodeEngine
   {
   public:
     enum ChangeType { ADDREF, RELEASE };
-    PlayingRefChanged(DelayNode& aNode, ChangeType aChange)
-      : mNode(aNode)
+    PlayingRefChanged(AudioNodeStream* aStream, ChangeType aChange)
+      : mStream(aStream)
       , mChange(aChange)
     {
     }
 
     NS_IMETHOD Run()
     {
-      if (mChange == ADDREF) {
-        mNode.mPlayingRef.Take(&mNode);
-      } else if (mChange == RELEASE) {
-        mNode.mPlayingRef.Drop(&mNode);
+      nsRefPtr<DelayNode> node = static_cast<DelayNode*>(mStream->Engine()->Node());
+      if (node) {
+        if (mChange == ADDREF) {
+          node->mPlayingRef.Take(node);
+        } else if (mChange == RELEASE) {
+          node->mPlayingRef.Drop(node);
+        }
       }
       return NS_OK;
     }
 
   private:
-    DelayNode& mNode;
+    nsRefPtr<AudioNodeStream> mStream;
     ChangeType mChange;
   };
 
 public:
-  DelayNodeEngine(AudioDestinationNode* aDestination, DelayNode& aDelay)
-    : mSource(nullptr)
+  DelayNodeEngine(AudioNode* aNode, AudioDestinationNode* aDestination)
+    : AudioNodeEngine(aNode)
+    , mSource(nullptr)
     , mDestination(static_cast<AudioNodeStream*> (aDestination->Stream()))
-    , mDelayNode(aDelay)
     // Keep the default value in sync with the default value in DelayNode::DelayNode.
     , mDelay(0.f)
     , mMaxDelay(0.)
@@ -136,7 +139,7 @@ public:
       mLeftOverData = static_cast<int32_t>(mCurrentDelayTime * IdealAudioRate());
 
       nsRefPtr<PlayingRefChanged> refchanged =
-        new PlayingRefChanged(mDelayNode, PlayingRefChanged::ADDREF);
+        new PlayingRefChanged(aStream, PlayingRefChanged::ADDREF);
       NS_DispatchToMainThread(refchanged);
     } else if (mLeftOverData != INT32_MIN) {
       mLeftOverData -= WEBAUDIO_BLOCK_SIZE;
@@ -145,7 +148,7 @@ public:
         playedBackAllLeftOvers = true;
 
         nsRefPtr<PlayingRefChanged> refchanged =
-          new PlayingRefChanged(mDelayNode, PlayingRefChanged::RELEASE);
+          new PlayingRefChanged(aStream, PlayingRefChanged::RELEASE);
         NS_DispatchToMainThread(refchanged);
       }
     }
@@ -244,7 +247,6 @@ public:
 
   AudioNodeStream* mSource;
   AudioNodeStream* mDestination;
-  DelayNode& mDelayNode;
   AudioParamTimeline mDelay;
   // Maximum delay time in seconds
   double mMaxDelay;
@@ -264,7 +266,7 @@ DelayNode::DelayNode(AudioContext* aContext, double aMaxDelay)
   : AudioNode(aContext)
   , mDelay(new AudioParam(this, SendDelayToStream, 0.0f))
 {
-  DelayNodeEngine* engine = new DelayNodeEngine(aContext->Destination(), *this);
+  DelayNodeEngine* engine = new DelayNodeEngine(this, aContext->Destination());
   mStream = aContext->Graph()->CreateAudioNodeStream(engine, MediaStreamGraph::INTERNAL_STREAM);
   engine->SetSourceStream(static_cast<AudioNodeStream*> (mStream.get()));
   AudioNodeStream* ns = static_cast<AudioNodeStream*>(mStream.get());

content/media/webaudio/DynamicsCompressorNode.cpp

@@ -31,8 +31,10 @@ NS_IMPL_RELEASE_INHERITED(DynamicsCompressorNode, AudioNode)
 class DynamicsCompressorNodeEngine : public AudioNodeEngine
 {
 public:
-  explicit DynamicsCompressorNodeEngine(AudioDestinationNode* aDestination)
-    : mSource(nullptr)
+  explicit DynamicsCompressorNodeEngine(AudioNode* aNode,
+                                        AudioDestinationNode* aDestination)
+    : AudioNodeEngine(aNode)
+    , mSource(nullptr)
     , mDestination(static_cast<AudioNodeStream*> (aDestination->Stream()))
     // Keep the default value in sync with the default value in
     // DynamicsCompressorNode::DynamicsCompressorNode.
@@ -120,7 +122,7 @@ DynamicsCompressorNode::DynamicsCompressorNode(AudioContext* aContext)
   , mAttack(new AudioParam(this, SendAttackToStream, 0.003f))
   , mRelease(new AudioParam(this, SendReleaseToStream, 0.25f))
 {
-  DynamicsCompressorNodeEngine* engine = new DynamicsCompressorNodeEngine(aContext->Destination());
+  DynamicsCompressorNodeEngine* engine = new DynamicsCompressorNodeEngine(this, aContext->Destination());
   mStream = aContext->Graph()->CreateAudioNodeStream(engine, MediaStreamGraph::INTERNAL_STREAM);
   engine->SetSourceStream(static_cast<AudioNodeStream*> (mStream.get()));
 }

content/media/webaudio/GainNode.cpp

@@ -26,8 +26,9 @@ NS_IMPL_RELEASE_INHERITED(GainNode, AudioNode)
 class GainNodeEngine : public AudioNodeEngine
 {
 public:
-  explicit GainNodeEngine(AudioDestinationNode* aDestination)
-    : mSource(nullptr)
+  GainNodeEngine(AudioNode* aNode, AudioDestinationNode* aDestination)
+    : AudioNodeEngine(aNode)
+    , mSource(nullptr)
     , mDestination(static_cast<AudioNodeStream*> (aDestination->Stream()))
     // Keep the default value in sync with the default value in GainNode::GainNode.
     , mGain(1.f)
@@ -97,7 +98,7 @@ GainNode::GainNode(AudioContext* aContext)
   : AudioNode(aContext)
   , mGain(new AudioParam(this, SendGainToStream, 1.0f))
 {
-  GainNodeEngine* engine = new GainNodeEngine(aContext->Destination());
+  GainNodeEngine* engine = new GainNodeEngine(this, aContext->Destination());
   mStream = aContext->Graph()->CreateAudioNodeStream(engine, MediaStreamGraph::INTERNAL_STREAM);
   engine->SetSourceStream(static_cast<AudioNodeStream*> (mStream.get()));
 }

content/media/webaudio/PannerNode.cpp

@@ -18,9 +18,10 @@ using namespace std;
 class PannerNodeEngine : public AudioNodeEngine
 {
 public:
-  PannerNodeEngine()
+  explicit PannerNodeEngine(AudioNode* aNode)
+    : AudioNodeEngine(aNode)
     // Please keep these default values consistent with PannerNode::PannerNode below.
-    : mPanningModel(PanningModelTypeValues::HRTF)
+    , mPanningModel(PanningModelTypeValues::HRTF)
     , mPanningModelFunction(&PannerNodeEngine::HRTFPanningFunction)
     , mDistanceModel(DistanceModelTypeValues::Inverse)
     , mDistanceModelFunction(&PannerNodeEngine::InverseGainFunction)
@@ -172,7 +173,7 @@ PannerNode::PannerNode(AudioContext* aContext)
   , mConeOuterAngle(360.)
   , mConeOuterGain(0.)
 {
-  mStream = aContext->Graph()->CreateAudioNodeStream(new PannerNodeEngine(),
+  mStream = aContext->Graph()->CreateAudioNodeStream(new PannerNodeEngine(this),
                                                      MediaStreamGraph::INTERNAL_STREAM);
   // We should register once we have set up our stream and engine.
   Context()->Listener()->RegisterPannerNode(this);