[Extensions] Update external extension warning to allow new platforms
Certain platforms (currently only Windows) prompt for externally-
installed, or sideloaded, extensions. In the future, we may want to
spread this to other platforms, but we'll want to grandfather in any
extensions that were already installed.
Update the external installation disablement and warning code to only
affect new installation of extensions. Add additional unit tests for:
- The general case of installing and disabling an external extension (I
couldn't find any for this already in place, though some probably test
it transitively?).
- Extensions installed before the feature is turned on not being
disabled.
- Not warning about extensions installed before the feature is turned
on.
BUG=669277
Review-Url: https://codereview.chromium.org/2607673002
Cr-Commit-Position: refs/heads/master@{#441000}
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index cce0d63..f63d065 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -2224,7 +2224,15 @@
}
int ExtensionService::GetDisableReasonsOnInstalled(const Extension* extension) {
- Extension::DisableReason disable_reason;
+ bool is_update_from_same_type = false;
+ {
+ const Extension* existing_extension =
+ GetInstalledExtension(extension->id());
+ is_update_from_same_type =
+ existing_extension &&
+ existing_extension->manifest()->type() == extension->manifest()->type();
+ }
+ Extension::DisableReason disable_reason = Extension::DISABLE_NONE;
// Extensions disabled by management policy should always be disabled, even
// if it's force-installed.
if (system_->management_policy()->MustRemainDisabled(
@@ -2253,10 +2261,15 @@
if (FeatureSwitch::prompt_for_external_extensions()->IsEnabled()) {
// External extensions are initially disabled. We prompt the user before
// enabling them. Hosted apps are excepted because they are not dangerous
- // (they need to be launched by the user anyway).
+ // (they need to be launched by the user anyway). We also don't prompt for
+ // extensions updating; this is because the extension will be disabled from
+ // the initial install if it is supposed to be, and this allows us to turn
+ // this on for other platforms without disabling already-installed
+ // extensions.
if (extension->GetType() != Manifest::TYPE_HOSTED_APP &&
Manifest::IsExternalLocation(extension->location()) &&
- !extension_prefs_->IsExternalExtensionAcknowledged(extension->id())) {
+ !extension_prefs_->IsExternalExtensionAcknowledged(extension->id()) &&
+ !is_update_from_same_type) {
return Extension::DISABLE_EXTERNAL_EXTENSION;
}
}
