879097 - Remove "FormHistory:Init" and "Passwords:Init" messages

Status: RESOLVED WONTFIX

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

Description

[:Margaret Leibovic]

Follow-up to bug 878692.

It looks like these DBs will always get created once Gecko is up and running, so we don't need to force it with these init messages.

Comment 1

[:Margaret Leibovic]

Attachment: WIP

Here's a quick WIP. I'm wondering if we still need to be doing something in PerProfileContentProvider to initalize Gecko when we find that the DBs don't exist.

It seems fishy that we're sending "Passwords:Init" in PasswordsProvider, but also sending it in GeckoMessageReceiver. Also, why are we only using that for passwords, but not for form history?

Comment 2

[Wesley Johnston (:wesj)]

Comment on attachment 757744 [details] [diff] [review]
WIP

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

I agree, I worry a little too. I guess the downside is, if your db is somehow erased, but your profile isn't, we wouldn't recreate it until you did something that created it, and syncs would fail. If you've lost your databases, but not your profile, something really strange is up, but I'm hoping rnewman knows better about it.

The passwords stuff is different because it runs in its own process and uses intents to communicate with the main one. This is probably the simplest way to do that.

Comment 3

[Richard Newman [:rnewman]]

Comment on attachment 757744 [details] [diff] [review]
WIP

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

As Wes mentioned, that chunk of passwords code is there to make sure pwmgr is running in its separate process when the password provider is (though I don't see how it will keep it alive, mm? Does it ever die?).

My concern here is that there's nothing connecting FormHistoryProvider to Gecko, unless you fill in that "XXX". If a sync tries to run but Fennec isn't in the background, the code you added to browser.js won't run.

So in theory:

* Upgrade Fennec such that a form history DB schema change happens.
* On another device, add form history items and sync.
* Accounts > Firefox Sync > Sync Now.

The sync should fail, or cause damage to the DB (because the schema upgrade didn't occur -- you're cross-mojinating SQLiteOpenHelper and mozStorage upgrades).

Probably that "XXX" would just be a lifting of the sendEventToGecko that you're deleting from FormHistoryProvider, no? And in that case, maybe this patch will shrink a lot.

So, in the spirit of Socratic reviewing: can you prove to me that accessing the FHP without first launching GeckoApp will cause the DB to be correctly initialized and upgraded?

(And, in the spirit of fairness, I don't know if it *currently* does so.)

If you can prove that there's no regression, then rock on.

Comment 4

[Wesley Johnston (:wesj)]

I don't think it happens. If you try to access form history (or passwords) and Gecko has never run (or the db needs an upgrade), the access will fail (since we can't throw across process, you just get a return value that indicates no insert/update/delete/etc. happened).

Comment 5

[:Margaret Leibovic]

I haven't had time to work on this.

Comment 6

[Richard Newman [:rnewman]]

Currently we spam

07-16 16:00:01.397 E/GeckoConsole(29384): [JavaScript Error: "storage.init is not a function" {file: "chrome://browser/content/browser.js" line: 1624}]

during startup.

From:

      case "Passwords:Init": {
        let storage = Cc["@mozilla.org/login-manager/storage/mozStorage;1"].
                      getService(Ci.nsILoginManagerStorage);
        storage.init();
        Services.obs.removeObserver(this, "Passwords:Init");
        break;
      }

Comment 7

[:Margaret Leibovic]

I don't think it's worth the time and energy to try to do this.