Closed
Bug 1369705
Opened 7 years ago
Closed 7 years ago
avoid search UI overhead before first paint (search-one-offs_XBL_Constructor, search service load, searchbar context menu)
Categories
(Firefox :: Address Bar, enhancement, P2)
Firefox
Address Bar
Tracking
()
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: florian, Assigned: florian)
References
(Blocks 1 open bug)
Details
(Whiteboard: [fxsearch])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
urlbar_XBL_Constructor is triggered before first paint, and spends 40% of its time in _enableOrDisableOneOffSearches (which apparently calls search-one-offs_XBL_Constructor, so I guess it causes the search one off binding to be attached synchronously). From the name of this method, it looks like it could wait until the first time we open the awesomebar panel.
See this profile https://perfht.ml/2s1cSMz captured on a slow netbook to have more detailed data.
Updated•7 years ago
|
Priority: -- → P2
Whiteboard: [fxsearch]
Assignee | ||
Comment 1•7 years ago
|
||
This patch delays:
- loading the search service until after first paint (and this is covered by test),
- running the one-off-buttons constructor until the relevant panels are showing,
- initializing the searchbar context menu until it's first showing.
All these things were visible in this startup profile: https://perfht.ml/2sjFIrz
Attachment #8875784 -
Flags: review?(adw)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → florian
Status: NEW → ASSIGNED
Comment 2•7 years ago
|
||
Comment on attachment 8875784 [details] [diff] [review]
Patch
Review of attachment 8875784 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/components/search/content/search.xml
@@ +593,5 @@
> "anonid", "textbox-input-box");
> var cxmenu = document.getAnonymousElementByAttribute(textBox,
> "anonid", "input-box-contextmenu");
> + cxmenu.addEventListener("popupshowing", () => {
> + let stringBundle = document.getBindingParent(this)._stringBundle;
While you're touching this, could you please pull out all the code that builds the context menu into its own method and call it here?
Attachment #8875784 -
Flags: review?(adw) → review+
Assignee | ||
Comment 3•7 years ago
|
||
- Moved the context menu initialization to its own method per comment 2.
- Moved the this._textbox.placeholder = this._stringBundle.getString("searchPlaceholder"); line to the searchbar constructor to avoid the placeholder flickering.
- Fixed the browser_426329.js test which was testing the controller but never opening the context menu.
Assignee | ||
Updated•7 years ago
|
Attachment #8875784 -
Attachment is obsolete: true
Assignee | ||
Comment 4•7 years ago
|
||
Seems green on try after I fixed an issue with the test on Linux:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=35e0b7be0f5e6d023be059253397da5a1cc37560
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/56ac88ccb3c6
avoid starting the search service or calling the search-one-offs XBL constructor before first paint, r=adw.
Assignee | ||
Comment 6•7 years ago
|
||
Updating the summary to reflect what the patch did.
Summary: urlbar_XBL_Constructor shows up in startup profiles → avoid search UI overhead before first paint (search-one-offs_XBL_Constructor, search service load, searchbar context menu)
Comment 7•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in
before you can comment on or make changes to this bug.
Description
•