diff options
author | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-26 23:29:19 +0000 |
---|---|---|
committer | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-26 23:29:19 +0000 |
commit | 844bf242eb8cf058a52dd523164278bfbde23f42 (patch) | |
tree | 2ed5864945e7983fdbe23be2bde8fa09d758a11e /chrome/browser | |
parent | 5bd1c88a74cefc0d924be9ba6b6f0eb23972f00c (diff) | |
download | chromium_src-844bf242eb8cf058a52dd523164278bfbde23f42.zip chromium_src-844bf242eb8cf058a52dd523164278bfbde23f42.tar.gz chromium_src-844bf242eb8cf058a52dd523164278bfbde23f42.tar.bz2 |
Revert 63962 (broke chromeos and chromium linux builder) - 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
TBR=mpcomplete@chromium.org
Review URL: http://codereview.chromium.org/4130004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63976 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
27 files changed, 285 insertions, 287 deletions
diff --git a/chrome/browser/cocoa/extension_installed_bubble_controller_unittest.mm b/chrome/browser/cocoa/extension_installed_bubble_controller_unittest.mm index ad68c85..adcc426 100644 --- a/chrome/browser/cocoa/extension_installed_bubble_controller_unittest.mm +++ b/chrome/browser/cocoa/extension_installed_bubble_controller_unittest.mm @@ -75,8 +75,7 @@ class ExtensionInstalledBubbleControllerTest : public CocoaTest { // Create a skeletal framework of either page action or browser action // type. This extension only needs to have a type and a name to initialize // the ExtensionInstalledBubble for unit testing. - scoped_refptr<Extension> CreateExtension( - extension_installed_bubble::ExtensionType type) { + Extension* CreateExtension(extension_installed_bubble::ExtensionType type) { FilePath path; PathService::Get(chrome::DIR_TEST_DATA, &path); path = path.AppendASCII("extensions").AppendASCII("dummy"); @@ -99,9 +98,10 @@ class ExtensionInstalledBubbleControllerTest : public CocoaTest { extension_input_value.Set(keys::kBrowserAction, browser_action); } + Extension* extension = new Extension(path); std::string error; - return Extension::Create( - path, Extension::INVALID, extension_input_value, false, &error); + extension->InitFromValue(extension_input_value, false, &error); + return extension; } // Allows us to create the window and browser for testing. @@ -114,7 +114,7 @@ class ExtensionInstalledBubbleControllerTest : public CocoaTest { Browser* browser_; // weak, owned by BrowserTestHelper. // Skeleton extension to be tested; reinitialized for each test. - scoped_refptr<Extension> extension_; + scoped_ptr<Extension> extension_; // The icon_ to be loaded into the bubble window. SkBitmap icon_; @@ -122,7 +122,8 @@ class ExtensionInstalledBubbleControllerTest : public CocoaTest { // Confirm that window sizes are set correctly for a page action extension. TEST_F(ExtensionInstalledBubbleControllerTest, PageActionTest) { - extension_ = CreateExtension(extension_installed_bubble::kPageAction); + extension_.reset( + CreateExtension(extension_installed_bubble::kPageAction)); ExtensionInstalledBubbleControllerForTest* controller = [[ExtensionInstalledBubbleControllerForTest alloc] initWithParentWindow:window_ @@ -165,7 +166,8 @@ TEST_F(ExtensionInstalledBubbleControllerTest, PageActionTest) { } TEST_F(ExtensionInstalledBubbleControllerTest, BrowserActionTest) { - extension_ = CreateExtension(extension_installed_bubble::kBrowserAction); + extension_.reset( + CreateExtension(extension_installed_bubble::kBrowserAction)); ExtensionInstalledBubbleControllerForTest* controller = [[ExtensionInstalledBubbleControllerForTest alloc] initWithParentWindow:window_ diff --git a/chrome/browser/cocoa/extensions/extension_install_prompt_controller_unittest.mm b/chrome/browser/cocoa/extensions/extension_install_prompt_controller_unittest.mm index 12eadff..f596851 100644 --- a/chrome/browser/cocoa/extensions/extension_install_prompt_controller_unittest.mm +++ b/chrome/browser/cocoa/extensions/extension_install_prompt_controller_unittest.mm @@ -60,18 +60,19 @@ public: return; } - extension_ = Extension::Create( - path.DirName(), Extension::INVALID, *value, false, &error); - if (!extension_.get()) { + scoped_ptr<Extension> extension(new Extension(path.DirName())); + if (!extension->InitFromValue(*value, false, &error)) { LOG(ERROR) << error; return; } + + extension_.reset(extension.release()); } BrowserTestHelper helper_; FilePath test_data_dir_; SkBitmap icon_; - scoped_refptr<Extension> extension_; + scoped_ptr<Extension> extension_; }; diff --git a/chrome/browser/extensions/convert_user_script.cc b/chrome/browser/extensions/convert_user_script.cc index 89caec4..d50bec1 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; -scoped_refptr<Extension> ConvertUserScriptToExtension( - const FilePath& user_script_path, const GURL& original_url, - std::string* error) { +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,13 +138,12 @@ scoped_refptr<Extension> ConvertUserScriptToExtension( return NULL; } - scoped_refptr<Extension> extension = Extension::Create( - temp_dir.path(), Extension::INTERNAL, *root, false, error); - if (!extension) { + scoped_ptr<Extension> extension(new Extension(temp_dir.path())); + if (!extension->InitFromValue(*root, false, error)) { NOTREACHED() << "Could not init extension " << *error; return NULL; } temp_dir.Take(); // The caller takes ownership of the directory. - return extension; + return extension.release(); } diff --git a/chrome/browser/extensions/convert_user_script.h b/chrome/browser/extensions/convert_user_script.h index d779de0..d392c4d 100644 --- a/chrome/browser/extensions/convert_user_script.h +++ b/chrome/browser/extensions/convert_user_script.h @@ -8,8 +8,6 @@ #include <string> -#include "base/ref_counted.h" - class Extension; class FilePath; class GURL; @@ -19,7 +17,8 @@ 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. -scoped_refptr<Extension> ConvertUserScriptToExtension( - const FilePath& user_script, const GURL& original_url, std::string* error); +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 a0e1b8b..299fee7 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_refptr<Extension> extension(ConvertUserScriptToExtension( + scoped_ptr<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_refptr<Extension> extension(ConvertUserScriptToExtension( + scoped_ptr<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 83fdcb7..8b523d5 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; - scoped_refptr<Extension> extension = - ConvertUserScriptToExtension(source_file_, original_url_, &error); + 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_ = extension; + extension_.reset(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_ = extension_file_util::LoadExtension( - version_dir, install_source_, true, &error); + extension_.reset(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_, allow_privilege_increase_); - extension_ = NULL; + frontend_->OnExtensionInstalled(extension_.release(), + allow_privilege_increase_); // 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 8263095..b1212a2 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_refptr<Extension> extension_; + scoped_ptr<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 8bd68a2..86c78fe 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_refptr<Extension> extension( + scoped_ptr<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 37f3453..51c76ea 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); - scoped_refptr<Extension> extension(Extension::Create( - manifest_path.DirName(), Extension::INVALID, *manifest.get(), - false, NULL)); - ASSERT_TRUE(extension.get()); + Extension extension(manifest_path.DirName()); + ASSERT_TRUE(extension.InitFromValue(*manifest.get(), + false /* require_key */, + NULL /* errors */)); TestIconManager icon_manager(this); // Load the icon and grab the bitmap. - icon_manager.LoadIcon(extension.get()); + icon_manager.LoadIcon(&extension); 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.get()); + icon_manager.LoadIcon(&extension); 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 e417d23..559febb 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 scoped_refptr<Extension> CreateExtension(const std::string& name) { +static 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; - scoped_refptr<Extension> extension = Extension::Create( - path.AppendASCII(name), Extension::INVALID, manifest, false, &error); - EXPECT_TRUE(extension) << error; + EXPECT_TRUE(extension->InitFromValue(manifest, false, &error)) << error; - return extension; + return extension.release(); } -static scoped_refptr<Extension> LoadManifest(const std::string& dir, - const std::string& test_file) { +static 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,12 +61,11 @@ static scoped_refptr<Extension> LoadManifest(const std::string& dir, return NULL; std::string error; - scoped_refptr<Extension> extension = Extension::Create( - path, Extension::INVALID, *static_cast<DictionaryValue*>(result.get()), - false, &error); - EXPECT_TRUE(extension) << error; + scoped_ptr<Extension> extension(new Extension(path)); + EXPECT_TRUE(extension->InitFromValue( + *static_cast<DictionaryValue*>(result.get()), false, &error)) << error; - return extension; + return extension.release(); } // Test that the ExtensionInfoMap handles refcounting properly. @@ -75,9 +74,9 @@ TEST_F(ExtensionInfoMapTest, RefCounting) { // New extensions should have a single reference holding onto their static // data. - scoped_refptr<Extension> extension1(CreateExtension("extension1")); - scoped_refptr<Extension> extension2(CreateExtension("extension2")); - scoped_refptr<Extension> extension3(CreateExtension("extension3")); + scoped_ptr<Extension> extension1(CreateExtension("extension1")); + scoped_ptr<Extension> extension2(CreateExtension("extension2")); + scoped_ptr<Extension> extension3(CreateExtension("extension3")); EXPECT_TRUE(extension1->static_data()->HasOneRef()); EXPECT_TRUE(extension2->static_data()->HasOneRef()); EXPECT_TRUE(extension3->static_data()->HasOneRef()); @@ -94,7 +93,7 @@ TEST_F(ExtensionInfoMapTest, RefCounting) { // Delete extension1, and the info map should have the only ref. const Extension::StaticData* data1 = extension1->static_data(); - extension1 = NULL; + extension1.reset(); EXPECT_TRUE(data1->HasOneRef()); // Remove extension2, and the extension2 object should have the only ref. @@ -110,8 +109,8 @@ TEST_F(ExtensionInfoMapTest, RefCounting) { TEST_F(ExtensionInfoMapTest, Properties) { scoped_refptr<ExtensionInfoMap> info_map(new ExtensionInfoMap()); - scoped_refptr<Extension> extension1(CreateExtension("extension1")); - scoped_refptr<Extension> extension2(CreateExtension("extension2")); + scoped_ptr<Extension> extension1(CreateExtension("extension1")); + scoped_ptr<Extension> extension2(CreateExtension("extension2")); extension1->static_data()->AddRef(); info_map->AddExtension(extension1->static_data()); @@ -133,9 +132,9 @@ TEST_F(ExtensionInfoMapTest, Properties) { TEST_F(ExtensionInfoMapTest, CheckPermissions) { scoped_refptr<ExtensionInfoMap> info_map(new ExtensionInfoMap()); - scoped_refptr<Extension> app(LoadManifest("manifest_tests", + scoped_ptr<Extension> app(LoadManifest("manifest_tests", "valid_app.json")); - scoped_refptr<Extension> extension(LoadManifest("manifest_tests", + scoped_ptr<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 489b5c6..0b653ba 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) { - scoped_refptr<Extension> extension = prefs_.AddExtension(name); + Extension* extension = prefs_.AddExtension(name); extensions_.push_back(extension); return extension; } protected: ExtensionMenuManager manager_; - ExtensionList extensions_; + ScopedVector<Extension> 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 90bed62..aebe5e6 100644 --- a/chrome/browser/extensions/extension_pref_store_unittest.cc +++ b/chrome/browser/extensions/extension_pref_store_unittest.cc @@ -17,8 +17,6 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" -namespace keys = extension_manifest_keys; - namespace { class TestExtensionPrefStore : public ExtensionPrefStore { @@ -34,25 +32,20 @@ class TestExtensionPrefStore : public ExtensionPrefStore { ADD_FAILURE() << "Failed to create temp dir"; return; } - DictionaryValue simple_dict; + DictionaryValue empty_dict; std::string error; - 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_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"))); 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; @@ -77,9 +70,9 @@ class TestExtensionPrefStore : public ExtensionPrefStore { private: ScopedTempDir temp_dir_; - scoped_refptr<Extension> ext1_scoped_; - scoped_refptr<Extension> ext2_scoped_; - scoped_refptr<Extension> ext3_scoped_; + scoped_ptr<Extension> ext1_scoped_; + scoped_ptr<Extension> ext2_scoped_; + scoped_ptr<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 c8d1967..8275a99 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 = prefs_.AddExtension("test"); + extension.reset(prefs_.AddExtension("test")); prefs()->SetExtensionState(extension.get(), Extension::DISABLED); } @@ -123,7 +123,7 @@ class ExtensionPrefsExtensionState : public ExtensionPrefsTest { } private: - scoped_refptr<Extension> extension; + scoped_ptr<Extension> extension; }; TEST_F(ExtensionPrefsExtensionState, ExtensionState) {} @@ -131,7 +131,7 @@ TEST_F(ExtensionPrefsExtensionState, ExtensionState) {} class ExtensionPrefsEscalatePermissions : public ExtensionPrefsTest { public: virtual void Initialize() { - extension = prefs_.AddExtension("test"); + extension.reset(prefs_.AddExtension("test")); prefs()->SetDidExtensionEscalatePermissions(extension.get(), true); } @@ -140,7 +140,7 @@ class ExtensionPrefsEscalatePermissions : public ExtensionPrefsTest { } private: - scoped_refptr<Extension> extension; + scoped_ptr<Extension> extension; }; TEST_F(ExtensionPrefsEscalatePermissions, EscalatePermissions) {} @@ -149,7 +149,7 @@ TEST_F(ExtensionPrefsEscalatePermissions, EscalatePermissions) {} class ExtensionPrefsVersionString : public ExtensionPrefsTest { public: virtual void Initialize() { - extension = prefs_.AddExtension("test"); + extension.reset(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_refptr<Extension> extension; + scoped_ptr<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(prefs_.AddExtension(name)); + extensions_.push_back(linked_ptr<Extension>(prefs_.AddExtension(name))); } EXPECT_EQ(NULL, prefs()->GetInstalledExtensionInfo(not_installed_id_)); - ExtensionList::const_iterator iter; + std::vector<linked_ptr<Extension> >::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. - ExtensionList::const_iterator iter; + std::vector<linked_ptr<Extension> >::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: - ExtensionList extensions_; + std::vector<linked_ptr<Extension> > 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_ = prefs_.AddExtension("on_extension_installed"); + extension_.reset(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_refptr<Extension> extension_; + scoped_ptr<Extension> extension_; }; TEST_F(ExtensionPrefsOnExtensionInstalled, ExtensionPrefsOnExtensionInstalled) {} @@ -341,7 +341,7 @@ public: // No extensions yet. EXPECT_EQ(0, prefs()->GetNextAppLaunchIndex()); - extension_ = prefs_.AddExtension("on_extension_installed"); + extension_.reset(prefs_.AddExtension("on_extension_installed")); EXPECT_EQ(Extension::ENABLED, prefs()->GetExtensionState(extension_->id())); prefs()->OnExtensionInstalled(extension_.get(), @@ -364,6 +364,6 @@ public: } private: - scoped_refptr<Extension> extension_; + scoped_ptr<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 1bda60a..ba429ba 100644 --- a/chrome/browser/extensions/extension_ui_unittest.cc +++ b/chrome/browser/extensions/extension_ui_unittest.cc @@ -33,6 +33,7 @@ namespace { #elif defined(OS_POSIX) FilePath path(FILE_PATH_LITERAL("/foo")); #endif + Extension extension(path); std::string error; FilePath manifest_path = extension_path.Append( @@ -40,10 +41,7 @@ namespace { scoped_ptr<DictionaryValue> extension_data(DeserializeJSONTestData( manifest_path, &error)); EXPECT_EQ("", error); - - scoped_refptr<Extension> extension(Extension::Create( - path, Extension::INVALID, *extension_data, true, &error)); - EXPECT_TRUE(extension.get()); + EXPECT_TRUE(extension.InitFromValue(*extension_data, true, &error)); EXPECT_EQ("", error); scoped_ptr<DictionaryValue> expected_output_data(DeserializeJSONTestData( @@ -52,7 +50,7 @@ namespace { // Produce test output. scoped_ptr<DictionaryValue> actual_output_data( - ExtensionsDOMHandler::CreateExtensionDetailValue(NULL, extension.get(), + ExtensionsDOMHandler::CreateExtensionDetailValue(NULL, &extension, pages, true)); // Compare the outputs. diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc index bae09c6..7989e7a 100644 --- a/chrome/browser/extensions/extension_updater_unittest.cc +++ b/chrome/browser/extensions/extension_updater_unittest.cc @@ -94,8 +94,7 @@ class MockService : public ExtensionUpdateService { base::StringPrintf("Extension %d", i)); if (update_url) manifest.SetString(extension_manifest_keys::kUpdateURL, *update_url); - scoped_refptr<Extension> e = - prefs_.AddExtensionWithManifest(manifest, location); + Extension* e = prefs_.AddExtensionWithManifest(manifest, location); ASSERT_TRUE(e != NULL); list->push_back(e); } @@ -344,6 +343,10 @@ class ExtensionUpdaterTest : public testing::Test { EXPECT_EQ(extensions[0]->VersionString(), params["v"]); } EXPECT_EQ("", params["uc"]); + + if (!pending) { + STLDeleteElements(&extensions); + } } static void TestBlacklistUpdateCheckRequests() { @@ -435,6 +438,7 @@ 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() { @@ -784,6 +788,8 @@ 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 @@ -815,6 +821,8 @@ 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); } }; @@ -886,6 +894,7 @@ 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 205611e..feb12941 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; - scoped_refptr<Extension> extension = extension_file_util::LoadExtension( + 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_refptr<Extension> extension(extension_file_util::LoadExtension( + scoped_ptr<Extension> extension(extension_file_util::LoadExtension( info->extension_path, info->extension_location, false, &error)); if (extension.get()) @@ -894,19 +894,20 @@ void ExtensionsService::LoadComponentExtensions() { continue; } + scoped_ptr<Extension> extension(new Extension(it->root_directory)); + extension->set_location(Extension::COMPONENT); + std::string error; - scoped_refptr<Extension> extension(Extension::Create( - it->root_directory, - Extension::COMPONENT, - *static_cast<DictionaryValue*>(manifest.get()), - true, // require key - &error)); - if (!extension.get()) { + if (!extension->InitFromValue( + *static_cast<DictionaryValue*>(manifest.get()), + true, // require key + &error)) { NOTREACHED() << error; return; } - OnExtensionLoaded(extension, false); // Don't allow privilege increase. + OnExtensionLoaded(extension.release(), false); // Don't allow privilege + // increase. } } @@ -1035,14 +1036,15 @@ void ExtensionsService::ContinueLoadAllExtensions( void ExtensionsService::LoadInstalledExtension(const ExtensionInfo& info, bool write_to_prefs) { std::string error; - scoped_refptr<Extension> extension(NULL); + 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; - extension = Extension::Create( - info.extension_path, info.extension_location, *info.extension_manifest, - require_key, &error); + tmp->set_location(info.extension_location); + if (tmp->InitFromValue(*info.extension_manifest, require_key, &error)) + extension = tmp.release(); } else { error = errors::kManifestUnreadable; } @@ -1310,7 +1312,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_refptr<Extension> extension( + scoped_ptr<Extension> extension( GetExtensionByIdInternal(extension_id, true, true)); // Callers should not send us nonexistent extensions. @@ -1348,7 +1350,11 @@ 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 @@ -1393,7 +1399,7 @@ void ExtensionsService::OnLoadedInstalledExtensions() { void ExtensionsService::OnExtensionLoaded(Extension* extension, bool allow_privilege_increase) { // Ensure extension is deleted unless we transfer ownership. - scoped_refptr<Extension> scoped_extension(extension); + scoped_ptr<Extension> scoped_extension(extension); // The extension is now loaded, remove its data from unloaded extension map. unloaded_extension_paths_.erase(extension->id()); @@ -1440,7 +1446,7 @@ void ExtensionsService::OnExtensionLoaded(Extension* extension, switch (extension_prefs_->GetExtensionState(extension->id())) { case Extension::ENABLED: - extensions_.push_back(scoped_extension); + extensions_.push_back(scoped_extension.release()); NotifyExtensionLoaded(extension); @@ -1448,7 +1454,7 @@ void ExtensionsService::OnExtensionLoaded(Extension* extension, extension->GetChromeURLOverrides()); break; case Extension::DISABLED: - disabled_extensions_.push_back(scoped_extension); + disabled_extensions_.push_back(scoped_extension.release()); NotificationService::current()->Notify( NotificationType::EXTENSION_UPDATE_DISABLED, Source<Profile>(profile_), @@ -1488,7 +1494,7 @@ void ExtensionsService::UpdateActiveExtensionsInCrashReporter() { void ExtensionsService::OnExtensionInstalled(Extension* extension, bool allow_privilege_increase) { // Ensure extension is deleted unless we transfer ownership. - scoped_refptr<Extension> scoped_extension(extension); + scoped_ptr<Extension> scoped_extension(extension); Extension::State initial_state = Extension::DISABLED; bool initial_enable_incognito = false; PendingExtensionMap::iterator it = @@ -1625,7 +1631,7 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension, } // Transfer ownership of |extension| to OnExtensionLoaded. - OnExtensionLoaded(scoped_extension, allow_privilege_increase); + OnExtensionLoaded(scoped_extension.release(), 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 d23e2f6..f796a36 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; } - scoped_refptr<Extension> CreateExtension() { + Extension* CreateExtension() { // Create and load an extension. FilePath test_file; if (!PathService::Get(chrome::DIR_TEST_DATA, &test_file)) { @@ -74,8 +74,11 @@ class ImageLoadingTrackerTest : public testing::Test, if (!valid_value.get()) return NULL; - return Extension::Create( - test_file, Extension::INVALID, *valid_value, false, &error); + scoped_ptr<Extension> extension(new Extension(test_file)); + if (!extension->InitFromValue(*valid_value, false, &error)) + return NULL; + + return extension.release(); } SkBitmap image_; @@ -96,7 +99,7 @@ class ImageLoadingTrackerTest : public testing::Test, // Tests asking ImageLoadingTracker to cache pushes the result to the Extension. TEST_F(ImageLoadingTrackerTest, Cache) { - scoped_refptr<Extension> extension(CreateExtension()); + scoped_ptr<Extension> extension(CreateExtension()); ASSERT_TRUE(extension.get() != NULL); ExtensionResource image_resource = @@ -143,7 +146,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_refptr<Extension> extension(CreateExtension()); + scoped_ptr<Extension> extension(CreateExtension()); ASSERT_TRUE(extension.get() != NULL); ExtensionResource image_resource = @@ -167,7 +170,7 @@ TEST_F(ImageLoadingTrackerTest, DeleteExtensionWhileWaitingForCache) { // Chuck the extension, that way if anyone tries to access it we should crash // or get valgrind errors. - extension = NULL; + extension.reset(); WaitForImageLoad(); diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc index ea5ec64..fc8aea8 100644 --- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc +++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc @@ -140,20 +140,18 @@ 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_root_, + if (!extension_l10n_util::LocalizeExtension(extension_.get(), final_manifest.get(), &error)) { ReportFailure(error); return; } - extension_ = Extension::Create( - extension_root_, Extension::INTERNAL, *final_manifest, true, &error); - - if (!extension_.get()) { + if (!extension_->InitFromValue(*final_manifest, true, &error)) { ReportFailure(std::string("Manifest is invalid: ") + error); return; } @@ -272,8 +270,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_); - extension_ = NULL; + client_->OnUnpackSuccess(temp_dir_.Take(), extension_root_, + extension_.release()); } DictionaryValue* SandboxedExtensionUnpacker::RewriteManifestFile( diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.h b/chrome/browser/extensions/sandboxed_extension_unpacker.h index 383156b..8df1414 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_refptr<Extension> extension_; + scoped_ptr<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 0ba87d5..d461ff4 100644 --- a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc +++ b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc @@ -26,6 +26,7 @@ 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 2cbb2d1..a32f847 100644 --- a/chrome/browser/extensions/test_extension_prefs.cc +++ b/chrome/browser/extensions/test_extension_prefs.cc @@ -45,25 +45,22 @@ void TestExtensionPrefs::RecreateExtensionPrefs() { prefs_.reset(new ExtensionPrefs(pref_service_.get(), temp_dir_.path())); } -scoped_refptr<Extension> TestExtensionPrefs::AddExtension(std::string name) { +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); } -scoped_refptr<Extension> TestExtensionPrefs::AddExtensionWithManifest( +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; - scoped_refptr<Extension> extension = Extension::Create( - path, location, manifest, false, &errors); - EXPECT_TRUE(extension); - if (!extension) - return NULL; - + extension->set_location(location); + EXPECT_TRUE(extension->InitFromValue(manifest, false, &errors)); EXPECT_TRUE(Extension::IdIsValid(extension->id())); const bool kInitialIncognitoEnabled = false; prefs_->OnExtensionInstalled(extension, Extension::ENABLED, @@ -72,6 +69,6 @@ scoped_refptr<Extension> TestExtensionPrefs::AddExtensionWithManifest( } std::string TestExtensionPrefs::AddExtensionAndReturnId(std::string name) { - scoped_refptr<Extension> extension(AddExtension(name)); + scoped_ptr<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 cb723b2..1523bee 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. - scoped_refptr<Extension> AddExtension(std::string name); + Extension* AddExtension(std::string name); // Similar to AddExtension, but takes a dictionary with manifest values. - scoped_refptr<Extension> AddExtensionWithManifest( - const DictionaryValue& manifest, Extension::Location location); + 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 diff --git a/chrome/browser/sync/glue/extension_util_unittest.cc b/chrome/browser/sync/glue/extension_util_unittest.cc index 47c7830..56cf24e 100644 --- a/chrome/browser/sync/glue/extension_util_unittest.cc +++ b/chrome/browser/sync/glue/extension_util_unittest.cc @@ -36,12 +36,10 @@ const char kName2[] = "MyExtension2"; class ExtensionUtilTest : public testing::Test { }; -scoped_refptr<Extension> MakeExtension( - bool is_theme, const GURL& update_url, - const GURL& launch_url, - bool converted_from_user_script, - Extension::Location location, int num_plugins, - const FilePath& extension_path) { +void MakeExtension(bool is_theme, const GURL& update_url, + const GURL& launch_url, bool converted_from_user_script, + Extension::Location location, int num_plugins, + Extension* extension) { DictionaryValue source; source.SetString(extension_manifest_keys::kName, "PossiblySyncableExtension"); @@ -70,123 +68,121 @@ scoped_refptr<Extension> MakeExtension( } std::string error; - scoped_refptr<Extension> extension = Extension::Create( - extension_path, location, source, false, &error); #if defined(OS_CHROMEOS) if (num_plugins > 0) { // plugins are illegal in extensions on chrome os. - EXPECT_FALSE(extension); - return NULL; + EXPECT_FALSE(extension->InitFromValue(source, false, &error)); + return; } #endif - EXPECT_TRUE(extension); + extension->set_location(location); + EXPECT_TRUE(extension->InitFromValue(source, false, &error)); EXPECT_EQ("", error); - return extension; } TEST_F(ExtensionUtilTest, GetExtensionType) { { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), GURL(), false, - Extension::INTERNAL, 0, file_path)); - EXPECT_EQ(EXTENSION, GetExtensionType(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL(), GURL(), false, + Extension::INTERNAL, 0, &extension); + EXPECT_EQ(EXTENSION, GetExtensionType(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(true, GURL(), GURL(), false, - Extension::INTERNAL, 0, file_path)); - EXPECT_EQ(THEME, GetExtensionType(*extension)); + Extension extension(file_path); + MakeExtension(true, GURL(), GURL(), false, + Extension::INTERNAL, 0, &extension); + EXPECT_EQ(THEME, GetExtensionType(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), GURL(), true, - Extension::INTERNAL, 0, file_path)); - EXPECT_EQ(LOCAL_USER_SCRIPT, GetExtensionType(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL(), GURL(), true, + Extension::INTERNAL, 0, &extension); + EXPECT_EQ(LOCAL_USER_SCRIPT, GetExtensionType(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL("http://www.google.com"), GURL(), true, - Extension::INTERNAL, 0, file_path)); - EXPECT_EQ(UPDATEABLE_USER_SCRIPT, GetExtensionType(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL("http://www.google.com"), GURL(), true, + Extension::INTERNAL, 0, &extension); + EXPECT_EQ(UPDATEABLE_USER_SCRIPT, GetExtensionType(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), - GURL("http://www.google.com"), false, - Extension::INTERNAL, 0, file_path)); - EXPECT_EQ(APP, GetExtensionType(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL(), + GURL("http://www.google.com"), false, + Extension::INTERNAL, 0, &extension); + EXPECT_EQ(APP, GetExtensionType(extension)); } } TEST_F(ExtensionUtilTest, IsExtensionValid) { { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), GURL(), false, - Extension::INTERNAL, 0, file_path)); - EXPECT_TRUE(IsExtensionValid(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL(), GURL(), false, + Extension::INTERNAL, 0, &extension); + EXPECT_TRUE(IsExtensionValid(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(kValidUpdateUrl1), GURL(), - true, Extension::INTERNAL, 0, file_path)); - EXPECT_TRUE(IsExtensionValid(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL(kValidUpdateUrl1), GURL(), + true, Extension::INTERNAL, 0, &extension); + EXPECT_TRUE(IsExtensionValid(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), GURL(), true, - Extension::INTERNAL, 0, file_path)); - EXPECT_TRUE(IsExtensionValid(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL(), GURL(), true, + Extension::INTERNAL, 0, &extension); + EXPECT_TRUE(IsExtensionValid(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(true, GURL(), GURL(), false, - Extension::INTERNAL, 0, file_path)); - EXPECT_TRUE(IsExtensionValid(*extension)); + Extension extension(file_path); + MakeExtension(true, GURL(), GURL(), false, + Extension::INTERNAL, 0, &extension); + EXPECT_TRUE(IsExtensionValid(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), - GURL("http://www.google.com"), false, - Extension::INTERNAL, 0, file_path)); - EXPECT_TRUE(IsExtensionValid(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL(), + GURL("http://www.google.com"), false, + Extension::INTERNAL, 0, &extension); + EXPECT_TRUE(IsExtensionValid(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), GURL(), false, - Extension::EXTERNAL_PREF, 0, file_path)); - EXPECT_FALSE(IsExtensionValid(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL(), GURL(), false, + Extension::EXTERNAL_PREF, 0, &extension); + EXPECT_FALSE(IsExtensionValid(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension( - false, GURL("http://third-party.update_url.com"), GURL(), true, - Extension::INTERNAL, 0, file_path)); - EXPECT_FALSE(IsExtensionValid(*extension)); + Extension extension(file_path); + MakeExtension( + false, GURL("http://third-party.update_url.com"), GURL(), true, + Extension::INTERNAL, 0, &extension); + EXPECT_FALSE(IsExtensionValid(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), GURL(), true, - Extension::INTERNAL, 1, file_path)); - EXPECT_FALSE(IsExtensionValid(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL(), GURL(), true, + Extension::INTERNAL, 1, &extension); + EXPECT_FALSE(IsExtensionValid(extension)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), GURL(), true, - Extension::INTERNAL, 2, file_path)); - EXPECT_FALSE(IsExtensionValid(*extension)); + Extension extension(file_path); + MakeExtension(false, GURL(), GURL(), true, + Extension::INTERNAL, 2, &extension); + EXPECT_FALSE(IsExtensionValid(extension)); } } @@ -196,36 +192,36 @@ TEST_F(ExtensionUtilTest, IsExtensionValidAndSyncable) { allowed_extension_types.insert(APP); { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), GURL(), false, - Extension::INTERNAL, 0, file_path)); + Extension extension(file_path); + MakeExtension(false, GURL(), GURL(), false, + Extension::INTERNAL, 0, &extension); EXPECT_TRUE(IsExtensionValidAndSyncable( - *extension, allowed_extension_types)); + extension, allowed_extension_types)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), - GURL("http://www.google.com"), false, - Extension::INTERNAL, 0, file_path)); + Extension extension(file_path); + MakeExtension(false, GURL(), + GURL("http://www.google.com"), false, + Extension::INTERNAL, 0, &extension); EXPECT_TRUE(IsExtensionValidAndSyncable( - *extension, allowed_extension_types)); + extension, allowed_extension_types)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), GURL(), true, - Extension::INTERNAL, 0, file_path)); + Extension extension(file_path); + MakeExtension(false, GURL(), GURL(), true, + Extension::INTERNAL, 0, &extension); EXPECT_FALSE(IsExtensionValidAndSyncable( - *extension, allowed_extension_types)); + extension, allowed_extension_types)); } { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeExtension(false, GURL(), GURL(), false, - Extension::EXTERNAL_PREF, 0, file_path)); + Extension extension(file_path); + MakeExtension(false, GURL(), GURL(), false, + Extension::EXTERNAL_PREF, 0, &extension); EXPECT_FALSE(IsExtensionValidAndSyncable( - *extension, allowed_extension_types)); + extension, allowed_extension_types)); } } @@ -454,32 +450,30 @@ TEST_F(ExtensionUtilTest, AreExtensionSpecificsNonUserPropertiesEqual) { EXPECT_TRUE(AreExtensionSpecificsNonUserPropertiesEqual(a, b)); } -scoped_refptr<Extension> MakeSyncableExtension( - const std::string& version_string, - const std::string& update_url_spec, - const std::string& name, - const FilePath& extension_path) { +void MakeSyncableExtension(const std::string& version_string, + const std::string& update_url_spec, + const std::string& name, + Extension* extension) { DictionaryValue source; source.SetString(extension_manifest_keys::kVersion, version_string); source.SetString(extension_manifest_keys::kUpdateURL, update_url_spec); source.SetString(extension_manifest_keys::kName, name); std::string error; - scoped_refptr<Extension> extension = Extension::Create( - extension_path, Extension::INTERNAL, source, false, &error); - EXPECT_TRUE(extension); + extension->set_location(Extension::INTERNAL); + EXPECT_TRUE(extension->InitFromValue(source, false, &error)); EXPECT_EQ("", error); - return extension; } TEST_F(ExtensionUtilTest, GetExtensionSpecificsHelper) { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeSyncableExtension(kValidVersion, kValidUpdateUrl1, kName, file_path)); + Extension extension(file_path); + MakeSyncableExtension(kValidVersion, kValidUpdateUrl1, kName, + &extension); sync_pb::ExtensionSpecifics specifics; - GetExtensionSpecificsHelper(*extension, true, false, &specifics); - EXPECT_EQ(extension->id(), specifics.id()); - EXPECT_EQ(extension->VersionString(), kValidVersion); - EXPECT_EQ(extension->update_url().spec(), kValidUpdateUrl1); + GetExtensionSpecificsHelper(extension, true, false, &specifics); + EXPECT_EQ(extension.id(), specifics.id()); + EXPECT_EQ(extension.VersionString(), kValidVersion); + EXPECT_EQ(extension.update_url().spec(), kValidUpdateUrl1); EXPECT_TRUE(specifics.enabled()); EXPECT_FALSE(specifics.incognito_enabled()); EXPECT_EQ(kName, specifics.name()); @@ -487,18 +481,19 @@ TEST_F(ExtensionUtilTest, GetExtensionSpecificsHelper) { TEST_F(ExtensionUtilTest, IsExtensionOutdated) { FilePath file_path(kExtensionFilePath); - scoped_refptr<Extension> extension( - MakeSyncableExtension(kVersion2, kValidUpdateUrl1, kName, file_path)); + Extension extension(file_path); + MakeSyncableExtension(kVersion2, kValidUpdateUrl1, kName, + &extension); sync_pb::ExtensionSpecifics specifics; specifics.set_id(kValidId); specifics.set_update_url(kValidUpdateUrl1); specifics.set_version(kVersion1); - EXPECT_FALSE(IsExtensionOutdated(*extension, specifics)); + EXPECT_FALSE(IsExtensionOutdated(extension, specifics)); specifics.set_version(kVersion2); - EXPECT_FALSE(IsExtensionOutdated(*extension, specifics)); + EXPECT_FALSE(IsExtensionOutdated(extension, specifics)); specifics.set_version(kVersion3); - EXPECT_TRUE(IsExtensionOutdated(*extension, specifics)); + EXPECT_TRUE(IsExtensionOutdated(extension, specifics)); } // TODO(akalin): Make ExtensionsService/ExtensionUpdater testable diff --git a/chrome/browser/sync/glue/theme_util_unittest.cc b/chrome/browser/sync/glue/theme_util_unittest.cc index 3eac301..fb6641f 100644 --- a/chrome/browser/sync/glue/theme_util_unittest.cc +++ b/chrome/browser/sync/glue/theme_util_unittest.cc @@ -23,20 +23,17 @@ using ::testing::Return; class ThemeUtilTest : public testing::Test { }; -scoped_refptr<Extension> MakeThemeExtension(const FilePath& extension_path, - const std::string& name, - const std::string& update_url) { +void MakeThemeExtension(Extension* extension, + const std::string& name, + const std::string& update_url) { DictionaryValue source; source.SetString(extension_manifest_keys::kName, name); source.Set(extension_manifest_keys::kTheme, new DictionaryValue()); source.SetString(extension_manifest_keys::kUpdateURL, update_url); source.SetString(extension_manifest_keys::kVersion, "0.0.0.0"); std::string error; - scoped_refptr<Extension> extension = Extension::Create( - extension_path, Extension::INTERNAL, source, false, &error); - EXPECT_TRUE(extension); + EXPECT_TRUE(extension->InitFromValue(source, false, &error)); EXPECT_EQ("", error); - return extension; } TEST_F(ThemeUtilTest, AreThemeSpecificsEqualHelper) { @@ -171,17 +168,17 @@ TEST_F(ThemeUtilTest, GetThemeSpecificsHelperCustomTheme) { theme_specifics.set_use_custom_theme(false); theme_specifics.set_use_system_theme_by_default(true); FilePath file_path(kExtensionFilePath); + Extension extension(file_path); const std::string kThemeName("name"); const std::string kThemeUpdateUrl("http://update.url/foo"); - scoped_refptr<Extension> extension( - MakeThemeExtension(file_path, kThemeName, kThemeUpdateUrl)); - GetThemeSpecificsFromCurrentThemeHelper(extension.get(), false, false, + MakeThemeExtension(&extension, kThemeName, kThemeUpdateUrl); + GetThemeSpecificsFromCurrentThemeHelper(&extension, false, false, &theme_specifics); EXPECT_TRUE(theme_specifics.use_custom_theme()); EXPECT_TRUE(theme_specifics.use_system_theme_by_default()); EXPECT_EQ(kThemeName, theme_specifics.custom_theme_name()); - EXPECT_EQ(extension->id(), theme_specifics.custom_theme_id()); + EXPECT_EQ(extension.id(), theme_specifics.custom_theme_id()); EXPECT_EQ(kThemeUpdateUrl, theme_specifics.custom_theme_update_url()); } @@ -189,18 +186,18 @@ TEST_F(ThemeUtilTest, GetThemeSpecificsHelperCustomThemeDistinct) { sync_pb::ThemeSpecifics theme_specifics; theme_specifics.set_use_custom_theme(false); FilePath file_path(kExtensionFilePath); + Extension extension(file_path); const std::string kThemeName("name"); const std::string kThemeUpdateUrl("http://update.url/foo"); - scoped_refptr<Extension> extension( - MakeThemeExtension(file_path, kThemeName, kThemeUpdateUrl)); - GetThemeSpecificsFromCurrentThemeHelper(extension.get(), true, false, + MakeThemeExtension(&extension, kThemeName, kThemeUpdateUrl); + GetThemeSpecificsFromCurrentThemeHelper(&extension, true, false, &theme_specifics); EXPECT_TRUE(theme_specifics.use_custom_theme()); EXPECT_TRUE(theme_specifics.has_use_system_theme_by_default()); EXPECT_FALSE(theme_specifics.use_system_theme_by_default()); EXPECT_EQ(kThemeName, theme_specifics.custom_theme_name()); - EXPECT_EQ(extension->id(), theme_specifics.custom_theme_id()); + EXPECT_EQ(extension.id(), theme_specifics.custom_theme_id()); EXPECT_EQ(kThemeUpdateUrl, theme_specifics.custom_theme_update_url()); } diff --git a/chrome/browser/sync/util/extensions_activity_monitor_unittest.cc b/chrome/browser/sync/util/extensions_activity_monitor_unittest.cc index d5f88d4..cffa963 100644 --- a/chrome/browser/sync/util/extensions_activity_monitor_unittest.cc +++ b/chrome/browser/sync/util/extensions_activity_monitor_unittest.cc @@ -53,7 +53,7 @@ class BookmarkAPIEventTask : public Task { done_->Signal(); } private: - scoped_refptr<Extension> extension_; + scoped_ptr<Extension> extension_; scoped_refptr<FunctionType> function_; size_t repeats_; base::WaitableEvent* done_; @@ -68,12 +68,13 @@ class BookmarkAPIEventGenerator { template <class T> void NewEvent(const FilePath::StringType& extension_path, T* bookmarks_function, size_t repeats) { + FilePath path(extension_path); + Extension* extension = new Extension(path); std::string error; DictionaryValue input; input.SetString(keys::kVersion, kTestExtensionVersion); input.SetString(keys::kName, kTestExtensionName); - scoped_refptr<Extension> extension(Extension::Create( - FilePath(extension_path), Extension::INVALID, input, false, &error)); + extension->InitFromValue(input, false, &error); bookmarks_function->set_name(T::function_name()); base::WaitableEvent done_event(false, false); BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, @@ -127,13 +128,14 @@ class ExtensionsActivityMonitorTest : public testing::Test { static std::string GetExtensionIdForPath( const FilePath::StringType& extension_path) { std::string error; + FilePath path(extension_path); + Extension e(path); DictionaryValue input; input.SetString(keys::kVersion, kTestExtensionVersion); input.SetString(keys::kName, kTestExtensionName); - scoped_refptr<Extension> extension(Extension::Create( - FilePath(extension_path), Extension::INVALID, input, false, &error)); + e.InitFromValue(input, false, &error); EXPECT_EQ("", error); - return extension->id(); + return e.id(); } private: NotificationService* service_; diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc index 906119c..976509f 100644 --- a/chrome/browser/tabs/tab_strip_model_unittest.cc +++ b/chrome/browser/tabs/tab_strip_model_unittest.cc @@ -1511,13 +1511,12 @@ TEST_F(TabStripModelTest, Apps) { #elif defined(OS_POSIX) FilePath path(FILE_PATH_LITERAL("/foo")); #endif - scoped_refptr<Extension> extension_app(new Extension(path, - Extension::INVALID)); - extension_app->mutable_static_data_->launch_web_url = "http://www.google.com"; + Extension extension_app(path); + extension_app.mutable_static_data_->launch_web_url = "http://www.google.com"; TabContents* contents1 = CreateTabContents(); - contents1->SetExtensionApp(extension_app); + contents1->SetExtensionApp(&extension_app); TabContents* contents2 = CreateTabContents(); - contents2->SetExtensionApp(extension_app); + contents2->SetExtensionApp(&extension_app); TabContents* contents3 = CreateTabContents(); SetID(contents1, 1); diff --git a/chrome/browser/themes/browser_theme_pack_unittest.cc b/chrome/browser/themes/browser_theme_pack_unittest.cc index 5da6acf..87fc2c7 100644 --- a/chrome/browser/themes/browser_theme_pack_unittest.cc +++ b/chrome/browser/themes/browser_theme_pack_unittest.cc @@ -397,6 +397,8 @@ TEST_F(BrowserThemePackTest, CanBuildAndReadPack) { // Part 1: Build the pack from an extension. { FilePath star_gazing_path = GetStarGazingPath(); + Extension extension(star_gazing_path); + FilePath manifest_path = star_gazing_path.AppendASCII("manifest.json"); std::string error; @@ -405,13 +407,11 @@ TEST_F(BrowserThemePackTest, CanBuildAndReadPack) { static_cast<DictionaryValue*>(serializer.Deserialize(NULL, &error))); EXPECT_EQ("", error); ASSERT_TRUE(valid_value.get()); - scoped_refptr<Extension> extension(Extension::Create( - star_gazing_path, Extension::INVALID, *valid_value, true, &error)); - ASSERT_TRUE(extension.get()); + ASSERT_TRUE(extension.InitFromValue(*valid_value, true, &error)); ASSERT_EQ("", error); scoped_refptr<BrowserThemePack> pack = - BrowserThemePack::BuildFromExtension(extension.get()); + BrowserThemePack::BuildFromExtension(&extension); ASSERT_TRUE(pack.get()); ASSERT_TRUE(pack->WriteToDisk(file)); VerifyStarGazing(pack.get()); |