WICG / document-policy

https://wicg.github.io/document-policy/
Other
19 stars 8 forks source link

[Feature policy: animations] Compositor accelerated animations should not be allowed #10

Open birtles opened 6 years ago

birtles commented 6 years ago

As per my message to blink-dev I have strong concerns with this feature allowing compositor animations but disallowing others including:

<iframe allow="animations(transform,translate,scale,rotate,opacity,background-color,filter,offset,offset-distance,clip-path)"></iframe>

I suggest this feature policy should restrict all declarative animations which would at least make it useful for limiting CPU usage.

ehsan-karamad commented 6 years ago

Thanks for posting here as well. I think the explainer needs revisions (as also requested in issue w3c/webappsec-permissions-policy#203).

I will try to respond to the concerns in line. Hopefully more people familiar with animations and web platform will chime in.

Doing so would require either adopting a lowest common denominator approach (whereby Blink would be required to disallow filter animations, contrary to what the explainer currently states)

Thanks. Based on the idea that animations policy should eventually be parametric, I believe the lowest common denominator approach is not necessarily an issue since it will be up to the developer to select the white list. If however we go for a non-parametric version and common denominator, then filter should not be part of the policy at this time.

or would produce different results in different engines (creating a Web compatibility nightmare since content that, for example, waits for a transitionend from a certain animation might never get it).

I don't think allowing for arbitrarily different behaviors across the browsers is the goal here. If an animation is disallowed it should still fire the same events but only jump from initial state to final state (as discussed in issue w3c/webappsec-permissions-policy#204 ). If it is allowed, it should run as expected regardless of it being actually composited.

On that note, if a website relies on something such as the computed style during the animation then there could be a problem.

Using a whitelist of properties puts the burden on site maintainers to update (or forever hold Web content back) and will quickly become unwieldy. In 2019 authors might need to write:

Yes admittedly this is an inconvenience, but, it also provides granularity in setting what is (not) allowed.

For development and testing I wouldn't think compiling a long list of animations is too taxing. I think this feature is very useful for that purpose.

If the feature is used for performance reasons, I agree that compiling an up-to-date list is cumbersome; but I'd argue that not having a perfect list is no worse (if not better) than not using the feature altogether. Also I'd assume a typical use case for the feature is setting it in the HTTP headers as opposed to individually setting allow for each <iframe>.

Also a random idea which might partially alleviate this problem: I am wondering if it also makes sense to use wildcards or even coin new terms for a selecting a group of animations - e.g., border-*-color or transforms for all of transform,translate,scale,rotate.

The conditions that govern whether a particular animation can be run on the compositor are very subtle and change over time.

I don't believe running on the compositor is (should be) a condition here (I will try to update the explainer soon to reflect the correct expectation from animations blocked/allowed). Current experimental implementation in Chromium only includes the set of composited animations (on Chrome) as those included with the policy. However the policy is expected to be parametric anyway.

I also think composited should refer to animations that would normally run on compositor. When a certain animation is allowed, then it should run as expected on the page; regardless of whether or not it actually runs composited.

I suggest this feature policy should restrict all declarative animations which would at least make it useful for limiting CPU usage.

This is a very good suggestion and could be introduced as a special use case of the parametric version.

To disable all animations for a frame, perhaps something like:

<iframe allow="animations() https://example.com'"></iframe>

would disallow all animations for https://example.com. To allow all animations for an origin we should just not set the feature for the origin.

birtles commented 6 years ago

I don't think allowing for arbitrarily different behaviors across the browsers is the goal here.

On the contrary, I think we very much want behavior to be consistent between browsers.

I'm still concerned about this policy. It might be easier to support if very specific and compelling use cases are made available.

I had thought it might be possible to try defining the set of allowed animations as something less engine-specific than, "Can be animated on the compositor" and more spec-related such as, "Does not influence layout" but there is feedback from developers that sometimes an animation that triggers layout can still perform better than one that runs on the compositor.

ehsan-karamad commented 6 years ago

On the contrary, I think we very much want behavior to be consistent between browsers.

Yes, this is exactly what I meant (sorry for convoluted wording).

I'm still concerned about this policy. It might be easier to support if very specific and compelling use cases are made available.

I think so far I can think of two use cases for this policy:

I had thought it might be possible to try defining the set of allowed animations as something less engine-specific than, "Can be animated on the compositor" and more spec-related such as, "Does not influence layout" but there is feedback from developers that sometimes an animation that triggers layout can still perform better than one that runs on the compositor.

I agree "being composited" or "does not influence layout" do not, universally, mean that performance gets better. But the parametric policy provides a reasonable tool to the developers to try and match different combinations that works best for their (possibly-too-large-to-individually-change-animations) project.

I think we should be more clear on what we expect in terms of performance here. On one hand, performance might hint at smoother animations; as argued this does not necessarily equal being composited. On the other hand, responsiveness of page could be improved by offloading or disabling some of the main thread work. The latter measure of performance seems to suggest animations that could potentially offload some work on GPU and/or a separate (compositor) thread (depending on the browser and some other factors mentioned before) should be prioritized over those that mainly run on CPU and the main thread.

birtles commented 6 years ago

I think so far I can think of two use cases for this policy:

Disable all or most animations on a page to reduce CPU usage. If the set of enabled animations is something as small as {transform(s), opacity} this policy could reduce CPU load by disabling the rest. Even going for disable all animations as v0 would serve that purpose.
The parametric version can be used in test and measurement by the developers.

These aren't particularly strong use cases. What is the motivation for this feature in the first place? Is there a business need? Customer request? etc.

ehsan-karamad commented 6 years ago

The initial motivation as I understand is blocking animations from using up CPU cycles (for main frame responsiveness).

As for an application right now I can think of <amp-carousel>s and <portal>s. I am not aware of any customer requests.

ehsan-karamad commented 6 years ago

To conclude this thread so far:

I will update the explainer and ping you to kindly take another look. The interim goal still is to come up with a set of animations to disable when the policy is used. The set will include the animations that cause layout (or can cause layout).