1497034 - FeaturePolicy: autoplay

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Andrea Marchesini [:baku]]

Implement the FeaturePolicy support for autoplay media elements.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: autoplay.patch

Comment 2

[Andrea Marchesini [:baku]]

Attachment: autoplay.patch

We don't pass so many WPTs because webdriver doesn't seem to have the support for user-interaction.

Comment 3

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

Comment on attachment 9015110 [details] [diff] [review]
autoplay.patch

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

::: dom/media/AutoplayPolicy.cpp
@@ +79,5 @@
>    if (!aWindow->GetExtantDoc()) {
>      return false;
>    }
>  
> +  if (!FeaturePolicyUtils::IsFeatureAllowed(aWindow->GetExtantDoc(),

Here you're (correctly) checking whether the current document is blocked via feature-policy, and further down we walk up the doc tree to the top level content document and check permissions etc on the top level content document. It would be incorrect to check the feature policy on the top level document, so can you add a comment here explaining why you're checking feature policy on the current document, rather than the "approver" document. Then we'll hopefully prevent future refactorings from making this incorrect.

Bonus points if you can write a mochitest to enforce this behaviour.

Comment 4

[Andrea Marchesini [:baku]]

> Bonus points if you can write a mochitest to enforce this behaviour.

Thanks for the review. About this extra mochitest, all the existing featurePolicy WPTs are based on nested iframes. Is it similar to what you suggested? Here an example:

https://searchfox.org/mozilla-central/rev/29aea2a2a3bd0f5e25ce0b60a76053fb25ba5149/testing/web-platform/tests/html/semantics/embedded-content/media-elements/autoplay-allowed-by-feature-policy.https.sub.html#27

which uses iframes here:

https://searchfox.org/mozilla-central/rev/29aea2a2a3bd0f5e25ce0b60a76053fb25ba5149/testing/web-platform/tests/feature-policy/resources/featurepolicy.js#28-52

If I'm wrong, I'll add an extra WPT test in a follow up.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f574c731fcb0
FeaturePolicy: autoplay, r=cpearce

Comment 6

[Stefan Hindli [:stefan_hindli]]

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

Comment 7

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

Yeah, those WPT look like they cover us here.

Comment 8

[Florian Scholz (Open Web Docs)]

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Feature-Policy
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Feature-Policy/autoplay