Closed
Bug 735912
Opened 10 years ago
Closed 10 years ago
Passwords database is not accessible for Sync through Content Provider on clean install.
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(blocking-fennec1.0 beta+)
VERIFIED
FIXED
Firefox 14
Tracking | Status | |
---|---|---|
blocking-fennec1.0 | --- | beta+ |
People
(Reporter: liuche, Assigned: wesj)
References
Details
(Whiteboard: [sync])
Attachments
(7 files)
6.24 KB,
application/msword
|
Details | |
1.90 KB,
application/msword
|
Details | |
31.41 KB,
patch
|
Details | Diff | Splinter Review | |
1.61 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
2.91 KB,
patch
|
mfinkle
:
review-
|
Details | Diff | Splinter Review |
762 bytes,
patch
|
bnicholson
:
review+
|
Details | Diff | Splinter Review |
5.68 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
Found while implementing password syncing in Android Sync. 1. Apply patch changes. 2. Start Fennec (to create ContentProviders) 3. Pair device to desktop Firefox with Sync. 4. Syncing starts. Expected result: Checking passwords database availability succeeds (fetch for a guid). Actual result: Null Cursor Exception is thrown. This indicates the column wasn't found. Note: If a password is saved before setting up sync, the passwords database check succeeds. Is the passwords database created lazily? It needs to be accessible on start, because users may set up sync w/o saving any passwords.
Reporter | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Yes, the db is created and upgraded lazily. If Gecko is running, we fire off a message to it and create the database (i.e. you can retry soon after and things should succeed). Otherwise, there's not much we can do without starting Gecko (unless we want to duplicate all the database creation and migration code in Java). If sync wants some method to determine the difference between these two positions, we can provide that.
Assignee | ||
Comment 3•10 years ago
|
||
Form History should be expecting this same problem (I think?), so I'm cc'ing nalexander to make sure we're aware that it can happen there too.
Reporter | ||
Comment 4•10 years ago
|
||
Reporter | ||
Comment 5•10 years ago
|
||
wesj, I think that would be fine as a workaround. Please provide a method for sync to check whether a db exists and fire off db creation if not; we can skip the initial passwords sync and reschedule.
Assignee | ||
Comment 6•10 years ago
|
||
Sorry. I don't think expressed that quite right. If 1.) sync tries to do anything with the Content Provider, 2.) the database doesn't exist or is the wrong version, and 3.) Gecko is running we will fire off the database creation for you. You should be able to try again very soon after (within seconds) and things will work. Maybe we should also just force database creation on first run as well. I think we can do that (just a bit worried about startup impact). If Gecko is not running, we don't do anything. You can keep trying, and we will keep failing. I've wondered if you guys need some way to query if Gecko is running. Are you looking for a way to start Gecko from Sync? I don't think we want to do that...
Comment 7•10 years ago
|
||
Nom'ing for triage. this will block password sync work: bug 709385
blocking-fennec1.0: --- → ?
Updated•10 years ago
|
blocking-fennec1.0: ? → beta+
Comment 8•10 years ago
|
||
Wes - it seems that the password DB is not created by Firefox until a password is saved via the UI. That's not good. Looks like we need an explicit "make the DB" method. If we get that method, we could try to do the silent install of the DB.
Updated•10 years ago
|
Whiteboard: [sync]
Assignee | ||
Comment 9•10 years ago
|
||
OK. Sounds like we want two things: 1.) Set up these databases earlier, without the user having to do something. That means, every time Fennec is upgraded/installed, we should do this at (delayed) startup. We can do this in either Java or JS, but I really want to make sure we only hit it on those "special" startups. 2.) Add a new content provider so that sync can query what's going on. filed Bug 736216 for that.
Assignee | ||
Comment 10•10 years ago
|
||
Using a trick similar to what BrowserCLH does: http://mxr.mozilla.org/mozilla-central/source/mobile/xul/components/BrowserCLH.js#87 Initializing the passwords database is pretty slow. 2-3s on my phone (and my phone's file system is usually pretty fast). I have a follow up to put these in a setTimeout and delay them.
Assignee: nobody → wjohnston
Attachment #606366 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 11•10 years ago
|
||
This adds a delayedActions object that builds its own queue of things to... delay.
Attachment #606367 -
Flags: review?(mark.finkle)
Comment 12•10 years ago
|
||
Comment on attachment 606366 [details] [diff] [review] Create the databases during startup >+ isUpdated: function() { isAppUpdated >+ onUpdate: function() { onAppUpdated
Attachment #606366 -
Flags: review?(mark.finkle) → review+
Comment 13•10 years ago
|
||
Comment on attachment 606367 [details] [diff] [review] Move to delayed startup >+var DelayedActions = { Let's hold off on this for now. We should consider using it for more in a separate bug. We should also try timing the affect on Ts. >- // Init LoginManager >- Cc["@mozilla.org/login-manager;1"].getService(Ci.nsILoginManager); >- // Init FormHistory >- Cc["@mozilla.org/satchel/form-history;1"].getService(Ci.nsIFormHistory2); Are we sure we can remove these? if we can safely remove these inits, then let's do it. Also: 916 } else if (aTopic == "Passwords:Init") { 917 var storage = Components.classes["@mozilla.org/login-manager/storage/mozStorage;1"]. 918 getService(Components.interfaces.nsILoginManagerStorage); 923 } else if (aTopic == "FormHistory:Init") { 924 var fh = Components.classes["@mozilla.org/satchel/form-history;1"]. 925 getService(Components.interfaces.nsIFormHistory2); 926 var db = fh.DBConnection; fix this code to use "Cc" and "Ci" and one-line and "var" -> "let" r- for new patch
Attachment #606367 -
Flags: review?(mark.finkle) → review-
Assignee | ||
Comment 14•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a46d0a93816
Assignee | ||
Comment 15•10 years ago
|
||
eek. backout the bad bits. https://hg.mozilla.org/integration/mozilla-inbound/rev/3849a7572a98
Comment 16•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2a46d0a93816 https://hg.mozilla.org/mozilla-central/rev/3849a7572a98
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
Assignee | ||
Comment 17•10 years ago
|
||
Landing on a Monday morning is apparently not advisable for me.
Attachment #608482 -
Flags: review?(bnicholson)
Updated•10 years ago
|
Attachment #608482 -
Flags: review?(bnicholson) → review+
Assignee | ||
Comment 18•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ba795535c17 thanks brian!
Assignee | ||
Comment 20•10 years ago
|
||
backout for test failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/197cccd053e9 will post a test fix in a second
Assignee | ||
Updated•10 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 21•10 years ago
|
||
This should fix the tests. Works locally. Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=7370272686db
Attachment #610756 -
Flags: review?(mark.finkle)
Updated•10 years ago
|
Attachment #610756 -
Flags: review?(mark.finkle) → review+
Comment 22•10 years ago
|
||
(In reply to Wesley Johnston (:wesj) from comment #21) > Created attachment 610756 [details] [diff] [review] > Test Fixes > > This should fix the tests. Works locally. Pushed to try: > > https://tbpl.mozilla.org/?tree=Try&rev=7370272686db Note: This try push did not include robocop. If in doubt, just run all the tests for Android Opt.
Comment 23•10 years ago
|
||
is this an issue?
Assignee | ||
Comment 24•10 years ago
|
||
Back on try: https://tbpl.mozilla.org/?tree=Try&rev=2ed200d7a807 I'l hopefully reland today sometime.
Assignee | ||
Comment 25•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/af9989ded605 https://hg.mozilla.org/integration/mozilla-inbound/rev/56d86fb6b887 Try looks good! Relanded with tests fixes.
Comment 26•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/56d86fb6b887 https://hg.mozilla.org/mozilla-central/rev/af9989ded605
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Status: RESOLVED → VERIFIED
Updated•1 year ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•