1421507 - Throttle animations on position:fixed element which is out-of-view

Status: RESOLVED FIXED

(Core :: DOM: Animation, defect, P2)

Description

[Hiroyuki Ikezoe (:hiro)]

Attachment: An example

Attachment file has an animation on an element which is out-of-view and position:absolute.  The animation does not run on the compositor, so it consumes the main thread unfortunately.

Comment 1

[Brian Birtles (:birtles, Feb 26~ parental leave)]

This appears to be causing high CPU usage for Tree Style Tabs (see bug 1423398). We should try to tackle this in Q1.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Actually the element is position:fixed.  We've already been throttling animations in position:absolute element.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5fd7b88114b6fd59db8e205ad07bdca8ca02b627

Comment 4

[Hiroyuki Ikezoe (:hiro)]

The try in comment 3 looks good.
CCing :tnikkel.  I believe calling GetNearestScrollableFrame with SCROLLABLE_FIXEDPOS_FINDS_ROOT is a right fix for this issue.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1421507 - Test cases for position:absolute element.

Review commit: https://reviewboard.mozilla.org/r/210246/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210246/

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1421507 - Throttle animations in position:fixed element that the element is out of view.

Review commit: https://reviewboard.mozilla.org/r/210248/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210248/

Comment 7

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8939973 [details]
Bug 1421507 - Throttle animations in position:fixed element that the element is out of view.

https://reviewboard.mozilla.org/r/210248/#review216032

Comment 8

[Boris Chiou [:boris]]

Comment on attachment 8939972 [details]
Bug 1421507 - Test cases for position:absolute element.

https://reviewboard.mozilla.org/r/210246/#review216034

::: dom/animation/test/mozilla/file_restyles.html:1153
(Diff revision 1)
> +      if (!offscreenThrottlingEnabled) {
> +        return;
> +      }
> +
> +      var parentDiv = addDiv(null,
> +                             { style: 'position: absolute; top: -1000px;'});

nit: need one extra space before `}`.

::: dom/animation/test/mozilla/file_restyles.html:1163
(Diff revision 1)
> +      var animation = targetDiv.getAnimations()[0];
> +      await animation.ready;
> +
> +      var markers = await observeStyling(5);
> +      is(markers.length, 0,
> +         'CSS animation in out-of-view position absolute element should be ' +

nit: "... in a out-of-view ..."

::: dom/animation/test/mozilla/file_restyles.html:1178
(Diff revision 1)
> +        return;
> +      }
> +
> +      var div = addDiv(null,
> +                       { style: 'animation: background-color 100s; ' +
> +                                'position: absolute; top: -1000px;'});

nit: need one extra space before `}`

Comment 9

[Boris Chiou [:boris]]

Comment on attachment 8939973 [details]
Bug 1421507 - Throttle animations in position:fixed element that the element is out of view.

https://reviewboard.mozilla.org/r/210248/#review216038

Based on tnikkel's r+ for nsFrame, the test case is good to me.

::: dom/animation/test/mozilla/file_restyles.html:1199
(Diff revision 1)
> +      if (!offscreenThrottlingEnabled) {
> +        return;
> +      }
> +
> +      var parentDiv = addDiv(null,
> +                             { style: 'position: fixed; top: -1000px;'});

nit: need one extra space before `}`

::: dom/animation/test/mozilla/file_restyles.html:1209
(Diff revision 1)
> +      var animation = targetDiv.getAnimations()[0];
> +      await animation.ready;
> +
> +      var markers = await observeStyling(5);
> +      is(markers.length, 0,
> +         'CSS animation on out-of-view position:fixed element should be ' +

nit: "... on an out-of-view ..."

::: dom/animation/test/mozilla/file_restyles.html:1224
(Diff revision 1)
> +        return;
> +      }
> +
> +      var div = addDiv(null,
> +                       { style: 'animation: background-color 100s; ' +
> +                                'position: fixed; top: -1000px;'});

nit: same here

::: dom/animation/test/mozilla/file_restyles.html:1231
(Diff revision 1)
> +      var animation = div.getAnimations()[0];
> +      await animation.ready;
> +
> +      var markers = await observeStyling(5);
> +      is(markers.length, 0,
> +         'CSS animation on out-of-view position:fixed element should be ' +

nit: "... on an out-of-view ..."

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8939972 [details]
Bug 1421507 - Test cases for position:absolute element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/210246/diff/1-2/

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8939973 [details]
Bug 1421507 - Throttle animations in position:fixed element that the element is out of view.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/210248/diff/1-2/

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Thanks you guys for the review!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7fb87e5f9a62
Test cases for position:absolute element. r=boris
https://hg.mozilla.org/integration/autoland/rev/d9cef5fcc2bb
Throttle animations in position:fixed element that the element is out of view. r=boris,tnikkel

Comment 14

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/7fb87e5f9a62
https://hg.mozilla.org/mozilla-central/rev/d9cef5fcc2bb