1354789 - Cache layout information needed in _positionPinnedTabs

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, enhancement)

Description

[Dão Gottwald [:dao]]

(In reply to Florian Quèze [:florian] [:flo] from bug 1354782 comment 4)
> > > (for a follow-up) Could we save the pinned tab's width once at the time the
> > > tab is pinned (or soon after), and avoid this getBoundingClientRect call
> > > completely?
> > 
> > Yes, but with two caveats:
> > 
> > - We already flush layout before the loop, so getBoundingClientRect is cheap
> > at this point. We'd have to cache scrollButtonWidth and paddingStart too.
> > 
> > - We'd have to clear the cache when the theme layout changes (e.g. when
> > enabling a compact theme, but we already call _positionPinnedTabs there for
> > the same reason, so it's probably not a big deal)
> > 
> > Do you think the added complexity would be worth it?
> 
> Yes. Any layout flush we can remove (especially when they are during user
> interactions where smooth visual feedback is expected like resizing) is
> worth it. But it's not worth blocking landing of removing the invalidation
> loop for several days, this is why I said "for a follow-up". Up to you to
> decide if you want to try here or file a lower priority follow-up :-).

Comment 1

[Dão Gottwald [:dao]]

Attachment: Bug 1354789 - Cache layout information needed in _positionPinnedTabs.

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

Comment 2

[Florian Quèze [:florian]]

Comment on attachment 8856170 [details]
Bug 1354789 - Cache layout information needed in _positionPinnedTabs.

https://reviewboard.mozilla.org/r/128096/#review130874

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 8856170 [details]
Bug 1354789 - Cache layout information needed in _positionPinnedTabs.

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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee9bd670aadf
Cache layout information needed in _positionPinnedTabs. r=florian

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ee9bd670aadf