Closed Bug 1354789 Opened 3 years ago Closed 3 years ago

Cache layout information needed in _positionPinnedTabs

Categories

(Firefox :: Tabbed Browser, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: dao, Assigned: dao)

References

(Blocks 2 open bugs)

Details

(Keywords: perf)

Attachments

(1 file)

(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 on attachment 8856170 [details]
Bug 1354789 - Cache layout information needed in _positionPinnedTabs.

https://reviewboard.mozilla.org/r/128096/#review130874
Attachment #8856170 - Flags: review?(florian) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ee9bd670aadf
Cache layout information needed in _positionPinnedTabs. r=florian
https://hg.mozilla.org/mozilla-central/rev/ee9bd670aadf
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Blocks: 1389744
You need to log in before you can comment on or make changes to this bug.