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

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Search
P1
normal
RESOLVED FIXED
7 months ago
10 days 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

7 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

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

Updated

a month ago
Blocks: 1353725
(Reporter)

Updated

a month ago
Whiteboard: [fxsearch] → [fxsearch][photon-performance]

Updated

a month ago
Flags: qe-verify?
Priority: P3 → P2
See Also: → bug 1357054
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: nobody → florian
Status: NEW → ASSIGNED

Updated

a month ago
Priority: P2 → P1

Updated

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

Comment 2

a month 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

a month 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

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

Updated

a month ago
Depends on: 1357800

Updated

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