Open Bug 784759 Opened 8 years ago Updated 5 years ago

Move database creation out of startup path


(Firefox for Android :: Data Providers, defect)

Not set



Firefox 29
Tracking Status
fennec + ---


(Reporter: wesj, Unassigned, NeedInfo)


(Blocks 2 open bugs)



(2 files, 2 obsolete files)

Attached patch Patch (obsolete) — Splinter Review
We create/upgrade signons.sqlite and formhistory.sqlite at startup on fist run or upgrade for sync. That can take anywhere for 1-3sec. We should move it to after first paint, specifically to improve webapp first run experience.
Attachment #654318 - Attachment is patch: true
Comment on attachment 654318 [details] [diff] [review]

Whoops. Please ignore that stupid comment. At least it doesn't say something embarrassing....
Attachment #654318 - Flags: review?(mark.finkle)
Comment on attachment 654318 [details] [diff] [review]

good first step to offloading some of the firstrun work
Attachment #654318 - Flags: review?(mark.finkle) → review+
I backed this out on suspicion that our password provider tests rely on this happening early.
Blocks: 789185
Duplicate of this bug: 789185
We should fix the problem and get this landed. This is 300ms or about 20% of our startup time.
Backed out again in for crashing in every reftest, crashtests and jsreftest, and timing out in startup (or, equally likely, crashing but talos doesn't know how to detect startup crashes) in every talos test.
Still a problem at ~250ms on a Galaxy Nexus.
Blocks: 807322
Attached patch Patch v2 (obsolete) — Splinter Review
Unbitrotted (and updated). Tossed onto try as well:
Attachment #654318 - Attachment is obsolete: true
Comment on attachment 8365282 [details] [diff] [review]
Patch v2

>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

>+    // Store the low-precision buffer pref {

'{' at end of comment is a typo

>     this.gUseLowPrecision = Services.prefs.getBoolPref("layers.low-precision-buffer");
>     // notify java that gecko has loaded

nit: keep the blank line for readability

>   onAppUpdated: function() {
>-    // initialize the form history and passwords databases on upgrades
>-    Services.obs.notifyObservers(null, "FormHistory:Init", "");
>-    Services.obs.notifyObservers(null, "Passwords:Init", "");

Why not add the "DOMContentLoaded" listener right here? Use BrowserApp.deck.addEventListener(.., true) ?

>   addTab: function addTab(aURI, aParams) {
>     aParams = aParams || {};
>     let newTab = new Tab(aURI, aParams);
>     this._tabs.push(newTab);
>+    if (this._doLazyUpdate) {

I don't like _doLazyUpdate. Can't we just do it above?
Attached patch Patch v3Splinter Review
Using browser-delayed-startup-finished. Depends on bug 964510.
Attachment #8365282 - Attachment is obsolete: true
Attachment #8366304 - Flags: review?(mark.finkle)
Comment on attachment 8366304 [details] [diff] [review]
Patch v3

I'd rather not move onAppUpdate, but move some of it's contents. Probably making a _delayedStartup method.
Attachment #8366304 - Flags: review?(mark.finkle) → review+
This was backed out:

I think the errors were likely caused by the ViewFlipper patch. Back in:
Hi, sorry backedout as part of for the suspicion of causing robocop testfailures on android like
Attached patch Patch 3.1Splinter Review
Same patch, but uses the Java notification to fix some (now broken) tests.
Attachment #8369090 - Flags: review?(mark.finkle)
Attachment #8369090 - Flags: review?(mark.finkle) → review+
Assignee: nobody → wjohnston
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
backed out for startup regressions:
Resolution: FIXED → ---
OS: Linux → Android
Hardware: x86 → All
Comment on attachment 8369090 [details] [diff] [review]
Patch 3.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This caused a startup regression. Requesting uplift of the backout.
User impact if declined: Startup regression
Testing completed (on m-c, etc.): Been on mc for a week.
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none.
Attachment #8369090 - Flags: approval-mozilla-aurora?
Attachment #8369090 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Never got this to land happily. I do see some lazy password manager initialization code in the tree now though (along with the old non-lazy code). Might be worth someone looking into what's going on? Chenxia, do you know anything about password manager startup?
Assignee: wjohnston → nobody
tracking-fennec: --- → ?
Flags: needinfo?(liuche)
I think the real fix for this is Bug 946857.
Component: General → Data Providers
tracking-fennec: ? → +
You need to log in before you can comment on or make changes to this bug.