From 99a9fd01df438f41d7192e6581fe7b1cb4f18d99 Mon Sep 17 00:00:00 2001 From: xiyuan Date: Fri, 25 Mar 2016 16:13:04 -0700 Subject: kiosk: Update meta data when cached crx is updated - Initialize KioskAppData with existing cached crx; - Extract meta data when the cached crx file is changed to ensure meta data is always up-to-date with the crx; Other changes: - Use vector> instead of deprecated ScopedVector; - Break down UpdateAppData into smaller chunks; BUG=585143 Review URL: https://codereview.chromium.org/1833293002 Cr-Commit-Position: refs/heads/master@{#383400} --- chrome/browser/chromeos/app_mode/kiosk_app_data.cc | 14 +++-- chrome/browser/chromeos/app_mode/kiosk_app_data.h | 5 +- .../browser/chromeos/app_mode/kiosk_app_manager.cc | 66 ++++++++++++---------- .../browser/chromeos/app_mode/kiosk_app_manager.h | 11 +++- 4 files changed, 55 insertions(+), 41 deletions(-) diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_data.cc b/chrome/browser/chromeos/app_mode/kiosk_app_data.cc index 0cb0c98..711f585 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_app_data.cc +++ b/chrome/browser/chromeos/app_mode/kiosk_app_data.cc @@ -396,12 +396,14 @@ class KioskAppData::WebstoreDataParser KioskAppData::KioskAppData(KioskAppDataDelegate* delegate, const std::string& app_id, const AccountId& account_id, - const GURL& update_url) + const GURL& update_url, + const base::FilePath& cached_crx) : delegate_(delegate), status_(STATUS_INIT), app_id_(app_id), account_id_(account_id), - update_url_(update_url) {} + update_url_(update_url), + crx_file_(cached_crx) {} KioskAppData::~KioskAppData() {} @@ -459,7 +461,7 @@ void KioskAppData::SetCachedCrx(const base::FilePath& crx_file) { return; crx_file_ = crx_file; - MaybeLoadFromCrx(); + LoadFromCrx(); } bool KioskAppData::IsLoading() const { @@ -612,7 +614,7 @@ void KioskAppData::OnWebstoreParseFailure() { void KioskAppData::StartFetch() { if (!IsFromWebStore()) { - MaybeLoadFromCrx(); + LoadFromCrx(); return; } @@ -681,8 +683,8 @@ bool KioskAppData::CheckResponseKeyValue(const base::DictionaryValue* response, return true; } -void KioskAppData::MaybeLoadFromCrx() { - if (status_ == STATUS_LOADED || crx_file_.empty()) +void KioskAppData::LoadFromCrx() { + if (crx_file_.empty()) return; scoped_refptr crx_loader(new CrxLoader(AsWeakPtr(), crx_file_)); diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_data.h b/chrome/browser/chromeos/app_mode/kiosk_app_data.h index edcf76f..81df9f6 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_app_data.h +++ b/chrome/browser/chromeos/app_mode/kiosk_app_data.h @@ -50,7 +50,8 @@ class KioskAppData : public base::SupportsWeakPtr, KioskAppData(KioskAppDataDelegate* delegate, const std::string& app_id, const AccountId& account_id, - const GURL& update_url); + const GURL& update_url, + const base::FilePath& cached_crx); ~KioskAppData() override; // Loads app data from cache. If there is no cached data, fetches it @@ -138,7 +139,7 @@ class KioskAppData : public base::SupportsWeakPtr, // Extracts meta data from crx file when loading from Webstore and local // cache fails. - void MaybeLoadFromCrx(); + void LoadFromCrx(); void OnCrxLoadFinished(const CrxLoader* crx_loader); diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc b/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc index 4a39547..62ebcdb 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc +++ b/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc @@ -5,8 +5,6 @@ #include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h" #include -#include -#include #include #include "base/barrier_closure.h" @@ -612,10 +610,9 @@ void KioskAppManager::CleanUp() { const KioskAppData* KioskAppManager::GetAppData( const std::string& app_id) const { - for (size_t i = 0; i < apps_.size(); ++i) { - const KioskAppData* data = apps_[i]; - if (data->app_id() == app_id) - return data; + for (const auto& app : apps_) { + if (app->app_id() == app_id) + return app.get(); } return nullptr; @@ -627,10 +624,10 @@ KioskAppData* KioskAppManager::GetAppDataMutable(const std::string& app_id) { void KioskAppManager::UpdateAppData() { // Gets app id to data mapping for existing apps. - std::map old_apps; - for (size_t i = 0; i < apps_.size(); ++i) - old_apps[apps_[i]->app_id()] = apps_[i]; - apps_.weak_clear(); // |old_apps| takes ownership + std::map> old_apps; + for (auto& app : apps_) + old_apps[app->app_id()] = std::move(app); + apps_.clear(); auto_launch_app_id_.clear(); std::string auto_login_account_id; @@ -652,20 +649,33 @@ void KioskAppManager::UpdateAppData() { // Note that app ids are not canonical, i.e. they can contain upper // case letters. const AccountId account_id(AccountId::FromUserEmail(it->user_id)); - std::map::iterator old_it = - old_apps.find(it->kiosk_app_id); + auto old_it = old_apps.find(it->kiosk_app_id); if (old_it != old_apps.end()) { - apps_.push_back(old_it->second); + apps_.push_back(std::move(old_it->second)); old_apps.erase(old_it); } else { - KioskAppData* new_app = new KioskAppData( - this, it->kiosk_app_id, account_id, GURL(it->kiosk_app_update_url)); - apps_.push_back(new_app); // Takes ownership of |new_app|. - new_app->Load(); + base::FilePath cached_crx; + std::string version; + GetCachedCrx(it->kiosk_app_id, &cached_crx, &version); + + apps_.push_back(make_scoped_ptr( + new KioskAppData(this, it->kiosk_app_id, account_id, + GURL(it->kiosk_app_update_url), cached_crx))); + apps_.back()->Load(); } CancelDelayedCryptohomeRemoval(cryptohome::Identification(account_id)); } + ClearRemovedApps(old_apps); + UpdateExternalCachePrefs(); + RetryFailedAppDataFetch(); + + FOR_EACH_OBSERVER(KioskAppManagerObserver, observers_, + OnKioskAppsSettingsChanged()); +} + +void KioskAppManager::ClearRemovedApps( + const std::map>& old_apps) { base::Closure cryptohomes_barrier_closure; const user_manager::User* active_user = @@ -684,20 +694,19 @@ void KioskAppManager::UpdateAppData() { // Clears cache and deletes the remaining old data. std::vector apps_to_remove; - for (std::map::iterator it = old_apps.begin(); - it != old_apps.end(); ++it) { - it->second->ClearCache(); - const cryptohome::Identification cryptohome_id(it->second->account_id()); + for (auto& entry : old_apps) { + entry.second->ClearCache(); + const cryptohome::Identification cryptohome_id(entry.second->account_id()); cryptohome::AsyncMethodCaller::GetInstance()->AsyncRemove( cryptohome_id, base::Bind(&OnRemoveAppCryptohomeComplete, cryptohome_id, - it->first, cryptohomes_barrier_closure)); - apps_to_remove.push_back(it->second->app_id()); + entry.first, cryptohomes_barrier_closure)); + apps_to_remove.push_back(entry.second->app_id()); } - STLDeleteValues(&old_apps); external_cache_->RemoveExtensions(apps_to_remove); +} - // Request external_cache_ to download new apps and update the existing - // apps. +void KioskAppManager::UpdateExternalCachePrefs() { + // Request external_cache_ to download new apps and update the existing apps. scoped_ptr prefs(new base::DictionaryValue); for (size_t i = 0; i < apps_.size(); ++i) { scoped_ptr entry(new base::DictionaryValue); @@ -713,11 +722,6 @@ void KioskAppManager::UpdateAppData() { prefs->Set(apps_[i]->app_id(), entry.release()); } external_cache_->UpdateExtensionsList(std::move(prefs)); - - RetryFailedAppDataFetch(); - - FOR_EACH_OBSERVER(KioskAppManagerObserver, observers_, - OnKioskAppsSettingsChanged()); } void KioskAppManager::GetKioskAppIconCacheDir(base::FilePath* cache_dir) { diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_manager.h b/chrome/browser/chromeos/app_mode/kiosk_app_manager.h index 1bc27f1..8862f0d 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_app_manager.h +++ b/chrome/browser/chromeos/app_mode/kiosk_app_manager.h @@ -5,6 +5,7 @@ #ifndef CHROME_BROWSER_CHROMEOS_APP_MODE_KIOSK_APP_MANAGER_H_ #define CHROME_BROWSER_CHROMEOS_APP_MODE_KIOSK_APP_MANAGER_H_ +#include #include #include #include @@ -13,7 +14,6 @@ #include "base/lazy_instance.h" #include "base/macros.h" #include "base/memory/scoped_ptr.h" -#include "base/memory/scoped_vector.h" #include "base/observer_list.h" #include "base/time/time.h" #include "chrome/browser/chromeos/app_mode/kiosk_app_data_delegate.h" @@ -261,6 +261,13 @@ class KioskAppManager : public KioskAppDataDelegate, // Updates app data |apps_| based on CrosSettings. void UpdateAppData(); + // Clear cached data and crx of the removed apps. + void ClearRemovedApps( + const std::map>& old_apps); + + // Updates the prefs of |external_cache_| from |apps_|. + void UpdateExternalCachePrefs(); + // KioskAppDataDelegate overrides: void GetKioskAppIconCacheDir(base::FilePath* cache_dir) override; void OnKioskAppDataChanged(const std::string& app_id) override; @@ -301,7 +308,7 @@ class KioskAppManager : public KioskAppDataDelegate, // True if machine ownership is already established. bool ownership_established_; - ScopedVector apps_; + std::vector> apps_; std::string auto_launch_app_id_; std::string currently_auto_launched_with_zero_delay_app_; base::ObserverList observers_; -- cgit v1.1