Load extensions that have their manifest in the Prefs on the UI thread.

Load extensions that have their manifest in the Prefs on the UI thread.

There is some migration code for loading old extensions which we do on the UI
thread as well, for simplicity. This will only happen the first time the user
starts Chrome, so I think it's OK.

Also cleaned up the EXTENSIONS_LOADED notification to send out a single
extension, instead of a 1-element list of extensions.

BUG=18293
TEST=loading extensions should still work

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=23997

[Aaron Boodman, 2009-08-21 01:06:41]

Drive-by: Why do we still need ValidateInstalledExtension()? It seems to me that
we should just trust that the extension is in a good state once it is installed.

[Erik does not do reviews, 2009-08-21 01:13:06]

http://codereview.chromium.org/174208/diff/1/4
File chrome/browser/extensions/extension_process_manager.cc (right):

http://codereview.chromium.org/174208/diff/1/4#newcode155
Line 155: CreateBackgroundHosts(this, &list);
add a TODO to make this just take a single Extension?

http://codereview.chromium.org/174208/diff/1/6
File chrome/browser/extensions/extensions_service.cc (right):

http://codereview.chromium.org/174208/diff/1/6#newcode408
Line 408: extension_prefs_->MigrateToPrefs(extension);
doesn't this mean that already migrated extensions will go through this path?

http://codereview.chromium.org/174208/diff/1/6#newcode596
Line 596: void ExtensionsServiceBackend::ValidateInstalledExtension(
It looks like the only reason this is called now is for CheckExternalUninstall. 
I think that that can be done from the UI thread (it could check the registry or
prefs).  It would also be nice to just call it that rather than Validate to
avoid confusion.

[Matt Perry, 2009-08-21 01:51:32]

http://codereview.chromium.org/174208/diff/1/4
File chrome/browser/extensions/extension_process_manager.cc (right):

http://codereview.chromium.org/174208/diff/1/4#newcode155
Line 155: CreateBackgroundHosts(this, &list);
On 2009/08/21 01:13:06, Erik Kay wrote:
> add a TODO to make this just take a single Extension?

I went ahead and did it in this CL.

http://codereview.chromium.org/174208/diff/1/6
File chrome/browser/extensions/extensions_service.cc (right):

http://codereview.chromium.org/174208/diff/1/6#newcode408
Line 408: extension_prefs_->MigrateToPrefs(extension);
On 2009/08/21 01:13:06, Erik Kay wrote:
> doesn't this mean that already migrated extensions will go through this path?

Yeah, but it just resets a pref so it doesn't hurt.

http://codereview.chromium.org/174208/diff/1/6#newcode596
Line 596: void ExtensionsServiceBackend::ValidateInstalledExtension(
On 2009/08/21 01:13:06, Erik Kay wrote:
> It looks like the only reason this is called now is for
CheckExternalUninstall. 
> I think that that can be done from the UI thread (it could check the registry
or
> prefs).  It would also be nice to just call it that rather than Validate to
> avoid confusion.

All the external_extension_providers_ stuff is part of the backend, so it would
take a bit of work to move this one function to the UI thread. I'd prefer to
just keep this function here for now to minimize the change.

I renamed it to CheckInstalledExtension... not sure what would be better.

[Erik does not do reviews, 2009-08-21 15:46:05]

http://codereview.chromium.org/174208/diff/1015/20
File chrome/browser/extensions/extensions_service.cc (right):

http://codereview.chromium.org/174208/diff/1015/20#newcode599
Line 599: if (CheckExternalUninstall(id, location)) {
since this is the only callsite, let's just merge the two methods

http://codereview.chromium.org/174208/diff/1015/21
File chrome/browser/extensions/extensions_service.h (right):

http://codereview.chromium.org/174208/diff/1015/21#newcode275
Line 275: // Runs a sanity check on each extension in the installed extension
list.
update comment

http://codereview.chromium.org/174208/diff/1015/21#newcode277
Line 277: void CheckInstalledExtensions(scoped_refptr<ExtensionsService>
frontend,
How about "CheckExternalUninstall" and only call this for external extensions?

http://codereview.chromium.org/174208/diff/1015/21#newcode278
Line 278: InstalledExtensions* installed);
indent

http://codereview.chromium.org/174208/diff/1015/21#newcode309
Line 309: // Sanity check a single installed extension.
update comment

http://codereview.chromium.org/174208/diff/1015/21#newcode311
Line 311: const std::string& id,
indent

[Matt Perry, 2009-08-21 18:10:59]

OK, I got rid of the Check* methods and now just call CheckExternalInstall
directly for external extensions.

http://codereview.chromium.org/174208/diff/1015/20
File chrome/browser/extensions/extensions_service.cc (right):

http://codereview.chromium.org/174208/diff/1015/20#newcode599
Line 599: if (CheckExternalUninstall(id, location)) {
On 2009/08/21 15:46:05, Erik Kay wrote:
> since this is the only callsite, let's just merge the two methods

Done.

[Erik does not do reviews, 2009-08-21 18:22:21]

LGTM