Bug 1160064 - Do watching/mirroring initialization on the state machine task queue. r=jww
authorBobby Holley <bobbyholley@gmail.com>
Thu, 30 Apr 2015 15:46:49 -0700
changeset 273328 6979a9e3e60a8b10c986d45cda2ed90b3d7efbab
parent 273327 fe579e65165e645aa6f92475a44a0c60a1c8f7a9
child 273329 924a8ed0b89cf23e388e2a673866354ff67f7d80
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjww
bugs1160064
milestone40.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 1160064 - Do watching/mirroring initialization on the state machine task queue. r=jww We take this as an opportunity to remove connect-during-initialization. Always connecting from the owner thread feels like a stronger invariant.
dom/media/MediaDecoderStateMachine.cpp
dom/media/MediaDecoderStateMachine.h
dom/media/StateMirroring.h
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -208,21 +208,19 @@ MediaDecoderStateMachine::MediaDecoderSt
   mDispatchedStateMachine(false),
   mDelayedScheduler(this),
   mState(DECODER_STATE_DECODING_NONE, "MediaDecoderStateMachine::mState"),
   mPlayDuration(0),
   mStartTime(-1),
   mEndTime(-1),
   mDurationSet(false),
   mPlayState(mTaskQueue, MediaDecoder::PLAY_STATE_LOADING,
-             "MediaDecoderStateMachine::mPlayState (Mirror)",
-             aDecoder->CanonicalPlayState()),
+             "MediaDecoderStateMachine::mPlayState (Mirror)"),
   mNextPlayState(mTaskQueue, MediaDecoder::PLAY_STATE_PAUSED,
-                 "MediaDecoderStateMachine::mNextPlayState (Mirror)",
-                 aDecoder->CanonicalNextPlayState()),
+                 "MediaDecoderStateMachine::mNextPlayState (Mirror)"),
   mNextFrameStatus(mTaskQueue, MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED,
                    "MediaDecoderStateMachine::mNextFrameStatus (Canonical)"),
   mFragmentEndTime(-1),
   mReader(aReader),
   mCurrentFrameTime(0),
   mAudioStartTime(-1),
   mAudioEndTime(-1),
   mDecodedAudioEndTime(-1),
@@ -254,19 +252,19 @@ MediaDecoderStateMachine::MediaDecoderSt
   mDecodingFrozenAtStateDecoding(false),
   mSentLoadedMetadataEvent(false),
   mSentFirstFrameLoadedEvent(false),
   mSentPlaybackEndedEvent(false)
 {
   MOZ_COUNT_CTOR(MediaDecoderStateMachine);
   NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
 
-  // Initialize watchers.
-  mWatchManager.Watch(mState, &MediaDecoderStateMachine::UpdateNextFrameStatus);
-  mWatchManager.Watch(mAudioCompleted, &MediaDecoderStateMachine::UpdateNextFrameStatus);
+  // Dispatch initialization that needs to happen on that task queue.
+  nsCOMPtr<nsIRunnable> r = NS_NewRunnableMethod(this, &MediaDecoderStateMachine::InitializationTask);
+  mTaskQueue->Dispatch(r.forget());
 
   static bool sPrefCacheInit = false;
   if (!sPrefCacheInit) {
     sPrefCacheInit = true;
     Preferences::AddUintVarCache(&sVideoQueueDefaultSize,
                                  "media.video-queue.default-size",
                                  MAX_VIDEO_QUEUE_SIZE);
     Preferences::AddUintVarCache(&sVideoQueueHWAccelSize,
@@ -296,16 +294,30 @@ MediaDecoderStateMachine::~MediaDecoderS
 
   mReader = nullptr;
 
 #ifdef XP_WIN
   timeEndPeriod(1);
 #endif
 }
 
+void
+MediaDecoderStateMachine::InitializationTask()
+{
+  MOZ_ASSERT(OnTaskQueue());
+
+  // Connect mirrors.
+  mPlayState.Connect(mDecoder->CanonicalPlayState());
+  mNextPlayState.Connect(mDecoder->CanonicalNextPlayState());
+
+  // Initialize watchers.
+  mWatchManager.Watch(mState, &MediaDecoderStateMachine::UpdateNextFrameStatus);
+  mWatchManager.Watch(mAudioCompleted, &MediaDecoderStateMachine::UpdateNextFrameStatus);
+}
+
 bool MediaDecoderStateMachine::HasFutureAudio() {
   AssertCurrentThreadInMonitor();
   NS_ASSERTION(HasAudio(), "Should only call HasFutureAudio() when we have audio");
   // We've got audio ready to play if:
   // 1. We've not completed playback of audio, and
   // 2. we either have more than the threshold of decoded audio available, or
   //    we've completely decoded all audio (but not finished playing it yet
   //    as per 1).
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/dom/media/MediaDecoderStateMachine.h
@@ -158,16 +158,21 @@ public:
   void SetAudioCaptured();
 
   // Check if the decoder needs to become dormant state.
   bool IsDormantNeeded();
   // Set/Unset dormant state.
   void SetDormant(bool aDormant);
 
 private:
+  // Initialization that needs to happen on the task queue. This is the first
+  // task that gets run on the task queue, and is dispatched from the MDSM
+  // constructor immediately after the task queue is created.
+  void InitializationTask();
+
   void Shutdown();
 public:
 
   void DispatchShutdown()
   {
     TaskQueue()->Dispatch(NS_NewRunnableMethod(this, &MediaDecoderStateMachine::Shutdown));
   }
 
--- a/dom/media/StateMirroring.h
+++ b/dom/media/StateMirroring.h
@@ -267,20 +267,19 @@ private:
  * Mirror<T> is intended to be used as a member variable, so it doesn't actually
  * inherit AbstractMirror<T> (a refcounted type). Rather, it contains an inner
  * class called |Impl| that implements most of the interesting logic.
  */
 template<typename T>
 class Mirror
 {
 public:
-  Mirror(AbstractThread* aThread, const T& aInitialValue, const char* aName,
-         AbstractCanonical<T>* aCanonical = nullptr)
+  Mirror(AbstractThread* aThread, const T& aInitialValue, const char* aName)
   {
-    mImpl = new Impl(aThread, aInitialValue, aName, aCanonical);
+    mImpl = new Impl(aThread, aInitialValue, aName);
   }
 
   ~Mirror()
   {
     if (mImpl->OwnerThread()->IsCurrentThreadIn()) {
       mImpl->DisconnectIfConnected();
     } else {
       // If holder destruction happens on a thread other than the mirror's
@@ -291,25 +290,20 @@ public:
   }
 
 private:
   class Impl : public AbstractMirror<T>, public WatchTarget
   {
   public:
     using AbstractMirror<T>::OwnerThread;
 
-    Impl(AbstractThread* aThread, const T& aInitialValue, const char* aName,
-         AbstractCanonical<T>* aCanonical)
+    Impl(AbstractThread* aThread, const T& aInitialValue, const char* aName)
       : AbstractMirror<T>(aThread), WatchTarget(aName), mValue(aInitialValue)
     {
       MIRROR_LOG("%s [%p] initialized", mName, this);
-
-      if (aCanonical) {
-        ConnectInternal(aCanonical);
-      }
     }
 
     operator const T&()
     {
       MOZ_ASSERT(OwnerThread()->IsCurrentThreadIn());
       return mValue;
     }
 
@@ -328,26 +322,18 @@ private:
       MOZ_ASSERT(OwnerThread()->IsCurrentThreadIn());
       mCanonical = nullptr;
     }
 
     bool IsConnected() const { return !!mCanonical; }
 
     void Connect(AbstractCanonical<T>* aCanonical)
     {
+      MIRROR_LOG("%s [%p] Connecting to %p", mName, this, aCanonical);
       MOZ_ASSERT(OwnerThread()->IsCurrentThreadIn());
-      ConnectInternal(aCanonical);
-    }
-
-  private:
-    // We separate the guts of Connect into a helper so that we can call it from
-    // initialization while not necessarily on the owner thread.
-    void ConnectInternal(AbstractCanonical<T>* aCanonical)
-    {
-      MIRROR_LOG("%s [%p] Connecting to %p", mName, this, aCanonical);
       MOZ_ASSERT(!IsConnected());
 
       nsCOMPtr<nsIRunnable> r = NS_NewRunnableMethodWithArg<StorensRefPtrPassByPtr<AbstractMirror<T>>>
                                   (aCanonical, &AbstractCanonical<T>::AddMirror, this);
       aCanonical->OwnerThread()->Dispatch(r.forget(), AbstractThread::DontAssertDispatchSuccess);
       mCanonical = aCanonical;
     }
   public: