Fixed memory leak in ExtensionsService.

Added coverage for disabled extensions in unit tests.

BUG=none
TEST=new unit tests

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

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

2 files changed, 66 insertions, 3 deletions

diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 108a214..b7c0800 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -8,6 +8,7 @@
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/histogram.h"
+#include "base/stl_util-inl.h"
 #include "base/string16.h"
 #include "base/string_util.h"
 #include "base/time.h"
@@ -779,11 +780,13 @@ void ExtensionsService::UnloadExtension(const std::string& extension_id) {
 }
 
 void ExtensionsService::UnloadAllExtensions() {
-  ExtensionList::iterator iter;
-  for (iter = extensions_.begin(); iter != extensions_.end(); ++iter)
-    delete *iter;
+  STLDeleteContainerPointers(extensions_.begin(), extensions_.end());
   extensions_.clear();
 
+  STLDeleteContainerPointers(disabled_extensions_.begin(),
+                             disabled_extensions_.end());
+  disabled_extensions_.clear();
+
   // TODO(erikkay) should there be a notification for this?  We can't use
   // EXTENSION_UNLOADED since that implies that the extension has been disabled
   // or uninstalled, and UnloadAll is just part of shutdown.
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 5618648..7f62914 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -1438,6 +1438,66 @@ TEST_F(ExtensionsServiceTest, WillNotLoadBlacklistedExtensionsFromDirectory) {
   EXPECT_NE(std::string(good0), loaded_[1]->id());
 }
 
+// Tests disabling extensions
+TEST_F(ExtensionsServiceTest, DisableExtension) {
+  InitializeEmptyExtensionsService();
+  FilePath extensions_path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
+  extensions_path = extensions_path.AppendASCII("extensions");
+
+  // A simple extension that should install without error.
+  FilePath path = extensions_path.AppendASCII("good.crx");
+  InstallExtension(path, true);
+
+  const char* extension_id = good_crx;
+  EXPECT_FALSE(service_->extensions()->empty());
+  EXPECT_TRUE(service_->GetExtensionById(extension_id, true) != NULL);
+  EXPECT_TRUE(service_->GetExtensionById(extension_id, false) != NULL);
+  EXPECT_TRUE(service_->disabled_extensions()->empty());
+
+  // Disable it.
+  service_->DisableExtension(extension_id);
+
+  EXPECT_TRUE(service_->extensions()->empty());
+  EXPECT_TRUE(service_->GetExtensionById(extension_id, true) != NULL);
+  EXPECT_FALSE(service_->GetExtensionById(extension_id, false) != NULL);
+  EXPECT_FALSE(service_->disabled_extensions()->empty());
+}
+
+// Tests reloading extensions
+TEST_F(ExtensionsServiceTest, ReloadExtensions) {
+  InitializeEmptyExtensionsService();
+  FilePath extensions_path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
+  extensions_path = extensions_path.AppendASCII("extensions");
+
+  // Simple extension that should install without error.
+  FilePath path = extensions_path.AppendASCII("good.crx");
+  InstallExtension(path, true);
+  const char* extension_id = good_crx;
+  service_->DisableExtension(extension_id);
+
+  EXPECT_EQ(0u, service_->extensions()->size());
+  EXPECT_EQ(1u, service_->disabled_extensions()->size());
+
+  service_->ReloadExtensions();
+
+  // Extension counts shouldn't change.
+  EXPECT_EQ(0u, service_->extensions()->size());
+  EXPECT_EQ(1u, service_->disabled_extensions()->size());
+
+  service_->EnableExtension(extension_id);
+
+  EXPECT_EQ(1u, service_->extensions()->size());
+  EXPECT_EQ(0u, service_->disabled_extensions()->size());
+
+  service_->ReloadExtensions();
+
+  // Extension counts shouldn't change.
+  EXPECT_EQ(1u, service_->extensions()->size());
+  EXPECT_EQ(0u, service_->disabled_extensions()->size());
+}
+
 // Tests uninstalling normal extensions
 TEST_F(ExtensionsServiceTest, UninstallExtension) {
   InitializeEmptyExtensionsService();