diff options
37 files changed, 463 insertions, 472 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()); diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc index 8bf9938..e0cdc5d 100644 --- a/chrome/common/extensions/extension.cc +++ b/chrome/common/extensions/extension.cc @@ -283,18 +283,6 @@ Extension::RuntimeData::~RuntimeData() { // // static -scoped_refptr<Extension> Extension::Create(const FilePath& path, - Location location, - const DictionaryValue& value, - bool require_key, - std::string* error) { - scoped_refptr<Extension> extension = new Extension(path, location); - if (!extension->InitFromValue(value, require_key, error)) - return NULL; - return extension; -} - -// static int Extension::GetPermissionMessageId(const std::string& permission) { return ExtensionConfig::GetSingleton()->GetPermissionMessageId(permission); } @@ -413,6 +401,9 @@ bool Extension::IsHostedAppPermission(const std::string& str) { return false; } +Extension::~Extension() { +} + const std::string Extension::VersionString() const { return version()->GetString(); } @@ -1007,16 +998,15 @@ bool Extension::EnsureNotHybridApp(const DictionaryValue* manifest, return true; } -Extension::Extension(const FilePath& path, Location location) - : mutable_static_data_(new StaticData) { +Extension::Extension(const FilePath& path) + : mutable_static_data_(new StaticData), + runtime_data_(new RuntimeData) { DCHECK(path.IsAbsolute()); static_data_ = mutable_static_data_; - mutable_static_data_->location = location; - mutable_static_data_->path = MaybeNormalizePath(path); -} + mutable_static_data_->location = INVALID; -Extension::~Extension() { + mutable_static_data_->path = MaybeNormalizePath(path); } ExtensionResource Extension::GetResource(const std::string& relative_path) { @@ -2246,7 +2236,7 @@ bool Extension::CanExecuteScriptEverywhere() const { Extension::RuntimeData* Extension::GetRuntimeData() const { // TODO(mpcomplete): it would be nice if I could verify we were on the UI // thread, but we're in common and don't have access to BrowserThread. - return const_cast<Extension::RuntimeData*>(&runtime_data_); + return const_cast<Extension::RuntimeData*>(runtime_data_.get()); } ExtensionInfo::ExtensionInfo(const DictionaryValue* manifest, diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h index 82c007b..c073987 100644 --- a/chrome/common/extensions/extension.h +++ b/chrome/common/extensions/extension.h @@ -30,7 +30,7 @@ class SkBitmap; class Version; // Represents a Chrome extension. -class Extension : public base::RefCountedThreadSafe<Extension> { +class Extension { public: typedef std::map<const std::string, GURL> URLOverrideMap; typedef std::vector<std::string> ScriptingWhitelist; @@ -105,6 +105,18 @@ class Extension : public base::RefCountedThreadSafe<Extension> { struct StaticData : public base::RefCountedThreadSafe<StaticData> { StaticData(); + // TODO(mpcomplete): RefCountedThreadSafe does not allow AddRef/Release on + // const objects. I think that is a mistake. Until we can fix that, here's + // a workaround. + void AddRef() const { + const_cast<StaticData*>(this)-> + base::RefCountedThreadSafe<StaticData>::AddRef(); + } + void Release() const { + const_cast<StaticData*>(this)-> + base::RefCountedThreadSafe<StaticData>::Release(); + } + // A persistent, globally unique ID. An extension's ID is used in things // like directory structures and URLs, and is expected to not change across // versions. It is generated as a SHA-256 hash of the extension's public @@ -274,12 +286,6 @@ class Extension : public base::RefCountedThreadSafe<Extension> { const int message_id; }; - static scoped_refptr<Extension> Create(const FilePath& path, - Location location, - const DictionaryValue& value, - bool require_key, - std::string* error); - // The install message id for |permission|. Returns 0 if none exists. static int GetPermissionMessageId(const std::string& permission); @@ -353,6 +359,9 @@ class Extension : public base::RefCountedThreadSafe<Extension> { // The mimetype used for extensions. static const char kMimeType[]; + explicit Extension(const FilePath& path); + virtual ~Extension(); + // Checks to see if the extension has a valid ID. static bool IdIsValid(const std::string& id); @@ -464,11 +473,22 @@ class Extension : public base::RefCountedThreadSafe<Extension> { // Adds an extension to the scripting whitelist. Used for testing only. static void SetScriptingWhitelist(const ScriptingWhitelist& whitelist); + // Initialize the extension from a parsed manifest. + // Usually, the id of an extension is generated by the "key" property of + // its manifest, but if |require_key| is |false|, a temporary ID will be + // generated based on the path. + bool InitFromValue(const DictionaryValue& value, bool require_key, + std::string* error); + const StaticData* static_data() const { return static_data_; } const FilePath& path() const { return static_data_->path; } const GURL& url() const { return static_data_->extension_url; } Location location() const { return static_data_->location; } + void set_location(Location location) { + mutable_static_data_->location = location; + } + const std::string& id() const { return static_data_->id; } const Version* version() const { return static_data_->version.get(); } // String representation of the version number. @@ -638,22 +658,10 @@ class Extension : public base::RefCountedThreadSafe<Extension> { bool CanExecuteScriptEverywhere() const; private: - friend class base::RefCountedThreadSafe<Extension>; - // Normalize the path for use by the extension. On Windows, this will make // sure the drive letter is uppercase. static FilePath MaybeNormalizePath(const FilePath& path); - Extension(const FilePath& path, Location location); - ~Extension(); - - // Initialize the extension from a parsed manifest. - // Usually, the id of an extension is generated by the "key" property of - // its manifest, but if |require_key| is |false|, a temporary ID will be - // generated based on the path. - bool InitFromValue(const DictionaryValue& value, bool require_key, - std::string* error); - // Helper function for implementing HasCachedImage/GetCachedImage. A return // value of NULL means there is no matching image cached (we allow caching an // empty SkBitmap). @@ -728,18 +736,15 @@ class Extension : public base::RefCountedThreadSafe<Extension> { scoped_refptr<const StaticData> static_data_; // Runtime data. - const RuntimeData runtime_data_; + scoped_ptr<const RuntimeData> runtime_data_; FRIEND_TEST_ALL_PREFIXES(ExtensionTest, LoadPageActionHelper); - FRIEND_TEST_ALL_PREFIXES(ExtensionTest, InitFromValueInvalid); - FRIEND_TEST_ALL_PREFIXES(ExtensionTest, InitFromValueValid); - FRIEND_TEST_ALL_PREFIXES(ExtensionTest, InitFromValueValidNameInRTL); FRIEND_TEST_ALL_PREFIXES(TabStripModelTest, Apps); DISALLOW_COPY_AND_ASSIGN(Extension); }; -typedef std::vector< scoped_refptr<Extension> > ExtensionList; +typedef std::vector<Extension*> ExtensionList; typedef std::set<std::string> ExtensionIdSet; // Handy struct to pass core extension info around. diff --git a/chrome/common/extensions/extension_file_util.cc b/chrome/common/extensions/extension_file_util.cc index c7be522..d530357 100644 --- a/chrome/common/extensions/extension_file_util.cc +++ b/chrome/common/extensions/extension_file_util.cc @@ -81,14 +81,15 @@ void UninstallExtension(const FilePath& extensions_dir, file_util::Delete(extensions_dir.AppendASCII(id), true); // recursive. } -scoped_refptr<Extension> LoadExtension(const FilePath& extension_path, - Extension::Location location, - bool require_key, - std::string* error) { +Extension* LoadExtension(const FilePath& extension_path, + Extension::Location location, + bool require_key, + std::string* error) { FilePath manifest_path = extension_path.Append(Extension::kManifestFilename); if (!file_util::PathExists(manifest_path)) { - *error = l10n_util::GetStringUTF8(IDS_EXTENSION_MANIFEST_UNREADABLE); + *error = + l10n_util::GetStringUTF8(IDS_EXTENSION_MANIFEST_UNREADABLE); return NULL; } @@ -100,7 +101,8 @@ scoped_refptr<Extension> LoadExtension(const FilePath& extension_path, // It would be cleaner to have the JSON reader give a specific error // in this case, but other code tests for a file error with // error->empty(). For now, be consistent. - *error = l10n_util::GetStringUTF8(IDS_EXTENSION_MANIFEST_UNREADABLE); + *error = + l10n_util::GetStringUTF8(IDS_EXTENSION_MANIFEST_UNREADABLE); } else { *error = StringPrintf("%s %s", errors::kManifestParseError, @@ -110,23 +112,26 @@ scoped_refptr<Extension> LoadExtension(const FilePath& extension_path, } if (!root->IsType(Value::TYPE_DICTIONARY)) { - *error = l10n_util::GetStringUTF8(IDS_EXTENSION_MANIFEST_INVALID); + *error = + l10n_util::GetStringUTF8(IDS_EXTENSION_MANIFEST_INVALID); return NULL; } DictionaryValue* manifest = static_cast<DictionaryValue*>(root.get()); - if (!extension_l10n_util::LocalizeExtension(extension_path, manifest, error)) + + scoped_ptr<Extension> extension(new Extension(extension_path)); + extension->set_location(location); + + if (!extension_l10n_util::LocalizeExtension(extension.get(), manifest, error)) return NULL; - scoped_refptr<Extension> extension(Extension::Create( - extension_path, location, *manifest, require_key, error)); - if (!extension.get()) + if (!extension->InitFromValue(*manifest, require_key, error)) return NULL; if (!ValidateExtension(extension.get(), error)) return NULL; - return extension; + return extension.release(); } bool ValidateExtension(Extension* extension, std::string* error) { diff --git a/chrome/common/extensions/extension_file_util.h b/chrome/common/extensions/extension_file_util.h index f6a72bc..1b8fb76 100644 --- a/chrome/common/extensions/extension_file_util.h +++ b/chrome/common/extensions/extension_file_util.h @@ -37,10 +37,10 @@ void UninstallExtension(const FilePath& extensions_dir, // Loads and validates an extension from the specified directory. Returns NULL // on failure, with a description of the error in |error|. -scoped_refptr<Extension> LoadExtension(const FilePath& extension_root, - Extension::Location location, - bool require_key, - std::string* error); +Extension* LoadExtension(const FilePath& extension_root, + Extension::Location location, + bool require_key, + std::string* error); // Returns true if the given extension object is valid and consistent. // Otherwise, a description of the error is returned in |error|. diff --git a/chrome/common/extensions/extension_file_util_unittest.cc b/chrome/common/extensions/extension_file_util_unittest.cc index 230712d..634df05 100644 --- a/chrome/common/extensions/extension_file_util_unittest.cc +++ b/chrome/common/extensions/extension_file_util_unittest.cc @@ -78,7 +78,7 @@ TEST(ExtensionFileUtil, LoadExtensionWithValidLocales) { .AppendASCII("1.0.0.0"); std::string error; - scoped_refptr<Extension> extension(extension_file_util::LoadExtension( + scoped_ptr<Extension> extension(extension_file_util::LoadExtension( install_dir, Extension::LOAD, false, &error)); ASSERT_TRUE(extension != NULL); EXPECT_EQ("The first extension that I made.", extension->description()); @@ -94,7 +94,7 @@ TEST(ExtensionFileUtil, LoadExtensionWithoutLocalesFolder) { .AppendASCII("1.0"); std::string error; - scoped_refptr<Extension> extension(extension_file_util::LoadExtension( + scoped_ptr<Extension> extension(extension_file_util::LoadExtension( install_dir, Extension::LOAD, false, &error)); ASSERT_FALSE(extension == NULL); EXPECT_TRUE(error.empty()); @@ -152,7 +152,7 @@ TEST(ExtensionFileUtil, LoadExtensionGivesHelpfullErrorOnMissingManifest) { .AppendASCII("1.0"); std::string error; - scoped_refptr<Extension> extension(extension_file_util::LoadExtension( + scoped_ptr<Extension> extension(extension_file_util::LoadExtension( install_dir, Extension::LOAD, false, &error)); ASSERT_TRUE(extension == NULL); ASSERT_FALSE(error.empty()); @@ -169,7 +169,7 @@ TEST(ExtensionFileUtil, LoadExtensionGivesHelpfullErrorOnBadManifest) { .AppendASCII("1.0"); std::string error; - scoped_refptr<Extension> extension(extension_file_util::LoadExtension( + scoped_ptr<Extension> extension(extension_file_util::LoadExtension( install_dir, Extension::LOAD, false, &error)); ASSERT_TRUE(extension == NULL); ASSERT_FALSE(error.empty()); @@ -185,7 +185,7 @@ TEST(ExtensionFileUtil, FailLoadingNonUTF8Scripts) { .AppendASCII("bad_encoding"); std::string error; - scoped_refptr<Extension> extension(extension_file_util::LoadExtension( + scoped_ptr<Extension> extension(extension_file_util::LoadExtension( install_dir, Extension::LOAD, false, &error)); ASSERT_TRUE(extension == NULL); ASSERT_STREQ("Could not load file 'bad_encoding.js' for content script. " diff --git a/chrome/common/extensions/extension_l10n_util.cc b/chrome/common/extensions/extension_l10n_util.cc index 53950cb..4187689 100644 --- a/chrome/common/extensions/extension_l10n_util.cc +++ b/chrome/common/extensions/extension_l10n_util.cc @@ -113,7 +113,7 @@ bool LocalizeManifest(const ExtensionMessageBundle& messages, return true; } -bool LocalizeExtension(const FilePath& extension_path, +bool LocalizeExtension(Extension* extension, DictionaryValue* manifest, std::string* error) { DCHECK(manifest); @@ -122,7 +122,7 @@ bool LocalizeExtension(const FilePath& extension_path, scoped_ptr<ExtensionMessageBundle> message_bundle( extension_file_util::LoadExtensionMessageBundle( - extension_path, default_locale, error)); + extension->path(), default_locale, error)); if (!message_bundle.get() && !error->empty()) return false; diff --git a/chrome/common/extensions/extension_l10n_util.h b/chrome/common/extensions/extension_l10n_util.h index e1fdfa0..48dc901 100644 --- a/chrome/common/extensions/extension_l10n_util.h +++ b/chrome/common/extensions/extension_l10n_util.h @@ -44,7 +44,7 @@ bool LocalizeManifest(const ExtensionMessageBundle& messages, // Load message catalogs, localize manifest and attach message bundle to the // extension. -bool LocalizeExtension(const FilePath& extension_path, +bool LocalizeExtension(Extension* extension, DictionaryValue* manifest, std::string* error); diff --git a/chrome/common/extensions/extension_manifests_unittest.cc b/chrome/common/extensions/extension_manifests_unittest.cc index c27e2dd..e800cd9 100644 --- a/chrome/common/extensions/extension_manifests_unittest.cc +++ b/chrome/common/extensions/extension_manifests_unittest.cc @@ -37,48 +37,53 @@ class ExtensionManifestTest : public testing::Test { return static_cast<DictionaryValue*>(serializer.Deserialize(NULL, error)); } - scoped_refptr<Extension> LoadExtensionWithLocation( - DictionaryValue* value, - Extension::Location location, - std::string* error) { + Extension* LoadExtensionWithLocation(DictionaryValue* value, + Extension::Location location, + std::string* error) { FilePath path; PathService::Get(chrome::DIR_TEST_DATA, &path); path = path.AppendASCII("extensions").AppendASCII("manifest_tests"); - return Extension::Create(path.DirName(), location, *value, false, error); + + scoped_ptr<Extension> extension(new Extension(path.DirName())); + extension->set_location(location); + + if (!extension->InitFromValue(*value, false, error)) + return NULL; + + return extension.release(); } - scoped_refptr<Extension> LoadExtension(const std::string& name, - std::string* error) { + Extension* LoadExtension(const std::string& name, + std::string* error) { return LoadExtensionWithLocation(name, Extension::INTERNAL, error); } - scoped_refptr<Extension> LoadExtension(DictionaryValue* value, - std::string* error) { + Extension* LoadExtension(DictionaryValue* value, + std::string* error) { return LoadExtensionWithLocation(value, Extension::INTERNAL, error); } - scoped_refptr<Extension> LoadExtensionWithLocation( - const std::string& name, - Extension::Location location, - std::string* error) { + Extension* LoadExtensionWithLocation(const std::string& name, + Extension::Location location, + std::string* error) { scoped_ptr<DictionaryValue> value(LoadManifestFile(name, error)); if (!value.get()) return NULL; return LoadExtensionWithLocation(value.get(), location, error); } - scoped_refptr<Extension> LoadAndExpectSuccess(const std::string& name) { + Extension* LoadAndExpectSuccess(const std::string& name) { std::string error; - scoped_refptr<Extension> extension = LoadExtension(name, &error); + Extension* extension = LoadExtension(name, &error); EXPECT_TRUE(extension) << name; EXPECT_EQ("", error) << name; return extension; } - scoped_refptr<Extension> LoadAndExpectSuccess(DictionaryValue* manifest, - const std::string& name) { + Extension* LoadAndExpectSuccess(DictionaryValue* manifest, + const std::string& name) { std::string error; - scoped_refptr<Extension> extension = LoadExtension(manifest, &error); + Extension* extension = LoadExtension(manifest, &error); EXPECT_TRUE(extension) << "Unexpected success for " << name; EXPECT_EQ("", error) << "Unexpected no error for " << name; return extension; @@ -98,7 +103,7 @@ class ExtensionManifestTest : public testing::Test { void LoadAndExpectError(const std::string& name, const std::string& expected_error) { std::string error; - scoped_refptr<Extension> extension(LoadExtension(name, &error)); + scoped_ptr<Extension> extension(LoadExtension(name, &error)); VerifyExpectedError(extension.get(), name, error, expected_error); } @@ -106,7 +111,7 @@ class ExtensionManifestTest : public testing::Test { const std::string& name, const std::string& expected_error) { std::string error; - scoped_refptr<Extension> extension(LoadExtension(manifest, &error)); + scoped_ptr<Extension> extension(LoadExtension(manifest, &error)); VerifyExpectedError(extension.get(), name, error, expected_error); } @@ -114,7 +119,7 @@ class ExtensionManifestTest : public testing::Test { }; TEST_F(ExtensionManifestTest, ValidApp) { - scoped_refptr<Extension> extension(LoadAndExpectSuccess("valid_app.json")); + scoped_ptr<Extension> extension(LoadAndExpectSuccess("valid_app.json")); ASSERT_EQ(2u, extension->web_extent().patterns().size()); EXPECT_EQ("http://www.google.com/mail/*", extension->web_extent().patterns()[0].GetAsString()); @@ -140,7 +145,7 @@ TEST_F(ExtensionManifestTest, AppWebUrls) { ExtensionErrorUtils::FormatErrorMessage( errors::kInvalidWebURL, "0")); - scoped_refptr<Extension> extension( + scoped_ptr<Extension> extension( LoadAndExpectSuccess("web_urls_default.json")); ASSERT_EQ(1u, extension->web_extent().patterns().size()); EXPECT_EQ("*://www.google.com/*", @@ -148,21 +153,21 @@ TEST_F(ExtensionManifestTest, AppWebUrls) { } TEST_F(ExtensionManifestTest, AppLaunchContainer) { - scoped_refptr<Extension> extension; + scoped_ptr<Extension> extension; - extension = LoadAndExpectSuccess("launch_tab.json"); + extension.reset(LoadAndExpectSuccess("launch_tab.json")); EXPECT_EQ(extension_misc::LAUNCH_TAB, extension->launch_container()); - extension = LoadAndExpectSuccess("launch_panel.json"); + extension.reset(LoadAndExpectSuccess("launch_panel.json")); EXPECT_EQ(extension_misc::LAUNCH_PANEL, extension->launch_container()); - extension = LoadAndExpectSuccess("launch_default.json"); + extension.reset(LoadAndExpectSuccess("launch_default.json")); EXPECT_EQ(extension_misc::LAUNCH_TAB, extension->launch_container()); - extension = LoadAndExpectSuccess("launch_width.json"); + extension.reset(LoadAndExpectSuccess("launch_width.json")); EXPECT_EQ(640, extension->launch_width()); - extension = LoadAndExpectSuccess("launch_height.json"); + extension.reset(LoadAndExpectSuccess("launch_height.json")); EXPECT_EQ(480, extension->launch_height()); LoadAndExpectError("launch_window.json", @@ -193,15 +198,15 @@ TEST_F(ExtensionManifestTest, AppLaunchURL) { LoadAndExpectError("launch_url_invalid_type.json", errors::kInvalidLaunchWebURL); - scoped_refptr<Extension> extension; - extension = LoadAndExpectSuccess("launch_local_path.json"); + scoped_ptr<Extension> extension; + extension.reset(LoadAndExpectSuccess("launch_local_path.json")); EXPECT_EQ(extension->url().spec() + "launch.html", extension->GetFullLaunchURL().spec()); LoadAndExpectError("launch_web_url_relative.json", errors::kInvalidLaunchWebURL); - extension = LoadAndExpectSuccess("launch_web_url_absolute.json"); + extension.reset(LoadAndExpectSuccess("launch_web_url_absolute.json")); EXPECT_EQ(GURL("http://www.google.com/launch.html"), extension->GetFullLaunchURL()); } @@ -212,13 +217,13 @@ TEST_F(ExtensionManifestTest, Override) { LoadAndExpectError("override_invalid_page.json", errors::kInvalidChromeURLOverrides); - scoped_refptr<Extension> extension; + scoped_ptr<Extension> extension; - extension = LoadAndExpectSuccess("override_new_tab.json"); + extension.reset(LoadAndExpectSuccess("override_new_tab.json")); EXPECT_EQ(extension->url().spec() + "newtab.html", extension->GetChromeURLOverrides().find("newtab")->second.spec()); - extension = LoadAndExpectSuccess("override_history.json"); + extension.reset(LoadAndExpectSuccess("override_history.json")); EXPECT_EQ(extension->url().spec() + "history.html", extension->GetChromeURLOverrides().find("history")->second.spec()); } @@ -232,11 +237,11 @@ TEST_F(ExtensionManifestTest, ChromeResourcesPermissionValidOnlyForComponents) { LoadAndExpectError("permission_chrome_resources_url.json", errors::kInvalidPermissionScheme); std::string error; - scoped_refptr<Extension> extension; - extension = LoadExtensionWithLocation( + scoped_ptr<Extension> extension; + extension.reset(LoadExtensionWithLocation( "permission_chrome_resources_url.json", Extension::COMPONENT, - &error); + &error)); EXPECT_EQ("", error); } @@ -255,8 +260,8 @@ TEST_F(ExtensionManifestTest, DevToolsExtensions) { CommandLine::ForCurrentProcess()->AppendSwitch( switches::kEnableExperimentalExtensionApis); - scoped_refptr<Extension> extension; - extension = LoadAndExpectSuccess("devtools_extension.json"); + scoped_ptr<Extension> extension; + extension.reset(LoadAndExpectSuccess("devtools_extension.json")); EXPECT_EQ(extension->url().spec() + "devtools.html", extension->devtools_url().spec()); *CommandLine::ForCurrentProcess() = old_command_line; @@ -270,10 +275,11 @@ TEST_F(ExtensionManifestTest, DisallowHybridApps) { } TEST_F(ExtensionManifestTest, OptionsPageInApps) { - scoped_refptr<Extension> extension; + scoped_ptr<Extension> extension; // Allow options page with absolute URL in hosed apps. - extension = LoadAndExpectSuccess("hosted_app_absolute_options.json"); + extension.reset( + LoadAndExpectSuccess("hosted_app_absolute_options.json")); EXPECT_STREQ("http", extension->options_url().scheme().c_str()); EXPECT_STREQ("example.com", @@ -309,8 +315,8 @@ TEST_F(ExtensionManifestTest, DisallowExtensionPermissions) { permissions->Append(p); std::string message_name = StringPrintf("permission-%s", name); if (Extension::IsHostedAppPermission(name)) { - scoped_refptr<Extension> extension; - extension = LoadAndExpectSuccess(manifest.get(), message_name); + scoped_ptr<Extension> extension; + extension.reset(LoadAndExpectSuccess(manifest.get(), message_name)); } else { LoadAndExpectError(manifest.get(), message_name, errors::kInvalidPermission); @@ -319,7 +325,7 @@ TEST_F(ExtensionManifestTest, DisallowExtensionPermissions) { } TEST_F(ExtensionManifestTest, NormalizeIconPaths) { - scoped_refptr<Extension> extension( + scoped_ptr<Extension> extension( LoadAndExpectSuccess("normalize_icon_paths.json")); EXPECT_EQ("16.png", extension->icons().Get(16, ExtensionIconSet::MATCH_EXACTLY)); @@ -334,8 +340,7 @@ TEST_F(ExtensionManifestTest, DisallowMultipleUISurfaces) { } TEST_F(ExtensionManifestTest, ParseHomepageURLs) { - scoped_refptr<Extension> extension( - LoadAndExpectSuccess("homepage_valid.json")); + scoped_ptr<Extension> extension(LoadAndExpectSuccess("homepage_valid.json")); LoadAndExpectError("homepage_empty.json", extension_manifest_errors::kInvalidHomepageURL); LoadAndExpectError("homepage_invalid.json", @@ -343,23 +348,22 @@ TEST_F(ExtensionManifestTest, ParseHomepageURLs) { } TEST_F(ExtensionManifestTest, GetHomepageURL) { - scoped_refptr<Extension> extension( - LoadAndExpectSuccess("homepage_valid.json")); + scoped_ptr<Extension> extension(LoadAndExpectSuccess("homepage_valid.json")); EXPECT_EQ(GURL("http://foo.com#bar"), extension->GetHomepageURL()); // The Google Gallery URL ends with the id, which depends on the path, which // can be different in testing, so we just check the part before id. - extension = LoadAndExpectSuccess("homepage_google_hosted.json"); + extension.reset(LoadAndExpectSuccess("homepage_google_hosted.json")); EXPECT_TRUE(StartsWithASCII(extension->GetHomepageURL().spec(), "https://chrome.google.com/extensions/detail/", false)); - extension = LoadAndExpectSuccess("homepage_externally_hosted.json"); + extension.reset(LoadAndExpectSuccess("homepage_externally_hosted.json")); EXPECT_EQ(GURL(), extension->GetHomepageURL()); } TEST_F(ExtensionManifestTest, DefaultPathForExtent) { - scoped_refptr<Extension> extension( + scoped_ptr<Extension> extension( LoadAndExpectSuccess("default_path_for_extent.json")); ASSERT_EQ(1u, extension->web_extent().patterns().size()); diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc index 3bad1dc..d2a7049 100644 --- a/chrome/common/extensions/extension_unittest.cc +++ b/chrome/common/extensions/extension_unittest.cc @@ -77,9 +77,7 @@ TEST(ExtensionTest, InitFromValueInvalid) { #elif defined(OS_POSIX) FilePath path(FILE_PATH_LITERAL("/foo")); #endif - scoped_refptr<Extension> extension_ptr(new Extension(path, - Extension::INVALID)); - Extension& extension = *extension_ptr; + Extension extension(path); int error_code = 0; std::string error; @@ -305,9 +303,7 @@ TEST(ExtensionTest, InitFromValueValid) { #elif defined(OS_POSIX) FilePath path(FILE_PATH_LITERAL("/foo")); #endif - scoped_refptr<Extension> extension_ptr(new Extension(path, - Extension::INVALID)); - Extension& extension = *extension_ptr; + Extension extension(path); std::string error; DictionaryValue input_value; @@ -370,9 +366,7 @@ TEST(ExtensionTest, InitFromValueValidNameInRTL) { #elif defined(OS_POSIX) FilePath path(FILE_PATH_LITERAL("/foo")); #endif - scoped_refptr<Extension> extension_ptr(new Extension(path, - Extension::INVALID)); - Extension& extension = *extension_ptr; + Extension extension(path); std::string error; DictionaryValue input_value; @@ -409,20 +403,18 @@ TEST(ExtensionTest, GetResourceURLAndPath) { #elif defined(OS_POSIX) FilePath path(FILE_PATH_LITERAL("/foo")); #endif + Extension extension(path); DictionaryValue input_value; input_value.SetString(keys::kVersion, "1.0.0.0"); input_value.SetString(keys::kName, "my extension"); - scoped_refptr<Extension> extension(Extension::Create( - path, Extension::INVALID, input_value, false, NULL)); - EXPECT_TRUE(extension.get()); - - EXPECT_EQ(extension->url().spec() + "bar/baz.js", - Extension::GetResourceURL(extension->url(), "bar/baz.js").spec()); - EXPECT_EQ(extension->url().spec() + "baz.js", - Extension::GetResourceURL(extension->url(), - "bar/../baz.js").spec()); - EXPECT_EQ(extension->url().spec() + "baz.js", - Extension::GetResourceURL(extension->url(), "../baz.js").spec()); + EXPECT_TRUE(extension.InitFromValue(input_value, false, NULL)); + + EXPECT_EQ(extension.url().spec() + "bar/baz.js", + Extension::GetResourceURL(extension.url(), "bar/baz.js").spec()); + EXPECT_EQ(extension.url().spec() + "baz.js", + Extension::GetResourceURL(extension.url(), "bar/../baz.js").spec()); + EXPECT_EQ(extension.url().spec() + "baz.js", + Extension::GetResourceURL(extension.url(), "../baz.js").spec()); } TEST(ExtensionTest, LoadPageActionHelper) { @@ -431,9 +423,7 @@ TEST(ExtensionTest, LoadPageActionHelper) { #else FilePath path(StringPrintf("/extension")); #endif - scoped_refptr<Extension> extension_ptr(new Extension(path, - Extension::INVALID)); - Extension& extension = *extension_ptr; + Extension extension(path); std::string error_msg; scoped_ptr<ExtensionAction> action; DictionaryValue input; @@ -665,15 +655,14 @@ TEST(ExtensionTest, UpdateUrls) { #else FilePath path(StringPrintf("/extension%" PRIuS, i)); #endif + Extension extension(path); std::string error; input_value.SetString(keys::kVersion, "1.0"); input_value.SetString(keys::kName, "Test"); input_value.SetString(keys::kUpdateURL, url.spec()); - scoped_refptr<Extension> extension(Extension::Create( - path, Extension::INVALID, input_value, false, &error)); - EXPECT_TRUE(extension.get()) << error; + EXPECT_TRUE(extension.InitFromValue(input_value, false, &error)); } // Test some invalid update urls @@ -690,14 +679,13 @@ TEST(ExtensionTest, UpdateUrls) { #else FilePath path(StringPrintf("/extension%" PRIuS, i)); #endif + Extension extension(path); std::string error; input_value.SetString(keys::kVersion, "1.0"); input_value.SetString(keys::kName, "Test"); input_value.SetString(keys::kUpdateURL, invalid[i]); - scoped_refptr<Extension> extension(Extension::Create( - path, Extension::INVALID, input_value, false, &error)); - EXPECT_FALSE(extension.get()); + EXPECT_FALSE(extension.InitFromValue(input_value, false, &error)); EXPECT_TRUE(MatchPattern(error, errors::kInvalidUpdateURL)); } } @@ -726,8 +714,8 @@ TEST(ExtensionTest, MimeTypeSniffing) { EXPECT_EQ("application/octet-stream", result); } -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") @@ -742,70 +730,74 @@ static scoped_refptr<Extension> LoadManifest(const std::string& dir, return NULL; } - scoped_refptr<Extension> extension = Extension::Create( - path.DirName(), Extension::INVALID, - *static_cast<DictionaryValue*>(result.get()), false, &error); - EXPECT_TRUE(extension) << error; - return extension; + scoped_ptr<Extension> extension(new Extension(path.DirName())); + EXPECT_TRUE(extension->InitFromValue( + *static_cast<DictionaryValue*>(result.get()), false, &error)) << error; + + return extension.release(); } TEST(ExtensionTest, EffectiveHostPermissions) { - scoped_refptr<Extension> extension; + scoped_ptr<Extension> extension; ExtensionExtent hosts; - extension = LoadManifest("effective_host_permissions", "empty.json"); + extension.reset(LoadManifest("effective_host_permissions", "empty.json")); EXPECT_EQ(0u, extension->GetEffectiveHostPermissions().patterns().size()); EXPECT_FALSE(hosts.ContainsURL(GURL("http://www.google.com"))); EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts()); - extension = LoadManifest("effective_host_permissions", "one_host.json"); + extension.reset(LoadManifest("effective_host_permissions", "one_host.json")); hosts = extension->GetEffectiveHostPermissions(); EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com"))); EXPECT_FALSE(hosts.ContainsURL(GURL("https://www.google.com"))); EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts()); - extension = LoadManifest("effective_host_permissions", - "one_host_wildcard.json"); + extension.reset(LoadManifest("effective_host_permissions", + "one_host_wildcard.json")); hosts = extension->GetEffectiveHostPermissions(); EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com"))); EXPECT_TRUE(hosts.ContainsURL(GURL("http://foo.google.com"))); EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts()); - extension = LoadManifest("effective_host_permissions", "two_hosts.json"); + extension.reset(LoadManifest("effective_host_permissions", + "two_hosts.json")); hosts = extension->GetEffectiveHostPermissions(); EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com"))); EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.reddit.com"))); EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts()); - extension = LoadManifest("effective_host_permissions", - "https_not_considered.json"); + extension.reset(LoadManifest("effective_host_permissions", + "https_not_considered.json")); hosts = extension->GetEffectiveHostPermissions(); EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com"))); EXPECT_TRUE(hosts.ContainsURL(GURL("https://google.com"))); EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts()); - extension = LoadManifest("effective_host_permissions", - "two_content_scripts.json"); + extension.reset(LoadManifest("effective_host_permissions", + "two_content_scripts.json")); hosts = extension->GetEffectiveHostPermissions(); EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com"))); EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.reddit.com"))); EXPECT_TRUE(hosts.ContainsURL(GURL("http://news.ycombinator.com"))); EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts()); - extension = LoadManifest("effective_host_permissions", "all_hosts.json"); + extension.reset(LoadManifest("effective_host_permissions", + "all_hosts.json")); hosts = extension->GetEffectiveHostPermissions(); EXPECT_TRUE(hosts.ContainsURL(GURL("http://test/"))); EXPECT_FALSE(hosts.ContainsURL(GURL("https://test/"))); EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com"))); EXPECT_TRUE(extension->HasEffectiveAccessToAllHosts()); - extension = LoadManifest("effective_host_permissions", "all_hosts2.json"); + extension.reset(LoadManifest("effective_host_permissions", + "all_hosts2.json")); hosts = extension->GetEffectiveHostPermissions(); EXPECT_TRUE(hosts.ContainsURL(GURL("http://test/"))); EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com"))); EXPECT_TRUE(extension->HasEffectiveAccessToAllHosts()); - extension = LoadManifest("effective_host_permissions", "all_hosts3.json"); + extension.reset(LoadManifest("effective_host_permissions", + "all_hosts3.json")); hosts = extension->GetEffectiveHostPermissions(); EXPECT_FALSE(hosts.ContainsURL(GURL("http://test/"))); EXPECT_TRUE(hosts.ContainsURL(GURL("https://test/"))); @@ -842,10 +834,10 @@ TEST(ExtensionTest, IsPrivilegeIncrease) { }; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTests); ++i) { - scoped_refptr<Extension> old_extension( + scoped_ptr<Extension> old_extension( LoadManifest("allow_silent_upgrade", std::string(kTests[i].base_name) + "_old.json")); - scoped_refptr<Extension> new_extension( + scoped_ptr<Extension> new_extension( LoadManifest("allow_silent_upgrade", std::string(kTests[i].base_name) + "_new.json")); @@ -920,12 +912,11 @@ TEST(ExtensionTest, ImageCaching) { // Initialize the Extension. std::string errors; + scoped_ptr<Extension> extension(new Extension(path)); DictionaryValue values; values.SetString(keys::kName, "test"); values.SetString(keys::kVersion, "0.1"); - scoped_refptr<Extension> extension(Extension::Create( - path, Extension::INVALID, values, false, &errors)); - ASSERT_TRUE(extension.get()); + ASSERT_TRUE(extension->InitFromValue(values, false, &errors)); // Create an ExtensionResource pointing at an icon. FilePath icon_relative_path(FILE_PATH_LITERAL("icon3.png")); @@ -1006,11 +997,10 @@ TEST(ExtensionTest, OldUnlimitedStoragePermission) { // Initialize the extension and make sure the permission for unlimited storage // is present. + Extension extension(extension_path); std::string errors; - scoped_refptr<Extension> extension(Extension::Create( - extension_path, Extension::INVALID, dictionary, false, &errors)); - EXPECT_TRUE(extension.get()); - EXPECT_TRUE(extension->HasApiPermission( + EXPECT_TRUE(extension.InitFromValue(dictionary, false, &errors)); + EXPECT_TRUE(extension.HasApiPermission( Extension::kUnlimitedStoragePermission)); } @@ -1046,8 +1036,8 @@ TEST(ExtensionTest, ApiPermissions) { { "tabs.getSelected", false}, }; - scoped_refptr<Extension> extension; - extension = LoadManifest("empty_manifest", "empty.json"); + scoped_ptr<Extension> extension; + extension.reset(LoadManifest("empty_manifest", "empty.json")); for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTests); ++i) { EXPECT_EQ(kTests[i].expect_success, diff --git a/chrome/common/extensions/extension_unpacker.cc b/chrome/common/extensions/extension_unpacker.cc index 3f718c9..d431bcb 100644 --- a/chrome/common/extensions/extension_unpacker.cc +++ b/chrome/common/extensions/extension_unpacker.cc @@ -175,21 +175,20 @@ bool ExtensionUnpacker::Run() { // InitFromValue is allowed to generate a temporary id for the extension. // ANY CODE THAT FOLLOWS SHOULD NOT DEPEND ON THE CORRECT ID OF THIS // EXTENSION. + Extension extension(temp_install_dir_); std::string error; - scoped_refptr<Extension> extension(Extension::Create( - temp_install_dir_, Extension::INVALID, *parsed_manifest_, false, &error)); - if (!extension.get()) { + if (!extension.InitFromValue(*parsed_manifest_, false, &error)) { SetError(error); return false; } - if (!extension_file_util::ValidateExtension(extension.get(), &error)) { + if (!extension_file_util::ValidateExtension(&extension, &error)) { SetError(error); return false; } // Decode any images that the browser needs to display. - std::set<FilePath> image_paths = extension->GetBrowserImages(); + std::set<FilePath> image_paths = extension.GetBrowserImages(); for (std::set<FilePath>::iterator it = image_paths.begin(); it != image_paths.end(); ++it) { if (!AddDecodedImage(*it)) @@ -198,8 +197,8 @@ bool ExtensionUnpacker::Run() { // Parse all message catalogs (if any). parsed_catalogs_.reset(new DictionaryValue); - if (!extension->default_locale().empty()) { - if (!ReadAllMessageCatalogs(extension->default_locale())) + if (!extension.default_locale().empty()) { + if (!ReadAllMessageCatalogs(extension.default_locale())) return false; // Error was already reported. } |