Cache one-off buttons instead of regenerating them at every popupopen

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Search
P1
normal
RESOLVED FIXED
8 months ago
a month ago

People

(Reporter: mak, Assigned: florian)

Tracking

(Blocks: 1 bug, {perf})

unspecified
Firefox 55
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [fxsearch][photon-performance])

Attachments

(1 attachment)

(Reporter)

Description

8 months ago
Now that the awesomebar uses one-off buttons, it would be better if we'd cache them. The search engines don't change so often and we should not pay the rebuild cost at every opening.
(Reporter)

Updated

5 months ago
Blocks: 1337003
No longer blocks: 1337003
(Reporter)

Updated

2 months ago
Blocks: 1353725
(Reporter)

Updated

2 months ago
Whiteboard: [fxsearch] → [fxsearch][photon-performance]

Updated

2 months ago
Flags: qe-verify?
Priority: P3 → P2
(Assignee)

Updated

2 months ago
See Also: → bug 1357054
(Assignee)

Comment 1

2 months ago
Created attachment 8859166 [details] [diff] [review]
Patch

We need to rebuild this list of one-offs either when the search settings change, or when the width of the panel changes.

I haven't found any simple way to get the current panel's width without flushing (that's bug 1357054 anyway), but we can get the textbox size without flushing, and that's good enough to decide if we can skip rebuilding the one-offs.

While I was at it, I also did some small optimizations:
- avoid doing any DOM calls related to open search items when building the one-offs for the location bar panel
- avoid using the Preferences.jsm module (it was used to support unicode characters in engine names, we can now use getStringPref instead).
Attachment #8859166 - Flags: review?(adw)
(Assignee)

Updated

2 months ago
Assignee: nobody → florian
Status: NEW → ASSIGNED

Updated

2 months ago
Priority: P2 → P1

Updated

2 months ago
Iteration: --- → 55.4 - May 1
Flags: qe-verify? → qe-verify+
QA Contact: adrian.florinescu

Comment 2

2 months ago
Comment on attachment 8859166 [details] [diff] [review]
Patch

Review of attachment 8859166 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM
Attachment #8859166 - Flags: review?(adw) → review+

Comment 3

2 months ago
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6187bdf62907
Cache one-off buttons instead of regenerating them at every popupopen, r=adw.

Comment 4

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6187bdf62907
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Updated

2 months ago
Depends on: 1357800

Updated

a month ago
Blocks: 1303462
You need to log in before you can comment on or make changes to this bug.