Disabled extensions should be uninstallable.

The code to display the dialog looks up an
Extension by id but doesn't return disabled ones.

So I added that option to the function. 

I also added a check so that we don't show the
Options button enabled when the extension is
disabled, since you need the extension to be
enabled for the Options page to work.

BUG=26910
TEST=Disable an extension that has an Option page,
make sure Options is not enabled after disabling,
then uninstall it. It should uninstall.

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

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

15 files changed, 34 insertions, 27 deletions

diff --git a/chrome/browser/dom_ui/dom_ui_factory.cc b/chrome/browser/dom_ui/dom_ui_factory.cc
index 150afe7..6f0c275 100644
--- a/chrome/browser/dom_ui/dom_ui_factory.cc
+++ b/chrome/browser/dom_ui/dom_ui_factory.cc
@@ -37,7 +37,8 @@ template<>
 DOMUI* NewDOMUI<ExtensionDOMUI>(TabContents* contents, const GURL& url) {
   // Don't use a DOMUI for non-existent extensions.
   ExtensionsService* service = contents->profile()->GetExtensionsService();
-  bool valid_extension = (service && service->GetExtensionById(url.host()));
+  bool valid_extension =
+      (service && service->GetExtensionById(url.host(), false));
   if (valid_extension)
     return new ExtensionDOMUI(contents);
   return NULL;
diff --git a/chrome/browser/extensions/extension_function_dispatcher.cc b/chrome/browser/extensions/extension_function_dispatcher.cc
index a3f8a4c..a99a362 100644
--- a/chrome/browser/extensions/extension_function_dispatcher.cc
+++ b/chrome/browser/extensions/extension_function_dispatcher.cc
@@ -247,7 +247,7 @@ Extension* ExtensionFunctionDispatcher::GetExtension() {
   ExtensionsService* service = profile()->GetExtensionsService();
   DCHECK(service);
 
-  Extension* extension = service->GetExtensionById(extension_id());
+  Extension* extension = service->GetExtensionById(extension_id(), false);
   DCHECK(extension);
 
   return extension;
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
index 64419bf..02587a4 100644
--- a/chrome/browser/extensions/extension_process_manager.cc
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -137,7 +137,8 @@ void ExtensionProcessManager::RegisterExtensionProcess(
       browsing_instance_->profile()->GetExtensionsService();
 
   std::vector<std::string> page_action_ids;
-  Extension* extension = extension_service->GetExtensionById(extension_id);
+  Extension* extension =
+      extension_service->GetExtensionById(extension_id, false);
   if (extension->page_action())
     page_action_ids.push_back(extension->page_action()->id());
 
diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc
index 58f4ac2..6578024 100644
--- a/chrome/browser/extensions/extension_updater.cc
+++ b/chrome/browser/extensions/extension_updater.cc
@@ -563,7 +563,7 @@ bool ExtensionUpdater::GetExistingVersion(const std::string& id,
       WideToASCII(prefs_->GetString(kExtensionBlacklistUpdateVersion));
     return true;
   }
-  Extension* extension = service_->GetExtensionById(id);
+  Extension* extension = service_->GetExtensionById(id, false);
   if (!extension) {
     return false;
   }
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index 2765b40..2af7b46 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -52,7 +52,7 @@ class MockService : public ExtensionUpdateService {
     EXPECT_TRUE(false);
   }
 
-  virtual Extension* GetExtensionById(const std::string& id) {
+  virtual Extension* GetExtensionById(const std::string& id, bool) {
     EXPECT_TRUE(false);
     return NULL;
   }
@@ -120,7 +120,7 @@ class ServiceForManifestTests : public MockService {
 
   virtual ~ServiceForManifestTests() {}
 
-  virtual Extension* GetExtensionById(const std::string& id) {
+  virtual Extension* GetExtensionById(const std::string& id, bool) {
     for (ExtensionList::iterator iter = extensions_.begin();
         iter != extensions_.end(); ++iter) {
      if ((*iter)->id() == id) {
@@ -148,7 +148,7 @@ class ServiceForDownloadTests : public MockService {
     install_path_ = extension_path;
   }
 
-  virtual Extension* GetExtensionById(const std::string& id) {
+  virtual Extension* GetExtensionById(const std::string& id, bool) {
     last_inquired_extension_id_ = id;
     return NULL;
   }
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 6103bab..4a8b7c3 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -173,7 +173,7 @@ void ExtensionsService::UpdateExtension(const std::string& id,
 
 void ExtensionsService::ReloadExtension(const std::string& extension_id) {
   FilePath path;
-  Extension* current_extension = GetExtensionById(extension_id);
+  Extension* current_extension = GetExtensionById(extension_id, false);
 
   // Unload the extension if it's loaded. It might not be loaded if it crashed.
   if (current_extension) {
@@ -637,7 +637,7 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
 }
 
 void ExtensionsService::OnExtensionOverinstallAttempted(const std::string& id) {
-  Extension* extension = GetExtensionById(id);
+  Extension* extension = GetExtensionById(id, false);
   if (extension && extension->IsTheme()) {
     NotificationService::current()->Notify(
         NotificationType::THEME_INSTALLED,
@@ -674,7 +674,7 @@ Extension* ExtensionsService::GetExtensionByIdInternal(const std::string& id,
 
 Extension* ExtensionsService::GetExtensionByURL(const GURL& url) {
   std::string host = url.host();
-  return GetExtensionById(host);
+  return GetExtensionById(host, false);
 }
 
 void ExtensionsService::ClearProvidersForTesting() {
@@ -700,7 +700,7 @@ void ExtensionsService::OnExternalExtensionFound(const std::string& id,
   // Before even bothering to unpack, check and see if we already have this
   // version. This is important because these extensions are going to get
   // installed on every startup.
-  Extension* existing = GetExtensionById(id);
+  Extension* existing = GetExtensionById(id, true);
   scoped_ptr<Version> other(Version::GetVersionFromString(version));
   if (existing) {
     switch (existing->version()->CompareTo(*other)) {
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index b5aad76..2393891 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -47,7 +47,8 @@ class ExtensionUpdateService {
   virtual ~ExtensionUpdateService() {}
   virtual const ExtensionList* extensions() const = 0;
   virtual void UpdateExtension(const std::string& id, const FilePath& path) = 0;
-  virtual Extension* GetExtensionById(const std::string& id) = 0;
+  virtual Extension* GetExtensionById(const std::string& id,
+                                      bool include_disabled) = 0;
   virtual void UpdateExtensionBlacklist(
     const std::vector<std::string>& blacklist) = 0;
 };
@@ -100,8 +101,8 @@ class ExtensionsService
   void Init();
 
   // Look up an extension by ID.
-  Extension* GetExtensionById(const std::string& id) {
-    return GetExtensionByIdInternal(id, true, false);
+  Extension* GetExtensionById(const std::string& id, bool include_disabled) {
+    return GetExtensionByIdInternal(id, true, include_disabled);
   }
 
   // Install the extension file at |extension_path|.  Will install as an
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 63a12a2..c992167e 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -338,7 +338,7 @@ class ExtensionsServiceTest
       EXPECT_EQ(0u, errors.size()) << path.value();
       EXPECT_EQ(total_successes_, service_->extensions()->size()) <<
           path.value();
-      EXPECT_TRUE(service_->GetExtensionById(loaded_[0]->id())) <<
+      EXPECT_TRUE(service_->GetExtensionById(loaded_[0]->id(), false)) <<
           path.value();
       for (std::vector<std::string>::iterator err = errors.begin();
         err != errors.end(); ++err) {
@@ -522,7 +522,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
   EXPECT_EQ(std::string("The first extension that I made."),
             loaded_[0]->description());
   EXPECT_EQ(Extension::INTERNAL, loaded_[0]->location());
-  EXPECT_TRUE(service_->GetExtensionById(loaded_[0]->id()));
+  EXPECT_TRUE(service_->GetExtensionById(loaded_[0]->id(), false));
   EXPECT_EQ(3u, service_->extensions()->size());
 
   ValidatePrefKeyCount(3);
@@ -1150,7 +1150,7 @@ TEST_F(ExtensionsServiceTest, UninstallExtension) {
   ValidatePrefKeyCount(0);
 
   // The extension should not be in the service anymore.
-  ASSERT_FALSE(service_->GetExtensionById(extension_id));
+  ASSERT_FALSE(service_->GetExtensionById(extension_id, false));
   loop_.RunAllPending();
 
   // The directory should be gone.
diff --git a/chrome/browser/extensions/extensions_ui.cc b/chrome/browser/extensions/extensions_ui.cc
index f66ba3e..867a7e8 100644
--- a/chrome/browser/extensions/extensions_ui.cc
+++ b/chrome/browser/extensions/extensions_ui.cc
@@ -383,7 +383,8 @@ void ExtensionsDOMHandler::HandleUninstallMessage(const Value* value) {
   std::string extension_id;
   CHECK(list->GetString(0, &extension_id));
 
-  Extension *extension = extensions_service_->GetExtensionById(extension_id);
+  Extension *extension =
+      extensions_service_->GetExtensionById(extension_id, true);
   if (!extension)
     return;
 
@@ -412,7 +413,8 @@ void ExtensionsDOMHandler::HandleOptionsMessage(const Value* value) {
   CHECK(list->GetSize() == 1);
   std::string extension_id;
   CHECK(list->GetString(0, &extension_id));
-  Extension *extension = extensions_service_->GetExtensionById(extension_id);
+  Extension *extension =
+      extensions_service_->GetExtensionById(extension_id, false);
   if (!extension || extension->options_url().is_empty()) {
     return;
   }
diff --git a/chrome/browser/extensions/theme_installed_infobar_delegate.cc b/chrome/browser/extensions/theme_installed_infobar_delegate.cc
index ab46d78..879cbb2 100644
--- a/chrome/browser/extensions/theme_installed_infobar_delegate.cc
+++ b/chrome/browser/extensions/theme_installed_infobar_delegate.cc
@@ -35,7 +35,7 @@ void ThemeInstalledInfoBarDelegate::InfoBarClosed() {
       uninstall_id = previous_theme_id_;
     // It's possible that the theme was already uninstalled by someone so make
     // sure it exists.
-    if (!uninstall_id.empty() && service->GetExtensionById(uninstall_id))
+    if (!uninstall_id.empty() && service->GetExtensionById(uninstall_id, true))
       service->UninstallExtension(uninstall_id, false);
   }
   delete this;
@@ -80,7 +80,8 @@ bool ThemeInstalledInfoBarDelegate::Cancel() {
   if (!previous_theme_id_.empty()) {
     ExtensionsService* service = profile_->GetExtensionsService();
     if (service) {
-      Extension* previous_theme = service->GetExtensionById(previous_theme_id_);
+      Extension* previous_theme =
+          service->GetExtensionById(previous_theme_id_, true);
       if (previous_theme) {
         profile_->SetTheme(previous_theme);
         return true;
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
index fb36c0d..b166f1c 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
@@ -253,7 +253,7 @@ void BrowserActionsToolbarGtk::CreateAllButtons() {
 
   for (size_t i = 0; i < extension_service->extensions()->size(); ++i) {
     Extension* extension = extension_service->GetExtensionById(
-        extension_service->extensions()->at(i)->id());
+        extension_service->extensions()->at(i)->id(), false);
     CreateButtonForExtension(extension);
   }
 }
diff --git a/chrome/browser/gtk/location_bar_view_gtk.cc b/chrome/browser/gtk/location_bar_view_gtk.cc
index 2c1f4ec..209ae6d 100644
--- a/chrome/browser/gtk/location_bar_view_gtk.cc
+++ b/chrome/browser/gtk/location_bar_view_gtk.cc
@@ -700,7 +700,7 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
   gtk_container_add(GTK_CONTAINER(event_box_.get()), image_.get());
 
   Extension* extension = profile->GetExtensionsService()->GetExtensionById(
-      page_action->extension_id());
+      page_action->extension_id(), false);
   DCHECK(extension);
 
   // Load all the icons declared in the manifest. This is the contents of the
diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index 112375a..59010e6 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -1166,7 +1166,7 @@ Extension* ProfileImpl::GetTheme() {
   if (id == BrowserThemeProvider::kDefaultThemeID)
     return NULL;
 
-  return extensions_service_->GetExtensionById(id);
+  return extensions_service_->GetExtensionById(id, false);
 }
 
 ThemeProvider* ProfileImpl::GetThemeProvider() {
diff --git a/chrome/browser/resources/extensions_ui.html b/chrome/browser/resources/extensions_ui.html
index cc7d95d..8acb98b 100644
--- a/chrome/browser/resources/extensions_ui.html
+++ b/chrome/browser/resources/extensions_ui.html
@@ -731,13 +731,14 @@ function autoUpdate() {
           </td>
           <td align="right" valign="center" width="90">
                 <button
-                  jsdisplay="options_url"
+                  jsdisplay="options_url && enabled"
                   jsvalues=".extensionId:id"
                   onclick="handleOptions(this)"
                   i18n-content="options"
                   >OPTIONS</button>
                 <button
-                  jsdisplay="typeof(options_url) == 'undefined' ||
+                  jsdisplay="!enabled ||
+                             typeof(options_url) == 'undefined' ||
                              options_url.length == 0"
                   disabled="true"
                   i18n-content="options"
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index 01c6b2b..b10acc1 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -1296,7 +1296,7 @@ LocationBarView::PageActionImageView::PageActionImageView(
       current_tab_id_(-1),
       preview_enabled_(false) {
   Extension* extension = profile->GetExtensionsService()->GetExtensionById(
-      page_action->extension_id());
+      page_action->extension_id(), false);
   DCHECK(extension);
 
   // Load all the icons declared in the manifest. This is the contents of the