Bug 1160064 - Switch mirror/canonical initialization to happen in the constructor. r=jww
authorBobby Holley <bobbyholley@gmail.com>
Thu, 30 Apr 2015 00:21:43 -0700
changeset 273327 fe579e65165e645aa6f92475a44a0c60a1c8f7a9
parent 273326 931d1e1603f482a7d2048ccd1bde0f7b6e781a68
child 273328 6979a9e3e60a8b10c986d45cda2ed90b3d7efbab
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 - Switch mirror/canonical initialization to happen in the constructor. r=jww The goal here is to hoist all meaningful watcher/mirror/canonical manipulation onto the owner thread. But since that must necessarily happen asynchronously, we need to make sure that canonicals are in a sane state immediately upon creation, since otherwise a mirror from another thread may attempt to connect to a not-yet-initialized canonical.
dom/media/MediaDecoder.cpp
dom/media/MediaDecoderStateMachine.cpp
dom/media/StateMirroring.h
--- a/dom/media/MediaDecoder.cpp
+++ b/dom/media/MediaDecoder.cpp
@@ -591,26 +591,33 @@ void MediaDecoder::SetInfinite(bool aInf
 bool MediaDecoder::IsInfinite()
 {
   MOZ_ASSERT(NS_IsMainThread());
   return mInfiniteStream;
 }
 
 MediaDecoder::MediaDecoder() :
   mWatchManager(this),
+  mNextFrameStatus(AbstractThread::MainThread(),
+                   MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED,
+                   "MediaDecoder::mNextFrameStatus (Mirror)"),
   mDecoderPosition(0),
   mPlaybackPosition(0),
   mCurrentTime(0.0),
   mInitialVolume(0.0),
   mInitialPlaybackRate(1.0),
   mInitialPreservesPitch(true),
   mDuration(-1),
   mMediaSeekable(true),
   mSameOriginMedia(false),
   mReentrantMonitor("media.decoder"),
+  mPlayState(AbstractThread::MainThread(), PLAY_STATE_LOADING,
+             "MediaDecoder::mPlayState (Canonical)"),
+  mNextState(AbstractThread::MainThread(), PLAY_STATE_PAUSED,
+             "MediaDecoder::mNextState (Canonical)"),
   mIgnoreProgressData(false),
   mInfiniteStream(false),
   mOwner(nullptr),
   mPlaybackStatistics(new MediaChannelStatistics()),
   mPinnedForSeek(false),
   mShuttingDown(false),
   mPausedForPlaybackRateNull(false),
   mMinimizePreroll(false),
@@ -623,25 +630,16 @@ MediaDecoder::MediaDecoder() :
     Preferences::GetInt("media.decoder.heuristic.dormant.timeout",
                         DEFAULT_HEURISTIC_DORMANT_TIMEOUT_MSECS)),
   mIsHeuristicDormant(false)
 {
   MOZ_COUNT_CTOR(MediaDecoder);
   MOZ_ASSERT(NS_IsMainThread());
   MediaMemoryTracker::AddMediaDecoder(this);
 
-  // Initialize canonicals.
-  mPlayState.Init(AbstractThread::MainThread(), PLAY_STATE_LOADING, "MediaDecoder::mPlayState (Canonical)");
-  mNextState.Init(AbstractThread::MainThread(), PLAY_STATE_PAUSED, "MediaDecoder::mNextState (Canonical)");
-
-  // Initialize mirrors.
-  mNextFrameStatus.Init(AbstractThread::MainThread(), MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED,
-                        "MediaDecoder::mNextFrameStatus (Mirror)");
-
-
   mAudioChannel = AudioChannelService::GetDefaultAudioChannel();
 
   // Initialize watchers.
   mWatchManager.Watch(mPlayState, &MediaDecoder::UpdateReadyState);
   mWatchManager.Watch(mNextFrameStatus, &MediaDecoder::UpdateReadyState);
 }
 
 bool MediaDecoder::Init(MediaDecoderOwner* aOwner)
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -207,16 +207,24 @@ MediaDecoderStateMachine::MediaDecoderSt
   mRealTime(aRealTime),
   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()),
+  mNextPlayState(mTaskQueue, MediaDecoder::PLAY_STATE_PAUSED,
+                 "MediaDecoderStateMachine::mNextPlayState (Mirror)",
+                 aDecoder->CanonicalNextPlayState()),
+  mNextFrameStatus(mTaskQueue, MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED,
+                   "MediaDecoderStateMachine::mNextFrameStatus (Canonical)"),
   mFragmentEndTime(-1),
   mReader(aReader),
   mCurrentFrameTime(0),
   mAudioStartTime(-1),
   mAudioEndTime(-1),
   mDecodedAudioEndTime(-1),
   mVideoFrameEndTime(-1),
   mDecodedVideoEndTime(-1),
@@ -246,26 +254,17 @@ MediaDecoderStateMachine::MediaDecoderSt
   mDecodingFrozenAtStateDecoding(false),
   mSentLoadedMetadataEvent(false),
   mSentFirstFrameLoadedEvent(false),
   mSentPlaybackEndedEvent(false)
 {
   MOZ_COUNT_CTOR(MediaDecoderStateMachine);
   NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
 
-  // Initialize canonicals.
-  mNextFrameStatus.Init(mTaskQueue, MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED,
-                        "MediaDecoderStateMachine::mNextFrameStatus (Canonical)");
-
-  // Initialize mirrors.
-  mPlayState.Init(mTaskQueue, MediaDecoder::PLAY_STATE_LOADING, "MediaDecoderStateMachine::mPlayState (Mirror)",
-                  aDecoder->CanonicalPlayState());
-  mNextPlayState.Init(mTaskQueue, MediaDecoder::PLAY_STATE_PAUSED, "MediaDecoderStateMachine::mNextPlayState (Mirror)",
-                  aDecoder->CanonicalNextPlayState());
-
+  // Initialize watchers.
   mWatchManager.Watch(mState, &MediaDecoderStateMachine::UpdateNextFrameStatus);
   mWatchManager.Watch(mAudioCompleted, &MediaDecoderStateMachine::UpdateNextFrameStatus);
 
   static bool sPrefCacheInit = false;
   if (!sPrefCacheInit) {
     sPrefCacheInit = true;
     Preferences::AddUintVarCache(&sVideoQueueDefaultSize,
                                  "media.video-queue.default-size",
--- a/dom/media/StateMirroring.h
+++ b/dom/media/StateMirroring.h
@@ -107,18 +107,23 @@ protected:
  * Canonical<T> is intended to be used as a member variable, so it doesn't actually
  * inherit AbstractCanonical<T> (a refcounted type). Rather, it contains an inner
  * class called |Impl| that implements most of the interesting logic.
  */
 template<typename T>
 class Canonical
 {
 public:
-  Canonical() {}
-  ~Canonical() { MOZ_DIAGNOSTIC_ASSERT(mImpl, "Should have initialized me"); }
+  Canonical(AbstractThread* aThread, const T& aInitialValue, const char* aName)
+  {
+    mImpl = new Impl(aThread, aInitialValue, aName);
+  }
+
+
+  ~Canonical() {}
 
 private:
   class Impl : public AbstractCanonical<T>, public WatchTarget
   {
   public:
     using AbstractCanonical<T>::OwnerThread;
 
     Impl(AbstractThread* aThread, const T& aInitialValue, const char* aName)
@@ -228,21 +233,16 @@ private:
     T mValue;
     Maybe<T> mInitialValue;
     nsTArray<nsRefPtr<AbstractMirror<T>>> mMirrors;
   };
 public:
 
   // NB: Because mirror-initiated disconnection can race with canonical-
   // initiated disconnection, a canonical should never be reinitialized.
-  void Init(AbstractThread* aThread, const T& aInitialValue, const char* aName)
-  {
-    mImpl = new Impl(aThread, aInitialValue, aName);
-  }
-
   // Forward control operations to the Impl.
   void DisconnectAll() { return mImpl->DisconnectAll(); }
 
   // Access to the Impl.
   operator Impl&() { return *mImpl; }
   Impl* operator&() { return mImpl; }
 
   // Access to the T.
@@ -267,20 +267,24 @@ 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() {}
+  Mirror(AbstractThread* aThread, const T& aInitialValue, const char* aName,
+         AbstractCanonical<T>* aCanonical = nullptr)
+  {
+    mImpl = new Impl(aThread, aInitialValue, aName, aCanonical);
+  }
+
   ~Mirror()
   {
-    MOZ_DIAGNOSTIC_ASSERT(mImpl, "Should have initialized me");
     if (mImpl->OwnerThread()->IsCurrentThreadIn()) {
       mImpl->DisconnectIfConnected();
     } else {
       // If holder destruction happens on a thread other than the mirror's
       // owner thread, manual disconnection is mandatory. We should make this
       // more automatic by hooking it up to task queue shutdown.
       MOZ_DIAGNOSTIC_ASSERT(!mImpl->IsConnected());
     }
@@ -366,24 +370,16 @@ private:
     ~Impl() { MOZ_DIAGNOSTIC_ASSERT(!IsConnected()); }
 
   private:
     T mValue;
     nsRefPtr<AbstractCanonical<T>> mCanonical;
   };
 public:
 
-  // NB: Because mirror-initiated disconnection can race with canonical-
-  // initiated disconnection, a mirror should never be reinitialized.
-  void Init(AbstractThread* aThread, const T& aInitialValue, const char* aName,
-            AbstractCanonical<T>* aCanonical = nullptr)
-  {
-    mImpl = new Impl(aThread, aInitialValue, aName, aCanonical);
-  }
-
   // Forward control operations to the Impl<T>.
   void Connect(AbstractCanonical<T>* aCanonical) { mImpl->Connect(aCanonical); }
   void DisconnectIfConnected() { mImpl->DisconnectIfConnected(); }
 
   // Access to the Impl<T>.
   operator Impl&() { return *mImpl; }
   Impl* operator&() { return mImpl; }