1365305 - 0.86ms uninterruptible reflow at _positionPinnedTabs@chrome://browser/content/tabbrowser.xml:6147:33

Status: NEW

(Firefox :: Tabbed Browser, defect, P5)

Description

[Mike Conley (:mconley) (:⚙️)]

Here's the stack:

_positionPinnedTabs@chrome://browser/content/tabbrowser.xml:6147:33
onxbloverflow@chrome://browser/content/tabbrowser.xml:5696:9

That's this stuff: http://searchfox.org/mozilla-central/rev/ae24a3c83d22e0e35aecfd9049c2b463ca7e045b/browser/base/content/tabbrowser.xml#6154-6158

Comment 1

[Perry Jiang [:perry] [no longer employee, use ni?]]

I wasn't able to detect this reflow with ohnoreflow on Nightly 57.0a1 (2017-08-02) when the code around this.childNodes[0].getBoundingClientRect().width gets called.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Hey Perry,

I'm going to re-open this. It's quite possible that:

1) Oh no! Reflow! is running with too high a threshold to detect it
2) The STR are very precise - likely also occurring once per browser window, because it looks like we're caching some results here: http://searchfox.org/mozilla-central/rev/ae24a3c83d22e0e35aecfd9049c2b463ca7e045b/browser/base/content/tabbrowser.xml#6151-6159

Still, we're using getBoundingClientRect(), which guarantees a layout flush if we ever hit that codepath with a dirty DOM. So I think this bug is still legitimate.

Comment 3

[Perry Jiang [:perry] [no longer employee, use ni?]]

Attachment: Bug 1365305 - wrap _positionPinnedTabs's getBoundingClientRect() in requestAnimationFrame

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 8895977 [details] [diff] [review]
Bug 1365305 - wrap _positionPinnedTabs's getBoundingClientRect() in requestAnimationFrame

This makes the pinned tab area flicker when the tab strip overflows.

Comment 5

[:Felipe Gomes (needinfo for replies!)]

I believe that we should downgrade this bug from p1 to a p2 or even a p3. There's a sync reflow here to get the pinned tab dimensions, but that only happens only once per session, and then that information is cached and the reflow won't happen again. Also, pinned tabs are not a widely known feature. And it's not clear there's a non-flickering way to avoid this reflow.

I'm directly marking as a p2. If anyone disagrees feel free to change it back.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Setting this to qf:p3 as per comment 5 for now.

Comment 8

[Florian Quèze [:florian]]

The code doing the sync layout flushes is still there: https://searchfox.org/mozilla-central/rev/a06d5a8871b1796f2dbd588ab518eaa98507e018/browser/base/content/tabbrowser-tabs.js#1463-1469

Interestingly, this branch of the code seems to have 0 test coverage. Is it possible that no test exercises pinned tabs with an overflowing tab bar?