Bug 1256520 - use SyncRunnable to create DecodedStreamData synchronously to ensure the creation and destruction of DecodedStreamData happen in order. r=kikuo.
authorJW Wang <jwwang@mozilla.com>
Tue, 15 Mar 2016 08:54:52 +0800
changeset 290498 9667b7f27279cfacf1c6edf88810a78d036a498c
parent 290497 c49c1cfcbc7c31eca92fd735ba1bc60c1cc5c450
child 290499 0bdd199c0911e5f60a84af9bb869832aa2eaf023
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskikuo
bugs1256520
milestone48.0a1
Bug 1256520 - use SyncRunnable to create DecodedStreamData synchronously to ensure the creation and destruction of DecodedStreamData happen in order. r=kikuo. This greatly simplify the code because: 1. we don't have to dispatch the newly created DecodedStreamData to the work thread and store it to |mData|. 2. no need to deal with dispatch failure incurred by 1 due to task queue shutdown. (see: https://hg.mozilla.org/mozilla-central/file/f0c0480732d36153e8839c7f17394d45f679f87d/dom/media/mediasink/DecodedStream.cpp#l392) MozReview-Commit-ID: FwySgwKp8dV
dom/media/mediasink/DecodedStream.cpp
dom/media/mediasink/DecodedStream.h
--- a/dom/media/mediasink/DecodedStream.cpp
+++ b/dom/media/mediasink/DecodedStream.cpp
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/CheckedInt.h"
 #include "mozilla/gfx/Point.h"
+#include "mozilla/SyncRunnable.h"
 
 #include "AudioSegment.h"
 #include "DecodedStream.h"
 #include "MediaData.h"
 #include "MediaQueue.h"
 #include "MediaStreamGraph.h"
 #include "OutputStreamManager.h"
 #include "SharedBuffer.h"
@@ -264,42 +265,61 @@ DecodedStream::Start(int64_t aStartTime,
 
   mStartTime.emplace(aStartTime);
   mInfo = aInfo;
   mPlaying = true;
   ConnectListener();
 
   class R : public nsRunnable {
     typedef MozPromiseHolder<GenericPromise> Promise;
-    typedef decltype(&DecodedStream::CreateData) Method;
   public:
-    R(DecodedStream* aThis, Method aMethod, PlaybackInfoInit&& aInit, Promise&& aPromise)
-      : mThis(aThis), mMethod(aMethod), mInit(Move(aInit))
+    R(PlaybackInfoInit&& aInit, Promise&& aPromise, OutputStreamManager* aManager)
+      : mInit(Move(aInit)), mOutputStreamManager(aManager)
     {
       mPromise = Move(aPromise);
     }
     NS_IMETHOD Run() override
     {
-      (mThis->*mMethod)(Move(mInit), Move(mPromise));
+      MOZ_ASSERT(NS_IsMainThread());
+      // No need to create a source stream when there are no output streams. This
+      // happens when RemoveOutput() is called immediately after StartPlayback().
+      if (!mOutputStreamManager->Graph()) {
+        // Resolve the promise to indicate the end of playback.
+        mPromise.Resolve(true, __func__);
+        return NS_OK;
+      }
+      mData = MakeUnique<DecodedStreamData>(
+        mOutputStreamManager, Move(mInit), Move(mPromise));
       return NS_OK;
     }
+    UniquePtr<DecodedStreamData> ReleaseData()
+    {
+      return Move(mData);
+    }
   private:
-    RefPtr<DecodedStream> mThis;
-    Method mMethod;
     PlaybackInfoInit mInit;
     Promise mPromise;
+    RefPtr<OutputStreamManager> mOutputStreamManager;
+    UniquePtr<DecodedStreamData> mData;
   };
 
   MozPromiseHolder<GenericPromise> promise;
   mFinishPromise = promise.Ensure(__func__);
   PlaybackInfoInit init {
     aStartTime, aInfo
   };
-  nsCOMPtr<nsIRunnable> r = new R(this, &DecodedStream::CreateData, Move(init), Move(promise));
-  AbstractThread::MainThread()->Dispatch(r.forget());
+  nsCOMPtr<nsIRunnable> r = new R(Move(init), Move(promise), mOutputStreamManager);
+  nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
+  SyncRunnable::DispatchToThread(mainThread, r);
+  mData = static_cast<R*>(r.get())->ReleaseData();
+
+  if (mData) {
+    mData->SetPlaying(mPlaying);
+    SendData();
+  }
 }
 
 void
 DecodedStream::Stop()
 {
   AssertOwnerThread();
   MOZ_ASSERT(mStartTime.isSome(), "playback not started.");
 
@@ -338,87 +358,16 @@ DecodedStream::DestroyData(UniquePtr<Dec
   DecodedStreamData* data = aData.release();
   nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
     delete data;
   });
   AbstractThread::MainThread()->Dispatch(r.forget());
 }
 
 void
-DecodedStream::CreateData(PlaybackInfoInit&& aInit, MozPromiseHolder<GenericPromise>&& aPromise)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-
-  // No need to create a source stream when there are no output streams. This
-  // happens when RemoveOutput() is called immediately after StartPlayback().
-  if (!mOutputStreamManager->Graph()) {
-    // Resolve the promise to indicate the end of playback.
-    aPromise.Resolve(true, __func__);
-    return;
-  }
-
-  auto data = new DecodedStreamData(mOutputStreamManager, Move(aInit), Move(aPromise));
-
-  class R : public nsRunnable {
-    typedef void(DecodedStream::*Method)(UniquePtr<DecodedStreamData>);
-  public:
-    R(DecodedStream* aThis, Method aMethod, DecodedStreamData* aData)
-      : mThis(aThis), mMethod(aMethod), mData(aData) {}
-    NS_IMETHOD Run() override
-    {
-      (mThis->*mMethod)(Move(mData));
-      return NS_OK;
-    }
-  private:
-    virtual ~R()
-    {
-      // mData is not transferred when dispatch fails and Run() is not called.
-      // We need to dispatch a task to ensure DecodedStreamData is destroyed
-      // properly on the main thread.
-      if (mData) {
-        DecodedStreamData* data = mData.release();
-        nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
-          delete data;
-        });
-        // We are in tail dispatching phase. Don't call
-        // AbstractThread::MainThread()->Dispatch() to avoid reentrant
-        // AutoTaskDispatcher.
-        NS_DispatchToMainThread(r.forget());
-      }
-    }
-    RefPtr<DecodedStream> mThis;
-    Method mMethod;
-    UniquePtr<DecodedStreamData> mData;
-  };
-
-  // Post a message to ensure |mData| is only updated on the worker thread.
-  // Note this could fail when MDSM begin to shut down the worker thread.
-  nsCOMPtr<nsIRunnable> r = new R(this, &DecodedStream::OnDataCreated, data);
-  mOwnerThread->Dispatch(r.forget(), AbstractThread::DontAssertDispatchSuccess);
-}
-
-void
-DecodedStream::OnDataCreated(UniquePtr<DecodedStreamData> aData)
-{
-  AssertOwnerThread();
-  MOZ_ASSERT(!mData, "Already created.");
-
-  // Start to send data to the stream immediately
-  if (mStartTime.isSome()) {
-    aData->SetPlaying(mPlaying);
-    mData = Move(aData);
-    SendData();
-    return;
-  }
-
-  // Playback has ended. Destroy aData which is not needed anymore.
-  DestroyData(Move(aData));
-}
-
-void
 DecodedStream::SetPlaying(bool aPlaying)
 {
   AssertOwnerThread();
 
   // Resume/pause matters only when playback started.
   if (mStartTime.isNothing()) {
     return;
   }
--- a/dom/media/mediasink/DecodedStream.h
+++ b/dom/media/mediasink/DecodedStream.h
@@ -61,19 +61,17 @@ public:
   void Stop() override;
   bool IsStarted() const override;
   bool IsPlaying() const override;
 
 protected:
   virtual ~DecodedStream();
 
 private:
-  void CreateData(PlaybackInfoInit&& aInit, MozPromiseHolder<GenericPromise>&& aPromise);
   void DestroyData(UniquePtr<DecodedStreamData> aData);
-  void OnDataCreated(UniquePtr<DecodedStreamData> aData);
   void AdvanceTracks();
   void SendAudio(double aVolume, bool aIsSameOrigin);
   void SendVideo(bool aIsSameOrigin);
   void SendData();
 
   void AssertOwnerThread() const {
     MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn());
   }