Disable extensions besides externally installed ones and themes.

This is mainly intended to be pulled to the 3.0 branch. It won't be submitted until another corresponding change to
undo the damage is ready.

BUG=13467
TEST=Added unit tests

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

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

5 files changed, 61 insertions, 41 deletions

diff --git a/chrome/browser/extensions/extension_shelf_model.cc b/chrome/browser/extensions/extension_shelf_model.cc
index 70013e5..40b4158 100644
--- a/chrome/browser/extensions/extension_shelf_model.cc
+++ b/chrome/browser/extensions/extension_shelf_model.cc
@@ -124,8 +124,10 @@ void ExtensionShelfModel::Observe(NotificationType type,
       RemoveExtension(Details<Extension>(details).ptr());
       break;
     case NotificationType::EXTENSIONS_READY:
-      AddExtensions(browser_->profile()->GetExtensionsService()->extensions());
-      SortToolstrips();
+      if (browser_->profile()->GetExtensionsService()) {
+        AddExtensions(browser_->profile()->GetExtensionsService()->extensions());
+        SortToolstrips();
+      }
       ready_ = true;
       break;
     case NotificationType::EXTENSION_SHELF_MODEL_CHANGED:
diff --git a/chrome/browser/extensions/extension_uitest.cc b/chrome/browser/extensions/extension_uitest.cc
index 359ba9a..7d63356 100644
--- a/chrome/browser/extensions/extension_uitest.cc
+++ b/chrome/browser/extensions/extension_uitest.cc
@@ -101,7 +101,7 @@ class SimpleApiCallExtensionTest : public SingleMessageExtensionUITest {
 };
 
 // TODO(port) Should become portable once ExternalTabMessageLoop is ported.
-#if defined(OS_WIN)
+#if 0
 TEST_F(SimpleApiCallExtensionTest, RunTest) {
   namespace keys = extension_automation_constants;
 
@@ -270,7 +270,7 @@ class RoundtripApiCallExtensionTest
 
 // TODO(port) Should become portable once
 // ExternalTabMessageLoop is ported.
-#if defined(OS_WIN)
+#if 0
 TEST_F(RoundtripApiCallExtensionTest, RunTest) {
   TestWithURL(GURL(
       "chrome-extension://ofoknjclcmghjfmbncljcnpjmfmldhno/test.html"));
@@ -435,7 +435,7 @@ class BrowserEventExtensionTest
 
 // TODO(port) Should become portable once
 // ExternalTabMessageLoop is ported.
-#if defined(OS_WIN)
+#if 0
 TEST_F(BrowserEventExtensionTest, RunTest) {
   // The extension for this test does not specify a "key" property in its
   // manifest file.  Therefore, the extension system will automatically assign
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index a9f81ca..7960664 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -216,22 +216,26 @@ ExtensionsService::ExtensionsService(Profile* profile,
     : extension_prefs_(new ExtensionPrefs(profile->GetPrefs())),
       backend_loop_(backend_loop),
       install_directory_(profile->GetPath().AppendASCII(kInstallDirectoryName)),
-      extensions_enabled_(
-          CommandLine::ForCurrentProcess()->
-          HasSwitch(switches::kEnableExtensions)),
+      extensions_enabled_(false),
       show_extensions_prompts_(true),
       ready_(false) {
   // We pass ownership of this object to the Backend.
   DictionaryValue* extensions = extension_prefs_->CopyCurrentExtensions();
   backend_ = new ExtensionsServiceBackend(
           install_directory_, g_browser_process->resource_dispatcher_host(),
-          frontend_loop, extensions);
+          frontend_loop, extensions, extensions_enabled());
 }
 
 ExtensionsService::~ExtensionsService() {
   UnloadAllExtensions();
 }
 
+void ExtensionsService::SetExtensionsEnabled(bool enabled) {
+  extensions_enabled_ = true;
+  backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(),
+    &ExtensionsServiceBackend::set_extensions_enabled, enabled));
+}
+
 void ExtensionsService::Init() {
   DCHECK(extensions_.size() == 0);
 
@@ -354,13 +358,14 @@ void ExtensionsService::OnLoadedInstalledExtensions() {
 void ExtensionsService::OnExtensionsLoaded(ExtensionList* new_extensions) {
   scoped_ptr<ExtensionList> cleanup(new_extensions);
 
-  // Filter out any extensions we don't want to enable. Themes are always
-  // enabled, but other extensions are only loaded if --enable-extensions is
-  // present.
+  // Filter out any extensions that shouldn't be loaded. Themes are always
+  // loaded, but other extensions are only loaded if the extensions system is
+  // enabled.
   ExtensionList enabled_extensions;
   for (ExtensionList::iterator iter = new_extensions->begin();
        iter != new_extensions->end(); ++iter) {
-    if (extensions_enabled() || (*iter)->IsTheme()) {
+    if (extensions_enabled() || (*iter)->IsTheme() ||
+        (*iter)->location() == Extension::EXTERNAL_REGISTRY) {
       Extension* old = GetExtensionById((*iter)->id());
       if (old) {
         if ((*iter)->version()->CompareTo(*(old->version())) > 0) {
@@ -451,12 +456,14 @@ void ExtensionsService::SetProviderForTesting(
 
 ExtensionsServiceBackend::ExtensionsServiceBackend(
     const FilePath& install_directory, ResourceDispatcherHost* rdh,
-    MessageLoop* frontend_loop, DictionaryValue* extension_prefs)
+    MessageLoop* frontend_loop, DictionaryValue* extension_prefs,
+    bool extensions_enabled)
         : frontend_(NULL),
           install_directory_(install_directory),
           resource_dispatcher_host_(rdh),
           alert_on_error_(false),
-          frontend_loop_(frontend_loop) {
+          frontend_loop_(frontend_loop),
+          extensions_enabled_(false) {
   external_extension_providers_[Extension::EXTERNAL_PREF] =
       linked_ptr<ExternalExtensionProvider>(
           new ExternalPrefExtensionProvider(extension_prefs));
@@ -956,28 +963,22 @@ void ExtensionsServiceBackend::OnExtensionUnpacked(
     return;
   }
 
-  if (!frontend_->extensions_enabled() && !extension.IsTheme()) {
-#if defined(OS_WIN)
-    if (frontend_->show_extensions_prompts()) {
-      win_util::MessageBox(GetForegroundWindow(),
-          L"Extensions are not enabled. Add --enable-extensions to the "
-          L"command-line to enable extensions.\n\n"
-          L"This is a temporary message and it will be removed when extensions "
-          L"UI is finalized.",
-          l10n_util::GetString(IDS_PRODUCT_NAME).c_str(), MB_OK);
-    }
-#endif
+  Extension::Location location = Extension::INTERNAL;
+  LookupExternalExtension(extension.id(), NULL, &location);
+
+  // We currently only allow themes and registry-installed extensions to be
+  // installed.
+  if (!extensions_enabled_ &&
+      !extension.IsTheme() &&
+      location != Extension::EXTERNAL_REGISTRY) {
     ReportExtensionInstallError(extension_path,
-        "Extensions are not enabled.");
+                                "Extensions are not enabled (yet!)");
     return;
   }
 
-  Extension::Location location = Extension::INTERNAL;
-  LookupExternalExtension(extension.id(), NULL, &location);
 #if defined(OS_WIN)
-  bool from_external = Extension::IsExternalLocation(location);
-
-  if (!extension.IsTheme() && !from_external &&
+  if (!extension.IsTheme() &&
+      !Extension::IsExternalLocation(location) &&
       frontend_->show_extensions_prompts() &&
       win_util::MessageBox(GetForegroundWindow(),
           L"Are you sure you want to install this extension?\n\n"
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index f2f0b56..87c387f 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -132,12 +132,13 @@ class ExtensionsService
   // The name of the file that the current active version number is stored in.
   static const char* kCurrentVersionFileName;
 
-  void set_extensions_enabled(bool enabled) { extensions_enabled_ = enabled; }
+  void SetExtensionsEnabled(bool enabled);
+  bool extensions_enabled() { return extensions_enabled_; }
+
   void set_show_extensions_prompts(bool enabled) {
     show_extensions_prompts_ = enabled;
   }
 
-  bool extensions_enabled() { return extensions_enabled_; }
   bool show_extensions_prompts() {
     return show_extensions_prompts_;
   }
@@ -209,10 +210,13 @@ class ExtensionsServiceBackend
   ExtensionsServiceBackend(const FilePath& install_directory,
                            ResourceDispatcherHost* rdh,
                            MessageLoop* frontend_loop,
-                           DictionaryValue* extension_prefs);
+                           DictionaryValue* extension_prefs,
+                           bool extensions_enabled);
 
   virtual ~ExtensionsServiceBackend();
 
+  void set_extensions_enabled(bool enabled) { extensions_enabled_ = enabled; }
+
   // Loads the installed extensions.
   // Errors are reported through ExtensionErrorReporter. On completion,
   // OnExtensionsLoaded() is called with any successfully loaded extensions.
@@ -399,6 +403,10 @@ class ExtensionsServiceBackend
   // The message loop to use to call the frontend.
   MessageLoop* frontend_loop_;
 
+  // Whether non-theme extensions are enabled (themes and externally registered
+  // extensions are always enabled).
+  bool extensions_enabled_;
+
   // A map of all external extension providers.
   typedef std::map<Extension::Location,
                    linked_ptr<ExternalExtensionProvider> > ProviderMap;
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index e2dd984..e41a84a 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -131,7 +131,7 @@ class ExtensionsServiceTest
 
     profile_.reset(new TestingProfile());
     service_ = new ExtensionsService(profile_.get(), &loop_, &loop_);
-    service_->set_extensions_enabled(true);
+    service_->SetExtensionsEnabled(true);
     service_->set_show_extensions_prompts(false);
 
     // When we start up, we want to make sure there is no external provider,
@@ -191,7 +191,7 @@ class ExtensionsServiceTest
   }
 
   void SetExtensionsEnabled(bool enabled) {
-    service_->set_extensions_enabled(enabled);
+    service_->SetExtensionsEnabled(enabled);
   }
 
   void SetMockExternalProvider(Extension::Location location,
@@ -498,8 +498,6 @@ TEST_F(ExtensionsServiceTest, InstallExtension) {
 // Test Packaging and installing an extension.
 // TODO(rafaelw): add more tests for failure cases.
 TEST_F(ExtensionsServiceTest, PackExtension) {
-  SetExtensionsEnabled(true);
-
   FilePath extensions_path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
   extensions_path = extensions_path.AppendASCII("extensions");
@@ -530,8 +528,6 @@ TEST_F(ExtensionsServiceTest, PackExtension) {
 // The privkey.pem is a PrivateKey, and the pcks8 -topk8 creates a
 // PrivateKeyInfo ASN.1 structure, we our RSAPrivateKey expects.
 TEST_F(ExtensionsServiceTest, PackExtensionOpenSSLKey) {
-  SetExtensionsEnabled(true);
-
   FilePath extensions_path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
   extensions_path = extensions_path.AppendASCII("extensions");
@@ -804,6 +800,9 @@ TEST_F(ExtensionsServiceTest, GenerateID) {
 #if defined(OS_WIN)
 
 TEST_F(ExtensionsServiceTest, ExternalInstallRegistry) {
+  // This should all work, even when normal extension installation is disabled.
+  SetExtensionsEnabled(false);
+
   // Verify that starting with no providers loads no extensions.
   service_->Init();
   loop_.RunAllPending();
@@ -1013,4 +1012,14 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
   extension_path = extension_path.AppendASCII(good_crx);
   EXPECT_FALSE(file_util::PathExists(extension_path)) <<
       extension_path.ToWStringHack();
+
+  // This shouldn't work if extensions are disabled.
+  SetExtensionsEnabled(false);
+
+  pref_provider->UpdateOrAddExtension(good_crx, "1.0", source_path);
+  service_->CheckForUpdates();
+  loop_.RunAllPending();
+
+  ASSERT_EQ(0u, loaded_.size());
+  ASSERT_EQ(1u, GetErrors().size());
 }