Backed out 3 changesets (bug 1499903) for causing bug 1502637 CLOSED TREE

Backed out changeset 3c7165875616 (bug 1499903)
Backed out changeset 106752b77131 (bug 1499903)
Backed out changeset 3f6d3c425b42 (bug 1499903)

dom/media/MediaDecoder.cpp

@@ -119,37 +119,6 @@ public:
   }
 };
 
-// When media is looping back to the head position, the spec [1] mentions that
-// MediaElement should dispatch `seeking` first, `timeupdate`, and `seeked` in
-// the end. This guard should be created before we fire `timeupdate` so that it
-// can ensure the event order.
-// [1]
-// https://html.spec.whatwg.org/multipage/media.html#playing-the-media-resource:attr-media-loop-2
-// https://html.spec.whatwg.org/multipage/media.html#seeking:dom-media-seek
-class MOZ_RAII SeekEventsGuard
-{
-public:
-  explicit SeekEventsGuard(MediaDecoderOwner* aOwner, bool aIsLoopingBack)
-    : mOwner(aOwner)
-    , mIsLoopingBack(aIsLoopingBack)
-  {
-    MOZ_ASSERT(mOwner);
-    if (mIsLoopingBack) {
-      mOwner->SeekStarted();
-    }
-  }
-
-  ~SeekEventsGuard() {
-    MOZ_ASSERT(mOwner);
-    if (mIsLoopingBack) {
-      mOwner->SeekCompleted();
-    }
-  }
-private:
-  MediaDecoderOwner* mOwner;
-  bool mIsLoopingBack;
-};
-
 StaticRefPtr<MediaMemoryTracker> MediaMemoryTracker::sUniqueInstance;
 
 LazyLogModule gMediaTimerLog("MediaTimer");
@@ -275,7 +244,6 @@ MediaDecoder::MediaDecoder(MediaDecoderInit& aInit)
   , mForcedHidden(false)
   , mHasSuspendTaint(aInit.mHasSuspendTaint)
   , mPlaybackRate(aInit.mPlaybackRate)
-  , mLogicallySeeking(false, "MediaDecoder::mLogicallySeeking")
   , INIT_MIRROR(mBuffered, TimeIntervals())
   , INIT_MIRROR(mCurrentPosition, TimeUnit::Zero())
   , INIT_MIRROR(mStateMachineDuration, NullableTimeUnit())
@@ -284,6 +252,7 @@ MediaDecoder::MediaDecoder(MediaDecoderInit& aInit)
   , INIT_CANONICAL(mPreservesPitch, aInit.mPreservesPitch)
   , INIT_CANONICAL(mLooping, aInit.mLooping)
   , INIT_CANONICAL(mPlayState, PLAY_STATE_LOADING)
+  , INIT_CANONICAL(mLogicallySeeking, false)
   , INIT_CANONICAL(mSameOriginMedia, false)
   , INIT_CANONICAL(mMediaPrincipalHandle, PRINCIPAL_HANDLE_NONE)
   , mVideoDecodingOberver(new BackgroundVideoDecodingPermissionObserver(this))
@@ -409,6 +378,10 @@ MediaDecoder::OnPlaybackEvent(MediaPlaybackEvent&& aEvent)
     case MediaPlaybackEvent::SeekStarted:
       SeekingStarted();
       break;
+    case MediaPlaybackEvent::Loop:
+      GetOwner()->DispatchAsyncEvent(NS_LITERAL_STRING("seeking"));
+      GetOwner()->DispatchAsyncEvent(NS_LITERAL_STRING("seeked"));
+      break;
     case MediaPlaybackEvent::Invalidate:
       Invalidate();
       break;
@@ -831,11 +804,12 @@ MediaDecoder::OnSeekResolved()
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_DIAGNOSTIC_ASSERT(!IsShutdown());
   AbstractThread::AutoEnter context(AbstractMainThread());
+  mSeekRequest.Complete();
+
   mLogicallySeeking = false;
 
   // Ensure logical position is updated after seek.
   UpdateLogicalPositionInternal();
-  mSeekRequest.Complete();
 
   GetOwner()->SeekCompleted();
   GetOwner()->AsyncResolveSeekDOMPromiseIfExists();
@@ -881,16 +855,6 @@ MediaDecoder::ChangeState(PlayState aState)
   }
 }
 
-bool
-MediaDecoder::IsLoopingBack(double aPrevPosition, double aCurPosition) const
-{
-  // If current position is early than previous position and we didn't do seek,
-  // that means we looped back to the start position.
-  return mLooping &&
-         !mSeekRequest.Exists() &&
-         aCurPosition < aPrevPosition;
-}
-
 void
 MediaDecoder::UpdateLogicalPositionInternal()
 {
@@ -902,8 +866,6 @@ MediaDecoder::UpdateLogicalPositionInternal()
     currentPosition = std::max(currentPosition, mDuration);
   }
   bool logicalPositionChanged = mLogicalPosition != currentPosition;
-  SeekEventsGuard guard(GetOwner(),
-                        IsLoopingBack(mLogicalPosition, currentPosition));
   mLogicalPosition = currentPosition;
   DDLOG(DDLogCategory::Property, "currentTime", mLogicalPosition);
 

dom/media/MediaDecoder.h

@@ -294,9 +294,6 @@ private:
   // outlined in the specification.
   void FireTimeUpdate();
 
-  // True if we're going to loop back to the head position when media is in looping.
-  bool IsLoopingBack(double aPrevPosition, double aCurPosition) const;
-
   // Returns true if we can play the entire media through without stopping
   // to buffer, given the current download and playback rates.
   bool CanPlayThrough();
@@ -602,9 +599,6 @@ protected:
   // PlaybackRate and pitch preservation status we should start at.
   double mPlaybackRate;
 
-  // True if the decoder is seeking.
-  Watchable<bool> mLogicallySeeking;
-
   // Buffered range, mirrored from the reader.
   Mirror<media::TimeIntervals> mBuffered;
 
@@ -637,6 +631,9 @@ protected:
   // This can only be changed on the main thread.
   PlayState mNextState = PLAY_STATE_PAUSED;
 
+  // True if the decoder is seeking.
+  Canonical<bool> mLogicallySeeking;
+
   // True if the media is same-origin with the element. Data can only be
   // passed to MediaStreams when this is true.
   Canonical<bool> mSameOriginMedia;
@@ -665,6 +662,10 @@ public:
     return &mLooping;
   }
   AbstractCanonical<PlayState>* CanonicalPlayState() { return &mPlayState; }
+  AbstractCanonical<bool>* CanonicalLogicallySeeking()
+  {
+    return &mLogicallySeeking;
+  }
   AbstractCanonical<bool>* CanonicalSameOriginMedia()
   {
     return &mSameOriginMedia;

dom/media/MediaDecoderStateMachine.cpp

@@ -2355,7 +2355,20 @@ DecodingState::Step()
   TimeUnit before = mMaster->GetMediaTime();
   mMaster->UpdatePlaybackPositionPeriodically();
 
-  if (mMaster->mMediaSink->IsStarted() && !mMaster->mLooping) {
+  // Fire the `seeking` and `seeked` events to meet the HTML spec
+  // when the media is looped back from the end to the beginning.
+  if (before > mMaster->GetMediaTime()) {
+    MOZ_ASSERT(mMaster->mLooping);
+    mMaster->mOnPlaybackEvent.Notify(MediaPlaybackEvent::Loop);
+  // After looping is cancelled, the time won't be corrected, and therefore we
+  // can check it to see if the end of the media track is reached. Make sure
+  // the media is started before comparing the time, or it's meaningless.
+  // Without checking IsStarted(), the media will be terminated immediately
+  // after seeking forward. When the state is just transited from seeking state,
+  // GetClock() is smaller than GetMediaTime(), since GetMediaTime() is updated
+  // upon seek is completed while GetClock() will be updated after the media is
+  // started again.
+  } else if (mMaster->mMediaSink->IsStarted() && !mMaster->mLooping) {
     TimeUnit adjusted = mMaster->GetClock();
     Reader()->AdjustByLooping(adjusted);
     if (adjusted < before) {

dom/media/MediaDecoderStateMachine.h

@@ -123,6 +123,7 @@ struct MediaPlaybackEvent
     PlaybackProgressed,
     PlaybackEnded,
     SeekStarted,
+    Loop,
     Invalidate,
     EnterVideoSuspend,
     ExitVideoSuspend,

dom/media/test/mochitest.ini

@@ -882,7 +882,6 @@ skip-if = toolkit == 'android' # bug 1274802, android(bug 1232305)
 skip-if = toolkit == 'android' # android(bug 1232305)
 [test_loop.html]
 skip-if = toolkit == 'android' # bug 1242112, android(bug 1232305)
-[test_looping_eventsOrder.html]
 [test_media_selection.html]
 [test_media_sniffer.html]
 skip-if = android_version == '17' # android(bug 1232305)

dom/media/test/test_looping_eventsOrder.html

@@ -1,47 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <title>Looping events order</title>
-  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
-  <script type="text/javascript" src="manifest.js"></script>
-</head>
-<script type="text/javascript">
-/**
- * This test is used to ensure the events order when media is looping back to
- * the start position. We should see events in following order.
- * 'seeking' -> 'timeupdate' -> 'seeked'.
- */
-const manager = new MediaTestManager;
-
-async function testTimeupdateChanged({name, type, duration}) {
-  info(`- start testPlay for name=${name} -`);
-  const element = document.createElement(getMajorMimeType(type));
-  element.src = name;
-  element.loop = true;
-
-  await once(element, "canplay");
-  ok(await element.play().then(() => true, () => false), `start playing ${name}`);
-
-  let gotTimeUpdated = false;
-  await once(element, "seeking");
-  element.addEventListener("timeupdate", function() {
-    gotTimeUpdated = true;
-  }, {once: true});
-  await once(element, "seeked");
-  ok(gotTimeUpdated, "Got timeupdate between seeking and seeked.");
-
-  removeNodeAndSource(element);
-}
-
-async function runTest(test, token) {
-  manager.started(token);
-  await testTimeupdateChanged(test);
-  manager.finished(token);
-}
-
-manager.runTests(gSmallTests, runTest);
-</script>
-</pre>
-</body>
-</html>