Unrevert r63919: "Part 2 of immutable Extension refactor."

I made Extension a refcounted object, and privitized the existing
con/destructor and InitFromValue. The only way to get an Extension is to call
a factory method.

In the next CL, I plan to make the factory method return a const Extension,
to guarantee that no one can modify the Extension object after creation.

Note: There was a tricky part of this CL because of the difference in
semantics between scoped_ptr and scoped_refptr. I had to be careful not to use
ptr.release(), since that would result in leaks (an un-Released AddRef).

BUG=56558
TEST=no functional change

Original Review URL: http://codereview.chromium.org/3982001

TBR=aa

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

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

20 files changed, 138 insertions, 140 deletions

diff --git a/chrome/browser/extensions/convert_user_script.cc b/chrome/browser/extensions/convert_user_script.cc
index d50bec1..89caec4 100644
--- a/chrome/browser/extensions/convert_user_script.cc
+++ b/chrome/browser/extensions/convert_user_script.cc
@@ -24,9 +24,9 @@
 
 namespace keys = extension_manifest_keys;
 
-Extension* ConvertUserScriptToExtension(const FilePath& user_script_path,
-                                        const GURL& original_url,
-                                        std::string* error) {
+scoped_refptr<Extension> ConvertUserScriptToExtension(
+    const FilePath& user_script_path, const GURL& original_url,
+    std::string* error) {
   std::string content;
   if (!file_util::ReadFileToString(user_script_path, &content)) {
     *error = "Could not read source file: " +
@@ -138,12 +138,13 @@ Extension* ConvertUserScriptToExtension(const FilePath& user_script_path,
     return NULL;
   }
 
-  scoped_ptr<Extension> extension(new Extension(temp_dir.path()));
-  if (!extension->InitFromValue(*root, false, error)) {
+  scoped_refptr<Extension> extension = Extension::Create(
+      temp_dir.path(), Extension::INTERNAL, *root, false, error);
+  if (!extension) {
     NOTREACHED() << "Could not init extension " << *error;
     return NULL;
   }
 
   temp_dir.Take();  // The caller takes ownership of the directory.
-  return extension.release();
+  return extension;
 }
diff --git a/chrome/browser/extensions/convert_user_script.h b/chrome/browser/extensions/convert_user_script.h
index d392c4d..d779de0 100644
--- a/chrome/browser/extensions/convert_user_script.h
+++ b/chrome/browser/extensions/convert_user_script.h
@@ -8,6 +8,8 @@
 
 #include <string>
 
+#include "base/ref_counted.h"
+
 class Extension;
 class FilePath;
 class GURL;
@@ -17,8 +19,7 @@ class GURL;
 // should take ownership on success, or NULL and |error| on failure.
 //
 // NOTE: This function does file IO and should not be called on the UI thread.
-Extension* ConvertUserScriptToExtension(const FilePath& user_script,
-                                        const GURL& original_url,
-                                        std::string* error);
+scoped_refptr<Extension> ConvertUserScriptToExtension(
+    const FilePath& user_script, const GURL& original_url, std::string* error);
 
 #endif  // CHROME_BROWSER_EXTENSIONS_CONVERT_USER_SCRIPT_H_
diff --git a/chrome/browser/extensions/convert_user_script_unittest.cc b/chrome/browser/extensions/convert_user_script_unittest.cc
index 299fee7..a0e1b8b 100644
--- a/chrome/browser/extensions/convert_user_script_unittest.cc
+++ b/chrome/browser/extensions/convert_user_script_unittest.cc
@@ -21,7 +21,7 @@ TEST(ExtensionFromUserScript, Basic) {
                        .AppendASCII("user_script_basic.user.js");
 
   std::string error;
-  scoped_ptr<Extension> extension(ConvertUserScriptToExtension(
+  scoped_refptr<Extension> extension(ConvertUserScriptToExtension(
       test_file, GURL("http://www.google.com/foo"), &error));
 
   ASSERT_TRUE(extension.get());
@@ -61,7 +61,7 @@ TEST(ExtensionFromUserScript, NoMetdata) {
                        .AppendASCII("user_script_no_metadata.user.js");
 
   std::string error;
-  scoped_ptr<Extension> extension(ConvertUserScriptToExtension(
+  scoped_refptr<Extension> extension(ConvertUserScriptToExtension(
       test_file, GURL("http://www.google.com/foo/bar.user.js?monkey"), &error));
 
   ASSERT_TRUE(extension.get());
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 8b523d5..83fdcb7 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -141,8 +141,8 @@ void CrxInstaller::InstallUserScript(const FilePath& source_file,
 
 void CrxInstaller::ConvertUserScriptOnFileThread() {
   std::string error;
-  Extension* extension = ConvertUserScriptToExtension(source_file_,
-                                                      original_url_, &error);
+  scoped_refptr<Extension> extension =
+      ConvertUserScriptToExtension(source_file_, original_url_, &error);
   if (!extension) {
     ReportFailureFromFileThread(error);
     return;
@@ -237,7 +237,7 @@ void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir,
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   // Note: We take ownership of |extension| and |temp_dir|.
-  extension_.reset(extension);
+  extension_ = extension;
   temp_dir_ = temp_dir;
 
   // We don't have to delete the unpack dir explicity since it is a child of
@@ -351,8 +351,8 @@ void CrxInstaller::CompleteInstall() {
   // TODO(aa): All paths to resources inside extensions should be created
   // lazily and based on the Extension's root path at that moment.
   std::string error;
-  extension_.reset(extension_file_util::LoadExtension(
-      version_dir, install_source_, true, &error));
+  extension_ = extension_file_util::LoadExtension(
+      version_dir, install_source_, true, &error);
   DCHECK(error.empty());
 
   ReportSuccessFromFileThread();
@@ -400,8 +400,8 @@ void CrxInstaller::ReportSuccessFromUIThread() {
 
   // Tell the frontend about the installation and hand off ownership of
   // extension_ to it.
-  frontend_->OnExtensionInstalled(extension_.release(),
-                                  allow_privilege_increase_);
+  frontend_->OnExtensionInstalled(extension_, allow_privilege_increase_);
+  extension_ = NULL;
 
   // We're done. We don't post any more tasks to ourselves so we are deleted
   // soon.
diff --git a/chrome/browser/extensions/crx_installer.h b/chrome/browser/extensions/crx_installer.h
index b1212a2..8263095 100644
--- a/chrome/browser/extensions/crx_installer.h
+++ b/chrome/browser/extensions/crx_installer.h
@@ -187,7 +187,7 @@ class CrxInstaller
 
   // The extension we're installing. We own this and either pass it off to
   // ExtensionsService on success, or delete it on failure.
-  scoped_ptr<Extension> extension_;
+  scoped_refptr<Extension> extension_;
 
   // If non-empty, contains the current version of the extension we're
   // installing (for upgrades).
diff --git a/chrome/browser/extensions/extension_creator.cc b/chrome/browser/extensions/extension_creator.cc
index 86c78fe..8bd68a2 100644
--- a/chrome/browser/extensions/extension_creator.cc
+++ b/chrome/browser/extensions/extension_creator.cc
@@ -56,7 +56,7 @@ bool ExtensionCreator::InitializeInput(
 
   // Load the extension once. We don't really need it, but this does a lot of
   // useful validation of the structure.
-  scoped_ptr<Extension> extension(
+  scoped_refptr<Extension> extension(
       extension_file_util::LoadExtension(extension_dir,
                                          Extension::INTERNAL,
                                          false,  // key not required
diff --git a/chrome/browser/extensions/extension_icon_manager_unittest.cc b/chrome/browser/extensions/extension_icon_manager_unittest.cc
index 51c76ea..37f3453 100644
--- a/chrome/browser/extensions/extension_icon_manager_unittest.cc
+++ b/chrome/browser/extensions/extension_icon_manager_unittest.cc
@@ -109,26 +109,26 @@ TEST_F(ExtensionIconManagerTest, LoadRemoveLoad) {
       static_cast<DictionaryValue*>(serializer.Deserialize(NULL, NULL)));
   ASSERT_TRUE(manifest.get() != NULL);
 
-  Extension extension(manifest_path.DirName());
-  ASSERT_TRUE(extension.InitFromValue(*manifest.get(),
-                                      false /* require_key */,
-                                      NULL /* errors */));
+  scoped_refptr<Extension> extension(Extension::Create(
+      manifest_path.DirName(), Extension::INVALID, *manifest.get(),
+      false, NULL));
+  ASSERT_TRUE(extension.get());
   TestIconManager icon_manager(this);
 
   // Load the icon and grab the bitmap.
-  icon_manager.LoadIcon(&extension);
+  icon_manager.LoadIcon(extension.get());
   WaitForImageLoad();
-  SkBitmap first_icon = icon_manager.GetIcon(extension.id());
+  SkBitmap first_icon = icon_manager.GetIcon(extension->id());
   EXPECT_FALSE(gfx::BitmapsAreEqual(first_icon, default_icon));
 
   // Remove the icon from the manager.
-  icon_manager.RemoveIcon(extension.id());
+  icon_manager.RemoveIcon(extension->id());
 
   // Now re-load the icon - we should get the same result bitmap (and not the
   // default icon).
-  icon_manager.LoadIcon(&extension);
+  icon_manager.LoadIcon(extension.get());
   WaitForImageLoad();
-  SkBitmap second_icon = icon_manager.GetIcon(extension.id());
+  SkBitmap second_icon = icon_manager.GetIcon(extension->id());
   EXPECT_FALSE(gfx::BitmapsAreEqual(second_icon, default_icon));
 
   EXPECT_TRUE(gfx::BitmapsAreEqual(first_icon, second_icon));
diff --git a/chrome/browser/extensions/extension_info_map_unittest.cc b/chrome/browser/extensions/extension_info_map_unittest.cc
index 559febb..e417d23 100644
--- a/chrome/browser/extensions/extension_info_map_unittest.cc
+++ b/chrome/browser/extensions/extension_info_map_unittest.cc
@@ -28,27 +28,27 @@ class ExtensionInfoMapTest : public testing::Test {
 };
 
 // Returns a barebones test Extension object with the given name.
-static Extension* CreateExtension(const std::string& name) {
+static scoped_refptr<Extension> CreateExtension(const std::string& name) {
 #if defined(OS_WIN)
   FilePath path(FILE_PATH_LITERAL("c:\\foo"));
 #elif defined(OS_POSIX)
   FilePath path(FILE_PATH_LITERAL("/foo"));
 #endif
 
-  scoped_ptr<Extension> extension(new Extension(path.AppendASCII(name)));
-
   DictionaryValue manifest;
   manifest.SetString(keys::kVersion, "1.0.0.0");
   manifest.SetString(keys::kName, name);
 
   std::string error;
-  EXPECT_TRUE(extension->InitFromValue(manifest, false, &error)) << error;
+  scoped_refptr<Extension> extension = Extension::Create(
+      path.AppendASCII(name), Extension::INVALID, manifest, false, &error);
+  EXPECT_TRUE(extension) << error;
 
-  return extension.release();
+  return extension;
 }
 
-static Extension* LoadManifest(const std::string& dir,
-                               const std::string& test_file) {
+static scoped_refptr<Extension> LoadManifest(const std::string& dir,
+                                             const std::string& test_file) {
   FilePath path;
   PathService::Get(chrome::DIR_TEST_DATA, &path);
   path = path.AppendASCII("extensions")
@@ -61,11 +61,12 @@ static Extension* LoadManifest(const std::string& dir,
     return NULL;
 
   std::string error;
-  scoped_ptr<Extension> extension(new Extension(path));
-  EXPECT_TRUE(extension->InitFromValue(
-      *static_cast<DictionaryValue*>(result.get()), false, &error)) << error;
+  scoped_refptr<Extension> extension = Extension::Create(
+      path, Extension::INVALID, *static_cast<DictionaryValue*>(result.get()),
+      false, &error);
+  EXPECT_TRUE(extension) << error;
 
-  return extension.release();
+  return extension;
 }
 
 // Test that the ExtensionInfoMap handles refcounting properly.
@@ -74,9 +75,9 @@ TEST_F(ExtensionInfoMapTest, RefCounting) {
 
   // New extensions should have a single reference holding onto their static
   // data.
-  scoped_ptr<Extension> extension1(CreateExtension("extension1"));
-  scoped_ptr<Extension> extension2(CreateExtension("extension2"));
-  scoped_ptr<Extension> extension3(CreateExtension("extension3"));
+  scoped_refptr<Extension> extension1(CreateExtension("extension1"));
+  scoped_refptr<Extension> extension2(CreateExtension("extension2"));
+  scoped_refptr<Extension> extension3(CreateExtension("extension3"));
   EXPECT_TRUE(extension1->static_data()->HasOneRef());
   EXPECT_TRUE(extension2->static_data()->HasOneRef());
   EXPECT_TRUE(extension3->static_data()->HasOneRef());
@@ -93,7 +94,7 @@ TEST_F(ExtensionInfoMapTest, RefCounting) {
 
   // Delete extension1, and the info map should have the only ref.
   const Extension::StaticData* data1 = extension1->static_data();
-  extension1.reset();
+  extension1 = NULL;
   EXPECT_TRUE(data1->HasOneRef());
 
   // Remove extension2, and the extension2 object should have the only ref.
@@ -109,8 +110,8 @@ TEST_F(ExtensionInfoMapTest, RefCounting) {
 TEST_F(ExtensionInfoMapTest, Properties) {
   scoped_refptr<ExtensionInfoMap> info_map(new ExtensionInfoMap());
 
-  scoped_ptr<Extension> extension1(CreateExtension("extension1"));
-  scoped_ptr<Extension> extension2(CreateExtension("extension2"));
+  scoped_refptr<Extension> extension1(CreateExtension("extension1"));
+  scoped_refptr<Extension> extension2(CreateExtension("extension2"));
 
   extension1->static_data()->AddRef();
   info_map->AddExtension(extension1->static_data());
@@ -132,9 +133,9 @@ TEST_F(ExtensionInfoMapTest, Properties) {
 TEST_F(ExtensionInfoMapTest, CheckPermissions) {
   scoped_refptr<ExtensionInfoMap> info_map(new ExtensionInfoMap());
 
-  scoped_ptr<Extension> app(LoadManifest("manifest_tests",
+  scoped_refptr<Extension> app(LoadManifest("manifest_tests",
                                          "valid_app.json"));
-  scoped_ptr<Extension> extension(LoadManifest("manifest_tests",
+  scoped_refptr<Extension> extension(LoadManifest("manifest_tests",
                                                "tabs_extension.json"));
 
   GURL app_url("http://www.google.com/mail/foo.html");
diff --git a/chrome/browser/extensions/extension_menu_manager_unittest.cc b/chrome/browser/extensions/extension_menu_manager_unittest.cc
index 0b653ba..489b5c6 100644
--- a/chrome/browser/extensions/extension_menu_manager_unittest.cc
+++ b/chrome/browser/extensions/extension_menu_manager_unittest.cc
@@ -44,14 +44,14 @@ class ExtensionMenuManagerTest : public testing::Test {
   // Creates and returns a test Extension. The caller does *not* own the return
   // value.
   Extension* AddExtension(std::string name) {
-    Extension* extension = prefs_.AddExtension(name);
+    scoped_refptr<Extension> extension = prefs_.AddExtension(name);
     extensions_.push_back(extension);
     return extension;
   }
 
  protected:
   ExtensionMenuManager manager_;
-  ScopedVector<Extension> extensions_;
+  ExtensionList extensions_;
   TestExtensionPrefs prefs_;
   int next_id_;
 
diff --git a/chrome/browser/extensions/extension_pref_store_unittest.cc b/chrome/browser/extensions/extension_pref_store_unittest.cc
index aebe5e6..90bed62 100644
--- a/chrome/browser/extensions/extension_pref_store_unittest.cc
+++ b/chrome/browser/extensions/extension_pref_store_unittest.cc
@@ -17,6 +17,8 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace keys = extension_manifest_keys;
+
 namespace {
 
 class TestExtensionPrefStore : public ExtensionPrefStore {
@@ -32,20 +34,25 @@ class TestExtensionPrefStore : public ExtensionPrefStore {
       ADD_FAILURE() << "Failed to create temp dir";
       return;
     }
-    DictionaryValue empty_dict;
+    DictionaryValue simple_dict;
     std::string error;
 
-    ext1_scoped_.reset(new Extension(temp_dir_.path().AppendASCII("ext1")));
-    ext2_scoped_.reset(new Extension(temp_dir_.path().AppendASCII("ext2")));
-    ext3_scoped_.reset(new Extension(temp_dir_.path().AppendASCII("ext3")));
+    simple_dict.SetString(keys::kVersion, "1.0.0.0");
+    simple_dict.SetString(keys::kName, "unused");
+
+    ext1_scoped_ = Extension::Create(
+        temp_dir_.path().AppendASCII("ext1"), Extension::INVALID,
+        simple_dict, false, &error);
+    ext2_scoped_ = Extension::Create(
+        temp_dir_.path().AppendASCII("ext2"), Extension::INVALID,
+        simple_dict, false, &error);
+    ext3_scoped_ = Extension::Create(
+        temp_dir_.path().AppendASCII("ext3"), Extension::INVALID,
+        simple_dict, false, &error);
 
     ext1 = ext1_scoped_.get();
     ext2 = ext2_scoped_.get();
     ext3 = ext3_scoped_.get();
-
-    EXPECT_FALSE(ext1->InitFromValue(empty_dict, false, &error));
-    EXPECT_FALSE(ext2->InitFromValue(empty_dict, false, &error));
-    EXPECT_FALSE(ext3->InitFromValue(empty_dict, false, &error));
   }
 
   typedef std::vector<std::string> ExtensionIDs;
@@ -70,9 +77,9 @@ class TestExtensionPrefStore : public ExtensionPrefStore {
  private:
   ScopedTempDir temp_dir_;
 
-  scoped_ptr<Extension> ext1_scoped_;
-  scoped_ptr<Extension> ext2_scoped_;
-  scoped_ptr<Extension> ext3_scoped_;
+  scoped_refptr<Extension> ext1_scoped_;
+  scoped_refptr<Extension> ext2_scoped_;
+  scoped_refptr<Extension> ext3_scoped_;
 
   // Weak reference.
   PrefService* pref_service_;
diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc
index 8275a99..c8d1967 100644
--- a/chrome/browser/extensions/extension_prefs_unittest.cc
+++ b/chrome/browser/extensions/extension_prefs_unittest.cc
@@ -114,7 +114,7 @@ TEST_F(ExtensionPrefsToolbarOrder, ToolbarOrder) {}
 class ExtensionPrefsExtensionState : public ExtensionPrefsTest {
  public:
   virtual void Initialize() {
-    extension.reset(prefs_.AddExtension("test"));
+    extension = prefs_.AddExtension("test");
     prefs()->SetExtensionState(extension.get(), Extension::DISABLED);
   }
 
@@ -123,7 +123,7 @@ class ExtensionPrefsExtensionState : public ExtensionPrefsTest {
   }
 
  private:
-  scoped_ptr<Extension> extension;
+  scoped_refptr<Extension> extension;
 };
 TEST_F(ExtensionPrefsExtensionState, ExtensionState) {}
 
@@ -131,7 +131,7 @@ TEST_F(ExtensionPrefsExtensionState, ExtensionState) {}
 class ExtensionPrefsEscalatePermissions : public ExtensionPrefsTest {
  public:
   virtual void Initialize() {
-    extension.reset(prefs_.AddExtension("test"));
+    extension = prefs_.AddExtension("test");
     prefs()->SetDidExtensionEscalatePermissions(extension.get(), true);
   }
 
@@ -140,7 +140,7 @@ class ExtensionPrefsEscalatePermissions : public ExtensionPrefsTest {
   }
 
  private:
-  scoped_ptr<Extension> extension;
+  scoped_refptr<Extension> extension;
 };
 TEST_F(ExtensionPrefsEscalatePermissions, EscalatePermissions) {}
 
@@ -149,7 +149,7 @@ TEST_F(ExtensionPrefsEscalatePermissions, EscalatePermissions) {}
 class ExtensionPrefsVersionString : public ExtensionPrefsTest {
  public:
   virtual void Initialize() {
-    extension.reset(prefs_.AddExtension("test"));
+    extension = prefs_.AddExtension("test");
     EXPECT_EQ("0.1", prefs()->GetVersionString(extension->id()));
     prefs()->OnExtensionUninstalled(extension->id(),
                                     Extension::INTERNAL, false);
@@ -160,7 +160,7 @@ class ExtensionPrefsVersionString : public ExtensionPrefsTest {
   }
 
  private:
-  scoped_ptr<Extension> extension;
+  scoped_refptr<Extension> extension;
 };
 TEST_F(ExtensionPrefsVersionString, VersionString) {}
 
@@ -173,11 +173,11 @@ class ExtensionPrefsBlacklist : public ExtensionPrefsTest {
     // Install 5 extensions.
     for (int i = 0; i < 5; i++) {
       std::string name = "test" + base::IntToString(i);
-      extensions_.push_back(linked_ptr<Extension>(prefs_.AddExtension(name)));
+      extensions_.push_back(prefs_.AddExtension(name));
     }
     EXPECT_EQ(NULL, prefs()->GetInstalledExtensionInfo(not_installed_id_));
 
-    std::vector<linked_ptr<Extension> >::const_iterator iter;
+    ExtensionList::const_iterator iter;
     for (iter = extensions_.begin(); iter != extensions_.end(); ++iter) {
       EXPECT_FALSE(prefs()->IsExtensionBlacklisted((*iter)->id()));
     }
@@ -194,7 +194,7 @@ class ExtensionPrefsBlacklist : public ExtensionPrefsTest {
     EXPECT_TRUE(prefs()->IsExtensionBlacklisted(not_installed_id_));
 
     // Make sure the other id's are not blacklisted.
-    std::vector<linked_ptr<Extension> >::const_iterator iter;
+    ExtensionList::const_iterator iter;
     for (iter = extensions_.begin() + 1; iter != extensions_.end(); ++iter) {
       EXPECT_FALSE(prefs()->IsExtensionBlacklisted((*iter)->id()));
     }
@@ -212,7 +212,7 @@ class ExtensionPrefsBlacklist : public ExtensionPrefsTest {
   }
 
  private:
-  std::vector<linked_ptr<Extension> > extensions_;
+  ExtensionList extensions_;
 
   // An id we'll make up that doesn't match any installed extension id.
   std::string not_installed_id_;
@@ -315,7 +315,7 @@ TEST_F(ExtensionPrefsIdleInstallInfo, IdleInstallInfo) {}
 class ExtensionPrefsOnExtensionInstalled : public ExtensionPrefsTest {
  public:
   virtual void Initialize() {
-    extension_.reset(prefs_.AddExtension("on_extension_installed"));
+    extension_ = prefs_.AddExtension("on_extension_installed");
     EXPECT_EQ(Extension::ENABLED,
               prefs()->GetExtensionState(extension_->id()));
     EXPECT_FALSE(prefs()->IsIncognitoEnabled(extension_->id()));
@@ -330,7 +330,7 @@ class ExtensionPrefsOnExtensionInstalled : public ExtensionPrefsTest {
   }
 
  private:
-  scoped_ptr<Extension> extension_;
+  scoped_refptr<Extension> extension_;
 };
 TEST_F(ExtensionPrefsOnExtensionInstalled,
        ExtensionPrefsOnExtensionInstalled) {}
@@ -341,7 +341,7 @@ public:
     // No extensions yet.
     EXPECT_EQ(0, prefs()->GetNextAppLaunchIndex());
 
-    extension_.reset(prefs_.AddExtension("on_extension_installed"));
+    extension_ = prefs_.AddExtension("on_extension_installed");
     EXPECT_EQ(Extension::ENABLED,
         prefs()->GetExtensionState(extension_->id()));
     prefs()->OnExtensionInstalled(extension_.get(),
@@ -364,6 +364,6 @@ public:
   }
 
 private:
-  scoped_ptr<Extension> extension_;
+  scoped_refptr<Extension> extension_;
 };
 TEST_F(ExtensionPrefsAppLaunchIndex, ExtensionPrefsAppLaunchIndex) {}
diff --git a/chrome/browser/extensions/extension_ui_unittest.cc b/chrome/browser/extensions/extension_ui_unittest.cc
index ba429ba..1bda60a 100644
--- a/chrome/browser/extensions/extension_ui_unittest.cc
+++ b/chrome/browser/extensions/extension_ui_unittest.cc
@@ -33,7 +33,6 @@ namespace {
 #elif defined(OS_POSIX)
     FilePath path(FILE_PATH_LITERAL("/foo"));
 #endif
-    Extension extension(path);
     std::string error;
 
     FilePath manifest_path = extension_path.Append(
@@ -41,7 +40,10 @@ namespace {
     scoped_ptr<DictionaryValue> extension_data(DeserializeJSONTestData(
         manifest_path, &error));
     EXPECT_EQ("", error);
-    EXPECT_TRUE(extension.InitFromValue(*extension_data, true, &error));
+
+    scoped_refptr<Extension> extension(Extension::Create(
+        path, Extension::INVALID, *extension_data, true, &error));
+    EXPECT_TRUE(extension.get());
     EXPECT_EQ("", error);
 
     scoped_ptr<DictionaryValue> expected_output_data(DeserializeJSONTestData(
@@ -50,7 +52,7 @@ namespace {
 
     // Produce test output.
     scoped_ptr<DictionaryValue> actual_output_data(
-        ExtensionsDOMHandler::CreateExtensionDetailValue(NULL, &extension,
+        ExtensionsDOMHandler::CreateExtensionDetailValue(NULL, extension.get(),
                                                          pages, true));
 
     // Compare the outputs.
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index 7989e7a..bae09c6 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -94,7 +94,8 @@ class MockService : public ExtensionUpdateService {
                          base::StringPrintf("Extension %d", i));
       if (update_url)
         manifest.SetString(extension_manifest_keys::kUpdateURL, *update_url);
-      Extension* e = prefs_.AddExtensionWithManifest(manifest, location);
+      scoped_refptr<Extension> e =
+          prefs_.AddExtensionWithManifest(manifest, location);
       ASSERT_TRUE(e != NULL);
       list->push_back(e);
     }
@@ -343,10 +344,6 @@ class ExtensionUpdaterTest : public testing::Test {
       EXPECT_EQ(extensions[0]->VersionString(), params["v"]);
     }
     EXPECT_EQ("", params["uc"]);
-
-    if (!pending) {
-      STLDeleteElements(&extensions);
-    }
   }
 
   static void TestBlacklistUpdateCheckRequests() {
@@ -438,7 +435,6 @@ class ExtensionUpdaterTest : public testing::Test {
     updateable = updater->DetermineUpdates(fetch_data, updates);
     EXPECT_EQ(1u, updateable.size());
     EXPECT_EQ(0, updateable[0]);
-    STLDeleteElements(&tmp);
   }
 
   static void TestDetermineUpdatesPending() {
@@ -788,8 +784,6 @@ class ExtensionUpdaterTest : public testing::Test {
       size_t pos = url1_query.find(search_string);
       EXPECT_TRUE(pos != std::string::npos);
     }
-
-    STLDeleteElements(&tmp);
   }
 
   // This makes sure that the extension updater properly stores the results
@@ -821,8 +815,6 @@ class ExtensionUpdaterTest : public testing::Test {
     EXPECT_FALSE(last_ping_day.is_null());
     int64 seconds_diff = (Time::Now() - last_ping_day).InSeconds();
     EXPECT_LT(seconds_diff - results.daystart_elapsed_seconds, 5);
-
-    STLDeleteElements(&tmp);
   }
 };
 
@@ -894,7 +886,6 @@ TEST(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) {
     ASSERT_FALSE(extensions.empty());
     builder.AddExtension(*extensions[0]);
     EXPECT_TRUE(builder.GetFetches().empty());
-    STLDeleteElements(&extensions);
   }
 
   // Extensions with invalid update URLs should be rejected.
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index feb12941..205611e 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -293,7 +293,7 @@ void ExtensionsServiceBackend::LoadSingleExtension(
   file_util::AbsolutePath(&extension_path);
 
   std::string error;
-  Extension* extension = extension_file_util::LoadExtension(
+  scoped_refptr<Extension> extension = extension_file_util::LoadExtension(
       extension_path,
       Extension::LOAD,
       false,  // Don't require id
@@ -421,7 +421,7 @@ void ExtensionsServiceBackend::ReloadExtensionManifests(
 
     // We need to reload original manifest in order to localize properly.
     std::string error;
-    scoped_ptr<Extension> extension(extension_file_util::LoadExtension(
+    scoped_refptr<Extension> extension(extension_file_util::LoadExtension(
         info->extension_path, info->extension_location, false, &error));
 
     if (extension.get())
@@ -894,20 +894,19 @@ void ExtensionsService::LoadComponentExtensions() {
       continue;
     }
 
-    scoped_ptr<Extension> extension(new Extension(it->root_directory));
-    extension->set_location(Extension::COMPONENT);
-
     std::string error;
-    if (!extension->InitFromValue(
-            *static_cast<DictionaryValue*>(manifest.get()),
-            true,  // require key
-            &error)) {
+    scoped_refptr<Extension> extension(Extension::Create(
+        it->root_directory,
+        Extension::COMPONENT,
+        *static_cast<DictionaryValue*>(manifest.get()),
+        true,  // require key
+        &error));
+    if (!extension.get()) {
       NOTREACHED() << error;
       return;
     }
 
-    OnExtensionLoaded(extension.release(), false);  // Don't allow privilege
-                                                    // increase.
+    OnExtensionLoaded(extension, false);  // Don't allow privilege increase.
   }
 }
 
@@ -1036,15 +1035,14 @@ void ExtensionsService::ContinueLoadAllExtensions(
 void ExtensionsService::LoadInstalledExtension(const ExtensionInfo& info,
                                                bool write_to_prefs) {
   std::string error;
-  Extension* extension = NULL;
+  scoped_refptr<Extension> extension(NULL);
   if (!extension_prefs_->IsExtensionAllowedByPolicy(info.extension_id)) {
     error = errors::kDisabledByPolicy;
   } else if (info.extension_manifest.get()) {
-    scoped_ptr<Extension> tmp(new Extension(info.extension_path));
     bool require_key = info.extension_location != Extension::LOAD;
-    tmp->set_location(info.extension_location);
-    if (tmp->InitFromValue(*info.extension_manifest, require_key, &error))
-      extension = tmp.release();
+    extension = Extension::Create(
+        info.extension_path, info.extension_location, *info.extension_manifest,
+        require_key, &error);
   } else {
     error = errors::kManifestUnreadable;
   }
@@ -1312,7 +1310,7 @@ void ExtensionsService::CheckForExternalUpdates() {
 
 void ExtensionsService::UnloadExtension(const std::string& extension_id) {
   // Make sure the extension gets deleted after we return from this function.
-  scoped_ptr<Extension> extension(
+  scoped_refptr<Extension> extension(
       GetExtensionByIdInternal(extension_id, true, true));
 
   // Callers should not send us nonexistent extensions.
@@ -1350,11 +1348,7 @@ void ExtensionsService::UnloadExtension(const std::string& extension_id) {
 }
 
 void ExtensionsService::UnloadAllExtensions() {
-  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
@@ -1399,7 +1393,7 @@ void ExtensionsService::OnLoadedInstalledExtensions() {
 void ExtensionsService::OnExtensionLoaded(Extension* extension,
                                           bool allow_privilege_increase) {
   // Ensure extension is deleted unless we transfer ownership.
-  scoped_ptr<Extension> scoped_extension(extension);
+  scoped_refptr<Extension> scoped_extension(extension);
 
   // The extension is now loaded, remove its data from unloaded extension map.
   unloaded_extension_paths_.erase(extension->id());
@@ -1446,7 +1440,7 @@ void ExtensionsService::OnExtensionLoaded(Extension* extension,
 
     switch (extension_prefs_->GetExtensionState(extension->id())) {
       case Extension::ENABLED:
-        extensions_.push_back(scoped_extension.release());
+        extensions_.push_back(scoped_extension);
 
         NotifyExtensionLoaded(extension);
 
@@ -1454,7 +1448,7 @@ void ExtensionsService::OnExtensionLoaded(Extension* extension,
             extension->GetChromeURLOverrides());
         break;
       case Extension::DISABLED:
-        disabled_extensions_.push_back(scoped_extension.release());
+        disabled_extensions_.push_back(scoped_extension);
         NotificationService::current()->Notify(
             NotificationType::EXTENSION_UPDATE_DISABLED,
             Source<Profile>(profile_),
@@ -1494,7 +1488,7 @@ void ExtensionsService::UpdateActiveExtensionsInCrashReporter() {
 void ExtensionsService::OnExtensionInstalled(Extension* extension,
                                              bool allow_privilege_increase) {
   // Ensure extension is deleted unless we transfer ownership.
-  scoped_ptr<Extension> scoped_extension(extension);
+  scoped_refptr<Extension> scoped_extension(extension);
   Extension::State initial_state = Extension::DISABLED;
   bool initial_enable_incognito = false;
   PendingExtensionMap::iterator it =
@@ -1631,7 +1625,7 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
   }
 
   // Transfer ownership of |extension| to OnExtensionLoaded.
-  OnExtensionLoaded(scoped_extension.release(), allow_privilege_increase);
+  OnExtensionLoaded(scoped_extension, allow_privilege_increase);
 }
 
 Extension* ExtensionsService::GetExtensionByIdInternal(const std::string& id,
diff --git a/chrome/browser/extensions/image_loading_tracker_unittest.cc b/chrome/browser/extensions/image_loading_tracker_unittest.cc
index f796a36..d23e2f6 100644
--- a/chrome/browser/extensions/image_loading_tracker_unittest.cc
+++ b/chrome/browser/extensions/image_loading_tracker_unittest.cc
@@ -51,7 +51,7 @@ class ImageLoadingTrackerTest : public testing::Test,
     return result;
   }
 
-  Extension* CreateExtension() {
+  scoped_refptr<Extension> CreateExtension() {
     // Create and load an extension.
     FilePath test_file;
     if (!PathService::Get(chrome::DIR_TEST_DATA, &test_file)) {
@@ -74,11 +74,8 @@ class ImageLoadingTrackerTest : public testing::Test,
     if (!valid_value.get())
       return NULL;
 
-    scoped_ptr<Extension> extension(new Extension(test_file));
-    if (!extension->InitFromValue(*valid_value, false, &error))
-      return NULL;
-
-    return extension.release();
+    return Extension::Create(
+        test_file, Extension::INVALID, *valid_value, false, &error);
   }
 
   SkBitmap image_;
@@ -99,7 +96,7 @@ class ImageLoadingTrackerTest : public testing::Test,
 
 // Tests asking ImageLoadingTracker to cache pushes the result to the Extension.
 TEST_F(ImageLoadingTrackerTest, Cache) {
-  scoped_ptr<Extension> extension(CreateExtension());
+  scoped_refptr<Extension> extension(CreateExtension());
   ASSERT_TRUE(extension.get() != NULL);
 
   ExtensionResource image_resource =
@@ -146,7 +143,7 @@ TEST_F(ImageLoadingTrackerTest, Cache) {
 // Tests deleting an extension while waiting for the image to load doesn't cause
 // problems.
 TEST_F(ImageLoadingTrackerTest, DeleteExtensionWhileWaitingForCache) {
-  scoped_ptr<Extension> extension(CreateExtension());
+  scoped_refptr<Extension> extension(CreateExtension());
   ASSERT_TRUE(extension.get() != NULL);
 
   ExtensionResource image_resource =
@@ -170,7 +167,7 @@ TEST_F(ImageLoadingTrackerTest, DeleteExtensionWhileWaitingForCache) {
 
   // Chuck the extension, that way if anyone tries to access it we should crash
   // or get valgrind errors.
-  extension.reset();
+  extension = NULL;
 
   WaitForImageLoad();
 
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
index fc8aea8..ea5ec64 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
@@ -140,18 +140,20 @@ void SandboxedExtensionUnpacker::OnUnpackExtensionSucceeded(
   // extension was unpacked to. We use this until the extension is finally
   // installed. For example, the install UI shows images from inside the
   // extension.
-  extension_.reset(new Extension(extension_root_));
 
   // Localize manifest now, so confirm UI gets correct extension name.
   std::string error;
-  if (!extension_l10n_util::LocalizeExtension(extension_.get(),
+  if (!extension_l10n_util::LocalizeExtension(extension_root_,
                                               final_manifest.get(),
                                               &error)) {
     ReportFailure(error);
     return;
   }
 
-  if (!extension_->InitFromValue(*final_manifest, true, &error)) {
+  extension_ = Extension::Create(
+      extension_root_, Extension::INTERNAL, *final_manifest, true, &error);
+
+  if (!extension_.get()) {
     ReportFailure(std::string("Manifest is invalid: ") + error);
     return;
   }
@@ -270,8 +272,8 @@ void SandboxedExtensionUnpacker::ReportFailure(const std::string& error) {
 
 void SandboxedExtensionUnpacker::ReportSuccess() {
   // Client takes ownership of temporary directory and extension.
-  client_->OnUnpackSuccess(temp_dir_.Take(), extension_root_,
-                           extension_.release());
+  client_->OnUnpackSuccess(temp_dir_.Take(), extension_root_, extension_);
+  extension_ = NULL;
 }
 
 DictionaryValue* SandboxedExtensionUnpacker::RewriteManifestFile(
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.h b/chrome/browser/extensions/sandboxed_extension_unpacker.h
index 8df1414..383156b 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.h
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.h
@@ -160,7 +160,7 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
   FilePath extension_root_;
 
   // Represents the extension we're unpacking.
-  scoped_ptr<Extension> extension_;
+  scoped_refptr<Extension> extension_;
 
   // Whether we've received a response from the utility process yet.
   bool got_response_;
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
index d461ff4..0ba87d5 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
@@ -26,7 +26,6 @@ using testing::Invoke;
 void OnUnpackSuccess(const FilePath& temp_dir,
                      const FilePath& extension_root,
                      Extension* extension) {
-  delete extension;
   // Don't delete temp_dir here, we need to do some post op checking.
 }
 
diff --git a/chrome/browser/extensions/test_extension_prefs.cc b/chrome/browser/extensions/test_extension_prefs.cc
index a32f847..2cbb2d1 100644
--- a/chrome/browser/extensions/test_extension_prefs.cc
+++ b/chrome/browser/extensions/test_extension_prefs.cc
@@ -45,22 +45,25 @@ void TestExtensionPrefs::RecreateExtensionPrefs() {
   prefs_.reset(new ExtensionPrefs(pref_service_.get(), temp_dir_.path()));
 }
 
-Extension* TestExtensionPrefs::AddExtension(std::string name) {
+scoped_refptr<Extension> TestExtensionPrefs::AddExtension(std::string name) {
   DictionaryValue dictionary;
   dictionary.SetString(extension_manifest_keys::kName, name);
   dictionary.SetString(extension_manifest_keys::kVersion, "0.1");
   return AddExtensionWithManifest(dictionary, Extension::INTERNAL);
 }
 
-Extension* TestExtensionPrefs::AddExtensionWithManifest(
+scoped_refptr<Extension> TestExtensionPrefs::AddExtensionWithManifest(
     const DictionaryValue& manifest, Extension::Location location) {
   std::string name;
   EXPECT_TRUE(manifest.GetString(extension_manifest_keys::kName, &name));
   FilePath path =  extensions_dir_.AppendASCII(name);
-  Extension* extension = new Extension(path);
   std::string errors;
-  extension->set_location(location);
-  EXPECT_TRUE(extension->InitFromValue(manifest, false, &errors));
+  scoped_refptr<Extension> extension = Extension::Create(
+      path, location, manifest, false, &errors);
+  EXPECT_TRUE(extension);
+  if (!extension)
+    return NULL;
+
   EXPECT_TRUE(Extension::IdIsValid(extension->id()));
   const bool kInitialIncognitoEnabled = false;
   prefs_->OnExtensionInstalled(extension, Extension::ENABLED,
@@ -69,6 +72,6 @@ Extension* TestExtensionPrefs::AddExtensionWithManifest(
 }
 
 std::string TestExtensionPrefs::AddExtensionAndReturnId(std::string name) {
-  scoped_ptr<Extension> extension(AddExtension(name));
+  scoped_refptr<Extension> extension(AddExtension(name));
   return extension->id();
 }
diff --git a/chrome/browser/extensions/test_extension_prefs.h b/chrome/browser/extensions/test_extension_prefs.h
index 1523bee..cb723b2 100644
--- a/chrome/browser/extensions/test_extension_prefs.h
+++ b/chrome/browser/extensions/test_extension_prefs.h
@@ -34,11 +34,11 @@ class TestExtensionPrefs {
 
   // Creates a new Extension with the given name in our temp dir, adds it to
   // our ExtensionPrefs, and returns it.
-  Extension* AddExtension(std::string name);
+  scoped_refptr<Extension> AddExtension(std::string name);
 
   // Similar to AddExtension, but takes a dictionary with manifest values.
-  Extension* AddExtensionWithManifest(const DictionaryValue& manifest,
-                                      Extension::Location location);
+  scoped_refptr<Extension> AddExtensionWithManifest(
+      const DictionaryValue& manifest, Extension::Location location);
 
   // Similar to AddExtension, this adds a new test Extension. This is useful for
   // cases when you don't need the Extension object, but just the id it was