Closed Bug 784759 Opened 12 years ago Closed 4 years ago

Move database creation out of startup path

Categories

(Firefox for Android Graveyard :: Data Providers, defect)

All
Android
defect
Not set
normal

Tracking

(fennec+)

RESOLVED INCOMPLETE
Firefox 29
Tracking Status
fennec + ---

People

(Reporter: wesj, Unassigned, NeedInfo)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached patch Patch (obsolete) (deleted) — 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] Patch 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] Patch 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. https://hg.mozilla.org/integration/mozilla-inbound/rev/bb414dc59262
Blocks: 789185
We should fix the problem and get this landed. This is 300ms or about 20% of our startup time.
Backed out again in http://hg.mozilla.org/integration/mozilla-inbound/rev/64a0ab78b261 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) (deleted) — Splinter Review
Unbitrotted (and updated). Tossed onto try as well: https://tbpl.mozilla.org/?tree=Try&rev=dd7a68a312ad
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 v3 (deleted) — Splinter 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: 76b9e0793b76 I think the errors were likely caused by the ViewFlipper patch. Back in: https://hg.mozilla.org/integration/fx-team/rev/33060d0581d6
Hi, sorry backedout as part of https://tbpl.mozilla.org/?tree=Fx-Team&rev=c15e29ead092 for the suspicion of causing robocop testfailures on android like https://tbpl.mozilla.org/php/getParsedLog.php?id=33734468&tree=Fx-Team
Attached patch Patch 3.1 (deleted) — Splinter 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
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Status: RESOLVED → REOPENED
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: ? → +
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: REOPENED → RESOLVED
Closed: 11 years ago4 years ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: