Bug 907162 - Fix MediaDecoderStateMachine might dispatch MediaDecoder::PlaybackEnded more than once and trigger multiple 'ended' events in HTMLMediaElement. r=cpearce, a=sledru
authorJW Wang <jwwang@mozilla.com>
Mon, 24 Mar 2014 11:41:18 -0400
changeset 192243 a5bb91ab9c00b415d51a0a22fcc865b3a4705810
parent 192242 dce0c66c663697e8aa3e97613c6396633a18fc43
child 192244 6a3cd07007aa31a8b04eae97cef386e51db24ee4
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, sledru
bugs907162
milestone30.0a2
Bug 907162 - Fix MediaDecoderStateMachine might dispatch MediaDecoder::PlaybackEnded more than once and trigger multiple 'ended' events in HTMLMediaElement. r=cpearce, a=sledru
content/media/MediaDecoderStateMachine.cpp
--- a/content/media/MediaDecoderStateMachine.cpp
+++ b/content/media/MediaDecoderStateMachine.cpp
@@ -1516,17 +1516,17 @@ MediaDecoderStateMachine::SetReaderActiv
   MOZ_ASSERT(OnDecodeThread());
   mReader->SetActive();
 }
 
 void
 MediaDecoderStateMachine::DispatchDecodeTasksIfNeeded()
 {
   AssertCurrentThreadInMonitor();
-  
+
   // NeedToDecodeAudio() can go from false to true while we hold the
   // monitor, but it can't go from true to false. This can happen because
   // NeedToDecodeAudio() takes into account the amount of decoded audio
   // that's been written to the AudioStream but not played yet. So if we
   // were calling NeedToDecodeAudio() twice and we thread-context switch
   // between the calls, audio can play, which can affect the return value
   // of NeedToDecodeAudio() giving inconsistent results. So we cache the
   // value returned by NeedToDecodeAudio(), and make decisions
@@ -2269,19 +2269,26 @@ nsresult MediaDecoderStateMachine::RunSt
       // When we're decoding to a stream, the stream's main-thread finish signal
       // will take care of calling MediaDecoder::PlaybackEnded.
       if (mDecoder->GetState() == MediaDecoder::PLAY_STATE_PLAYING &&
           !mDecoder->GetDecodedStream()) {
         int64_t videoTime = HasVideo() ? mVideoFrameEndTime : 0;
         int64_t clockTime = std::max(mEndTime, std::max(videoTime, GetAudioClock()));
         UpdatePlaybackPosition(clockTime);
 
-        nsCOMPtr<nsIRunnable> event =
-          NS_NewRunnableMethod(mDecoder, &MediaDecoder::PlaybackEnded);
-        NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
+        {
+          // Wait for the state change is completed in the main thread,
+          // otherwise we might see |mDecoder->GetState() == MediaDecoder::PLAY_STATE_PLAYING|
+          // in next loop and send |MediaDecoder::PlaybackEnded| again to trigger 'ended'
+          // event twice in the media element.
+          ReentrantMonitorAutoExit exitMon(mDecoder->GetReentrantMonitor());
+          nsCOMPtr<nsIRunnable> event =
+            NS_NewRunnableMethod(mDecoder, &MediaDecoder::PlaybackEnded);
+          NS_DispatchToMainThread(event, NS_DISPATCH_SYNC);
+        }
       }
       return NS_OK;
     }
   }
 
   return NS_OK;
 }
 
@@ -2751,16 +2758,22 @@ nsresult MediaDecoderStateMachine::Sched
       return GetStateMachineThread()->Dispatch(this, NS_DISPATCH_NORMAL);
     }
     // We're not currently running this state machine on the state machine
     // thread, but something has already dispatched an event to run it again,
     // so just exit; it's going to run real soon.
     return NS_OK;
   }
 
+  // Since there is already a pending task that will run immediately,
+  // we don't need to schedule a timer task.
+  if (mRunAgain) {
+    return NS_OK;
+  }
+
   mTimeout = timeout;
 
   nsresult res;
   if (!mTimer) {
     mTimer = do_CreateInstance("@mozilla.org/timer;1", &res);
     if (NS_FAILED(res)) return res;
     mTimer->SetTarget(GetStateMachineThread());
   }