Hiroyuki Ikezoe (:hiro) Assignee
	Description • 7 years ago

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.

	Brian Birtles (:birtles)
	Comment 1 • 7 years ago

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

	Hiroyuki Ikezoe (:hiro) Assignee
	Comment 2 • 7 years ago

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

	Hiroyuki Ikezoe (:hiro) Assignee
	Comment 3 • 7 years ago

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

	Hiroyuki Ikezoe (:hiro) Assignee
	Comment 4 • 7 years ago

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.

	Timothy Nikkel (:tnikkel)
	Comment 7 • 7 years ago
mozreview-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/#review216032

	Boris Chiou [:boris]
	Comment 8 • 7 years ago
mozreview-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 `}`

	Boris Chiou [:boris]
	Comment 9 • 7 years ago
mozreview-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 ..."

	Hiroyuki Ikezoe (:hiro) Assignee
	Comment 12 • 7 years ago

Thanks you guys for the review!

	Pulsebot
	Comment 13 • 7 years ago

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

	Tiberius Oros[:tiberius_oros]
	Comment 14 • 7 years ago
bugherder

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