avoid search UI overhead before first paint (search-one-offs_XBL_Constructor, search service load, searchbar context menu)

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Location Bar
P2
normal
RESOLVED FIXED
3 months ago
27 days ago

People

(Reporter: florian, Assigned: florian)

Tracking

(Blocks: 2 bugs)

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

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [fxsearch])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 months ago
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]
(Assignee)

Comment 1

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

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

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

Comment 2

2 months 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

2 months ago
Created attachment 8875927 [details] [diff] [review]
Patch v2

- 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

2 months ago
Attachment #8875784 - Attachment is obsolete: true
(Assignee)

Comment 4

2 months ago
Seems green on try after I fixed an issue with the test on Linux:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=35e0b7be0f5e6d023be059253397da5a1cc37560

Comment 5

2 months ago
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

2 months 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

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

Updated

27 days ago
Depends on: 1383749
You need to log in before you can comment on or make changes to this bug.