There is an uninterruptible reflow in setToolbarButtonHeightProperty when the `forceLayoutFlushIfNeeded` option is used. With the patches from bug 1383367 we won't need to use this and can instead wait for the reflow before starting the animation.

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #0) > There is an uninterruptible reflow in setToolbarButtonHeightProperty when > the `forceLayoutFlushIfNeeded` option is used. > > With the patches from bug 1383367 we won't need to use this and can instead > wait for the reflow before starting the animation. It might mean that the animation starts one frame later (see also bug 1385729). I'm not sure that's desirable. :/

(In reply to Dão Gottwald [::dao] from comment #1) > (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #0) > > There is an uninterruptible reflow in setToolbarButtonHeightProperty when > > the `forceLayoutFlushIfNeeded` option is used. > > > > With the patches from bug 1383367 we won't need to use this and can instead > > wait for the reflow before starting the animation. > > It might mean that the animation starts one frame later (see also bug > 1385729). I'm not sure that's desirable. :/ For this animation it will not be a problem. The pin-to-overflow animation shows the overflow arrow in its original position, then rotates the arrow down, then rotates the arrow back to the original position. Painting the arrow in its original position for the first frame will not cause any awkwardness with this animation, but we may want to reduce the animation by one frame at the beginning if we think it looks like it is too slow to begin moving.

I'm not concerned about some kind of awkwardness. It's a matter of perceived performance. Animations should start ASAP after the user interaction they respond to. If there are idle frames at the beginning, we should remove those regardless of this bug.

chrome://browser/skin/chevron-animation.svg is the animation, which at first glance has what appears to be roughly 3 idle frames at the beginning, though inspecting the SVG shows that there are minor pixel differences between each frame. I believe we could remove the first frame of the animation as part of fixing this bug and there would be no issue.

Comment on attachment 8892580 [details] Bug 1383689 - Remove uninterruptible reflow at setToolbarButtonHeightProperty@resource://gre/modules/BrowserUtils.jsm. https://reviewboard.mozilla.org/r/163584/#review168938 ::: browser/themes/shared/toolbarbutton-icons.inc.css:392 (Diff revision 1) > } > > #nav-bar-overflow-button[animate] > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image { > animation-name: overflow-animation; > - animation-timing-function: steps(70); > - animation-duration: 1.1s; > + animation-timing-function: steps(69); > + animation-duration: 1104ms; Why has this inreased when the number of frames has decreased? ::: toolkit/modules/BrowserUtils.jsm:437 (Diff revision 1) > } > let bounds = dwu.getBoundsWithoutFlushing(toolbarItem); > - if (!bounds.height && options.forceLayoutFlushIfNeeded) { > + if (!bounds.height) { > + let document = element.ownerDocument; > + await BrowserUtils.promiseLayoutFlushed(document, "layout", () => { > - bounds = toolbarItem.getBoundingClientRect(); > + bounds = toolbarItem.getBoundingClientRect(); Why can't we use dwu again here?

Comment on attachment 8892580 [details] Bug 1383689 - Remove uninterruptible reflow at setToolbarButtonHeightProperty@resource://gre/modules/BrowserUtils.jsm. https://reviewboard.mozilla.org/r/163584/#review168938 > Why has this inreased when the number of frames has decreased? It should have been 1120ms before at 70 frames. This is now 1104ms at 69 frames. It is now slightly longer but it is more correct now than it was before. > Why can't we use dwu again here? Yeah, we can. I went back and forth on this and didn't think it would be so expensive to put in something that would guarantee we would get the value we wanted since we had just waited for layout flush. I'll change this to use dwu.

Comment on attachment 8892580 [details] Bug 1383689 - Remove uninterruptible reflow at setToolbarButtonHeightProperty@resource://gre/modules/BrowserUtils.jsm. https://reviewboard.mozilla.org/r/163584/#review168970

Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/daca6e4033c7 Remove uninterruptible reflow at setToolbarButtonHeightProperty@resource://gre/modules/BrowserUtils.jsm. r=Gijs