Bug 1495904 - Have HTMLMediaElement::Seek() return synchronous errors synchronously instead of causing uncatchable promise rejections in web console. Enforce existing fastSeek() and mediaElement.currentPosition = value behavior (which is to ignore... r=pehrsons
authorJan-Ivar Bruaroey <jib@mozilla.com>
Fri, 05 Oct 2018 15:43:32 +0000
changeset 498252 8ba29ee081caa2de560c4b68f548fb04d6cbb567
parent 498251 7335b0ef725a3d154ea91ff99ce3c91605ec7e49
child 498253 3e1d1b6a529eb202af9a579f08e6bc293d1e4b12
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspehrsons
bugs1495904
milestone64.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 1495904 - Have HTMLMediaElement::Seek() return synchronous errors synchronously instead of causing uncatchable promise rejections in web console. Enforce existing fastSeek() and mediaElement.currentPosition = value behavior (which is to ignore... r=pehrsons ...errors) if srcObject is a MediaStream. r?pehrsons Differential Revision: https://phabricator.services.mozilla.com/D7566
dom/html/HTMLMediaElement.cpp
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -2695,16 +2695,17 @@ HTMLMediaElement::CurrentTime() const
 
 void
 HTMLMediaElement::FastSeek(double aTime, ErrorResult& aRv)
 {
   LOG(LogLevel::Debug, ("%p FastSeek(%f) called by JS", this, aTime));
   LOG(LogLevel::Debug, ("Reporting telemetry VIDEO_FASTSEEK_USED"));
   Telemetry::Accumulate(Telemetry::VIDEO_FASTSEEK_USED, 1);
   RefPtr<Promise> tobeDropped = Seek(aTime, SeekTarget::PrevSyncPoint, aRv);
+  aRv.SuppressException();
 }
 
 already_AddRefed<Promise>
 HTMLMediaElement::SeekToNextFrame(ErrorResult& aRv)
 {
   /* This will cause JIT code to be kept around longer, to help performance
    * when using SeekToNextFrame to iterate through every frame of a video.
    */
@@ -2720,16 +2721,17 @@ HTMLMediaElement::SeekToNextFrame(ErrorR
 }
 
 void
 HTMLMediaElement::SetCurrentTime(double aCurrentTime, ErrorResult& aRv)
 {
   LOG(LogLevel::Debug,
       ("%p SetCurrentTime(%f) called by JS", this, aCurrentTime));
   RefPtr<Promise> tobeDropped = Seek(aCurrentTime, SeekTarget::Accurate, aRv);
+  aRv.SuppressException();
 }
 
 /**
  * Check if aValue is inside a range of aRanges, and if so returns true
  * and puts the range index in aIntervalIndex. If aValue is not
  * inside a range, returns false, and aIntervalIndex
  * is set to the index of the range which starts immediately after aValue
  * (and can be aRanges.Length() if aValue is after the last range).
@@ -2755,37 +2757,39 @@ IsInRanges(TimeRanges& aRanges, double a
   return false;
 }
 
 already_AddRefed<Promise>
 HTMLMediaElement::Seek(double aTime,
                        SeekTarget::Type aSeekType,
                        ErrorResult& aRv)
 {
+  // Note: Seek is called both by synchronous code that expects errors thrown in
+  // aRv, as well as asynchronous code that expects a promise. Make sure all
+  // synchronous errors are returned using aRv, not promise rejections.
+
   // aTime should be non-NaN.
   MOZ_ASSERT(!mozilla::IsNaN(aTime));
 
-  RefPtr<Promise> promise = CreateDOMPromise(aRv);
-
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   // Detect if user has interacted with element by seeking so that
   // play will not be blocked when initiated by a script.
   if (EventStateManager::IsHandlingUserInput()) {
     mIsBlessed = true;
   }
 
   StopSuspendingAfterFirstFrame();
 
   if (mSrcStream) {
     // do nothing since media streams have an empty Seekable range.
-    promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
-    return promise.forget();
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return nullptr;
   }
 
   if (mPlayed && mCurrentPlayRangeStart != -1.0) {
     double rangeEndTime = CurrentTime();
     LOG(LogLevel::Debug,
         ("%p Adding \'played\' a range : [%f, %f]",
          this,
          mCurrentPlayRangeStart,
@@ -2796,39 +2800,39 @@ HTMLMediaElement::Seek(double aTime,
     }
     // Reset the current played range start time. We'll re-set it once
     // the seek completes.
     mCurrentPlayRangeStart = -1.0;
   }
 
   if (mReadyState == HAVE_NOTHING) {
     mDefaultPlaybackStartPosition = aTime;
-    promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
-    return promise.forget();
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return nullptr;
   }
 
   if (!mDecoder) {
     // mDecoder must always be set in order to reach this point.
     NS_ASSERTION(mDecoder, "SetCurrentTime failed: no decoder");
-    promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
-    return promise.forget();
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return nullptr;
   }
 
   // Clamp the seek target to inside the seekable ranges.
   media::TimeIntervals seekableIntervals = mDecoder->GetSeekable();
   if (seekableIntervals.IsInvalid()) {
-    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR); // This will reject the promise.
-    return promise.forget();
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return nullptr;
   }
   RefPtr<TimeRanges> seekable =
     new TimeRanges(ToSupports(OwnerDoc()), seekableIntervals);
   uint32_t length = seekable->Length();
   if (length == 0) {
-    promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
-    return promise.forget();
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return nullptr;
   }
 
   // If the position we want to seek to is not in a seekable range, we seek
   // to the closest position in the seekable ranges instead. If two positions
   // are equally close, we seek to the closest position from the currentTime.
   // See seeking spec, point 7 :
   // http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#seeking
   uint32_t range = 0;
@@ -2881,19 +2885,17 @@ HTMLMediaElement::Seek(double aTime,
   // event if it changes the playback position as a result of the seek.
   LOG(LogLevel::Debug, ("%p SetCurrentTime(%f) starting seek", this, aTime));
   mDecoder->Seek(aTime, aSeekType);
 
   // We changed whether we're seeking so we need to AddRemoveSelfReference.
   AddRemoveSelfReference();
 
   // Keep the DOM promise.
-  mSeekDOMPromise = promise;
-
-  return promise.forget();
+  return do_AddRef(mSeekDOMPromise = CreateDOMPromise(aRv));
 }
 
 double
 HTMLMediaElement::Duration() const
 {
   if (mSrcStream) {
     return std::numeric_limits<double>::infinity();
   }