718586 - Profile Migrators should not need to call doStartup

Status: RESOLVED WONTFIX

(Toolkit :: Startup and Profile System, defect)

Description

[Asaf Romano (gone)]

I'm not sure how and why this happened.

1. All profile migrators call doStartup before they do any migration.
2. XRE also calls DoStartup as well after migration (or after deciding not to call migration). In the case of migration, DoStartup filters the second call.

Wouldn't it be simpler to call DoStartup before calling migration?

On a side note, nsIProfileStartup is passed to nsIProfileMigrator, in order to allow migrators to get the profile folder before "ProfD" can be retrieved from the directory service. Questions:
1. After calling DoStartup, isn't ProfD set already?
2. The new Firefox migrator "ignores" the provided aStartup and uses ProfDS. Is that bad?

So, it seems we can get rid of nsIProfileStartup altogether.

Comment 1

[Benjamin Smedberg]

I'm sure *why* it happened: some of the previous profile migrators wanted to be able to copy files (the pref file, as well as bookmarks files, IIRC) from an existing profile before "starting" the current profile. For example the phoenix/firebird->firefox migrators. I thought that the seamonkey migrator did this as well.

Doesn't the new firefox->firefox migrator also rely on this feature?

Comment 2

[Asaf Romano (gone)]

Thanks for the explanation.  Those migrators are in the graveyard for some time now, and are very likely to stay there (even phoenix, yes).

The new Firefox->Firefox migrator definitely *could* take advantage of this feature, but it doesn't. See
http://hg.mozilla.org/mozilla-central/annotate/c550ffd3ba74/browser/components/migration/src/FirefoxProfileMigrator.js#l206

While I'm pretty sure that's just a result of copying code from the chrome migrator, I don't think we'd benefit much from relying on this optimization. It's optimizing the Ts for a very rare start-up scenario, in the price of an error-prone, untested API.

Comment 3

[Asaf Romano (gone)]

As for the second issue, getting "ProfD" after doStartup works fine, meaning we can indeed remove nsIProfileStartup altogether.

Comment 4

[Asaf Romano (gone)]

Other than the Firefox Migrator (which doesn't use it), the SeaMonkey profile migrator of Thunderbird is also a legitimate use case of this feature.  However, I think they can manage the same way the Firefox Migrator does.

Comment 5

[Asaf Romano (gone)]

Attachment: Proposed removal (just the toolkit part)

I'll patch browser/ and mail/ when/if we generally agree this change is desired.

Comment 6

[Matthew N. [:MattN]]

(In reply to Mano from comment #2)
> The new Firefox->Firefox migrator definitely *could* take advantage of this
> feature, but it doesn't.

I've actually noticed a bug where the key3.db is not being used after migrating and I suspect it's because of a race condition between copying the file to the destination profile and starting up the component that loads it.  I'm going to fix this in bug 717070 by moving the call to DoStartup after password migration. There will need to be a way to migrate some data before DoStartup if this is the actual cause.

Comment 7

[Asaf Romano (gone)]

Sigh, I didn't realize you've already commented on this bug. Sorry!

So, as we both are the only ones concerned, I won't repeat my words :)

Comment 8

[Matthew N. [:MattN]]

Do most migrators need DoStartup called before they begin migration? It's not going to be trivial to get PSM to read the new key3.db from what I can tell. 

I filed bug 721242 to track the Firefox migrator password decryption issue.

Comment 9

[Asaf Romano (gone)]

WONTFIX, for the time being.