Closed Bug 1129607 Opened 5 years ago Closed 5 years ago

Regression: Time it takes to show engines/results on first launch initial search has regressed

Categories

(Firefox for Android :: General, defect)

38 Branch
ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 38
Tracking Status
firefox36 --- fixed
firefox37 --- fixed
firefox38 --- fixed
fennec 38+ ---

People

(Reporter: aaronmt, Assigned: mfinkle)

References

Details

(Keywords: perf, regression, reproducible)

Attachments

(2 files)

[Tracking Requested - why for this release]:

Currently on trunk, it takes longer to retrieve engine list and results on first time search.
Attached video video.mp4
Blocks: 1117186
Summary: Regression: Time it takes to show engines/results on first time search has regressed → Regression: Time it takes to show engines/results on first launch initial search has regressed
> 03:41  mfinkle:	it's the async init
> 03:41  mfinkle:	if i do: first run, awesome screen, then settings
> 03:41  mfinkle:	the list is shown normally
Assignee: nobody → mark.finkle
tracking-fennec: ? → 38+
This patch will delay init the Search service along with some other performance impacting services. Because we are now delay-initing the service instead of lazy-initing the service, the country code fetch happens before we attempt to display the list of search engines (or use them in the awesomescreen). 

We still need to wait for Gecko to load before the engines are available though, but the regression should be fixed.
Attachment #8560014 - Flags: review?(margaret.leibovic)
Comment on attachment 8560014 [details] [diff] [review]
early-init-search v0.1

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

Looks fine to me. Let's just monitor startup performance to make sure this isn't regressing that by accident.

::: mobile/android/chrome/content/browser.js
@@ +373,4 @@
>            Cc["@mozilla.org/login-manager;1"].getService(Ci.nsILoginManager);
> +          Services.search.init();
> +
> +          // Spin up some features which impact performance.

We don't need this comment twice.
Attachment #8560014 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8560014 [details] [diff] [review]
early-init-search v0.1

Approval Request Comment
[Feature/regressing bug #]: bug 1117186 exposes this regression because we now fetch the country code
[User impact if declined]: Noticeable delay when displaying the Awesomescreen search or the Setting search liist
[Describe test coverage new/current, TreeHerder]: Minimal so far
[Risks and why]: Low risk since we aren't doing anything new, just doing it a bit sooner. Might cause a startup regression that would be caught by Autophone tests.
[String/UUID change made/needed]: None
Attachment #8560014 - Flags: approval-mozilla-beta?
Attachment #8560014 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e743d6237be9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Attachment #8560014 - Flags: approval-mozilla-beta?
Attachment #8560014 - Flags: approval-mozilla-beta+
Attachment #8560014 - Flags: approval-mozilla-aurora?
Attachment #8560014 - Flags: approval-mozilla-aurora+
Needs rebasing for uplift.
Flags: needinfo?(mark.finkle)
You need to log in before you can comment on or make changes to this bug.