Bug 1044514 - AudioDestinationNode should not be kept alive by the event listener, r=ehsan
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 29 Jul 2014 16:30:40 +0100
changeset 219523 e8a6a7a2212502f15564c671a363601b7dcd5a19
parent 219522 8f4b3b59f1a4d7dc52aa63c04e54eadfc5e27006
child 219524 f163c6ca88ee4c080e56533326e42e10be637143
push id583
push userbhearsum@mozilla.com
push dateMon, 24 Nov 2014 19:04:58 +0000
treeherdermozilla-release@c107e74250f4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1044514
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1044514 - AudioDestinationNode should not be kept alive by the event listener, r=ehsan
content/media/webaudio/AudioContext.cpp
content/media/webaudio/AudioDestinationNode.cpp
content/media/webaudio/AudioDestinationNode.h
--- a/content/media/webaudio/AudioContext.cpp
+++ b/content/media/webaudio/AudioContext.cpp
@@ -91,19 +91,20 @@ AudioContext::AudioContext(nsPIDOMWindow
   , mIsShutDown(false)
 {
   aWindow->AddAudioContext(this);
 
   // Note: AudioDestinationNode needs an AudioContext that must already be
   // bound to the window.
   mDestination = new AudioDestinationNode(this, aIsOffline, aChannel,
                                           aNumberOfChannels, aLength, aSampleRate);
-  // We skip calling SetIsOnlyNodeForContext during mDestination's constructor,
-  // because we can only call SetIsOnlyNodeForContext after mDestination has
-  // been set up.
+  // We skip calling SetIsOnlyNodeForContext and the creation of the
+  // audioChannelAgent during mDestination's constructor, because we can only
+  // call them after mDestination has been set up.
+  mDestination->CreateAudioChannelAgent();
   mDestination->SetIsOnlyNodeForContext(true);
 }
 
 AudioContext::~AudioContext()
 {
   nsPIDOMWindow* window = GetOwner();
   if (window) {
     window->RemoveAudioContext(this);
--- a/content/media/webaudio/AudioDestinationNode.cpp
+++ b/content/media/webaudio/AudioDestinationNode.cpp
@@ -253,18 +253,50 @@ private:
   bool mLastInputMuted;
 };
 
 static bool UseAudioChannelService()
 {
   return Preferences::GetBool("media.useAudioChannelService");
 }
 
+class EventProxyHandler MOZ_FINAL : public nsIDOMEventListener
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  explicit EventProxyHandler(nsIDOMEventListener* aNode)
+  {
+    MOZ_ASSERT(aNode);
+    mWeakNode = do_GetWeakReference(aNode);
+  }
+
+  // nsIDOMEventListener
+  NS_IMETHOD HandleEvent(nsIDOMEvent* aEvent) MOZ_OVERRIDE
+  {
+    nsCOMPtr<nsIDOMEventListener> listener = do_QueryReferent(mWeakNode);
+    if (!listener) {
+      return NS_OK;
+    }
+
+    auto node = static_cast<AudioDestinationNode*>(listener.get());
+    return node->HandleEvent(aEvent);
+  }
+
+private:
+  ~EventProxyHandler()
+  { }
+
+  nsWeakPtr mWeakNode;
+};
+
+NS_IMPL_ISUPPORTS(EventProxyHandler, nsIDOMEventListener)
+
 NS_IMPL_CYCLE_COLLECTION_INHERITED(AudioDestinationNode, AudioNode,
-                                   mAudioChannelAgent)
+                                   mAudioChannelAgent, mEventProxyHelper)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(AudioDestinationNode)
   NS_INTERFACE_MAP_ENTRY(nsIDOMEventListener)
   NS_INTERFACE_MAP_ENTRY(nsIAudioChannelAgentCallback)
 NS_INTERFACE_MAP_END_INHERITING(AudioNode)
 
 NS_IMPL_ADDREF_INHERITED(AudioDestinationNode, AudioNode)
 NS_IMPL_RELEASE_INHERITED(AudioDestinationNode, AudioNode)
@@ -300,27 +332,16 @@ AudioDestinationNode::AudioDestinationNo
   mStream->SetAudioChannelType(aChannel);
   mStream->AddMainThreadListener(this);
   mStream->AddAudioOutput(&gWebAudioOutputKey);
 
   if (aChannel != AudioChannel::Normal) {
     ErrorResult rv;
     SetMozAudioChannelType(aChannel, rv);
   }
-
-  if (!aIsOffline && UseAudioChannelService()) {
-    nsCOMPtr<nsIDOMEventTarget> target = do_QueryInterface(GetOwner());
-    if (target) {
-      target->AddSystemEventListener(NS_LITERAL_STRING("visibilitychange"), this,
-                                     /* useCapture = */ true,
-                                     /* wantsUntrusted = */ false);
-    }
-
-    CreateAudioChannelAgent();
-  }
 }
 
 AudioDestinationNode::~AudioDestinationNode()
 {
 }
 
 size_t
 AudioDestinationNode::SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const
@@ -342,17 +363,18 @@ AudioDestinationNode::DestroyMediaStream
 {
   if (mAudioChannelAgent && !Context()->IsOffline()) {
     mAudioChannelAgent->StopPlaying();
     mAudioChannelAgent = nullptr;
 
     nsCOMPtr<nsIDOMEventTarget> target = do_QueryInterface(GetOwner());
     NS_ENSURE_TRUE_VOID(target);
 
-    target->RemoveSystemEventListener(NS_LITERAL_STRING("visibilitychange"), this,
+    target->RemoveSystemEventListener(NS_LITERAL_STRING("visibilitychange"),
+                                      mEventProxyHelper,
                                       /* useCapture = */ true);
   }
 
   if (!mStream)
     return;
 
   mStream->RemoveMainThreadListener(this);
   MediaStreamGraph* graph = mStream->Graph();
@@ -560,16 +582,34 @@ AudioDestinationNode::CheckAudioChannelP
     &perm);
 
   return perm == nsIPermissionManager::ALLOW_ACTION;
 }
 
 void
 AudioDestinationNode::CreateAudioChannelAgent()
 {
+  if (mIsOffline || !UseAudioChannelService()) {
+    return;
+  }
+
+  if (!mEventProxyHelper) {
+    nsCOMPtr<nsIDOMEventTarget> target = do_QueryInterface(GetOwner());
+    if (target) {
+      // We use a proxy because otherwise the event listerner would hold a
+      // reference of the destination node, and by extension, everything
+      // connected to it.
+      mEventProxyHelper = new EventProxyHandler(this);
+      target->AddSystemEventListener(NS_LITERAL_STRING("visibilitychange"),
+                                     mEventProxyHelper,
+                                     /* useCapture = */ true,
+                                     /* wantsUntrusted = */ false);
+    }
+  }
+
   if (mAudioChannelAgent) {
     mAudioChannelAgent->StopPlaying();
   }
 
   mAudioChannelAgent = new AudioChannelAgent();
   mAudioChannelAgent->InitWithWeakCallback(GetOwner(),
                                            static_cast<int32_t>(mAudioChannel),
                                            this);
--- a/content/media/webaudio/AudioDestinationNode.h
+++ b/content/media/webaudio/AudioDestinationNode.h
@@ -12,16 +12,17 @@
 #include "nsIDOMEventListener.h"
 #include "nsIAudioChannelAgent.h"
 #include "AudioChannelCommon.h"
 
 namespace mozilla {
 namespace dom {
 
 class AudioContext;
+class EventProxyHandler;
 
 class AudioDestinationNode : public AudioNode
                            , public nsIDOMEventListener
                            , public nsIAudioChannelAgentCallback
                            , public MainThreadMediaStreamListener
 {
 public:
   // This node type knows what MediaStreamGraph to use based on
@@ -52,59 +53,62 @@ public:
 
   void Mute();
   void Unmute();
 
   void StartRendering();
 
   void OfflineShutdown();
 
-  // nsIDOMEventListener
+  // nsIDOMEventListener - by proxy
   NS_IMETHOD HandleEvent(nsIDOMEvent* aEvent);
 
   AudioChannel MozAudioChannelType() const;
   void SetMozAudioChannelType(AudioChannel aValue, ErrorResult& aRv);
 
   virtual void NotifyMainThreadStateChanged() MOZ_OVERRIDE;
   void FireOfflineCompletionEvent();
 
   // An amount that should be added to the MediaStream's current time to
   // get the AudioContext.currentTime.
   double ExtraCurrentTime();
 
   // When aIsOnlyNode is true, this is the only node for the AudioContext.
   void SetIsOnlyNodeForContext(bool aIsOnlyNode);
 
+  void CreateAudioChannelAgent();
+
   virtual const char* NodeType() const
   {
     return "AudioDestinationNode";
   }
 
   virtual size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const MOZ_OVERRIDE;
   virtual size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const MOZ_OVERRIDE;
 
   void InputMuted(bool aInputMuted);
 
 protected:
   virtual ~AudioDestinationNode();
 
 private:
   bool CheckAudioChannelPermissions(AudioChannel aValue);
-  void CreateAudioChannelAgent();
 
   void SetCanPlay(bool aCanPlay);
 
   void NotifyStableState();
   void ScheduleStableStateNotification();
 
   SelfReference<AudioDestinationNode> mOfflineRenderingRef;
   uint32_t mFramesToProduce;
 
   nsCOMPtr<nsIAudioChannelAgent> mAudioChannelAgent;
 
+  nsRefPtr<EventProxyHandler> mEventProxyHelper;
+
   // Audio Channel Type.
   AudioChannel mAudioChannel;
   bool mIsOffline;
   bool mHasFinished;
   bool mAudioChannelAgentPlaying;
 
   TimeStamp mStartedBlockingDueToBeingOnlyNode;
   double mExtraCurrentTime;