Bug 998168 - nsITimer functions should be only called in the event target thread of the timer. r=cpearce.
authorJW Wang <jwwang@mozilla.com>
Wed, 23 Apr 2014 05:29:14 -0400
changeset 179744 243bf2777fa46e67016265f46ee2e23e889805a6
parent 179743 d97882e9c503b30a66c3e842156d3a152a8adfb1
child 179745 b9e4dac08b6e202a363b14cd7ae01bd3260b9892
push id26639
push userryanvm@gmail.com
push dateWed, 23 Apr 2014 20:42:51 +0000
treeherdermozilla-central@ed0236a51ed3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs998168
milestone31.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 998168 - nsITimer functions should be only called in the event target thread of the timer. r=cpearce.
content/media/MediaDecoderStateMachine.cpp
content/media/MediaDecoderStateMachine.h
--- a/content/media/MediaDecoderStateMachine.cpp
+++ b/content/media/MediaDecoderStateMachine.cpp
@@ -202,17 +202,18 @@ MediaDecoderStateMachine::MediaDecoderSt
   mAudioCompleted(false),
   mGotDurationFromMetaData(false),
   mDispatchedEventToDecode(false),
   mStopAudioThread(true),
   mQuickBuffering(false),
   mMinimizePreroll(false),
   mDecodeThreadWaiting(false),
   mRealTime(aRealTime),
-  mLastFrameStatus(MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED)
+  mLastFrameStatus(MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED),
+  mTimerId(0)
 {
   MOZ_COUNT_CTOR(MediaDecoderStateMachine);
   NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
 
   // Only enable realtime mode when "media.realtime_decoder.enabled" is true.
   if (Preferences::GetBool("media.realtime_decoder.enabled", false) == false)
     mRealTime = false;
 
@@ -242,19 +243,18 @@ MediaDecoderStateMachine::~MediaDecoderS
   NS_ASSERTION(!mPendingWakeDecoder.get(),
                "WakeDecoder should have been revoked already");
 
   if (mDecodeTaskQueue) {
     mDecodeTaskQueue->Shutdown();
     mDecodeTaskQueue = nullptr;
   }
 
-  if (mTimer) {
-    mTimer->Cancel();
-  }
+  // No need to cancel the timer here for we've done that in
+  // TimeoutExpired() triggered by Shutdown()
   mTimer = nullptr;
   mReader = nullptr;
 
 #ifdef XP_WIN
   timeEndPeriod(1);
 #endif
 }
 
@@ -2672,36 +2672,54 @@ nsresult MediaDecoderStateMachine::CallR
   MOZ_ASSERT(!mInRunningStateMachine, "State machine cycles must run in sequence!");
   mTimeout = TimeStamp();
   mInRunningStateMachine = true;
   nsresult res = RunStateMachine();
   mInRunningStateMachine = false;
   return res;
 }
 
-static void TimeoutExpired(nsITimer *aTimer, void *aClosure) {
-  MediaDecoderStateMachine *machine =
-    static_cast<MediaDecoderStateMachine*>(aClosure);
-  NS_ASSERTION(machine, "Must have been passed state machine");
-  machine->TimeoutExpired();
-}
-
-void MediaDecoderStateMachine::TimeoutExpired()
+nsresult MediaDecoderStateMachine::TimeoutExpired(int aTimerId)
 {
   ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
   NS_ASSERTION(OnStateMachineThread(), "Must be on state machine thread");
-  CallRunStateMachine();
+  mTimer->Cancel();
+  if (mTimerId == aTimerId) {
+    return CallRunStateMachine();
+  } else {
+    return NS_OK;
+  }
 }
 
 void MediaDecoderStateMachine::ScheduleStateMachineWithLockAndWakeDecoder() {
   ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
   DispatchAudioDecodeTaskIfNeeded();
   DispatchVideoDecodeTaskIfNeeded();
 }
 
+class TimerEvent : public nsITimerCallback, public nsRunnable {
+  NS_DECL_THREADSAFE_ISUPPORTS
+public:
+  TimerEvent(MediaDecoderStateMachine* aStateMachine, int aTimerId)
+    : mStateMachine(aStateMachine), mTimerId(aTimerId) {}
+
+  NS_IMETHOD Run() MOZ_OVERRIDE {
+    return mStateMachine->TimeoutExpired(mTimerId);
+  }
+
+  NS_IMETHOD Notify(nsITimer* aTimer) {
+    return mStateMachine->TimeoutExpired(mTimerId);
+  }
+private:
+  const nsRefPtr<MediaDecoderStateMachine> mStateMachine;
+  int mTimerId;
+};
+
+NS_IMPL_ISUPPORTS2(TimerEvent, nsITimerCallback, nsIRunnable);
+
 nsresult MediaDecoderStateMachine::ScheduleStateMachine(int64_t aUsecs) {
   AssertCurrentThreadInMonitor();
   NS_ABORT_IF_FALSE(GetStateMachineThread(),
     "Must have a state machine thread to schedule");
 
   if (mState == DECODER_STATE_SHUTDOWN) {
     return NS_ERROR_FAILURE;
   }
@@ -2713,25 +2731,42 @@ nsresult MediaDecoderStateMachine::Sched
     // or have an event dispatched to run the state machine.
     return NS_OK;
   }
 
   uint32_t ms = static_cast<uint32_t>((aUsecs / USECS_PER_MS) & 0xFFFFFFFF);
   if (mRealTime && ms > 40) {
     ms = 40;
   }
-  mTimeout = timeout;
-  // Cancel existing timer if any since we are going to schedule a new one.
-  mTimer->Cancel();
-  nsresult rv = mTimer->InitWithFuncCallback(mozilla::TimeoutExpired,
-                                             this,
-                                             ms,
-                                             nsITimer::TYPE_ONE_SHOT);
-  NS_ENSURE_SUCCESS(rv, rv);
-  return NS_OK;
+
+  // Don't cancel the timer here for this function will be called from
+  // different threads.
+
+  nsresult rv = NS_ERROR_FAILURE;
+  nsRefPtr<TimerEvent> event = new TimerEvent(this, mTimerId+1);
+
+  if (ms == 0) {
+    // Dispatch a runnable to the state machine thread when delay is 0.
+    // It will has less latency than dispatching a runnable to the state
+    // machine thread which will then schedule a zero-delay timer.
+    rv = GetStateMachineThread()->Dispatch(event, NS_DISPATCH_NORMAL);
+  } else if (OnStateMachineThread()) {
+    rv = mTimer->InitWithCallback(event, ms, nsITimer::TYPE_ONE_SHOT);
+  } else {
+    MOZ_ASSERT(false, "non-zero delay timer should be only scheduled in state machine thread");
+  }
+
+  if (NS_SUCCEEDED(rv)) {
+    mTimeout = timeout;
+    ++mTimerId;
+  } else {
+    NS_WARNING("Failed to schedule state machine");
+  }
+
+  return rv;
 }
 
 bool MediaDecoderStateMachine::OnDecodeThread() const
 {
   return mDecodeTaskQueue->IsCurrentThreadIn();
 }
 
 bool MediaDecoderStateMachine::OnStateMachineThread() const
--- a/content/media/MediaDecoderStateMachine.h
+++ b/content/media/MediaDecoderStateMachine.h
@@ -302,17 +302,17 @@ public:
   void ScheduleStateMachineWithLockAndWakeDecoder();
 
   // Schedules the shared state machine thread to run the state machine
   // in aUsecs microseconds from now, if it's not already scheduled to run
   // earlier, in which case the request is discarded.
   nsresult ScheduleStateMachine(int64_t aUsecs = 0);
 
   // Timer function to implement ScheduleStateMachine(aUsecs).
-  void TimeoutExpired();
+  nsresult TimeoutExpired(int aGeneration);
 
   // Set the media fragment end time. aEndTime is in microseconds.
   void SetFragmentEndTime(int64_t aEndTime);
 
   // Drop reference to decoder.  Only called during shutdown dance.
   void ReleaseDecoder() {
     MOZ_ASSERT(mReader);
     if (mReader) {
@@ -932,12 +932,15 @@ private:
 
   // Stores presentation info required for playback. The decoder monitor
   // must be held when accessing this.
   MediaInfo mInfo;
 
   mozilla::MediaMetadataManager mMetadataManager;
 
   MediaDecoderOwner::NextFrameStatus mLastFrameStatus;
+
+  // The id of timer tasks, used to ignore tasks that are scheduled previously.
+  int mTimerId;
 };
 
 } // namespace mozilla;
 #endif