Reworked ExtensionsService::AddPendingExtension().

Reworked ExtensionsService::AddPendingExtension().

Made it take options for enabled/incognito enabled state to apply
on install.

BUG=46515
TEST=unittests, manual (newly-synced disabled extensions should stay
disabled)

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

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

[akalin, 2010-06-24 01:50:06]

+asargent for extensions stuff, +nick for sync stuff

The changes to crx_installer.cc are temporary workarounds for issues I found
with http://codereview.chromium.org/2855009/show (see my comments on that
issue).  I expect those will disappear after those issues are fixed.

[ncarter (slow), 2010-06-24 01:56:26]

Sync files LGTM

[asargent_no_longer_on_chrome, 2010-06-24 15:54:45]

Mostly LG with one question.

http://codereview.chromium.org/2819023/diff/1/2
File chrome/browser/extensions/crx_installer.cc (right):

http://codereview.chromium.org/2819023/diff/1/2#newcode140
chrome/browser/extensions/crx_installer.cc:140: // gallery.
Have you checked with rafaelw on this part (ignoring the scheme)?

I guess in the case of initial install of an extension, ideally we'd want to
keep the scheme check in place, but in the case of auto-updates it should be
safe to ignore (since the download urls there are http). I don't know if it's
worth it to push that complexity down into CrxInstaller, but Rafael probably has
an opinion.

[rafaelw, 2010-06-24 18:13:55]

http://codereview.chromium.org/2819023/diff/1/2
File chrome/browser/extensions/crx_installer.cc (right):

http://codereview.chromium.org/2819023/diff/1/2#newcode140
chrome/browser/extensions/crx_installer.cc:140: // gallery.
This is fine. I was actually preparing a patch for this, but Fred has beat me to
it.

Fred, while you are doing this, can you include the same logic in
ExtensionsService::IsDownloadFromGallery?

I talked with Carl and the issue is that Omaha has logic in there that will
respond with an http download if requested via http, and likewise with https.

It seems better to prepare for the possibility that purchasing apps results in
making an https request from Omaha and thus getting an https download url.
Regardless of the scheme, if the host & path match, we should consider it as
downloaded from the gallery.

On 2010/06/24 15:54:46, Antony Sargent wrote:
> Have you checked with rafaelw on this part (ignoring the scheme)?
> 
> I guess in the case of initial install of an extension, ideally we'd want to
> keep the scheme check in place, but in the case of auto-updates it should be
> safe to ignore (since the download urls there are http). I don't know if it's
> worth it to push that complexity down into CrxInstaller, but Rafael probably
has
> an opinion.
> 
>

[akalin (wrong akalin), 2010-06-24 18:20:51]

On Thu, Jun 24, 2010 at 11:13 AM,  <rafaelw@chromium.org> wrote:
>
> http://codereview.chromium.org/2819023/diff/1/2
> File chrome/browser/extensions/crx_installer.cc (right):
>
> http://codereview.chromium.org/2819023/diff/1/2#newcode140
> chrome/browser/extensions/crx_installer.cc:140: // gallery.
> This is fine. I was actually preparing a patch for this, but Fred has
> beat me to it.

I actually didn't intend to check it in as part of this patch, but
just a temporary workaround that'll disappear once it gets fixed for
real.  If you're preparing a patch, I'd rather you check it in so that
that fix and this patch are cleanly separated. :)

-- 
Frederick Akalin
Software Engineer

[asargent_no_longer_on_chrome, 2010-06-24 20:58:33]

Ok, everything else LGTM.

[akalin, 2010-06-24 21:57:51]

Reverted temporary fix, fixed test compile error, submitting as soon as trybots
pass

On 2010/06/24 20:58:33, Antony Sargent wrote:
> Ok, everything else LGTM.