Closed Bug 1421507 Opened 2 years ago Closed 2 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: hiro, Assigned: hiro)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(3 files)

Attached file 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.
Priority: -- → P3
This appears to be causing high CPU usage for Tree Style Tabs (see bug 1423398). We should try to tackle this in Q1.
Priority: P3 → P2
Depends on: 1425213
Actually the element is position:fixed.  We've already been throttling animations in position:absolute element.
Summary: Throttle animations on position:absolute element which is out-of-view → Throttle animations on position:fixed element which is out-of-view
Keywords: perf
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 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
Attachment #8939973 - Flags: review?(tnikkel) → review+
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 `}`
Attachment #8939972 - Flags: review?(boris.chiou) → review+
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 ..."
Attachment #8939973 - Flags: review?(boris.chiou) → review+
Thanks you guys for the review!
Pushed by hikezoe@mozilla.com:
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
Assignee: nobody → hikezoe
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/7fb87e5f9a62
https://hg.mozilla.org/mozilla-central/rev/d9cef5fcc2bb
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Blocks: 1520277
You need to log in before you can comment on or make changes to this bug.