corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1507193 - Don't leave dangling js promises after seeking. r=jya

Browse Source 
Only SeekToNextFrame cares about promises, but prior to this patch the common
method HTMLMediaElement::Seek() would always return a promise.

When the caller was not SeekToNextFrame (e.g., SetCurrentTime, FastSeek), the
promise would end up *not* being exposed. When later rejected, we would catch
this and write an error to the js console.

This patch lifts the handling of the promise out of Seek() and into
SeekToNextFrame() so that we avoid creating promises that would not get exposed
to js.

Differential Revision: https://phabricator.services.mozilla.com/D42511
This commit is contained in:
Andreas Pehrson
2019-08-28 14:57:13 +00:00
parent 4cd23a2d0a
commit 7876127b0c
5 changed files with 50 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

85
dom/html/HTMLMediaElement.cpp
View File

@@ -2804,8 +2804,7 @@ double HTMLMediaElement::CurrentTime() const {
void HTMLMediaElement::FastSeek(double aTime, ErrorResult& aRv) {
LOG(LogLevel::Debug, ("%p FastSeek(%f) called by JS", this, aTime));
RefPtr<Promise> tobeDropped =
Seek(aTime, SeekTarget::PrevSyncPoint, IgnoreErrors());
Seek(aTime, SeekTarget::PrevSyncPoint, IgnoreErrors());
}
already_AddRefed<Promise> HTMLMediaElement::SeekToNextFrame(ErrorResult& aRv) {
@@ -2820,14 +2819,23 @@ already_AddRefed<Promise> HTMLMediaElement::SeekToNextFrame(ErrorResult& aRv) {
}
}
return Seek(CurrentTime(), SeekTarget::NextFrame, aRv);
Seek(CurrentTime(), SeekTarget::NextFrame, aRv);
if (aRv.Failed()) {
return nullptr;
}
mSeekDOMPromise = CreateDOMPromise(aRv);
if (NS_WARN_IF(aRv.Failed())) {
return nullptr;
}
return do_AddRef(mSeekDOMPromise);
}
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, IgnoreErrors());
Seek(aCurrentTime, SeekTarget::Accurate, IgnoreErrors());
}
/**
@@ -2857,9 +2865,8 @@ static bool IsInRanges(TimeRanges& aRanges, double aValue,
return false;
}
already_AddRefed<Promise> HTMLMediaElement::Seek(double aTime,
SeekTarget::Type aSeekType,
ErrorResult& aRv) {
void 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.
@@ -2871,11 +2878,6 @@ already_AddRefed<Promise> HTMLMediaElement::Seek(double aTime,
// https://html.spec.whatwg.org/multipage/media.html#dom-media-seek
mShowPoster = false;
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()) {
@@ -2887,7 +2889,7 @@ already_AddRefed<Promise> HTMLMediaElement::Seek(double aTime,
if (mSrcAttrStream) {
// do nothing since media streams have an empty Seekable range.
aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
return nullptr;
return;
}
if (mPlayed && mCurrentPlayRangeStart != -1.0) {
@@ -2906,28 +2908,28 @@ already_AddRefed<Promise> HTMLMediaElement::Seek(double aTime,
if (mReadyState == HAVE_NOTHING) {
mDefaultPlaybackStartPosition = aTime;
aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
return nullptr;
return;
}
if (!mDecoder) {
// mDecoder must always be set in order to reach this point.
NS_ASSERTION(mDecoder, "SetCurrentTime failed: no decoder");
aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
return nullptr;
return;
}
// 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);
return nullptr;
return;
}
RefPtr<TimeRanges> seekable =
new TimeRanges(ToSupports(OwnerDoc()), seekableIntervals);
uint32_t length = seekable->Length();
if (length == 0) {
aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
return nullptr;
return;
}
// If the position we want to seek to is not in a seekable range, we seek
@@ -2988,11 +2990,6 @@ already_AddRefed<Promise> HTMLMediaElement::Seek(double aTime,
// We changed whether we're seeking so we need to AddRemoveSelfReference.
AddRemoveSelfReference();
// Keep the DOM promise.
mSeekDOMPromise = promise;
return promise.forget();
}
double HTMLMediaElement::Duration() const {
@@ -5252,6 +5249,24 @@ void HTMLMediaElement::SeekCompleted() {
if (IsAudioTrackCurrentlySilent()) {
UpdateAudioTrackSilenceRange(mIsAudioTrackAudible);
}
if (mSeekDOMPromise) {
mAbstractMainThread->Dispatch(NS_NewRunnableFunction(
__func__, [promise = std::move(mSeekDOMPromise)] {
promise->MaybeResolveWithUndefined();
}));
}
MOZ_ASSERT(!mSeekDOMPromise);
}
void HTMLMediaElement::SeekAborted() {
if (mSeekDOMPromise) {
mAbstractMainThread->Dispatch(NS_NewRunnableFunction(
__func__, [promise = std::move(mSeekDOMPromise)] {
promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
}));
}
MOZ_ASSERT(!mSeekDOMPromise);
}
void HTMLMediaElement::NotifySuspendedByCache(bool aSuspendedByCache) {
@@ -7221,30 +7236,6 @@ bool HasDebuggerOrTabsPrivilege(JSContext* aCx, JSObject* aObj) {
nsContentUtils::CallerHasPermission(aCx, nsGkAtoms::tabs);
}
void HTMLMediaElement::AsyncResolveSeekDOMPromiseIfExists() {
MOZ_ASSERT(NS_IsMainThread());
if (mSeekDOMPromise) {
RefPtr<dom::Promise> promise = mSeekDOMPromise.forget();
nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
"dom::HTMLMediaElement::AsyncResolveSeekDOMPromiseIfExists",
[promise]() { promise->MaybeResolveWithUndefined(); });
mAbstractMainThread->Dispatch(r.forget());
mSeekDOMPromise = nullptr;
}
}
void HTMLMediaElement::AsyncRejectSeekDOMPromiseIfExists() {
MOZ_ASSERT(NS_IsMainThread());
if (mSeekDOMPromise) {
RefPtr<dom::Promise> promise = mSeekDOMPromise.forget();
nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
"dom::HTMLMediaElement::AsyncRejectSeekDOMPromiseIfExists",
[promise]() { promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR); });
mAbstractMainThread->Dispatch(r.forget());
mSeekDOMPromise = nullptr;
}
}
void HTMLMediaElement::ReportCanPlayTelemetry() {
LOG(LogLevel::Debug, ("%s", __func__));