diff options
author | joaodasilva@chromium.org <joaodasilva@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-08 22:10:59 +0000 |
---|---|---|
committer | joaodasilva@chromium.org <joaodasilva@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-08 22:10:59 +0000 |
commit | 3b087116dd1d0df149bc4966bdf328616e024a8f (patch) | |
tree | de69dd5267cb4a03a690c116815c6c63d0837f12 /chrome | |
parent | a20e3871e3a26552d533d318e0d538e81edd03c4 (diff) | |
download | chromium_src-3b087116dd1d0df149bc4966bdf328616e024a8f.zip chromium_src-3b087116dd1d0df149bc4966bdf328616e024a8f.tar.gz chromium_src-3b087116dd1d0df149bc4966bdf328616e024a8f.tar.bz2 |
Retry to download the AppPack when the network changes.
This fixes an issue where the download starts too early and always fails.
BUG=130602
TEST=Bug doesn't trigger anymore
Review URL: https://chromiumcodereview.appspot.com/10532072
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141300 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r-- | chrome/browser/policy/app_pack_updater.cc | 65 | ||||
-rw-r--r-- | chrome/browser/policy/app_pack_updater.h | 18 |
2 files changed, 70 insertions, 13 deletions
diff --git a/chrome/browser/policy/app_pack_updater.cc b/chrome/browser/policy/app_pack_updater.cc index ac66728..6d32cb1 100644 --- a/chrome/browser/policy/app_pack_updater.cc +++ b/chrome/browser/policy/app_pack_updater.cc @@ -100,6 +100,7 @@ AppPackUpdater::AppPackUpdater(net::URLRequestContextGetter* request_context, AppPackUpdater::~AppPackUpdater() { chromeos::CrosSettings::Get()->RemoveSettingsObserver( chromeos::kAppPack, this); + net::NetworkChangeNotifier::RemoveIPAddressObserver(this); } ExternalExtensionLoader* AppPackUpdater::CreateExternalExtensionLoader() { @@ -135,6 +136,7 @@ void AppPackUpdater::Init() { this, chrome::NOTIFICATION_EXTENSION_INSTALL_ERROR, content::NotificationService::AllBrowserContextsAndSources()); + net::NetworkChangeNotifier::AddIPAddressObserver(this); LoadPolicy(); } @@ -166,6 +168,28 @@ void AppPackUpdater::Observe(int type, } } +void AppPackUpdater::OnIPAddressChanged() { + // Check if the AppPack has been fully downloaded whenever the network + // changes. This allows the AppPack to recover in case the network wasn't + // ready early during startup. + // To avoid performing too many update checks in case the network conditions + // change too often, an update is only triggered now if there are extensions + // configured via policy that haven't been checked for updates yet. + for (PolicyEntryMap::iterator it = app_pack_extensions_.begin(); + it != app_pack_extensions_.end(); ++it) { + if (!it->second.update_checked) { + // |id| is configured via policy, but hasn't been updated before. + // Drop any pending requests and start a full check now. + VLOG(1) << "Extension " << it->first << " hasn't been checked yet, " + << "new update triggered now by network change notification."; + downloader_.reset(); + weak_ptr_factory_.InvalidateWeakPtrs(); + LoadPolicy(); + break; + } + } +} + void AppPackUpdater::LoadPolicy() { chromeos::CrosSettings* settings = chromeos::CrosSettings::Get(); if (chromeos::CrosSettingsProvider::TRUSTED != settings->PrepareTrustedValues( @@ -189,7 +213,8 @@ void AppPackUpdater::LoadPolicy() { std::string update_url; if (dict->GetString(kExtensionId, &id) && dict->GetString(kUpdateUrl, &update_url)) { - app_pack_extensions_[id] = update_url; + app_pack_extensions_[id].update_url = update_url; + app_pack_extensions_[id].update_checked = false; } else { LOG(WARNING) << "Failed to read required fields for an AppPack entry, " << "ignoring."; @@ -233,6 +258,7 @@ void AppPackUpdater::BlockingCheckCache( base::Owned(entries))); } +// static void AppPackUpdater::BlockingCheckCacheInternal( const std::set<std::string>* valid_ids, CacheEntryMap* entries) { @@ -334,12 +360,10 @@ void AppPackUpdater::OnCacheUpdated(CacheEntryMap* cache_entries) { cached_extensions_.swap(*cache_entries); CacheEntryMap::iterator it = cached_extensions_.find(screen_saver_id_); - if (it != cached_extensions_.end()) { + if (it != cached_extensions_.end()) SetScreenSaverPath(FilePath(it->second.path)); - cached_extensions_.erase(it); - } else { + else SetScreenSaverPath(FilePath()); - } VLOG(1) << "Updated AppPack cache, there are " << cached_extensions_.size() << " extensions cached and " @@ -361,6 +385,10 @@ void AppPackUpdater::UpdateExtensionLoader() { scoped_ptr<base::DictionaryValue> prefs(new base::DictionaryValue()); for (CacheEntryMap::iterator it = cached_extensions_.begin(); it != cached_extensions_.end(); ++it) { + // The screensaver isn't installed into the Profile. + if (it->first == screen_saver_id_) + continue; + base::DictionaryValue* dict = new base::DictionaryValue(); dict->SetString(ExternalExtensionProviderImpl::kExternalCrx, it->second.path); @@ -384,8 +412,9 @@ void AppPackUpdater::DownloadMissingExtensions() { } for (PolicyEntryMap::iterator it = app_pack_extensions_.begin(); it != app_pack_extensions_.end(); ++it) { - downloader_->AddPendingExtension(it->first, GURL(it->second)); + downloader_->AddPendingExtension(it->first, GURL(it->second.update_url)); } + VLOG(1) << "Downloading AppPack update manifest now"; downloader_->StartAllPending(); } @@ -393,7 +422,11 @@ void AppPackUpdater::OnExtensionDownloadFailed( const std::string& id, extensions::ExtensionDownloaderDelegate::Error error, const extensions::ExtensionDownloaderDelegate::PingResult& ping_result) { - if (error != NO_UPDATE_AVAILABLE) { + if (error == NO_UPDATE_AVAILABLE) { + if (!ContainsKey(cached_extensions_, id)) + LOG(ERROR) << "AppPack extension " << id << " not found on update server"; + SetUpdateChecked(id); + } else { LOG(ERROR) << "AppPack failed to download extension " << id << ", error " << error; } @@ -405,6 +438,10 @@ void AppPackUpdater::OnExtensionDownloadFinished( const GURL& download_url, const std::string& version, const extensions::ExtensionDownloaderDelegate::PingResult& ping_result) { + // Just downloaded the latest version, no need to do further update checks + // for this extension. + SetUpdateChecked(id); + // The explicit copy ctors are to make sure that Bind() binds a copy and not // a reference to the arguments. PostBlockingTask(FROM_HERE, @@ -493,15 +530,15 @@ void AppPackUpdater::OnCacheEntryInstalled(const std::string& id, const std::string& path, const std::string& version) { VLOG(1) << "AppPack installed a new extension in the cache: " << path; + // Add to the list of cached extensions. + CacheEntry& entry = cached_extensions_[id]; + entry.path = path; + entry.cached_version = version; if (id == screen_saver_id_) { VLOG(1) << "AppPack got the screen saver extension at " << path; SetScreenSaverPath(FilePath(path)); } else { - // Add to the list of cached extensions. - CacheEntry& entry = cached_extensions_[id]; - entry.path = path; - entry.cached_version = version; UpdateExtensionLoader(); } } @@ -546,4 +583,10 @@ void AppPackUpdater::SetScreenSaverPath(const FilePath& path) { } } +void AppPackUpdater::SetUpdateChecked(const std::string& id) { + PolicyEntryMap::iterator entry = app_pack_extensions_.find(id); + if (entry != app_pack_extensions_.end()) + entry->second.update_checked = true; +} + } // namespace policy diff --git a/chrome/browser/policy/app_pack_updater.h b/chrome/browser/policy/app_pack_updater.h index b34a2f0..c8f5733 100644 --- a/chrome/browser/policy/app_pack_updater.h +++ b/chrome/browser/policy/app_pack_updater.h @@ -20,6 +20,7 @@ #include "chrome/browser/policy/cloud_policy_subsystem.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" +#include "net/base/network_change_notifier.h" class CrxInstaller; class ExternalExtensionLoader; @@ -48,6 +49,7 @@ class BrowserPolicyConnector; // at login time. class AppPackUpdater : public CloudPolicySubsystem::Observer, public content::NotificationObserver, + public net::NetworkChangeNotifier::IPAddressObserver, public extensions::ExtensionDownloaderDelegate { public: // Callback to listen for updates to the screensaver extension's path. @@ -74,13 +76,18 @@ class AppPackUpdater : public CloudPolicySubsystem::Observer, void SetScreenSaverUpdateCallback(const ScreenSaverUpdateCallback& callback); private: + struct AppPackEntry { + std::string update_url; + bool update_checked; + }; + struct CacheEntry { std::string path; std::string cached_version; }; - // Maps an extension ID to its update URL. - typedef std::map<std::string, std::string> PolicyEntryMap; + // Maps an extension ID to its update URL and update information. + typedef std::map<std::string, AppPackEntry> PolicyEntryMap; // Maps an extension ID to a CacheEntry. typedef std::map<std::string, CacheEntry> CacheEntryMap; @@ -97,6 +104,9 @@ class AppPackUpdater : public CloudPolicySubsystem::Observer, const content::NotificationSource& source, const content::NotificationDetails& details) OVERRIDE; + // net::NetworkChangeNotifier::IPAddressObserver: + virtual void OnIPAddressChanged() OVERRIDE; + // Loads the current policy and schedules a cache update. void LoadPolicy(); @@ -177,6 +187,10 @@ class AppPackUpdater : public CloudPolicySubsystem::Observer, // appropriate. void SetScreenSaverPath(const FilePath& path); + // Marks extension |id| in |app_pack_extensions_| as having already been + // checked for updates, if it exists. + void SetUpdateChecked(const std::string& id); + base::WeakPtrFactory<AppPackUpdater> weak_ptr_factory_; // Observes updates to the |device_cloud_policy_subsystem_|, to detect |