diff options
19 files changed, 511 insertions, 324 deletions
diff --git a/chrome/browser/extensions/extension_management_browsertest.cc b/chrome/browser/extensions/extension_management_browsertest.cc index 93a6ab0..14c0ac4 100644 --- a/chrome/browser/extensions/extension_management_browsertest.cc +++ b/chrome/browser/extensions/extension_management_browsertest.cc @@ -347,11 +347,15 @@ 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. - service->AddPendingExtensionFromExternalUpdateUrl( + + pending_extension_manager->AddFromExternalUpdateUrl( kExtensionId, GURL("http://localhost/autoupdate/manifest"), Extension::EXTERNAL_PREF_DOWNLOAD); @@ -374,13 +378,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, ExternalUrlUpdate) { // Try to install the extension again from an external source. It should fail // because of the killbit. - service->AddPendingExtensionFromExternalUpdateUrl( + pending_extension_manager->AddFromExternalUpdateUrl( kExtensionId, GURL("http://localhost/autoupdate/manifest"), Extension::EXTERNAL_PREF_DOWNLOAD); - const PendingExtensionMap& pending_extensions = - service->pending_extensions(); - EXPECT_TRUE( - pending_extensions.find(kExtensionId) == pending_extensions.end()) + EXPECT_FALSE(pending_extension_manager->IsIdPending(kExtensionId)) << "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 5619c89..1e1a3d6 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) { +bool ExtensionPrefs::IsExtensionKilled(const std::string& id) const { 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 24d4943..84c52a1 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); + bool IsExtensionKilled(const std::string& id) const; // 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 6349267..ed0d286 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -43,6 +43,7 @@ #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" @@ -269,7 +270,8 @@ void ExtensionService::OnExternalExtensionUpdateUrlFound( // Already installed. Do not change the update URL that the extension set. return; } - AddPendingExtensionFromExternalUpdateUrl(id, update_url, location); + pending_extension_manager()->AddFromExternalUpdateUrl( + id, update_url, location); external_extension_url_added_ |= true; } @@ -372,6 +374,7 @@ 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), @@ -434,8 +437,8 @@ const ExtensionList* ExtensionService::terminated_extensions() const { return &terminated_extensions_; } -const PendingExtensionMap& ExtensionService::pending_extensions() const { - return pending_extensions_; +PendingExtensionManager* ExtensionService::pending_extension_manager() { + return &pending_extension_manager_; } bool ExtensionService::HasInstalledExtensions() { @@ -472,8 +475,8 @@ void ExtensionService::InitEventRouters() { event_routers_initialized_ = true; } -const Extension* ExtensionService::GetExtensionById(const std::string& id, - bool include_disabled) { +const Extension* ExtensionService::GetExtensionById( + const std::string& id, bool include_disabled) const { return GetExtensionByIdInternal(id, true, include_disabled); } @@ -502,8 +505,9 @@ void ExtensionService::UpdateExtension(const std::string& id, const GURL& download_url) { CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - PendingExtensionMap::const_iterator it = pending_extensions_.find(id); - bool is_pending_extension = (it != pending_extensions_.end()); + PendingExtensionInfo pending_extension_info; + bool is_pending_extension = pending_extension_manager_.GetById( + id, &pending_extension_info); const Extension* extension = GetExtensionByIdInternal(id, true, true); if (!is_pending_extension && !extension) { @@ -521,7 +525,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 || it->second.install_silently()) ? + (!is_pending_extension || pending_extension_info.install_silently()) ? NULL : new ExtensionInstallUI(profile_); scoped_refptr<CrxInstaller> installer( @@ -529,7 +533,7 @@ void ExtensionService::UpdateExtension(const std::string& id, client)); installer->set_expected_id(id); if (is_pending_extension) - installer->set_install_source(it->second.install_source()); + installer->set_install_source(pending_extension_info.install_source()); else if (extension) installer->set_install_source(extension->location()); installer->set_delete_source(true); @@ -537,114 +541,6 @@ 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; @@ -1121,6 +1017,10 @@ 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. @@ -1535,12 +1435,11 @@ void ExtensionService::OnExtensionInstalled(const Extension* extension) { scoped_refptr<const Extension> scoped_extension(extension); Extension::State initial_state = Extension::DISABLED; bool initial_enable_incognito = false; - 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(); + + PendingExtensionInfo pending_extension_info; + if (pending_extension_manager()->GetById(extension->id(), + &pending_extension_info)) { + pending_extension_manager()->Remove(extension->id()); if (!pending_extension_info.ShouldAllowInstall(*extension)) { LOG(WARNING) @@ -1552,7 +1451,8 @@ 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; } @@ -1615,7 +1515,7 @@ void ExtensionService::OnExtensionInstalled(const Extension* extension) { } const Extension* ExtensionService::GetExtensionByIdInternal( - const std::string& id, bool include_enabled, bool include_disabled) { + const std::string& id, bool include_enabled, bool include_disabled) const { std::string lowercase_id = StringToLowerASCII(id); if (include_enabled) { for (ExtensionList::const_iterator iter = extensions_.begin(); @@ -1742,19 +1642,7 @@ void ExtensionService::OnExternalExtensionFileFound( } } - 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); + pending_extension_manager()->AddFromExternalFile(id, 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 a6081d6..e1d1cbe 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_info.h" +#include "chrome/browser/extensions/pending_extension_manager.h" #include "chrome/browser/extensions/sandboxed_extension_unpacker.h" #include "chrome/browser/prefs/pref_change_registrar.h" #include "chrome/common/extensions/extension.h" @@ -42,6 +42,7 @@ class ExtensionServiceBackend; class ExtensionToolbarModel; class ExtensionUpdater; class GURL; +class PendingExtensionManager; class Profile; class Version; @@ -51,17 +52,22 @@ class ExtensionUpdateService { public: virtual ~ExtensionUpdateService() {} virtual const ExtensionList* extensions() const = 0; - virtual const PendingExtensionMap& pending_extensions() const = 0; - virtual void UpdateExtension(const std::string& id, const FilePath& path, + virtual PendingExtensionManager* pending_extension_manager() = 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) = 0; + bool include_disabled) const = 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; }; @@ -133,8 +139,8 @@ class ExtensionService virtual const ExtensionList* disabled_extensions() const; virtual const ExtensionList* terminated_extensions() const; - // Gets the set of pending extensions. - virtual const PendingExtensionMap& pending_extensions() const; + // Gets the object managing the set of pending extensions. + virtual PendingExtensionManager* pending_extension_manager(); // Registers an extension to be loaded as a component extension. void register_component_extension(const ComponentExtensionInfo& info) { @@ -186,7 +192,7 @@ class ExtensionService // Look up an extension by ID. virtual const Extension* GetExtensionById(const std::string& id, - bool include_disabled); + bool include_disabled) const; // Looks up a terminated (crashed) extension by ID. GetExtensionById does // not include terminated extensions. @@ -200,30 +206,6 @@ 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); @@ -356,6 +338,7 @@ 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 @@ -449,22 +432,13 @@ class ExtensionService // and disabled extensions. const Extension* GetExtensionByIdInternal(const std::string& id, bool include_enabled, - bool include_disabled); + bool include_disabled) const; // 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); @@ -497,8 +471,8 @@ class ExtensionService // Used to quickly check if an extension was terminated. std::set<std::string> terminated_extension_ids_; - // The set of pending extensions. - PendingExtensionMap pending_extensions_; + // Hold the set of pending extensions. + PendingExtensionManager pending_extension_manager_; // The map of extension IDs to their runtime data. ExtensionRuntimeDataMap extension_runtime_data_; @@ -581,8 +555,6 @@ 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 c819d63..02f8e6c 100644 --- a/chrome/browser/extensions/extension_service_unittest.cc +++ b/chrome/browser/extensions/extension_service_unittest.cc @@ -553,7 +553,8 @@ 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)); + ASSERT_TRUE(file_util::PathExists(crx_path)) + << "Path does not exist: "<< crx_path.value().c_str(); scoped_refptr<CrxInstaller> installer( new CrxInstaller(service_, // frontend NULL)); // no client (silent install) @@ -562,7 +563,6 @@ 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,13 +1148,12 @@ TEST_F(ExtensionServiceTest, KilledExtensions) { ValidateIntegerPref(good_crx, "location", Extension::KILLBIT); // Try adding the same extension from an external update URL. - service_->AddPendingExtensionFromExternalUpdateUrl( + service_->pending_extension_manager()->AddFromExternalUpdateUrl( good_crx, GURL("http:://fake.update/url"), Extension::EXTERNAL_PREF_DOWNLOAD); - const PendingExtensionMap& pending_extensions = - service_->pending_extensions(); - ASSERT_TRUE(pending_extensions.find(good_crx) == pending_extensions.end()); + + ASSERT_FALSE(service_->pending_extension_manager()->IsIdPending(good_crx)); } // Test that external extensions with incorrect IDs are not installed. @@ -2119,15 +2118,16 @@ TEST_F(ExtensionServiceTest, AddPendingExtensionFromSync) { const Extension::State kFakeInitialState(Extension::ENABLED); const bool kFakeInitialIncognitoEnabled(false); - service_->AddPendingExtensionFromSync( + service_->pending_extension_manager()->AddFromSync( kFakeId, kFakeUpdateURL, &IsExtension, kFakeInstallSilently, kFakeInitialState, kFakeInitialIncognitoEnabled); - 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()); + + 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()); } namespace { @@ -2142,11 +2142,11 @@ const bool kGoodInitialIncognitoEnabled = true; // Test updating a pending extension. TEST_F(ExtensionServiceTest, UpdatePendingExtension) { InitializeEmptyExtensionService(); - service_->AddPendingExtensionFromSync( + service_->pending_extension_manager()->AddFromSync( kGoodId, GURL(kGoodUpdateURL), &IsExtension, kGoodInstallSilently, kGoodInitialState, kGoodInitialIncognitoEnabled); - EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId)); + EXPECT_TRUE(service_->pending_extension_manager()->IsIdPending(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(ContainsKey(service_->pending_extensions(), kGoodId)); + EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(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_->AddPendingExtensionFromSync( + service_->pending_extension_manager()->AddFromSync( theme_crx, GURL(), &IsTheme, false, Extension::ENABLED, false); - EXPECT_TRUE(ContainsKey(service_->pending_extensions(), theme_crx)); + EXPECT_TRUE(service_->pending_extension_manager()->IsIdPending(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(ContainsKey(service_->pending_extensions(), theme_crx)); + EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(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_->AddPendingExtensionFromExternalUpdateUrl( + service_->pending_extension_manager()->AddFromExternalUpdateUrl( theme_crx, GURL(), Extension::EXTERNAL_PREF_DOWNLOAD); - EXPECT_TRUE(ContainsKey(service_->pending_extensions(), theme_crx)); + EXPECT_TRUE(service_->pending_extension_manager()->IsIdPending(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(ContainsKey(service_->pending_extensions(), theme_crx)); + EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(theme_crx)); const Extension* extension = service_->GetExtensionById(theme_crx, true); ASSERT_TRUE(extension); @@ -2232,49 +2232,51 @@ TEST_F(ExtensionServiceTest, UpdatePendingExternalCrxWinsOverSync) { InitializeEmptyExtensionService(); // Add a crx to be installed from the update mechanism. - service_->AddPendingExtensionFromSync( + service_->pending_extension_manager()->AddFromSync( kGoodId, GURL(kGoodUpdateURL), &IsExtension, kGoodInstallSilently, kGoodInitialState, kGoodInitialIncognitoEnabled); // Check that there is a pending crx, with is_from_sync set to true. - PendingExtensionMap::const_iterator it; - it = service_->pending_extensions().find(kGoodId); - ASSERT_TRUE(it != service_->pending_extensions().end()); - EXPECT_TRUE(it->second.is_from_sync()); + PendingExtensionInfo pending_extension_info; + ASSERT_TRUE(service_->pending_extension_manager()->GetById( + kGoodId, &pending_extension_info)); + EXPECT_TRUE(pending_extension_info.is_from_sync()); // Add a crx to be updated, with the same ID, from a non-sync source. - service_->AddPendingExtensionFromExternalUpdateUrl( + service_->pending_extension_manager()->AddFromExternalUpdateUrl( kGoodId, GURL(kGoodUpdateURL), Extension::EXTERNAL_PREF_DOWNLOAD); // Check that there is a pending crx, with is_from_sync set to false. - 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()); + 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()); // Add a crx to be installed from the update mechanism. - service_->AddPendingExtensionFromSync( + service_->pending_extension_manager()->AddFromSync( kGoodId, GURL(kGoodUpdateURL), &IsExtension, kGoodInstallSilently, kGoodInitialState, kGoodInitialIncognitoEnabled); // Check that the external, non-sync update was not overridden. - 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()); + 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()); } // Updating a theme should fail if the updater is explicitly told that // the CRX is not a theme. TEST_F(ExtensionServiceTest, UpdatePendingCrxThemeMismatch) { InitializeEmptyExtensionService(); - service_->AddPendingExtensionFromSync( + service_->pending_extension_manager()->AddFromSync( theme_crx, GURL(), &IsExtension, true, Extension::ENABLED, false); - EXPECT_TRUE(ContainsKey(service_->pending_extensions(), theme_crx)); + EXPECT_TRUE(service_->pending_extension_manager()->IsIdPending(theme_crx)); FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); @@ -2282,7 +2284,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingCrxThemeMismatch) { FilePath path = extensions_path.AppendASCII("theme.crx"); UpdateExtension(theme_crx, path, FAILED_SILENTLY); - EXPECT_FALSE(ContainsKey(service_->pending_extensions(), theme_crx)); + EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(theme_crx)); const Extension* extension = service_->GetExtensionById(theme_crx, true); ASSERT_FALSE(extension); @@ -2296,11 +2298,11 @@ TEST_F(ExtensionServiceTest, UpdatePendingCrxThemeMismatch) { TEST_F(ExtensionServiceTest, UpdatePendingExtensionFailedShouldInstallTest) { InitializeEmptyExtensionService(); // Add pending extension with a flipped is_theme. - service_->AddPendingExtensionFromSync( + service_->pending_extension_manager()->AddFromSync( kGoodId, GURL(kGoodUpdateURL), &IsTheme, kGoodInstallSilently, kGoodInitialState, kGoodInitialIncognitoEnabled); - EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId)); + EXPECT_TRUE(service_->pending_extension_manager()->IsIdPending(kGoodId)); FilePath extensions_path; ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path)); @@ -2311,7 +2313,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingExtensionFailedShouldInstallTest) { // TODO(akalin): Figure out how to check that the extensions // directory is cleaned up properly in OnExtensionInstalled(). - EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId)); + EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(kGoodId)); } // TODO(akalin): Figure out how to test that installs of pending @@ -2327,7 +2329,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingExtensionNotPending) { FilePath path = extensions_path.AppendASCII("good.crx"); UpdateExtension(kGoodId, path, UPDATED); - EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId)); + EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(kGoodId)); } // Test updating a pending extension for one that is already @@ -2345,15 +2347,14 @@ TEST_F(ExtensionServiceTest, UpdatePendingExtensionAlreadyInstalled) { EXPECT_FALSE(good->is_theme()); - // Use AddPendingExtensionInternal() as AddPendingExtension() would - // balk. - service_->AddPendingExtensionInternal( + // Use AddExtensionImpl() as AddFrom*() would balk. + service_->pending_extension_manager()->AddExtensionImpl( good->id(), good->update_url(), &IsExtension, kGoodIsFromSync, kGoodInstallSilently, kGoodInitialState, kGoodInitialIncognitoEnabled, Extension::INTERNAL); UpdateExtension(good->id(), path, INSTALLED); - EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId)); + EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(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 ab01886..98e4fd6 100644 --- a/chrome/browser/extensions/extension_updater.cc +++ b/chrome/browser/extensions/extension_updater.cc @@ -847,10 +847,14 @@ void ExtensionUpdater::CheckNow() { fetches_builder.AddExtension(**iter); } - const PendingExtensionMap& pending_extensions = - service_->pending_extensions(); - for (PendingExtensionMap::const_iterator iter = pending_extensions.begin(); - iter != pending_extensions.end(); ++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. Extension::Location location = iter->second.install_source(); if (location != Extension::EXTERNAL_PREF && location != Extension::EXTERNAL_REGISTRY) @@ -915,6 +919,8 @@ 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]; @@ -922,8 +928,7 @@ std::vector<int> ExtensionUpdater::DetermineUpdates( if (!fetch_data.Includes(update->extension_id)) continue; - if (service_->pending_extensions().find(update->extension_id) == - service_->pending_extensions().end()) { + if (!pending_extension_manager->IsIdPending(update->extension_id)) { // 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. @@ -959,9 +964,8 @@ 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 bc98520..4c08abb 100644 --- a/chrome/browser/extensions/extension_updater_unittest.cc +++ b/chrome/browser/extensions/extension_updater_unittest.cc @@ -50,7 +50,8 @@ const ManifestFetchData::PingData kNeverPingedData( // Base class for further specialized test classes. class MockService : public ExtensionUpdateService { public: - MockService() {} + MockService() + : pending_extension_manager_(ALLOW_THIS_IN_INITIALIZER_LIST(*this)) {} virtual ~MockService() {} virtual const ExtensionList* extensions() const { @@ -58,18 +59,20 @@ class MockService : public ExtensionUpdateService { return NULL; } - virtual const PendingExtensionMap& pending_extensions() const { - ADD_FAILURE(); - return pending_extensions_; + virtual PendingExtensionManager* pending_extension_manager() { + ADD_FAILURE() << "Subclass should override this if it will " + << "be accessed by a test."; + return &pending_extension_manager_; } virtual void UpdateExtension(const std::string& id, - const FilePath& extension_path, + const FilePath& path, const GURL& download_url) { FAIL(); } - virtual const Extension* GetExtensionById(const std::string& id, bool) { + virtual const Extension* GetExtensionById(const std::string& id, + bool include_disabled) const { ADD_FAILURE(); return NULL; } @@ -89,6 +92,9 @@ 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_; } @@ -118,7 +124,7 @@ class MockService : public ExtensionUpdateService { } protected: - PendingExtensionMap pending_extensions_; + PendingExtensionManager pending_extension_manager_; TestExtensionPrefs prefs_; TestingProfile profile_; @@ -145,10 +151,11 @@ bool ShouldAlwaysInstall(const Extension& extension) { return true; } -// 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) { +// Loads some pending extension records into a pending extension manager. +void SetupPendingExtensionManagerForTest( + int count, + const GURL& update_url, + PendingExtensionManager* pending_extension_manager) { for (int i = 1; i <= count; i++) { PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install = (i % 2 == 0) ? &ShouldInstallThemesOnly : &ShouldInstallExtensionsOnly; @@ -157,10 +164,16 @@ void CreateTestPendingExtensions(int count, const GURL& update_url, const Extension::State kInitialState = Extension::ENABLED; const bool kInitialIncognitoEnabled = false; std::string id = GenerateId(base::StringPrintf("extension%i", i)); - (*pending_extensions)[id] = - PendingExtensionInfo(update_url, should_allow_install, - kIsFromSync, kInstallSilently, kInitialState, - kInitialIncognitoEnabled, Extension::INTERNAL); + + pending_extension_manager->AddForTesting( + id, + PendingExtensionInfo(update_url, + should_allow_install, + kIsFromSync, + kInstallSilently, + kInitialState, + kInitialIncognitoEnabled, + Extension::INTERNAL)); } } @@ -170,31 +183,27 @@ class ServiceForManifestTests : public MockService { virtual ~ServiceForManifestTests() {} - virtual const Extension* GetExtensionById(const std::string& id, bool) { - for (ExtensionList::iterator iter = extensions_.begin(); + virtual const Extension* GetExtensionById(const std::string& id, + bool include_disabled) const { + for (ExtensionList::const_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 const PendingExtensionMap& pending_extensions() const { - return pending_extensions_; + virtual PendingExtensionManager* pending_extension_manager() { + return &pending_extension_manager_; } 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_; } @@ -218,24 +227,19 @@ class ServiceForDownloadTests : public MockService { download_url_ = download_url; } - virtual const PendingExtensionMap& pending_extensions() const { - return pending_extensions_; + virtual PendingExtensionManager* pending_extension_manager() { + return &pending_extension_manager_; } - virtual const Extension* GetExtensionById(const std::string& id, bool) { + virtual const Extension* GetExtensionById(const std::string& id, bool) const { last_inquired_extension_id_ = id; return NULL; } - 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() { + 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 { return last_inquired_extension_id_; } @@ -244,8 +248,11 @@ class ServiceForDownloadTests : public MockService { FilePath install_path_; GURL download_url_; - // The last extension_id that GetExtensionById was called with. - std::string last_inquired_extension_id_; + // 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_; }; class ServiceForBlacklistTests : public MockService { @@ -324,10 +331,11 @@ class ExtensionUpdaterTest : public testing::Test { ServiceForManifestTests service; std::string update_url("http://foo.com/bar"); ExtensionList extensions; - PendingExtensionMap pending_extensions; + PendingExtensionManager* pending_extension_manager = + service.pending_extension_manager(); if (pending) { - CreateTestPendingExtensions(1, GURL(update_url), &pending_extensions); - service.set_pending_extensions(pending_extensions); + SetupPendingExtensionManagerForTest(1, GURL(update_url), + pending_extension_manager); } else { service.CreateTestExtensions(1, 1, &extensions, &update_url, Extension::INTERNAL); @@ -366,7 +374,7 @@ class ExtensionUpdaterTest : public testing::Test { std::map<std::string, std::string> params; ExtractParameters(decoded, ¶ms); if (pending) { - EXPECT_EQ(pending_extensions.begin()->first, params["id"]); + EXPECT_EQ(pending_extension_manager->begin()->first, params["id"]); EXPECT_EQ("0.0.0.0", params["v"]); } else { EXPECT_EQ(extensions[0]->id(), params["id"]); @@ -538,9 +546,9 @@ class ExtensionUpdaterTest : public testing::Test { static void TestDetermineUpdatesPending() { // Create a set of test extensions ServiceForManifestTests service; - PendingExtensionMap pending_extensions; - CreateTestPendingExtensions(3, GURL(), &pending_extensions); - service.set_pending_extensions(pending_extensions); + PendingExtensionManager* pending_extension_manager = + service.pending_extension_manager(); + SetupPendingExtensionManagerForTest(3, GURL(), pending_extension_manager); MessageLoop message_loop; scoped_refptr<ExtensionUpdater> updater( @@ -550,8 +558,9 @@ class ExtensionUpdaterTest : public testing::Test { ManifestFetchData fetch_data(GURL("http://localhost/foo")); UpdateManifest::Results updates; - for (PendingExtensionMap::const_iterator it = pending_extensions.begin(); - it != pending_extensions.end(); ++it) { + PendingExtensionManager::const_iterator it; + for (it = pending_extension_manager->begin(); + it != pending_extension_manager->end(); ++it) { fetch_data.AddExtension(it->first, "1.0.0.0", kNeverPingedData, kEmptyUpdateUrlData); @@ -669,12 +678,13 @@ class ExtensionUpdaterTest : public testing::Test { const bool kInstallSilently = true; const Extension::State kInitialState = Extension::ENABLED; const bool kInitialIncognitoEnabled = false; - PendingExtensionMap pending_extensions; - pending_extensions[id] = + PendingExtensionManager* pending_extension_manager = + service->pending_extension_manager(); + pending_extension_manager->AddForTesting( + id, PendingExtensionInfo(test_url, &ShouldAlwaysInstall, kIsFromSync, kInstallSilently, kInitialState, - kInitialIncognitoEnabled, Extension::INTERNAL); - service->set_pending_extensions(pending_extensions); + kInitialIncognitoEnabled, Extension::INTERNAL)); } // 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 6f74ecf..0841210 100644 --- a/chrome/browser/extensions/pending_extension_info.h +++ b/chrome/browser/extensions/pending_extension_info.h @@ -6,9 +6,6 @@ #define CHROME_BROWSER_EXTENSIONS_PENDING_EXTENSION_INFO_H_ #pragma once -#include <map> -#include <string> - #include "chrome/common/extensions/extension.h" class GURL; @@ -17,6 +14,8 @@ 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&); @@ -30,7 +29,7 @@ class PendingExtensionInfo { bool enable_incognito_on_install, Extension::Location install_source); - // Required for STL container membership. Should not be used. + // Required for STL container membership. Should not be used directly. PendingExtensionInfo(); const GURL& update_url() const { return update_url_; } @@ -69,8 +68,4 @@ 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 new file mode 100644 index 0000000..17290bf --- /dev/null +++ b/chrome/browser/extensions/pending_extension_manager.cc @@ -0,0 +1,184 @@ +// 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) { +} + +PendingExtensionManager::~PendingExtensionManager() {} + +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 new file mode 100644 index 0000000..e163945 --- /dev/null +++ b/chrome/browser/extensions/pending_extension_manager.h @@ -0,0 +1,125 @@ +// 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); + ~PendingExtensionManager(); + + // 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 1fa2351..af6a522 100644 --- a/chrome/browser/extensions/test_extension_prefs.h +++ b/chrome/browser/extensions/test_extension_prefs.h @@ -25,6 +25,7 @@ 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 cc8eb7d..12f23d0 100644 --- a/chrome/browser/profiles/profile_impl.cc +++ b/chrome/browser/profiles/profile_impl.cc @@ -484,9 +484,11 @@ 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) { - extension_service->AddPendingExtensionFromDefaultAppList(*iter); + pending_extension_manager->AddFromDefaultAppList(*iter); } } diff --git a/chrome/browser/sync/glue/extension_sync.cc b/chrome/browser/sync/glue/extension_sync.cc index 3933515..11decc7 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->AddPendingExtensionFromSync( + extensions_service->pending_extension_manager()->AddFromSync( 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 9ec1fc4..26ffe43 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->AddPendingExtensionFromSync( + extensions_service->pending_extension_manager()->AddFromSync( 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 136fd8f..a3cc6a6 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1058,6 +1058,8 @@ '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 9b2e64a..ab3b692 100644 --- a/chrome/test/live_sync/live_extensions_sync_test.cc +++ b/chrome/test/live_sync/live_extensions_sync_test.cc @@ -54,10 +54,11 @@ ExtensionStateMap GetExtensionStates(ExtensionService* extensions_service) { extension_states[(*it)->id()] = DISABLED; } - const PendingExtensionMap& pending_extensions = - extensions_service->pending_extensions(); - for (PendingExtensionMap::const_iterator it = pending_extensions.begin(); - it != pending_extensions.end(); ++it) { + 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) { 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 97e245a..ced3663 100644 --- a/chrome/test/live_sync/live_extensions_sync_test_base.cc +++ b/chrome/test/live_sync/live_extensions_sync_test_base.cc @@ -114,10 +114,11 @@ void LiveExtensionsSyncTestBase::InstallAllPendingExtensions( // We make a copy here since InstallExtension() removes the // extension from the extensions service's copy. - PendingExtensionMap pending_extensions = - profile->GetExtensionService()->pending_extensions(); - for (PendingExtensionMap::const_iterator it = pending_extensions.begin(); - it != pending_extensions.end(); ++it) { + 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) { 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 0537565..14994a3 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 PendingExtensionMap& pending_extensions = - profile->GetExtensionService()->pending_extensions(); - return pending_extensions.find(extension->id()) != pending_extensions.end(); + const PendingExtensionManager* pending_extension_manager = + profile->GetExtensionService()->pending_extension_manager(); + return pending_extension_manager->IsIdPending(extension->id()); } bool LiveThemesSyncTest::HasOrWillHaveCustomTheme( |