868660 - Missing document inactive check in media element

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[John Schoenick [:johns]]

I've noticed a few, unreproducible instances of <video> tags continuing to play after navigating, and suspect this is the culprit

Comment 1

[John Schoenick [:johns]]

Attachment: Add missing check for media paused in inactive documents

@cpearce would you be the right one to review this?

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 745425 [details] [diff] [review]
Add missing check for media paused in inactive documents

Review of attachment 745425 [details] [diff] [review]:
-----------------------------------------------------------------

I think roc should review this.

::: content/html/content/src/HTMLMediaElement.cpp
@@ +3062,5 @@
>        SetPlayedOrSeeked(true);
>        if (mCurrentPlayRangeStart == -1.0) {
>          mCurrentPlayRangeStart = CurrentTime();
>        }
> +      if (!mPausedForInactiveDocumentOrChannel) {

Maybe the !mPausedForInactiveDocumentOrChannel check should be added to CanActivateAutoplay() instead?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 745425 [details] [diff] [review]
Add missing check for media paused in inactive documents

Review of attachment 745425 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/HTMLMediaElement.cpp
@@ +3062,5 @@
>        SetPlayedOrSeeked(true);
>        if (mCurrentPlayRangeStart == -1.0) {
>          mCurrentPlayRangeStart = CurrentTime();
>        }
> +      if (!mPausedForInactiveDocumentOrChannel) {

Yes, I think that would be slightly better.

This check should definitely be done for the mSrcStream case as well.

Comment 4

[John Schoenick [:johns]]

Attachment: Add missing check for media paused in inactive documents

Moved to CanActivateAutoplay

Comment 5

[John Schoenick [:johns]]

Comment on attachment 751222 [details] [diff] [review]
Add missing check for media paused in inactive documents

https://hg.mozilla.org/integration/mozilla-inbound/rev/c294faeff580

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c294faeff580