Cache layout information needed in _positionPinnedTabs

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: dao, Assigned: dao)

Tracking

(Blocks: 2 bugs, {perf})

Trunk
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

3 months ago
(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 hidden (mozreview-request)

Comment 2

3 months ago
mozreview-review
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+
Comment hidden (mozreview-request)

Comment 4

3 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ee9bd670aadf
Cache layout information needed in _positionPinnedTabs. r=florian

Comment 5

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ee9bd670aadf
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
No longer blocks: 1348289
Blocks: 1348289
You need to log in before you can comment on or make changes to this bug.