diff options
19 files changed, 324 insertions, 508 deletions
diff --git a/chrome/browser/extensions/extension_management_browsertest.cc b/chrome/browser/extensions/extension_management_browsertest.cc index 14c0ac4..93a6ab0 100644 --- a/chrome/browser/extensions/extension_management_browsertest.cc +++ b/chrome/browser/extensions/extension_management_browsertest.cc @@ -347,15 +347,11 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, ExternalUrlUpdate) { const size_t size_before = service->extensions()->size(); ASSERT_TRUE(service->disabled_extensions()->empty()); - PendingExtensionManager* pending_extension_manager = - service->pending_extension_manager(); - // The code that reads external_extensions.json uses this method to inform // the ExtensionService of an extension to download. Using the real code // is race-prone, because instantating the ExtensionService starts a read // of external_extensions.json before this test function starts. - - pending_extension_manager->AddFromExternalUpdateUrl( + service->AddPendingExtensionFromExternalUpdateUrl( kExtensionId, GURL("http://localhost/autoupdate/manifest"), Extension::EXTERNAL_PREF_DOWNLOAD); @@ -378,10 +374,13 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, ExternalUrlUpdate) { // Try to install the extension again from an external source. It should fail // because of the killbit. - pending_extension_manager->AddFromExternalUpdateUrl( + service->AddPendingExtensionFromExternalUpdateUrl( kExtensionId, GURL("http://localhost/autoupdate/manifest"), Extension::EXTERNAL_PREF_DOWNLOAD); - EXPECT_FALSE(pending_extension_manager->IsIdPending(kExtensionId)) + const PendingExtensionMap& pending_extensions = + service->pending_extensions(); + EXPECT_TRUE( + pending_extensions.find(kExtensionId) == pending_extensions.end()) << "External reinstall of a killed extension shouldn't work."; EXPECT_TRUE(extension_prefs->IsExtensionKilled(kExtensionId)) << "External reinstall of a killed extension should leave it killed."; diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc index 1e1a3d6..5619c89 100644 --- a/chrome/browser/extensions/extension_prefs.cc +++ b/chrome/browser/extensions/extension_prefs.cc @@ -717,7 +717,7 @@ void ExtensionPrefs::SetLaunchType(const std::string& extension_id, SavePrefsAndNotify(); } -bool ExtensionPrefs::IsExtensionKilled(const std::string& id) const { +bool ExtensionPrefs::IsExtensionKilled(const std::string& id) { DictionaryValue* extension = GetExtensionPref(id); if (!extension) return false; diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h index 84c52a1..24d4943 100644 --- a/chrome/browser/extensions/extension_prefs.h +++ b/chrome/browser/extensions/extension_prefs.h @@ -77,7 +77,7 @@ class ExtensionPrefs { // Returns true if the specified extension has an entry in prefs // and its killbit is on. - bool IsExtensionKilled(const std::string& id) const; + bool IsExtensionKilled(const std::string& id); // Get the order that toolstrip URLs appear in the shelf. typedef std::vector<GURL> URLList; diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index ed0d286..6349267 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -43,7 +43,6 @@ #include "chrome/browser/extensions/extension_webnavigation_api.h" #include "chrome/browser/extensions/external_extension_provider_impl.h" #include "chrome/browser/extensions/external_extension_provider_interface.h" -#include "chrome/browser/extensions/pending_extension_manager.h" #include "chrome/browser/net/chrome_url_request_context.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" @@ -270,8 +269,7 @@ void ExtensionService::OnExternalExtensionUpdateUrlFound( // Already installed. Do not change the update URL that the extension set. return; } - pending_extension_manager()->AddFromExternalUpdateUrl( - id, update_url, location); + AddPendingExtensionFromExternalUpdateUrl(id, update_url, location); external_extension_url_added_ |= true; } @@ -374,7 +372,6 @@ ExtensionService::ExtensionService(Profile* profile, bool autoupdate_enabled) : profile_(profile), extension_prefs_(extension_prefs), - ALLOW_THIS_IN_INITIALIZER_LIST(pending_extension_manager_(*this)), install_directory_(install_directory), extensions_enabled_(true), show_extensions_prompts_(true), @@ -437,8 +434,8 @@ const ExtensionList* ExtensionService::terminated_extensions() const { return &terminated_extensions_; } -PendingExtensionManager* ExtensionService::pending_extension_manager() { - return &pending_extension_manager_; +const PendingExtensionMap& ExtensionService::pending_extensions() const { + return pending_extensions_; } bool ExtensionService::HasInstalledExtensions() { @@ -475,8 +472,8 @@ void ExtensionService::InitEventRouters() { event_routers_initialized_ = true; } -const Extension* ExtensionService::GetExtensionById( - const std::string& id, bool include_disabled) const { +const Extension* ExtensionService::GetExtensionById(const std::string& id, + bool include_disabled) { return GetExtensionByIdInternal(id, true, include_disabled); } @@ -505,9 +502,8 @@ void ExtensionService::UpdateExtension(const std::string& id, const GURL& download_url) { CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - PendingExtensionInfo pending_extension_info; - bool is_pending_extension = pending_extension_manager_.GetById( - id, &pending_extension_info); + PendingExtensionMap::const_iterator it = pending_extensions_.find(id); + bool is_pending_extension = (it != pending_extensions_.end()); const Extension* extension = GetExtensionByIdInternal(id, true, true); if (!is_pending_extension && !extension) { @@ -525,7 +521,7 @@ void ExtensionService::UpdateExtension(const std::string& id, // We want a silent install only for non-pending extensions and // pending extensions that have install_silently set. ExtensionInstallUI* client = - (!is_pending_extension || pending_extension_info.install_silently()) ? + (!is_pending_extension || it->second.install_silently()) ? NULL : new ExtensionInstallUI(profile_); scoped_refptr<CrxInstaller> installer( @@ -533,7 +529,7 @@ void ExtensionService::UpdateExtension(const std::string& id, client)); installer->set_expected_id(id); if (is_pending_extension) - installer->set_install_source(pending_extension_info.install_source()); + installer->set_install_source(it->second.install_source()); else if (extension) installer->set_install_source(extension->location()); installer->set_delete_source(true); @@ -541,6 +537,114 @@ void ExtensionService::UpdateExtension(const std::string& id, installer->InstallCrx(extension_path); } +void ExtensionService::AddPendingExtensionFromSync( + const std::string& id, const GURL& update_url, + PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install, + bool install_silently, bool enable_on_install, + bool enable_incognito_on_install) { + if (GetExtensionByIdInternal(id, true, true)) { + LOG(DFATAL) << "Trying to add pending extension " << id + << " which already exists"; + return; + } + + AddPendingExtensionInternal(id, update_url, should_allow_install, true, + install_silently, enable_on_install, + enable_incognito_on_install, + Extension::INTERNAL); +} + +namespace { + +bool AlwaysInstall(const Extension& extension) { + return true; +} + +} // namespace + +void ExtensionService::AddPendingExtensionFromExternalUpdateUrl( + const std::string& id, const GURL& update_url, + Extension::Location location) { + const bool kIsFromSync = false; + const bool kInstallSilently = true; + const bool kEnableOnInstall = true; + const bool kEnableIncognitoOnInstall = false; + if (extension_prefs_->IsExtensionKilled(id)) + return; + + if (GetExtensionByIdInternal(id, true, true)) { + LOG(DFATAL) << "Trying to add extension " << id + << " by external update, but it is already installed."; + return; + } + + AddPendingExtensionInternal(id, update_url, &AlwaysInstall, + kIsFromSync, kInstallSilently, + kEnableOnInstall, kEnableIncognitoOnInstall, + location); +} + +namespace { + +bool IsApp(const Extension& extension) { + return extension.is_app(); +} + +} // namespace + +// TODO(akalin): Change DefaultAppList to DefaultExtensionList and +// remove the IsApp() check. + +void ExtensionService::AddPendingExtensionFromDefaultAppList( + const std::string& id) { + const bool kIsFromSync = false; + const bool kInstallSilently = true; + const bool kEnableOnInstall = true; + const bool kEnableIncognitoOnInstall = true; + + // This can legitimately happen if the user manually installed one of the + // default apps before this code ran. + if (GetExtensionByIdInternal(id, true, true)) + return; + + AddPendingExtensionInternal(id, GURL(), &IsApp, + kIsFromSync, kInstallSilently, + kEnableOnInstall, kEnableIncognitoOnInstall, + Extension::INTERNAL); +} + +void ExtensionService::AddPendingExtensionInternal( + const std::string& id, const GURL& update_url, + PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install, + bool is_from_sync, bool install_silently, + bool enable_on_install, bool enable_incognito_on_install, + Extension::Location install_source) { + CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + // If a non-sync update is pending, a sync request should not + // overwrite it. This is important for external extensions. + // If an external extension download is pending, and the user has + // the extension in their sync profile, the install should set the + // type to be external. An external extension should not be + // rejected if it fails the safty checks for a syncable extension. + // TODO(skerner): Work out other potential overlapping conditions. + // (crbug.com/61000) + PendingExtensionMap::iterator it = pending_extensions_.find(id); + if (it != pending_extensions_.end()) { + VLOG(1) << "Extension id " << id + << " was entered for update more than once." + << " old is_from_sync = " << it->second.is_from_sync() + << " new is_from_sync = " << is_from_sync; + if (!it->second.is_from_sync() && is_from_sync) + return; + } + + pending_extensions_[id] = + PendingExtensionInfo(update_url, should_allow_install, + is_from_sync, install_silently, enable_on_install, + enable_incognito_on_install, install_source); +} + void ExtensionService::ReloadExtension(const std::string& extension_id) { CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); FilePath path; @@ -1017,10 +1121,6 @@ ExtensionPrefs* ExtensionService::extension_prefs() { return extension_prefs_; } -const ExtensionPrefs& ExtensionService::const_extension_prefs() const { - return *extension_prefs_; -} - void ExtensionService::CheckAdminBlacklist() { std::vector<std::string> to_be_removed; // Loop through extensions list, unload installed extensions. @@ -1435,11 +1535,12 @@ void ExtensionService::OnExtensionInstalled(const Extension* extension) { scoped_refptr<const Extension> scoped_extension(extension); Extension::State initial_state = Extension::DISABLED; bool initial_enable_incognito = false; - - PendingExtensionInfo pending_extension_info; - if (pending_extension_manager()->GetById(extension->id(), - &pending_extension_info)) { - pending_extension_manager()->Remove(extension->id()); + PendingExtensionMap::iterator it = + pending_extensions_.find(extension->id()); + if (it != pending_extensions_.end()) { + PendingExtensionInfo pending_extension_info = it->second; + pending_extensions_.erase(it); + it = pending_extensions_.end(); if (!pending_extension_info.ShouldAllowInstall(*extension)) { LOG(WARNING) @@ -1451,8 +1552,7 @@ void ExtensionService::OnExtensionInstalled(const Extension* extension) { // load it. BrowserThread::PostTask( BrowserThread::FILE, FROM_HERE, - NewRunnableFunction(&extension_file_util::DeleteFile, - extension->path(), true)); + NewRunnableFunction(&extension_file_util::DeleteFile, extension->path(), true)); return; } @@ -1515,7 +1615,7 @@ void ExtensionService::OnExtensionInstalled(const Extension* extension) { } const Extension* ExtensionService::GetExtensionByIdInternal( - const std::string& id, bool include_enabled, bool include_disabled) const { + const std::string& id, bool include_enabled, bool include_disabled) { std::string lowercase_id = StringToLowerASCII(id); if (include_enabled) { for (ExtensionList::const_iterator iter = extensions_.begin(); @@ -1642,7 +1742,19 @@ void ExtensionService::OnExternalExtensionFileFound( } } - pending_extension_manager()->AddFromExternalFile(id, location); + GURL update_url = GURL(); + bool is_from_sync = false; + bool install_silently = true; + bool enable_on_install = true; + bool enable_incognito_on_install = false; + pending_extensions_[id] = PendingExtensionInfo( + update_url, + &AlwaysInstall, + is_from_sync, + install_silently, + enable_on_install, + enable_incognito_on_install, + location); scoped_refptr<CrxInstaller> installer( new CrxInstaller(this, // frontend diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h index e1d1cbe..a6081d6 100644 --- a/chrome/browser/extensions/extension_service.h +++ b/chrome/browser/extensions/extension_service.h @@ -27,7 +27,7 @@ #include "chrome/browser/extensions/extension_toolbar_model.h" #include "chrome/browser/extensions/extensions_quota_service.h" #include "chrome/browser/extensions/external_extension_provider_interface.h" -#include "chrome/browser/extensions/pending_extension_manager.h" +#include "chrome/browser/extensions/pending_extension_info.h" #include "chrome/browser/extensions/sandboxed_extension_unpacker.h" #include "chrome/browser/prefs/pref_change_registrar.h" #include "chrome/common/extensions/extension.h" @@ -42,7 +42,6 @@ class ExtensionServiceBackend; class ExtensionToolbarModel; class ExtensionUpdater; class GURL; -class PendingExtensionManager; class Profile; class Version; @@ -52,22 +51,17 @@ class ExtensionUpdateService { public: virtual ~ExtensionUpdateService() {} virtual const ExtensionList* extensions() const = 0; - virtual PendingExtensionManager* pending_extension_manager() = 0; - virtual void UpdateExtension(const std::string& id, - const FilePath& path, + virtual const PendingExtensionMap& pending_extensions() const = 0; + virtual void UpdateExtension(const std::string& id, const FilePath& path, const GURL& download_url) = 0; virtual const Extension* GetExtensionById(const std::string& id, - bool include_disabled) const = 0; + bool include_disabled) = 0; virtual void UpdateExtensionBlacklist( const std::vector<std::string>& blacklist) = 0; virtual void CheckAdminBlacklist() = 0; virtual bool HasInstalledExtensions() = 0; - // TODO(skerner): Change to const ExtensionPrefs& extension_prefs() const, - // ExtensionPrefs* mutable_extension_prefs(). virtual ExtensionPrefs* extension_prefs() = 0; - virtual const ExtensionPrefs& const_extension_prefs() const = 0; - virtual Profile* profile() = 0; }; @@ -139,8 +133,8 @@ class ExtensionService virtual const ExtensionList* disabled_extensions() const; virtual const ExtensionList* terminated_extensions() const; - // Gets the object managing the set of pending extensions. - virtual PendingExtensionManager* pending_extension_manager(); + // Gets the set of pending extensions. + virtual const PendingExtensionMap& pending_extensions() const; // Registers an extension to be loaded as a component extension. void register_component_extension(const ComponentExtensionInfo& info) { @@ -192,7 +186,7 @@ class ExtensionService // Look up an extension by ID. virtual const Extension* GetExtensionById(const std::string& id, - bool include_disabled) const; + bool include_disabled); // Looks up a terminated (crashed) extension by ID. GetExtensionById does // not include terminated extensions. @@ -206,6 +200,30 @@ class ExtensionService const FilePath& extension_path, const GURL& download_url); + // Adds an extension in a pending state; the extension with the + // given info will be installed on the next auto-update cycle. + // + // It is an error to call this with an already-installed extension + // (even a disabled one). + // + // TODO(akalin): Replace |install_silently| with a list of + // pre-enabled permissions. + void AddPendingExtensionFromSync( + const std::string& id, const GURL& update_url, + PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install, + bool install_silently, bool enable_on_install, + bool enable_incognito_on_install); + + // Given an extension id and an update URL, schedule the extension + // to be fetched, installed, and activated. + void AddPendingExtensionFromExternalUpdateUrl(const std::string& id, + const GURL& update_url, + Extension::Location location); + + // Like the above. Always installed silently, and defaults update url + // from extension id. + void AddPendingExtensionFromDefaultAppList(const std::string& id); + // Reloads the specified extension. void ReloadExtension(const std::string& extension_id); @@ -338,7 +356,6 @@ class ExtensionService void DestroyingProfile(); virtual ExtensionPrefs* extension_prefs(); - virtual const ExtensionPrefs& const_extension_prefs() const; // Whether the extension service is ready. // TODO(skerner): Get rid of this method. crbug.com/63756 @@ -432,13 +449,22 @@ class ExtensionService // and disabled extensions. const Extension* GetExtensionByIdInternal(const std::string& id, bool include_enabled, - bool include_disabled) const; + bool include_disabled); // Keep track of terminated extensions. void TrackTerminatedExtension(const Extension* extension); void UntrackTerminatedExtension(const std::string& id); + // Like AddPendingExtension*() functions above, but assumes an + // extension with the same id is not already installed. + void AddPendingExtensionInternal( + const std::string& id, const GURL& update_url, + PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install, + bool is_from_sync, bool install_silently, + bool enable_on_install, bool enable_incognito_on_install, + Extension::Location install_source); + // Handles sending notification that |extension| was loaded. void NotifyExtensionLoaded(const Extension* extension); @@ -471,8 +497,8 @@ class ExtensionService // Used to quickly check if an extension was terminated. std::set<std::string> terminated_extension_ids_; - // Hold the set of pending extensions. - PendingExtensionManager pending_extension_manager_; + // The set of pending extensions. + PendingExtensionMap pending_extensions_; // The map of extension IDs to their runtime data. ExtensionRuntimeDataMap extension_runtime_data_; @@ -555,6 +581,8 @@ class ExtensionService bool external_extension_url_added_; FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, + UpdatePendingExtensionAlreadyInstalled); + FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, InstallAppsWithUnlimtedStorage); FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, InstallAppsAndCheckStorageProtection); diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc index 02f8e6c..c819d63 100644 --- a/chrome/browser/extensions/extension_service_unittest.cc +++ b/chrome/browser/extensions/extension_service_unittest.cc @@ -553,8 +553,7 @@ class ExtensionServiceTest // method directly. Instead, use InstallExtension(), which waits for // the crx to be installed and does extra error checking. void StartCrxInstall(const FilePath& crx_path) { - ASSERT_TRUE(file_util::PathExists(crx_path)) - << "Path does not exist: "<< crx_path.value().c_str(); + ASSERT_TRUE(file_util::PathExists(crx_path)); scoped_refptr<CrxInstaller> installer( new CrxInstaller(service_, // frontend NULL)); // no client (silent install) @@ -563,6 +562,7 @@ class ExtensionServiceTest void InstallExtension(const FilePath& path, bool should_succeed) { + ASSERT_TRUE(file_util::PathExists(path)); StartCrxInstall(path); loop_.RunAllPending(); std::vector<std::string> errors = GetErrors(); @@ -1148,12 +1148,13 @@ TEST_F(ExtensionServiceTest, KilledExtensions) { ValidateIntegerPref(good_crx, "location", Extension::KILLBIT); // Try adding the same extension from an external update URL. - service_->pending_extension_manager()->AddFromExternalUpdateUrl( + service_->AddPendingExtensionFromExternalUpdateUrl( good_crx, GURL("http:://fake.update/url"), Extension::EXTERNAL_PREF_DOWNLOAD); - - ASSERT_FALSE(service_->pending_extension_manager()->IsIdPending(good_crx)); + const PendingExtensionMap& pending_extensions = + service_->pending_extensions(); + ASSERT_TRUE(pending_extensions.find(good_crx) == pending_extensions.end()); } // Test that external extensions with incorrect IDs are not installed. @@ -2118,16 +2119,15 @@ TEST_F(ExtensionServiceTest, AddPendingExtensionFromSync) { const Extension::State kFakeInitialState(Extension::ENABLED); const bool kFakeInitialIncognitoEnabled(false); - service_->pending_extension_manager()->AddFromSync( + service_->AddPendingExtensionFromSync( kFakeId, kFakeUpdateURL, &IsExtension, kFakeInstallSilently, kFakeInitialState, kFakeInitialIncognitoEnabled); - - PendingExtensionInfo pending_extension_info; - ASSERT_TRUE(service_->pending_extension_manager()->GetById( - kFakeId, &pending_extension_info)); - EXPECT_EQ(kFakeUpdateURL, pending_extension_info.update_url()); - EXPECT_EQ(&IsExtension, pending_extension_info.should_allow_install_); - EXPECT_EQ(kFakeInstallSilently, pending_extension_info.install_silently()); + PendingExtensionMap::const_iterator it = + service_->pending_extensions().find(kFakeId); + ASSERT_TRUE(it != service_->pending_extensions().end()); + EXPECT_EQ(kFakeUpdateURL, it->second.update_url()); + EXPECT_EQ(&IsExtension, it->second.should_allow_install_); + EXPECT_EQ(kFakeInstallSilently, it->second.install_silently()); } namespace { @@ -2142,11 +2142,11 @@ const bool kGoodInitialIncognitoEnabled = true; // Test updating a pending extension. TEST_F(ExtensionServiceTest, UpdatePendingExtension) { InitializeEmptyExtensionService(); - service_->pending_extension_manager()->AddFromSync( + service_->AddPendingExtensionFromSync( kGoodId, GURL(kGoodUpdateURL), &IsExtension, kGoodInstallSilently, kGoodInitialState, kGoodInitialIncognitoEnabled); - EXPECT_TRUE(service_->pending_extension_manager()->IsIdPending(kGoodId)); + EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId)); FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); @@ -2154,7 +2154,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingExtension) { FilePath path = extensions_path.AppendASCII("good.crx"); UpdateExtension(kGoodId, path, INSTALLED); - EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(kGoodId)); + EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId)); const Extension* extension = service_->GetExtensionById(kGoodId, true); ASSERT_TRUE(extension); @@ -2178,10 +2178,10 @@ bool IsTheme(const Extension& extension) { // Test updating a pending theme. TEST_F(ExtensionServiceTest, UpdatePendingTheme) { InitializeEmptyExtensionService(); - service_->pending_extension_manager()->AddFromSync( + service_->AddPendingExtensionFromSync( theme_crx, GURL(), &IsTheme, false, Extension::ENABLED, false); - EXPECT_TRUE(service_->pending_extension_manager()->IsIdPending(theme_crx)); + EXPECT_TRUE(ContainsKey(service_->pending_extensions(), theme_crx)); FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); @@ -2189,7 +2189,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingTheme) { FilePath path = extensions_path.AppendASCII("theme.crx"); UpdateExtension(theme_crx, path, ENABLED); - EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(theme_crx)); + EXPECT_FALSE(ContainsKey(service_->pending_extensions(), theme_crx)); const Extension* extension = service_->GetExtensionById(theme_crx, true); ASSERT_TRUE(extension); @@ -2204,10 +2204,10 @@ TEST_F(ExtensionServiceTest, UpdatePendingTheme) { // or not. TEST_F(ExtensionServiceTest, UpdatePendingExternalCrx) { InitializeEmptyExtensionService(); - service_->pending_extension_manager()->AddFromExternalUpdateUrl( + service_->AddPendingExtensionFromExternalUpdateUrl( theme_crx, GURL(), Extension::EXTERNAL_PREF_DOWNLOAD); - EXPECT_TRUE(service_->pending_extension_manager()->IsIdPending(theme_crx)); + EXPECT_TRUE(ContainsKey(service_->pending_extensions(), theme_crx)); FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); @@ -2215,7 +2215,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingExternalCrx) { FilePath path = extensions_path.AppendASCII("theme.crx"); UpdateExtension(theme_crx, path, ENABLED); - EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(theme_crx)); + EXPECT_FALSE(ContainsKey(service_->pending_extensions(), theme_crx)); const Extension* extension = service_->GetExtensionById(theme_crx, true); ASSERT_TRUE(extension); @@ -2232,51 +2232,49 @@ TEST_F(ExtensionServiceTest, UpdatePendingExternalCrxWinsOverSync) { InitializeEmptyExtensionService(); // Add a crx to be installed from the update mechanism. - service_->pending_extension_manager()->AddFromSync( + service_->AddPendingExtensionFromSync( kGoodId, GURL(kGoodUpdateURL), &IsExtension, kGoodInstallSilently, kGoodInitialState, kGoodInitialIncognitoEnabled); // Check that there is a pending crx, with is_from_sync set to true. - PendingExtensionInfo pending_extension_info; - ASSERT_TRUE(service_->pending_extension_manager()->GetById( - kGoodId, &pending_extension_info)); - EXPECT_TRUE(pending_extension_info.is_from_sync()); + PendingExtensionMap::const_iterator it; + it = service_->pending_extensions().find(kGoodId); + ASSERT_TRUE(it != service_->pending_extensions().end()); + EXPECT_TRUE(it->second.is_from_sync()); // Add a crx to be updated, with the same ID, from a non-sync source. - service_->pending_extension_manager()->AddFromExternalUpdateUrl( + service_->AddPendingExtensionFromExternalUpdateUrl( kGoodId, GURL(kGoodUpdateURL), Extension::EXTERNAL_PREF_DOWNLOAD); // Check that there is a pending crx, with is_from_sync set to false. - ASSERT_TRUE(service_->pending_extension_manager()->GetById( - kGoodId, &pending_extension_info)); - EXPECT_FALSE(pending_extension_info.is_from_sync()); - EXPECT_EQ(Extension::EXTERNAL_PREF_DOWNLOAD, - pending_extension_info.install_source()); + it = service_->pending_extensions().find(kGoodId); + ASSERT_TRUE(it != service_->pending_extensions().end()); + EXPECT_FALSE(it->second.is_from_sync()); + EXPECT_EQ(Extension::EXTERNAL_PREF_DOWNLOAD, it->second.install_source()); // Add a crx to be installed from the update mechanism. - service_->pending_extension_manager()->AddFromSync( + service_->AddPendingExtensionFromSync( kGoodId, GURL(kGoodUpdateURL), &IsExtension, kGoodInstallSilently, kGoodInitialState, kGoodInitialIncognitoEnabled); // Check that the external, non-sync update was not overridden. - ASSERT_TRUE(service_->pending_extension_manager()->GetById( - kGoodId, &pending_extension_info)); - EXPECT_FALSE(pending_extension_info.is_from_sync()); - EXPECT_EQ(Extension::EXTERNAL_PREF_DOWNLOAD, - pending_extension_info.install_source()); + it = service_->pending_extensions().find(kGoodId); + ASSERT_TRUE(it != service_->pending_extensions().end()); + EXPECT_FALSE(it->second.is_from_sync()); + EXPECT_EQ(Extension::EXTERNAL_PREF_DOWNLOAD, it->second.install_source()); } // Updating a theme should fail if the updater is explicitly told that // the CRX is not a theme. TEST_F(ExtensionServiceTest, UpdatePendingCrxThemeMismatch) { InitializeEmptyExtensionService(); - service_->pending_extension_manager()->AddFromSync( + service_->AddPendingExtensionFromSync( theme_crx, GURL(), &IsExtension, true, Extension::ENABLED, false); - EXPECT_TRUE(service_->pending_extension_manager()->IsIdPending(theme_crx)); + EXPECT_TRUE(ContainsKey(service_->pending_extensions(), theme_crx)); FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); @@ -2284,7 +2282,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingCrxThemeMismatch) { FilePath path = extensions_path.AppendASCII("theme.crx"); UpdateExtension(theme_crx, path, FAILED_SILENTLY); - EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(theme_crx)); + EXPECT_FALSE(ContainsKey(service_->pending_extensions(), theme_crx)); const Extension* extension = service_->GetExtensionById(theme_crx, true); ASSERT_FALSE(extension); @@ -2298,11 +2296,11 @@ TEST_F(ExtensionServiceTest, UpdatePendingCrxThemeMismatch) { TEST_F(ExtensionServiceTest, UpdatePendingExtensionFailedShouldInstallTest) { InitializeEmptyExtensionService(); // Add pending extension with a flipped is_theme. - service_->pending_extension_manager()->AddFromSync( + service_->AddPendingExtensionFromSync( kGoodId, GURL(kGoodUpdateURL), &IsTheme, kGoodInstallSilently, kGoodInitialState, kGoodInitialIncognitoEnabled); - EXPECT_TRUE(service_->pending_extension_manager()->IsIdPending(kGoodId)); + EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId)); FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); @@ -2313,7 +2311,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingExtensionFailedShouldInstallTest) { // TODO(akalin): Figure out how to check that the extensions // directory is cleaned up properly in OnExtensionInstalled(). - EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(kGoodId)); + EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId)); } // TODO(akalin): Figure out how to test that installs of pending @@ -2329,7 +2327,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingExtensionNotPending) { FilePath path = extensions_path.AppendASCII("good.crx"); UpdateExtension(kGoodId, path, UPDATED); - EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(kGoodId)); + EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId)); } // Test updating a pending extension for one that is already @@ -2347,14 +2345,15 @@ TEST_F(ExtensionServiceTest, UpdatePendingExtensionAlreadyInstalled) { EXPECT_FALSE(good->is_theme()); - // Use AddExtensionImpl() as AddFrom*() would balk. - service_->pending_extension_manager()->AddExtensionImpl( + // Use AddPendingExtensionInternal() as AddPendingExtension() would + // balk. + service_->AddPendingExtensionInternal( good->id(), good->update_url(), &IsExtension, kGoodIsFromSync, kGoodInstallSilently, kGoodInitialState, kGoodInitialIncognitoEnabled, Extension::INTERNAL); UpdateExtension(good->id(), path, INSTALLED); - EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(kGoodId)); + EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId)); } // Test pref settings for blacklist and unblacklist extensions. diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc index 98e4fd6..ab01886 100644 --- a/chrome/browser/extensions/extension_updater.cc +++ b/chrome/browser/extensions/extension_updater.cc @@ -847,14 +847,10 @@ void ExtensionUpdater::CheckNow() { fetches_builder.AddExtension(**iter); } - const PendingExtensionManager* pending_extension_manager = - service_->pending_extension_manager(); - - PendingExtensionManager::const_iterator iter; - for (iter = pending_extension_manager->begin(); - iter != pending_extension_manager->end(); iter++) { - // TODO(skerner): Move the determination of what gets fetched into - // class PendingExtensionManager. + const PendingExtensionMap& pending_extensions = + service_->pending_extensions(); + for (PendingExtensionMap::const_iterator iter = pending_extensions.begin(); + iter != pending_extensions.end(); ++iter) { Extension::Location location = iter->second.install_source(); if (location != Extension::EXTERNAL_PREF && location != Extension::EXTERNAL_REGISTRY) @@ -919,8 +915,6 @@ std::vector<int> ExtensionUpdater::DetermineUpdates( // This will only get set if one of possible_updates specifies // browser_min_version. scoped_ptr<Version> browser_version; - PendingExtensionManager* pending_extension_manager = - service_->pending_extension_manager(); for (size_t i = 0; i < possible_updates.list.size(); i++) { const UpdateManifest::Result* update = &possible_updates.list[i]; @@ -928,7 +922,8 @@ std::vector<int> ExtensionUpdater::DetermineUpdates( if (!fetch_data.Includes(update->extension_id)) continue; - if (!pending_extension_manager->IsIdPending(update->extension_id)) { + if (service_->pending_extensions().find(update->extension_id) == + service_->pending_extensions().end()) { // If we're not installing pending extension, and the update // version is the same or older than what's already installed, // we don't want it. @@ -964,8 +959,9 @@ std::vector<int> ExtensionUpdater::DetermineUpdates( // TODO(asargent) - We may want this to show up in the extensions UI // eventually. (http://crbug.com/12547). LOG(WARNING) << "Updated version of extension " << update->extension_id - << " available, but requires chrome version " - << update->browser_min_version; + << " available, but requires chrome version " + << update->browser_min_version; + continue; } } diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc index 4c08abb..bc98520 100644 --- a/chrome/browser/extensions/extension_updater_unittest.cc +++ b/chrome/browser/extensions/extension_updater_unittest.cc @@ -50,8 +50,7 @@ const ManifestFetchData::PingData kNeverPingedData( // Base class for further specialized test classes. class MockService : public ExtensionUpdateService { public: - MockService() - : pending_extension_manager_(ALLOW_THIS_IN_INITIALIZER_LIST(*this)) {} + MockService() {} virtual ~MockService() {} virtual const ExtensionList* extensions() const { @@ -59,20 +58,18 @@ class MockService : public ExtensionUpdateService { return NULL; } - virtual PendingExtensionManager* pending_extension_manager() { - ADD_FAILURE() << "Subclass should override this if it will " - << "be accessed by a test."; - return &pending_extension_manager_; + virtual const PendingExtensionMap& pending_extensions() const { + ADD_FAILURE(); + return pending_extensions_; } virtual void UpdateExtension(const std::string& id, - const FilePath& path, + const FilePath& extension_path, const GURL& download_url) { FAIL(); } - virtual const Extension* GetExtensionById(const std::string& id, - bool include_disabled) const { + virtual const Extension* GetExtensionById(const std::string& id, bool) { ADD_FAILURE(); return NULL; } @@ -92,9 +89,6 @@ class MockService : public ExtensionUpdateService { } virtual ExtensionPrefs* extension_prefs() { return prefs_.prefs(); } - virtual const ExtensionPrefs& const_extension_prefs() const { - return prefs_.const_prefs(); - } virtual Profile* profile() { return &profile_; } @@ -124,7 +118,7 @@ class MockService : public ExtensionUpdateService { } protected: - PendingExtensionManager pending_extension_manager_; + PendingExtensionMap pending_extensions_; TestExtensionPrefs prefs_; TestingProfile profile_; @@ -151,11 +145,10 @@ bool ShouldAlwaysInstall(const Extension& extension) { return true; } -// Loads some pending extension records into a pending extension manager. -void SetupPendingExtensionManagerForTest( - int count, - const GURL& update_url, - PendingExtensionManager* pending_extension_manager) { +// Creates test pending extensions and inserts them into list. The +// name and version are all based on their index. +void CreateTestPendingExtensions(int count, const GURL& update_url, + PendingExtensionMap* pending_extensions) { for (int i = 1; i <= count; i++) { PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install = (i % 2 == 0) ? &ShouldInstallThemesOnly : &ShouldInstallExtensionsOnly; @@ -164,16 +157,10 @@ void SetupPendingExtensionManagerForTest( const Extension::State kInitialState = Extension::ENABLED; const bool kInitialIncognitoEnabled = false; std::string id = GenerateId(base::StringPrintf("extension%i", i)); - - pending_extension_manager->AddForTesting( - id, - PendingExtensionInfo(update_url, - should_allow_install, - kIsFromSync, - kInstallSilently, - kInitialState, - kInitialIncognitoEnabled, - Extension::INTERNAL)); + (*pending_extensions)[id] = + PendingExtensionInfo(update_url, should_allow_install, + kIsFromSync, kInstallSilently, kInitialState, + kInitialIncognitoEnabled, Extension::INTERNAL); } } @@ -183,27 +170,31 @@ class ServiceForManifestTests : public MockService { virtual ~ServiceForManifestTests() {} - virtual const Extension* GetExtensionById(const std::string& id, - bool include_disabled) const { - for (ExtensionList::const_iterator iter = extensions_.begin(); + virtual const Extension* GetExtensionById(const std::string& id, bool) { + for (ExtensionList::iterator iter = extensions_.begin(); iter != extensions_.end(); ++iter) { - if ((*iter)->id() == id) { - return *iter; - } + if ((*iter)->id() == id) { + return *iter; + } } return NULL; } virtual const ExtensionList* extensions() const { return &extensions_; } - virtual PendingExtensionManager* pending_extension_manager() { - return &pending_extension_manager_; + virtual const PendingExtensionMap& pending_extensions() const { + return pending_extensions_; } void set_extensions(ExtensionList extensions) { extensions_ = extensions; } + void set_pending_extensions( + const PendingExtensionMap& pending_extensions) { + pending_extensions_ = pending_extensions; + } + virtual bool HasInstalledExtensions() { return has_installed_extensions_; } @@ -227,19 +218,24 @@ class ServiceForDownloadTests : public MockService { download_url_ = download_url; } - virtual PendingExtensionManager* pending_extension_manager() { - return &pending_extension_manager_; + virtual const PendingExtensionMap& pending_extensions() const { + return pending_extensions_; } - virtual const Extension* GetExtensionById(const std::string& id, bool) const { + virtual const Extension* GetExtensionById(const std::string& id, bool) { last_inquired_extension_id_ = id; return NULL; } - const std::string& extension_id() const { return extension_id_; } - const FilePath& install_path() const { return install_path_; } - const GURL& download_url() const { return download_url_; } - const std::string& last_inquired_extension_id() const { + void set_pending_extensions( + const PendingExtensionMap& pending_extensions) { + pending_extensions_ = pending_extensions; + } + + const std::string& extension_id() { return extension_id_; } + const FilePath& install_path() { return install_path_; } + const GURL& download_url() { return download_url_; } + const std::string& last_inquired_extension_id() { return last_inquired_extension_id_; } @@ -248,11 +244,8 @@ class ServiceForDownloadTests : public MockService { FilePath install_path_; GURL download_url_; - // The last extension ID that GetExtensionById was called with. - // Mutable because the method that sets it (GetExtensionById) is const - // in the actual extension service, but must record the last extension - // ID in this test class. - mutable std::string last_inquired_extension_id_; + // The last extension_id that GetExtensionById was called with. + std::string last_inquired_extension_id_; }; class ServiceForBlacklistTests : public MockService { @@ -331,11 +324,10 @@ class ExtensionUpdaterTest : public testing::Test { ServiceForManifestTests service; std::string update_url("http://foo.com/bar"); ExtensionList extensions; - PendingExtensionManager* pending_extension_manager = - service.pending_extension_manager(); + PendingExtensionMap pending_extensions; if (pending) { - SetupPendingExtensionManagerForTest(1, GURL(update_url), - pending_extension_manager); + CreateTestPendingExtensions(1, GURL(update_url), &pending_extensions); + service.set_pending_extensions(pending_extensions); } else { service.CreateTestExtensions(1, 1, &extensions, &update_url, Extension::INTERNAL); @@ -374,7 +366,7 @@ class ExtensionUpdaterTest : public testing::Test { std::map<std::string, std::string> params; ExtractParameters(decoded, ¶ms); if (pending) { - EXPECT_EQ(pending_extension_manager->begin()->first, params["id"]); + EXPECT_EQ(pending_extensions.begin()->first, params["id"]); EXPECT_EQ("0.0.0.0", params["v"]); } else { EXPECT_EQ(extensions[0]->id(), params["id"]); @@ -546,9 +538,9 @@ class ExtensionUpdaterTest : public testing::Test { static void TestDetermineUpdatesPending() { // Create a set of test extensions ServiceForManifestTests service; - PendingExtensionManager* pending_extension_manager = - service.pending_extension_manager(); - SetupPendingExtensionManagerForTest(3, GURL(), pending_extension_manager); + PendingExtensionMap pending_extensions; + CreateTestPendingExtensions(3, GURL(), &pending_extensions); + service.set_pending_extensions(pending_extensions); MessageLoop message_loop; scoped_refptr<ExtensionUpdater> updater( @@ -558,9 +550,8 @@ class ExtensionUpdaterTest : public testing::Test { ManifestFetchData fetch_data(GURL("http://localhost/foo")); UpdateManifest::Results updates; - PendingExtensionManager::const_iterator it; - for (it = pending_extension_manager->begin(); - it != pending_extension_manager->end(); ++it) { + for (PendingExtensionMap::const_iterator it = pending_extensions.begin(); + it != pending_extensions.end(); ++it) { fetch_data.AddExtension(it->first, "1.0.0.0", kNeverPingedData, kEmptyUpdateUrlData); @@ -678,13 +669,12 @@ class ExtensionUpdaterTest : public testing::Test { const bool kInstallSilently = true; const Extension::State kInitialState = Extension::ENABLED; const bool kInitialIncognitoEnabled = false; - PendingExtensionManager* pending_extension_manager = - service->pending_extension_manager(); - pending_extension_manager->AddForTesting( - id, + PendingExtensionMap pending_extensions; + pending_extensions[id] = PendingExtensionInfo(test_url, &ShouldAlwaysInstall, kIsFromSync, kInstallSilently, kInitialState, - kInitialIncognitoEnabled, Extension::INTERNAL)); + kInitialIncognitoEnabled, Extension::INTERNAL); + service->set_pending_extensions(pending_extensions); } // Call back the ExtensionUpdater with a 200 response and some test data diff --git a/chrome/browser/extensions/pending_extension_info.h b/chrome/browser/extensions/pending_extension_info.h index 0841210..6f74ecf 100644 --- a/chrome/browser/extensions/pending_extension_info.h +++ b/chrome/browser/extensions/pending_extension_info.h @@ -6,6 +6,9 @@ #define CHROME_BROWSER_EXTENSIONS_PENDING_EXTENSION_INFO_H_ #pragma once +#include <map> +#include <string> + #include "chrome/common/extensions/extension.h" class GURL; @@ -14,8 +17,6 @@ class GURL; // and is intended to be installed in the next auto-update cycle. The // update URL of a pending extension may be blank, in which case a // default one is assumed. -// TODO(skerner): Make this class an implementation detail of -// PendingExtensionManager, and remove all other users. class PendingExtensionInfo { public: typedef bool (*ShouldAllowInstallPredicate)(const Extension&); @@ -29,7 +30,7 @@ class PendingExtensionInfo { bool enable_incognito_on_install, Extension::Location install_source); - // Required for STL container membership. Should not be used directly. + // Required for STL container membership. Should not be used. PendingExtensionInfo(); const GURL& update_url() const { return update_url_; } @@ -68,4 +69,8 @@ class PendingExtensionInfo { FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, AddPendingExtensionFromSync); }; +// A PendingExtensionMap is a map from IDs of pending extensions to +// their info. +typedef std::map<std::string, PendingExtensionInfo> PendingExtensionMap; + #endif // CHROME_BROWSER_EXTENSIONS_PENDING_EXTENSION_INFO_H_ diff --git a/chrome/browser/extensions/pending_extension_manager.cc b/chrome/browser/extensions/pending_extension_manager.cc deleted file mode 100644 index 37041d0..0000000 --- a/chrome/browser/extensions/pending_extension_manager.cc +++ /dev/null @@ -1,182 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "base/logging.h" -#include "base/stl_util-inl.h" -#include "chrome/browser/extensions/extension_service.h" -#include "chrome/browser/extensions/pending_extension_manager.h" -#include "content/browser/browser_thread.h" - -namespace { - -// Install predicate used by AddFromDefaultAppList(). -bool IsApp(const Extension& extension) { - return extension.is_app(); -} - -// Install predicate used by AddFromExternalUpdateUrl(). -bool AlwaysInstall(const Extension& extension) { - return true; -} - -} // namespace - -PendingExtensionManager::PendingExtensionManager( - const ExtensionUpdateService& service) - : service_(service) { -} - -bool PendingExtensionManager::GetById( - const std::string& id, - PendingExtensionInfo* out_pending_extension_info) const { - - PendingExtensionMap::const_iterator it = pending_extension_map_.find(id); - if (it != pending_extension_map_.end()) { - *out_pending_extension_info = it->second; - return true; - } - - return false; -} - -void PendingExtensionManager::Remove(const std::string& id) { - pending_extension_map_.erase(id); -} - -bool PendingExtensionManager::IsIdPending(const std::string& id) const { - return ContainsKey(pending_extension_map_, id); -} - -void PendingExtensionManager::AddFromSync( - const std::string& id, - const GURL& update_url, - PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install, - bool install_silently, - bool enable_on_install, - bool enable_incognito_on_install) { - CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - - if (service_.GetExtensionById(id, true)) { - LOG(DFATAL) << "Trying to add pending extension " << id - << " which already exists"; - return; - } - - AddExtensionImpl(id, update_url, should_allow_install, true, - install_silently, enable_on_install, - enable_incognito_on_install, - Extension::INTERNAL); -} - -void PendingExtensionManager::AddFromExternalUpdateUrl( - const std::string& id, const GURL& update_url, - Extension::Location location) { - CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - - const bool kIsFromSync = false; - const bool kInstallSilently = true; - const bool kEnableOnInstall = true; - const bool kEnableIncognitoOnInstall = false; - - if (service_.const_extension_prefs().IsExtensionKilled(id)) - return; - - if (service_.GetExtensionById(id, true)) { - LOG(DFATAL) << "Trying to add extension " << id - << " by external update, but it is already installed."; - return; - } - - AddExtensionImpl(id, update_url, &AlwaysInstall, - kIsFromSync, kInstallSilently, - kEnableOnInstall, kEnableIncognitoOnInstall, - location); -} - - -// TODO(akalin): Change DefaultAppList to DefaultExtensionList and -// remove the IsApp() check. -void PendingExtensionManager::AddFromDefaultAppList( - const std::string& id) { - CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - - const bool kIsFromSync = false; - const bool kInstallSilently = true; - const bool kEnableOnInstall = true; - const bool kEnableIncognitoOnInstall = true; - - // This can legitimately happen if the user manually installed one of the - // default apps before this code ran. - if (service_.GetExtensionById(id, true)) - return; - - AddExtensionImpl(id, GURL(), &IsApp, - kIsFromSync, kInstallSilently, - kEnableOnInstall, kEnableIncognitoOnInstall, - Extension::INTERNAL); -} - -void PendingExtensionManager::AddFromExternalFile( - const std::string& id, - Extension::Location location) { - - GURL kUpdateUrl = GURL(); - bool kIsFromSync = false; - bool kInstallSilently = true; - bool kEnableOnInstall = true; - bool kEnableIncognitoOnInstall = false; - - pending_extension_map_[id] = - PendingExtensionInfo(kUpdateUrl, - &AlwaysInstall, - kIsFromSync, - kInstallSilently, - kEnableOnInstall, - kEnableIncognitoOnInstall, - location); -} - -void PendingExtensionManager::AddExtensionImpl( - const std::string& id, const GURL& update_url, - PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install, - bool is_from_sync, bool install_silently, - bool enable_on_install, bool enable_incognito_on_install, - Extension::Location install_source) { - CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - - // If a non-sync update is pending, a sync request should not - // overwrite it. This is important for external extensions. - // If an external extension download is pending, and the user has - // the extension in their sync profile, the install should set the - // type to be external. An external extension should not be - // rejected if it fails the safty checks for a syncable extension. - // TODO(skerner): Work out other potential overlapping conditions. - // (crbug.com/61000) - - PendingExtensionInfo pending_extension_info; - bool has_pending_ext = GetById(id, &pending_extension_info); - if (has_pending_ext) { - VLOG(1) << "Extension id " << id - << " was entered for update more than once." - << " old is_from_sync = " << pending_extension_info.is_from_sync() - << " new is_from_sync = " << is_from_sync; - if (!pending_extension_info.is_from_sync() && is_from_sync) - return; - } - - pending_extension_map_[id] = - PendingExtensionInfo(update_url, - should_allow_install, - is_from_sync, - install_silently, - enable_on_install, - enable_incognito_on_install, - install_source); -} - -void PendingExtensionManager::AddForTesting( - const std::string& id, - const PendingExtensionInfo& pending_extension_info) { - pending_extension_map_[id] = pending_extension_info; -} diff --git a/chrome/browser/extensions/pending_extension_manager.h b/chrome/browser/extensions/pending_extension_manager.h deleted file mode 100644 index c155ed4..0000000 --- a/chrome/browser/extensions/pending_extension_manager.h +++ /dev/null @@ -1,124 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_EXTENSIONS_PENDING_EXTENSION_MANAGER_H_ -#define CHROME_BROWSER_EXTENSIONS_PENDING_EXTENSION_MANAGER_H_ -#pragma once - -#include <map> -#include <string> - -#include "chrome/browser/extensions/pending_extension_info.h" -#include "chrome/common/extensions/extension.h" - -class ExtensionUpdateService; -class GURL; - -// Class PendingExtensionManager manages the set of extensions which are -// being installed or updated. In general, instalation and updates take -// time, because they involve downloading, unpacking, and installing. -// This class allows us to avoid race cases where multiple sources install -// the same extension. -// The extensions service creates an instance of this class, and manages -// its lifetime. This class should only be used from the UI thread. -class PendingExtensionManager { - public: - // TODO(skerner): Make PendingExtensionMap a private implementation - // detail of this class. - typedef std::map<std::string, PendingExtensionInfo> PendingExtensionMap; - typedef PendingExtensionMap::const_iterator const_iterator; - - // |service| is a reference to the ExtensionService whose pending - // extensions we are managing. The service creates an instance of - // this class on construction, and destroys it on destruction. - // The service remains valid over the entire lifetime of this class. - explicit PendingExtensionManager(const ExtensionUpdateService& service); - - // TODO(skerner): Many of these methods can be private once code in - // ExtensionService is moved into methods of this class. - - // Remove |id| from the set of pending extensions. - void Remove(const std::string& id); - - // Get the information for a pending extension. Returns true and sets - // |out_pending_extension_info| if there is a pending extension with id - // |id|. Returns false otherwise. - bool GetById(const std::string& id, - PendingExtensionInfo* out_pending_extension_info) const; - - // Is |id| in the set of pending extensions? - bool IsIdPending(const std::string& id) const; - - // Iterate over the pending extensions. - const_iterator begin() const { return pending_extension_map_.begin(); } - const_iterator end() const { return pending_extension_map_.end(); } - - // Adds an extension in a pending state; the extension with the - // given info will be installed on the next auto-update cycle. - // - // It is an error to call this with an already-installed extension - // (even a disabled one). - // - // TODO(akalin): Replace |install_silently| with a list of - // pre-enabled permissions. - void AddFromSync( - const std::string& id, - const GURL& update_url, - PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install, - bool install_silently, - bool enable_on_install, - bool enable_incognito_on_install); - - // Given an extension id and an update URL, schedule the extension - // to be fetched, installed, and activated. - void AddFromExternalUpdateUrl(const std::string& id, - const GURL& update_url, - Extension::Location location); - - // Add a default app, using the default update url. - void AddFromDefaultAppList(const std::string& id); - - // Add a pending extension record for an external CRX file. - void AddFromExternalFile( - const std::string& id, - Extension::Location location); - - private: - // Assumes an extension with id |id| is not already installed. - void AddExtensionImpl( - const std::string& id, - const GURL& update_url, - PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install, - bool is_from_sync, - bool install_silently, - bool enable_on_install, - bool enable_incognito_on_install, - Extension::Location install_source); - - // Add a pending extension record directly. Used for unit tests that need - // to set an inital state. Use friendship to allow the tests to call this - // method. - void AddForTesting(const std::string& id, - const PendingExtensionInfo& pending_etension_info); - - // Reference to the extension service whose pending extensions this class is - // managing. Because this class is a member of |service_|, it is created - // and destroyed with |service_|. We only use methods from the interface - // ExtensionUpdateService. - const ExtensionUpdateService& service_; - - // A map from extension id to the pending extension info for that extension. - PendingExtensionMap pending_extension_map_; - - FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, - UpdatePendingExtensionAlreadyInstalled); - friend class ExtensionUpdaterTest; - friend void SetupPendingExtensionManagerForTest( - int count, const GURL& update_url, - PendingExtensionManager* pending_extension_manager); - - DISALLOW_COPY_AND_ASSIGN(PendingExtensionManager); -}; - -#endif // CHROME_BROWSER_EXTENSIONS_PENDING_EXTENSION_MANAGER_H_ diff --git a/chrome/browser/extensions/test_extension_prefs.h b/chrome/browser/extensions/test_extension_prefs.h index af6a522..1fa2351 100644 --- a/chrome/browser/extensions/test_extension_prefs.h +++ b/chrome/browser/extensions/test_extension_prefs.h @@ -25,7 +25,6 @@ class TestExtensionPrefs { virtual ~TestExtensionPrefs(); ExtensionPrefs* prefs() { return prefs_.get(); } - const ExtensionPrefs& const_prefs() const { return *prefs_.get(); } PrefService* pref_service() { return pref_service_.get(); } const FilePath& temp_dir() const { return temp_dir_.path(); } diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc index 12f23d0..cc8eb7d 100644 --- a/chrome/browser/profiles/profile_impl.cc +++ b/chrome/browser/profiles/profile_impl.cc @@ -484,11 +484,9 @@ void ProfileImpl::InstallDefaultApps() { return; const ExtensionIdSet& app_ids = default_apps->default_apps(); - PendingExtensionManager* pending_extension_manager = - extension_service->pending_extension_manager(); for (ExtensionIdSet::const_iterator iter = app_ids.begin(); iter != app_ids.end(); ++iter) { - pending_extension_manager->AddFromDefaultAppList(*iter); + extension_service->AddPendingExtensionFromDefaultAppList(*iter); } } diff --git a/chrome/browser/sync/glue/extension_sync.cc b/chrome/browser/sync/glue/extension_sync.cc index 11decc7..3933515 100644 --- a/chrome/browser/sync/glue/extension_sync.cc +++ b/chrome/browser/sync/glue/extension_sync.cc @@ -291,7 +291,7 @@ void TryUpdateClient( GURL update_url(specifics.update_url()); // TODO(akalin): Replace silent update with a list of enabled // permissions. - extensions_service->pending_extension_manager()->AddFromSync( + extensions_service->AddPendingExtensionFromSync( id, update_url, is_valid_and_syncable, true, // install_silently diff --git a/chrome/browser/sync/glue/theme_util.cc b/chrome/browser/sync/glue/theme_util.cc index 26ffe43..9ec1fc4 100644 --- a/chrome/browser/sync/glue/theme_util.cc +++ b/chrome/browser/sync/glue/theme_util.cc @@ -140,7 +140,7 @@ void SetCurrentThemeFromThemeSpecifics( const bool kInstallSilently = false; const bool kEnableOnInstall = true; const bool kEnableIncognitoOnInstall = false; - extensions_service->pending_extension_manager()->AddFromSync( + extensions_service->AddPendingExtensionFromSync( id, update_url, &IsTheme, kInstallSilently, kEnableOnInstall, kEnableIncognitoOnInstall); ExtensionUpdater* extension_updater = extensions_service->updater(); diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index a3cc6a6..136fd8f 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1058,8 +1058,6 @@ 'browser/extensions/pack_extension_job.h', 'browser/extensions/pending_extension_info.h', 'browser/extensions/pending_extension_info.cc', - 'browser/extensions/pending_extension_manager.h', - 'browser/extensions/pending_extension_manager.cc', 'browser/extensions/sandboxed_extension_unpacker.cc', 'browser/extensions/sandboxed_extension_unpacker.h', 'browser/extensions/theme_installed_infobar_delegate.cc', diff --git a/chrome/test/live_sync/live_extensions_sync_test.cc b/chrome/test/live_sync/live_extensions_sync_test.cc index ab3b692..9b2e64a 100644 --- a/chrome/test/live_sync/live_extensions_sync_test.cc +++ b/chrome/test/live_sync/live_extensions_sync_test.cc @@ -54,11 +54,10 @@ ExtensionStateMap GetExtensionStates(ExtensionService* extensions_service) { extension_states[(*it)->id()] = DISABLED; } - const PendingExtensionManager* pending_extension_manager = - extensions_service->pending_extension_manager(); - PendingExtensionManager::const_iterator it; - for (it = pending_extension_manager->begin(); - it != pending_extension_manager->end(); ++it) { + const PendingExtensionMap& pending_extensions = + extensions_service->pending_extensions(); + for (PendingExtensionMap::const_iterator it = pending_extensions.begin(); + it != pending_extensions.end(); ++it) { extension_states[it->first] = PENDING; } diff --git a/chrome/test/live_sync/live_extensions_sync_test_base.cc b/chrome/test/live_sync/live_extensions_sync_test_base.cc index ced3663..97e245a 100644 --- a/chrome/test/live_sync/live_extensions_sync_test_base.cc +++ b/chrome/test/live_sync/live_extensions_sync_test_base.cc @@ -114,11 +114,10 @@ void LiveExtensionsSyncTestBase::InstallAllPendingExtensions( // We make a copy here since InstallExtension() removes the // extension from the extensions service's copy. - const PendingExtensionManager* pending_extension_manager = - profile->GetExtensionService()->pending_extension_manager(); - PendingExtensionManager::const_iterator it; - for (it = pending_extension_manager->begin(); - it != pending_extension_manager->end(); ++it) { + PendingExtensionMap pending_extensions = + profile->GetExtensionService()->pending_extensions(); + for (PendingExtensionMap::const_iterator it = pending_extensions.begin(); + it != pending_extensions.end(); ++it) { ExtensionIdMap::const_iterator it2 = extensions_by_id_.find(it->first); CHECK(it2 != extensions_by_id_.end()); InstallExtension(profile, it2->second); diff --git a/chrome/test/live_sync/live_themes_sync_test.cc b/chrome/test/live_sync/live_themes_sync_test.cc index 14994a3..0537565 100644 --- a/chrome/test/live_sync/live_themes_sync_test.cc +++ b/chrome/test/live_sync/live_themes_sync_test.cc @@ -59,9 +59,9 @@ bool LiveThemesSyncTest::UsingNativeTheme(Profile* profile) { bool LiveThemesSyncTest::ExtensionIsPendingInstall( Profile* profile, const Extension* extension) { - const PendingExtensionManager* pending_extension_manager = - profile->GetExtensionService()->pending_extension_manager(); - return pending_extension_manager->IsIdPending(extension->id()); + const PendingExtensionMap& pending_extensions = + profile->GetExtensionService()->pending_extensions(); + return pending_extensions.find(extension->id()) != pending_extensions.end(); } bool LiveThemesSyncTest::HasOrWillHaveCustomTheme( |