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)

enhancement

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)

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.
Priority: -- → P2
Whiteboard: [fxsearch]
Attached patch Patch (obsolete) — Splinter Review
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: nobody → florian
Status: NEW → ASSIGNED
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+
Attached patch Patch v2Splinter Review
- 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.
Attachment #8875784 - Attachment is obsolete: true
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.
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)
https://hg.mozilla.org/mozilla-central/rev/56ac88ccb3c6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Depends on: 1383749
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: