Bug 1420608. P1 - don't switch off blank decoder when seeking begins. r=gerald a=gchang
authorJW Wang <jwwang@mozilla.com>
Tue, 19 Dec 2017 15:43:55 +0800
changeset 445411 5a2fd74ee373d938c4c15b5a92f470fe813428bf
parent 445410 914ddbe4a5f0d569e4d67798db3a031d582d9c94
child 445412 ccbe2255e35a2ec77a9c49ada482cf8193138549
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald, gchang
bugs1420608
milestone58.0
Bug 1420608. P1 - don't switch off blank decoder when seeking begins. r=gerald a=gchang When looping videos in a background tab, SeekingState::Enter() will switch off blank decoder and then DecodingState::Enter() will switch it on again. It is a waste of CPU cycles when the tab never goes to the foreground. The overhread is even more significant when looping short files. We should resume video decoding only when necessary that is we check in DecodingState::Enter() to see if mVideoDecodeSuspended matches mVideoDecodeMode. Bug 1420608. P2 - fix the test timeout. r=alwu See comment 50 for the cause. Since file_silentAudioTrack.html calls play() to start playback immediately, it is possible that 'mozentervideosuspend' has been fired before check_video_decoding_state() has a chance to register event handlers. We call load() and play() to start playback from the beginning so we won't miss any events. MozReview-Commit-ID: 9sKygfIxEtS
dom/media/MediaDecoderStateMachine.cpp
toolkit/content/tests/browser/browser_resume_bkg_video_on_tab_hover.js
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -842,25 +842,16 @@ public:
   explicit SeekingState(Master* aPtr) : StateObject(aPtr) { }
 
   RefPtr<MediaDecoder::SeekPromise> Enter(SeekJob&& aSeekJob,
                                           EventVisibility aVisibility)
   {
     mSeekJob = Move(aSeekJob);
     mVisibility = aVisibility;
 
-    // Always switch off the blank decoder otherwise we might become visible
-    // in the middle of seeking and won't have a valid video frame to show
-    // when seek is done.
-    if (mMaster->mVideoDecodeSuspended) {
-      mMaster->mVideoDecodeSuspended = false;
-      mMaster->mOnPlaybackEvent.Notify(MediaEventType::ExitVideoSuspend);
-      Reader()->SetVideoBlankDecode(false);
-    }
-
     // Suppressed visibility comes from two cases: (1) leaving dormant state,
     // and (2) resuming suspended video decoder. We want both cases to be
     // transparent to the user. So we only notify the change when the seek
     // request is from the user.
     if (mVisibility == EventVisibility::Observable) {
       // Don't stop playback for a video-only seek since we want to keep playing
       // audio and we don't need to stop playback while leaving dormant for the
       // playback should has been stopped.
@@ -893,18 +884,17 @@ public:
 
   void HandleVideoSuspendTimeout() override
   {
     // Do nothing since we want a valid video frame to show when seek is done.
   }
 
   void HandleResumeVideoDecoding(const TimeUnit&) override
   {
-    // We set mVideoDecodeSuspended to false in Enter().
-    MOZ_ASSERT(false, "Shouldn't have suspended video decoding.");
+    // Do nothing. We will resume video decoding in the decoding state.
   }
 
 protected:
   SeekJob mSeekJob;
   EventVisibility mVisibility;
 
   virtual void DoSeek() = 0;
   // Transition to the next state (defined by the subclass) when seek is completed.
@@ -2117,16 +2107,20 @@ ReportRecoveryTelemetry(const TimeStamp&
 }
 
 void
 MediaDecoderStateMachine::
 StateObject::HandleResumeVideoDecoding(const TimeUnit& aTarget)
 {
   MOZ_ASSERT(mMaster->mVideoDecodeSuspended);
 
+  mMaster->mVideoDecodeSuspended = false;
+  mMaster->mOnPlaybackEvent.Notify(MediaEventType::ExitVideoSuspend);
+  Reader()->SetVideoBlankDecode(false);
+
   // Start counting recovery time from right now.
   TimeStamp start = TimeStamp::Now();
 
   // Local reference to mInfo, so that it will be copied in the lambda below.
   auto& info = Info();
   bool hw = Reader()->VideoIsHardwareAccelerated();
 
   // Start video-only seek to the current time.
@@ -2267,16 +2261,22 @@ DecodingFirstFrameState::MaybeFinishDeco
 }
 
 void
 MediaDecoderStateMachine::
 DecodingState::Enter()
 {
   MOZ_ASSERT(mMaster->mSentFirstFrameLoadedEvent);
 
+  if (mMaster->mVideoDecodeSuspended &&
+      mMaster->mVideoDecodeMode == VideoDecodeMode::Normal) {
+    StateObject::HandleResumeVideoDecoding(mMaster->GetMediaTime());
+    return;
+  }
+
   if (mMaster->mVideoDecodeMode == VideoDecodeMode::Suspend &&
       !mMaster->mVideoDecodeSuspendTimer.IsScheduled() &&
       !mMaster->mVideoDecodeSuspended) {
     // If the VideoDecodeMode is Suspend and the timer is not schedule, it means
     // the timer has timed out and we should suspend video decoding now if
     // necessary.
     HandleVideoSuspendTimeout();
   }
--- a/toolkit/content/tests/browser/browser_resume_bkg_video_on_tab_hover.js
+++ b/toolkit/content/tests/browser/browser_resume_bkg_video_on_tab_hover.js
@@ -1,16 +1,27 @@
 const PAGE = "https://example.com/browser/toolkit/content/tests/browser/file_silentAudioTrack.html";
 
-async function check_video_decoding_state(isSuspended) {
+async function check_video_decoding_state(args) {
   let video = content.document.getElementById("autoplay");
   if (!video) {
     ok(false, "Can't get the video element!");
   }
 
+  let isSuspended = args.suspend;
+  let reload = args.reload;
+
+  if (reload) {
+    // It is too late to register event handlers when playback is half
+    // way done. Let's start playback from the beginning so we won't
+    // miss any events.
+    video.load();
+    video.play();
+  }
+
   let state = isSuspended ? "suspended" : "resumed";
   let event = isSuspended ? "mozentervideosuspend" : "mozexitvideosuspend";
   return new Promise(resolve => {
     video.addEventListener(event, function() {
       ok(true, `Video decoding is ${state}.`);
       resolve();
     }, {once: true});
   });
@@ -41,23 +52,26 @@ function check_should_not_send_unselecte
   return new Promise(resolve => {
     browser.messageManager.addMessageListener("UnselectedTabHoverMsg:Disabled", function() {
       ok(true, "Should not send unselected tab hover msg, no one is listening for it.");
       resolve();
     });
   });
 }
 
-function get_video_decoding_suspend_promise(browser) {
-  return ContentTask.spawn(browser, true /* suspend */,
+function get_video_decoding_suspend_promise(browser, reload) {
+  let suspend = true;
+  return ContentTask.spawn(browser, { suspend, reload },
                            check_video_decoding_state);
 }
 
 function get_video_decoding_resume_promise(browser) {
-  return ContentTask.spawn(browser, false /* resume */,
+  let suspend = false;
+  let reload = false;
+  return ContentTask.spawn(browser, { suspend, reload },
                            check_video_decoding_state);
 }
 
 /**
  * Because of bug1029451, we can't receive "mouseover" event correctly when
  * we disable non-test mouse event. Therefore, we can't synthesize mouse event
  * to simulate cursor hovering, so we temporarily use a hacky way to resume and
  * suspend video decoding.
@@ -92,17 +106,17 @@ add_task(async function resume_and_suspe
   let browser = tab.linkedBrowser;
 
   info("- before loading media, we shoudn't send the tab hover msg for tab -");
   await check_should_not_send_unselected_tab_hover_msg(browser);
   browser.loadURI(PAGE);
   await BrowserTestUtils.browserLoaded(browser);
 
   info("- should suspend background video decoding -");
-  await get_video_decoding_suspend_promise(browser);
+  await get_video_decoding_suspend_promise(browser, true);
   await check_should_send_unselected_tab_hover_msg(browser);
 
   info("- when cursor is hovering over the tab, resuming the video decoding -");
   let promise = get_video_decoding_resume_promise(browser);
   await cursor_hover_over_tab_and_resume_video_decoding(browser);
   await promise;
   await check_should_send_unselected_tab_hover_msg(browser);