Switch mirror/canonical initialization to happen in the constructor

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

@@ -591,6 +591,9 @@ bool MediaDecoder::IsInfinite()
 
 MediaDecoder::MediaDecoder() :
   mWatchManager(this),
+  mNextFrameStatus(AbstractThread::MainThread(),
+                   MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED,
+                   "MediaDecoder::mNextFrameStatus (Mirror)"),
   mDecoderPosition(0),
   mPlaybackPosition(0),
   mCurrentTime(0.0),
@@ -601,6 +604,10 @@ MediaDecoder::MediaDecoder() :
   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),
@@ -623,14 +630,6 @@ MediaDecoder::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.

dom/media/MediaDecoderStateMachine.cpp

@@ -213,6 +213,14 @@ MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder,
   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),
@@ -253,16 +261,7 @@ MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder,
   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);
 

dom/media/StateMirroring.h

@@ -112,8 +112,12 @@ 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
@@ -231,13 +235,6 @@ private:
   };
 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(); }
 
@@ -272,10 +269,14 @@ 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 {
@@ -371,14 +372,6 @@ private:
   };
 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(); }