We'd like to add telemetry to help inform the decision as to how enabling
block autoplay will affect video playback in the wild.
Our data science team would also like some input to help them estimate the
rate at which our shield study would receive pings, and the telemetry
collected here will help them estimate that.
We'd like to collect the following, on a per session basis:
* Count of the number of top level content documents loaded, as denominator for
other stats collected here.
* Count of the number of top level content documents which contained (directly
or in a descendant document) playback of an audible media element.
* Count of the number of top level content documents which contained (directly
or in a descendant document) a muted media element that was paused by the
autoplay policy because it tried to unmute and it wasn't allowed to autoplay
audibly.
* Count of the total number of audible autoplay videos that would have not been
allowed to play if block autoplay was enabled. We'd either prompt for
permission on these videos, or block outright depending on user's settings.
* Count of the total number of audible autoplay videos that would have been
allowed to play if block autoplay was enabled.
MozReview-Commit-ID: vHWJPyqHjT
Everything that goes in a PLDHashtable (and its derivatives, like
nsTHashtable) needs to inherit from PLDHashEntryHdr. But through a lack
of enforcement, copy constructors for these derived classes didn't
explicitly invoke the copy constructor for PLDHashEntryHdr (and the
compiler didn't invoke the copy constructor for us). Instead,
PLDHashTable explicitly copied around the bits that the copy constructor
would have.
The current setup has two problems:
1) Derived classes should be using move construction, not copy
construction, since anything that's shuffling hash table keys/entries
around will be using move construction.
2) Derived classes should take responsibility for transferring bits of
superclass state around, and not rely on something else to handle
that.
The second point is not a huge problem for PLDHashTable (PLDHashTable
only has to copy PLDHashEntryHdr's bits in a single place), but future
hash table implementations that might move entries around more
aggressively would have to insert compensation code all over the place.
Additionally, if moving entries is implemented via memcpy (which is
quite common), PLDHashTable copying around bits *again* is inefficient.
Let's fix all these problems in one go, by:
1) Explicitly declaring the set of constructors that PLDHashEntryHdr
implements (and does not implement). In particular, the copy
constructor is deleted, so any derived classes that attempt to make
themselves copyable will be detected at compile time: the compiler
will complain that the superclass type is not copyable.
This change on its own will result in many compiler errors, so...
2) Change any derived classes to implement move constructors instead
of copy constructors. Note that some of these move constructors are,
strictly speaking, unnecessary, since the relevant classes are moved
via memcpy in nsTHashtable and its derivatives.
We can start playing while we're awaiting a response to an autoplay-media
permission prompt, for example if the user clicks on a play button. In such
cases, it doesn't make sense to keep the autoplay permission request promise
connected in HTMLMediaElement, as since we're playing we'll be resolving the
play() promises and thus we won't be taking action on the autoplay request
promise's result. So we should just disconnect the autoplay permission request
promise if it's connected when we start playing.
MozReview-Commit-ID: 1aiCLXV7Ja9
The front end code can't always guarantee to give us an allow/cancel response
to a permission request. In particular in these cases:
* if we close a tab while showing a doorhanger, or
* if we navigate a tab while showing a doorhanger, or
* if the permission prompt requested in a background tab and never shown.
Handling all of these cases is problematic; we don't get events for all of
these where it's easy and cheap to determine that we should cancel the
permission request.
Canceling the permission request is important in the autoplay-media permission
request case as there's objects waiting on the resolution of the permission
request, and they leak in ASan builds while running chrome tests if the Gecko
size of the permission request doesn't get a notification telling it to stop
waiting.
But we can however rely on the doorhanger code to drop its reference to the
nsIContentPermissionRequest object that we pass to it when the doorhanger goes
away. So we can cancel the permission request in our
nsIContentPermissionRequest's implementation's destructor in order to easily
catch all the above cases.
In order to do that, we need to split AutoplayRequest into two; one part being
the implementation of nsIContentPermissionRequest (AutoplayPermissionRequest),
and the other part being the code to own the PromiseHolder and manage the
permission request (AutoplayPermissionManager).
AutoplayPermissionRequest keeps a weak reference to AutoplayPermissionManager,
so that it can tell the AutoplayPermissionManager to reject the request promise
when it's destroyed.
This fixes the ASan leak for which I got backed out from earlier in this bug,
and also fixes the cases above.
MozReview-Commit-ID: KoVkgIqDleW
Test that a video which tries to autoplay via either a play() call or via
an autoplay attribute:
* Plays when it has a pre-existing "allow" autoplay-media permission.
* Is blocked when it has a pre-existing "block" autoplay-media permission.
* Plays when it doesn't have a pre-existing autoplay-media permission and
"allow" is pressed on the door hanger.
* Is blocked when it doesn't have a pre-existing autoplay-media permission and
"block" is pressed on the door hanger.
MozReview-Commit-ID: CpftV6RQbtU
When autoplay is requested by setting the "autoplay" attribute, we should
check whether autoplay is allowed in HTMLMediaElement::CheckAutoplayDataReady()
and if not we should prompt for user consent.
This ensures that <video ... autoplay/> will prompt for consent when used on
a page without a pre-existing allow/block permission.
MozReview-Commit-ID: 77pJR2Ybn2i
It's possible that if the HTMLMediaElement is loading while we're loading a new
document into a docshell, that the HTMLMediaElement can reach readyState
HAVE_FUTURE_DATA just after its OwnerDoc is removed from the docshell. If the
HTMLMediaElement wasn't paused, then it may start playing due to the readyState
change in HTMLMediaElement::ChangeReadyState().
For years we've had hard to reproduce issues where media started playing after
we've closed the tab; I bet this was the cause!
When we detect that the document has been removed from its DocShell,
HTMLMediaElement::NotifyOwnerDocumentActivityChanged() is called, and that
suspends the MediaDecoder just in case we need to resurrect the media element,
for example if the tab comes out of the BF cache. When we suspend we set
mPausedForInactiveDocumentOrChannel=true, and all other calls to
MediaDecoder::Play() are guarded by checks on
mPausedForInactiveDocumentOrChannel.
So we should also guard the MediaDecoder::Play() call in ChangeReadyState()
with a check on mPausedForInactiveDocumentOrChannel too.
MozReview-Commit-ID: GfmZasT9jdr
It's possible that if the HTMLMediaElement is loading while we're loading a new
document into a docshell, that the HTMLMediaElement can reach readyState
HAVE_FUTURE_DATA just after its OwnerDoc is removed from the docshell. If the
HTMLMediaElement wasn't paused, then it may start playing due to the readyState
change in HTMLMediaElement::ChangeReadyState().
For years we've had hard to reproduce issues where media started playing after
we've closed the tab; I bet this was the cause!
When we detect that the document has been removed from its DocShell,
HTMLMediaElement::NotifyOwnerDocumentActivityChanged() is called, and that
suspends the MediaDecoder just in case we need to resurrect the media element,
for example if the tab comes out of the BF cache. When we suspend we set
mPausedForInactiveDocumentOrChannel=true, and all other calls to
MediaDecoder::Play() are guarded by checks on
mPausedForInactiveDocumentOrChannel.
So we should also guard the MediaDecoder::Play() call in ChangeReadyState()
with a check on mPausedForInactiveDocumentOrChannel too.
MozReview-Commit-ID: GfmZasT9jdr
This patch is an automatic replacement of s/NS_NOTREACHED/MOZ_ASSERT_UNREACHABLE/. Reindenting long lines and whitespace fixups follow in patch 6b.
MozReview-Commit-ID: 5UQVHElSpCr
Sometimes when video is playing, a preroll ad plays, and that may be in a cross
origin iframe. If autoplay media is disabled, we require a user gesture in a
document before playback in that document is permitted, and we require each
origin to be gesture activated separately. So in the cross origin preroll video
add case, then the user will have to click once to unblock playback for the
cross origin ad, and then once the preroll ad finishes, the user will have to
click again to activate playback of the same origin content video.
This is a bad user experience.
So we should instead make gesture activation propagate up the doc tree
irrespective of crossing origins. This way, when the user clicks to activate,
all documents in that tab are also also effectively gesture activated, and so
can autoplay.
MozReview-Commit-ID: 1HZQ5zkubR
SeekToNextFrame is handled differently than other seeks by the
MediaDecoderStateMachine, and should not take place while other seeks already
are. Bug 1410225 implemented some changes in HTMLMediaElement to prevent this,
but it's still possible to move to a seeking state in the MDSM and accept
SeekToNextFrame (as in this bug).
This changeset changes the MDSM to reject SeekToNextFrame if a seek is already
happening. Since the MDSM now does this the changes from bug 1410225 can be
removed.
This has the functional change of the promise from SeekToNextFrame being
rejected if the seek in not performed due to another seek. Previously the
promise would succeed when the other seek completed. This seems sensible as the
next frame seek does not actually take place.
MozReview-Commit-ID: HD9WRFq3LZV
We currently observe changes to HTMLMediaElement::mPaused via a hand-rolled
wrapper class. We can use use mozilla::Watchable<> and avoid rolling our
own equivalent here.
This also paves the way for using state watching on other observable state
in HTMLMediaElement.
MozReview-Commit-ID: 4lBlJiV15iG
We don't need to track this state anymore, as we don't need to delay calling
MediaDecoder::Play() or delay firing the "playing" event anymore.
MozReview-Commit-ID: E3B9l6ep7FP
I don't think we should allow media without audio tracks to autoplay because:
* It means play() before loaded metadata behaves differently than play()
called after loaded metadata.
* With the current impl we dispatch the "play" event and then the "pause"
event when we decide we should block, which may confuse some sites' controls.
* Delaying running the play() algorithm until we've loaded metadata would add
significant complexity, and may break sites.
* Chrome doesn't have this provision, meaning the complexity required to
support it will not result in much benefit to us.
MozReview-Commit-ID: FSVlDJAOisw
This patch splits FontTableURI and BlobURL in 2 classes:
FontTableURIProtocolHandler and BlobURLProtocolHandler
both under mozilla::dom.
It also removes a memory reporter because that report is already covered by the
BlobURL one.
Same approach as the other bug, mostly replacing automatically by removing
'using mozilla::Forward;' and then:
s/mozilla::Forward/std::forward/
s/Forward</std::forward</
The only file that required manual fixup was TestTreeTraversal.cpp, which had
a class called TestNodeForward with template parameters :)
MozReview-Commit-ID: A88qFG5AccP
This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
When using a media element with a Media Source, the resource fetching algorithm is to be called in "local" mode:
https://www.w3.org/TR/media-source/#mediasource-attach
"Continue the resource fetch algorithm by running the remaining "Otherwise (mode is local)" steps, with these clarifications"
https://html.spec.whatwg.org/multipage/media.html#concept-media-load-resource
Under the local mode, the steps that would cause the element to fire suspend, stalled or progress can never occur.
We only prevent the stalled event to be fired, many websites rely on the progress event to be fired (such as when the init segment has been parsed). The HTML5 media spec will be amended to clearly indicate that progress is to be fired even with mediasource
MozReview-Commit-ID: DkoQzoV0JzO
Currently we can end up dispatching a 'playing' event right before we reject
play() promises, and this confuses YouTube's controls, and it doesn't make
sense to dispatch a 'playing' event when we're not playing anyway.
This is because the logic to delay resolving the play() promise until after
we've reached loadedmetadata doesn't prevent the 'playing' event from being
dispatched. We shouldn't dispatch 'playing' until we resolve the play()
promise(s).
MozReview-Commit-ID: 5H4dcObfu4M
Bug 1435133 introduced a new path where we block autoplay and reject the play()
promise, but we didn't fire a "pause" event. This confuses YouTube's controls.
Additionally, even if we're not in a user generated event handler, we
unilaterally consider the media element blessed if execution reaches here:
https://searchfox.org/mozilla-central/rev/11a2ae294f50049e12515b5821f5a396d951aacb/dom/html/HTMLMediaElement.cpp#4110
We previously rejected before reaching here when not in a user generated event
handler, but now if play() is called before we've reached loadedmetadata, we
reject the promise if we're not in a non-event handler and bail out early, and
so we'll bless even if not in a user generated event handler. Meaning when we
do reach loadedmetadata, we think we were in a user generated event handler
when play() was originally called, and so we won't reject the play promise.
So this patch ensures we dispatch a "pause" event when we reject the play()
promise here. The WHATWG spec says we should do this when pausing anyway.
Note: calling our interal Pause() function when rejecting the play() promise
here breaks YouTube, as if we do that we fire a "timeupdate" event. So I opted
to manually code to fire the event here instead of just calling Pause()
everywhere we want to ensure we're paused.
MozReview-Commit-ID: 1snkiTnPGih
