Load external extensions even when --disable-extensions is used. We do this since different environments may require users to have certain external policy extensions installed. This patch also fixes a crash when --disable-extensions is used with external extensions installed.

BUG=66070
TEST=ExtensionsServiceTest.ExternalUninstall

Review URL: http://codereview.chromium.org/5695004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@68892 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 71 insertions, 13 deletions

diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 14027c7..73955fc 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -190,11 +190,8 @@ class ExtensionsServiceBackend
       public ExternalExtensionProvider::Visitor {
  public:
   // |install_directory| is a path where to look for extensions to load.
-  // |load_external_extensions| indicates whether or not backend should load
-  // external extensions listed in JSON file and Windows registry.
   ExtensionsServiceBackend(PrefService* prefs,
-                           const FilePath& install_directory,
-                           bool load_external_extensions);
+                           const FilePath& install_directory);
 
   // Loads a single extension from |path| where |path| is the top directory of
   // a specific extension where its manifest file lives.
@@ -285,15 +282,11 @@ class ExtensionsServiceBackend
 
 ExtensionsServiceBackend::ExtensionsServiceBackend(
     PrefService* prefs,
-    const FilePath& install_directory,
-    bool load_external_extensions)
+    const FilePath& install_directory)
         : frontend_(NULL),
           install_directory_(install_directory),
           alert_on_error_(false),
           external_extension_added_(false) {
-  if (!load_external_extensions)
-    return;
-
   // TODO(aa): This ends up doing blocking IO on the UI thread because it reads
   // pref data in the ctor and that is called on the UI thread. Would be better
   // to re-read data each time we list external extensions, anyway.
@@ -595,8 +588,7 @@ ExtensionsService::ExtensionsService(Profile* profile,
   }
 
   backend_ = new ExtensionsServiceBackend(profile->GetPrefs(),
-                                          install_directory_,
-                                          extensions_enabled_);
+                                          install_directory_);
 
   // Use monochrome icons for Omnibox icons.
   omnibox_popup_icon_manager_.set_monochrome(true);
@@ -855,7 +847,7 @@ void ExtensionsService::UninstallExtension(const std::string& extension_id,
       GetExtensionByIdInternal(extension_id, true, true);
 
   // Callers should not send us nonexistent extensions.
-  DCHECK(extension);
+  CHECK(extension);
 
   // Get hold of information we need after unloading, since the extension
   // pointer will be invalid then.
@@ -1570,7 +1562,8 @@ void ExtensionsService::OnExtensionLoaded(const Extension* extension) {
   // is set (http://crbug.com/29067).
   if (!extensions_enabled() &&
       !extension->is_theme() &&
-      extension->location() != Extension::COMPONENT)
+      extension->location() != Extension::COMPONENT &&
+      !Extension::IsExternalLocation(extension->location()))
     return;
 
   // Check if the extension's privileges have changed and disable the
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index a1bf7e6..9ac3aa7 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -2892,7 +2892,9 @@ void ExtensionsServiceTest::TestExternalProvider(
 // Tests the external installation feature
 #if defined(OS_WIN)
 TEST_F(ExtensionsServiceTest, ExternalInstallRegistry) {
+  // This should all work, even when normal extension installation is disabled.
   InitializeEmptyExtensionsService();
+  set_extensions_enabled(false);
 
   // Now add providers. Extension system takes ownership of the objects.
   MockExtensionProvider* reg_provider =
@@ -2914,7 +2916,9 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
 }
 
 TEST_F(ExtensionsServiceTest, ExternalInstallPrefUpdateUrl) {
+  // This should all work, even when normal extension installation is disabled.
   InitializeEmptyExtensionsService();
+  set_extensions_enabled(false);
 
   // TODO(skerner): The mock provider is not a good model of a provider
   // that works with update URLs, because it adds file and version info.
@@ -2929,6 +2933,39 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPrefUpdateUrl) {
   TestExternalProvider(pref_provider, Extension::EXTERNAL_PREF_DOWNLOAD);
 }
 
+// Tests that external extensions get uninstalled when the external extension
+// providers can't account for them.
+TEST_F(ExtensionsServiceTest, ExternalUninstall) {
+  // Start the extensions service with one external extension already installed.
+  FilePath source_install_dir;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_install_dir));
+  source_install_dir = source_install_dir
+      .AppendASCII("extensions")
+      .AppendASCII("good")
+      .AppendASCII("Extensions");
+  FilePath pref_path = source_install_dir
+      .DirName()
+      .AppendASCII("PreferencesExternal");
+
+  // This initializes the extensions service with no ExternalExtensionProviders.
+  InitializeInstalledExtensionsService(pref_path, source_install_dir);
+  set_extensions_enabled(false);
+
+  service_->Init();
+  loop_.RunAllPending();
+
+  ASSERT_EQ(0u, GetErrors().size());
+  ASSERT_EQ(0u, loaded_.size());
+
+  // Verify that it's not the disabled extensions flag causing it not to load.
+  set_extensions_enabled(true);
+  service_->ReloadExtensions();
+  loop_.RunAllPending();
+
+  ASSERT_EQ(0u, GetErrors().size());
+  ASSERT_EQ(0u, loaded_.size());
+}
+
 TEST_F(ExtensionsServiceTest, ExternalPrefProvider) {
   InitializeEmptyExtensionsService();
   std::string json_data =
diff --git a/chrome/test/data/extensions/good/PreferencesExternal b/chrome/test/data/extensions/good/PreferencesExternal
new file mode 100644
index 0000000..12dfe87
--- /dev/null
+++ b/chrome/test/data/extensions/good/PreferencesExternal
@@ -0,0 +1,28 @@
+{
+   "extensions": {
+      "settings": {
+         "ldnnhddmnhbkjipkidpdiheffobcpfmf": {
+            "app_launcher_index": 18,
+            "incognito": false,
+            "install_time": "12936409370321102",
+            "location": 2,
+            "manifest": {
+               "content_scripts": [ {
+                  "js": [ "script1.js" ],
+                  "matches": [ "http://*.google.com/*", "https://*.google.com/*" ]
+               }, {
+                  "js": [ "script2.js" ],
+                  "matches": [ "http://*.yahoo.com/*" ]
+               } ],
+               "description": "The first extension that I made.",
+               "format_version": 1,
+               "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC8c4fBSPZ6utYoZ8NiWF/DSaimBhihjwgOsskyleFGaurhi3TDClTVSGPxNkgCzrz0wACML7M4aNjpd05qupdbR2d294jkDuI7caxEGUucpP7GJRRHnm8Sx+y0ury28n8jbN0PnInKKWcxpIXXmNQyC19HBuO3QIeUq9Dqc+7YFQIDAQAB",
+               "name": "My extension 1",
+               "version": "1.0.0.0"
+            },
+            "path": "ldnnhddmnhbkjipkidpdiheffobcpfmf/1.0.0.0_0",
+            "state": 1
+         }
+      }
+   }
+}