Reinstall extensions if they become policy-controlled

Reinstall extensions if they become policy-controlled

BUG=86519
TEST=please see instructions in the bug

[gfeher, 2011-06-24 17:43:00]

Hi, what do you think about this?

This code would essentially issue an uninstall and then quickly again an install
of the extension, without waiting for the uninstall to complete. Is this a safe
thing? Would it also be possible to just modify the location of an
already-installed extension?

Thanks for the help,
Gábor

[Sam Kerner (Chrome), 2011-06-24 20:03:34]

FYI: Aaron is on leave.

http://codereview.chromium.org/7237028/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_service.cc (right):

http://codereview.chromium.org/7237028/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_service.cc:415: if (location !=
extension->location() &&
Uninstalling wipes any extension state in localstorage.  It would be better to
install the new CRX without uninstalling the old one.  A quick read of the code
makes me think that if you let the install run without uninstalling, the new
location be used.  There may be some logic to stop the fetch or install if the
extension is already installed.  You could use a check that the pending
extension's location is different to override such a check.



Testing against Extension::EXTERNAL_POLICY_DOWNLOAD works, but it can be made
more general.  The function Extension::GetHigherPriorityLocation() takes two
Extension::Locations, and returns the one chrome should choose if the same
extension is installed with the two locations.  Policy is the highest priority. 
You could use that function to see if the new location outranks the current one,
and allow the install to continue if it does.

It is important not to allow the install if the locations are the same, because
that happens on every reading of the external sources.  Something like:

if (extension)
  Extension::Location current = extension->location();
  // If the current location outranks the new one, skip this install.
  if (current == Extension::GetHigherPriorityLocation(location, current))
    return;
}

This will do the right thing when a user-installed extension is added as
EXTERNAL_UPDATE_URL, which also outranks a user install.

[gfeher, 2011-06-27 12:59:49]

http://codereview.chromium.org/7237028/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_service.cc (right):

http://codereview.chromium.org/7237028/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_service.cc:415: if (location !=
extension->location() &&
Your comment got me thinking. In the case you describe, consider:
1. user installs extension X
2. an application registers extension X as EXTERNAL_UPDATE_URL
3. the application gets uninstalled and removes extension X as
EXTERNAL_UPDATE_URL
4. extension X gets uninstalled
(I guess what we are missing is extensions having multiple locations, including
user install.)

This is actually also a problem for policy-controlled extensions, although it
may be acceptable in their special case. (I'll have to talk with gwilson about
it.)

WDYT?

[Sam Kerner (Chrome), 2011-06-27 14:26:30]

http://codereview.chromium.org/7237028/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_service.cc (right):

http://codereview.chromium.org/7237028/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_service.cc:415: if (location !=
extension->location() &&
On 2011/06/27 12:59:49, gfeher wrote:
> Your comment got me thinking. In the case you describe, consider:
> 1. user installs extension X
> 2. an application registers extension X as EXTERNAL_UPDATE_URL
> 3. the application gets uninstalled and removes extension X as
> EXTERNAL_UPDATE_URL
> 4. extension X gets uninstalled
> (I guess what we are missing is extensions having multiple locations,
including
> user install.)

Yes, we can't do the right thing here without some way to represent the fact
that extensions are installed for more than one reason.  It was decided that the
complexity of supporting multiple locations was not worth the benefit of fixing
this.  At the time, we were not thinking about policy extensions.

If you feel that this is a significant issue for policy extensions, send an
email to Erik Kay and me, and we can reevaluate.

> This is actually also a problem for policy-controlled extensions, although it
> may be acceptable in their special case. (I'll have to talk with gwilson about
> it.)
> 
> WDYT?
>