Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(47)

Unified Diff: trunk/src/chrome/browser/chromeos/policy/device_local_account_policy_service.cc

Issue 32513006: Revert 229896 "Cache force-installed apps/extensions in device-l..." (Closed) Base URL: svn://svn.chromium.org/chrome/
Patch Set: Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: trunk/src/chrome/browser/chromeos/policy/device_local_account_policy_service.cc
===================================================================
--- trunk/src/chrome/browser/chromeos/policy/device_local_account_policy_service.cc (revision 229896)
+++ trunk/src/chrome/browser/chromeos/policy/device_local_account_policy_service.cc (working copy)
@@ -7,24 +7,18 @@
#include <vector>
#include "base/bind.h"
-#include "base/file_util.h"
-#include "base/files/file_enumerator.h"
-#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/message_loop/message_loop.h"
#include "base/message_loop/message_loop_proxy.h"
-#include "base/path_service.h"
-#include "base/sequenced_task_runner.h"
-#include "base/strings/string_number_conversions.h"
#include "chrome/browser/chromeos/policy/device_local_account.h"
#include "chrome/browser/chromeos/policy/device_local_account_policy_store.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
#include "chrome/browser/chromeos/settings/device_settings_service.h"
#include "chrome/browser/policy/cloud/cloud_policy_client.h"
#include "chrome/browser/policy/cloud/cloud_policy_constants.h"
#include "chrome/browser/policy/cloud/cloud_policy_refresh_scheduler.h"
#include "chrome/browser/policy/cloud/device_management_service.h"
#include "chrome/browser/policy/proto/cloud/device_management_backend.pb.h"
-#include "chromeos/chromeos_paths.h"
#include "chromeos/dbus/session_manager_client.h"
#include "chromeos/settings/cros_settings_names.h"
#include "chromeos/settings/cros_settings_provider.h"
@@ -59,86 +53,23 @@
return client.Pass();
}
-// Get the subdirectory of the cache directory in which force-installed
-// extensions are cached for |account_id|.
-std::string GetCacheSubdirectoryForAccountID(const std::string& account_id) {
- return base::HexEncode(account_id.c_str(), account_id.size());
-}
-
-// Cleans up the cache directory by removing subdirectories that are not found
-// in |subdirectories_to_keep|. Only caches whose cache directory is found in
-// |subdirectories_to_keep| may be running while the clean-up is in progress.
-void DeleteOrphanedExtensionCaches(
- const std::set<std::string>& subdirectories_to_keep) {
- base::FilePath cache_root_dir;
- CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_CACHE,
- &cache_root_dir));
- base::FileEnumerator enumerator(cache_root_dir,
- false,
- base::FileEnumerator::DIRECTORIES);
- for (base::FilePath path = enumerator.Next(); !path.empty();
- path = enumerator.Next()) {
- const std::string subdirectory(path.BaseName().MaybeAsASCII());
- if (subdirectories_to_keep.find(subdirectory) ==
- subdirectories_to_keep.end()) {
- base::DeleteFile(path, true);
- }
- }
-}
-
-// Removes the subdirectory belonging to |account_id_to_delete| from the cache
-// directory. No cache belonging to |account_id_to_delete| may be running while
-// the removal is in progress.
-void DeleteObsoleteExtensionCache(const std::string& account_id_to_delete) {
- base::FilePath cache_root_dir;
- CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_CACHE,
- &cache_root_dir));
- const base::FilePath path = cache_root_dir
- .Append(GetCacheSubdirectoryForAccountID(account_id_to_delete));
- if (base::DirectoryExists(path))
- base::DeleteFile(path, true);
-}
-
} // namespace
DeviceLocalAccountPolicyBroker::DeviceLocalAccountPolicyBroker(
- const DeviceLocalAccount& account,
+ const std::string& user_id,
scoped_ptr<DeviceLocalAccountPolicyStore> store,
const scoped_refptr<base::SequencedTaskRunner>& task_runner)
- : account_id_(account.account_id),
- user_id_(account.user_id),
+ : user_id_(user_id),
store_(store.Pass()),
core_(PolicyNamespaceKey(dm_protocol::kChromePublicAccountPolicyType,
store_->account_id()),
store_.get(),
- task_runner) {
- base::FilePath cache_root_dir;
- CHECK(PathService::Get(chromeos::DIR_DEVICE_LOCAL_ACCOUNT_CACHE,
- &cache_root_dir));
- extension_loader_ = new chromeos::DeviceLocalAccountExternalPolicyLoader(
- store_.get(),
- cache_root_dir.Append(
- GetCacheSubdirectoryForAccountID(account.account_id)));
-}
+ task_runner) {}
-DeviceLocalAccountPolicyBroker::~DeviceLocalAccountPolicyBroker() {
-}
+DeviceLocalAccountPolicyBroker::~DeviceLocalAccountPolicyBroker() {}
-void DeviceLocalAccountPolicyBroker::Initialize() {
- store_->Load();
-}
-
-void DeviceLocalAccountPolicyBroker::ConnectIfPossible(
- chromeos::DeviceSettingsService* device_settings_service,
- DeviceManagementService* device_management_service) {
- if (core_.client())
- return;
-
- scoped_ptr<CloudPolicyClient> client(CreateClient(device_settings_service,
- device_management_service));
- if (!client)
- return;
-
+void DeviceLocalAccountPolicyBroker::Connect(
+ scoped_ptr<CloudPolicyClient> client) {
core_.Connect(client.Pass());
core_.StartRefreshScheduler();
UpdateRefreshDelay();
@@ -167,26 +98,66 @@
return display_name;
}
+DeviceLocalAccountPolicyService::PolicyBrokerWrapper::PolicyBrokerWrapper()
+ : parent(NULL), broker(NULL) {}
+
+DeviceLocalAccountPolicyService::PolicyBrokerWrapper::~PolicyBrokerWrapper() {}
+
+DeviceLocalAccountPolicyBroker*
+ DeviceLocalAccountPolicyService::PolicyBrokerWrapper::GetBroker() {
+ if (!broker) {
+ scoped_ptr<DeviceLocalAccountPolicyStore> store(
+ new DeviceLocalAccountPolicyStore(account_id,
+ parent->session_manager_client_,
+ parent->device_settings_service_,
+ parent->background_task_runner_));
+ broker = new DeviceLocalAccountPolicyBroker(
+ user_id, store.Pass(), base::MessageLoopProxy::current());
+ broker->core()->store()->AddObserver(parent);
+ broker->core()->store()->Load();
+ }
+ return broker;
+}
+
+void DeviceLocalAccountPolicyService::PolicyBrokerWrapper::ConnectIfPossible() {
+ if (broker && broker->core()->client())
+ return;
+ scoped_ptr<CloudPolicyClient> client(CreateClient(
+ parent->device_settings_service_,
+ parent->device_management_service_));
+ if (client)
+ GetBroker()->Connect(client.Pass());
+}
+
+void DeviceLocalAccountPolicyService::PolicyBrokerWrapper::Disconnect() {
+ if (broker)
+ broker->Disconnect();
+}
+
+void DeviceLocalAccountPolicyService::PolicyBrokerWrapper::DeleteBroker() {
+ if (!broker)
+ return;
+ broker->core()->store()->RemoveObserver(parent);
+ delete broker;
+ broker = NULL;
+}
+
DeviceLocalAccountPolicyService::DeviceLocalAccountPolicyService(
chromeos::SessionManagerClient* session_manager_client,
chromeos::DeviceSettingsService* device_settings_service,
chromeos::CrosSettings* cros_settings,
- scoped_refptr<base::SequencedTaskRunner> store_background_task_runner,
- scoped_refptr<base::SequencedTaskRunner> extension_cache_task_runner)
+ scoped_refptr<base::SequencedTaskRunner> background_task_runner)
: session_manager_client_(session_manager_client),
device_settings_service_(device_settings_service),
cros_settings_(cros_settings),
device_management_service_(NULL),
- waiting_for_cros_settings_(false),
- orphan_cache_deletion_state_(NOT_STARTED),
- store_background_task_runner_(store_background_task_runner),
- extension_cache_task_runner_(extension_cache_task_runner),
- local_accounts_subscription_(cros_settings_->AddSettingsObserver(
- chromeos::kAccountsPrefDeviceLocalAccounts,
- base::Bind(&DeviceLocalAccountPolicyService::
- UpdateAccountListIfNonePending,
- base::Unretained(this)))),
- weak_factory_(this) {
+ background_task_runner_(background_task_runner),
+ cros_settings_callback_factory_(this) {
+ local_accounts_subscription_ = cros_settings_->AddSettingsObserver(
+ chromeos::kAccountsPrefDeviceLocalAccounts,
+ base::Bind(&DeviceLocalAccountPolicyService::
+ UpdateAccountListIfNonePending,
+ base::Unretained(this)));
UpdateAccountList();
}
@@ -202,8 +173,7 @@
// Connect the brokers.
for (PolicyBrokerMap::iterator it(policy_brokers_.begin());
it != policy_brokers_.end(); ++it) {
- it->second->ConnectIfPossible(device_settings_service_,
- device_management_service_);
+ it->second.ConnectIfPossible();
}
}
@@ -214,7 +184,7 @@
// Disconnect the brokers.
for (PolicyBrokerMap::iterator it(policy_brokers_.begin());
it != policy_brokers_.end(); ++it) {
- it->second->Disconnect();
+ it->second.Disconnect();
}
}
@@ -225,7 +195,7 @@
if (entry == policy_brokers_.end())
return NULL;
- return entry->second;
+ return entry->second.GetBroker();
}
bool DeviceLocalAccountPolicyService::IsPolicyAvailableForUser(
@@ -259,214 +229,52 @@
FOR_EACH_OBSERVER(Observer, observers_, OnPolicyUpdated(broker->user_id()));
}
-bool DeviceLocalAccountPolicyService::IsExtensionCacheDirectoryBusy(
- const std::string& account_id) {
- return busy_extension_cache_directories_.find(account_id) !=
- busy_extension_cache_directories_.end();
-}
-
-void DeviceLocalAccountPolicyService::StartExtensionCachesIfPossible() {
- for (PolicyBrokerMap::iterator it = policy_brokers_.begin();
- it != policy_brokers_.end(); ++it) {
- if (!it->second->extension_loader()->IsCacheRunning() &&
- !IsExtensionCacheDirectoryBusy(it->second->account_id())) {
- it->second->extension_loader()->StartCache(extension_cache_task_runner_);
- }
- }
-}
-
-bool DeviceLocalAccountPolicyService::StartExtensionCacheForAccountIfPresent(
- const std::string& account_id) {
- for (PolicyBrokerMap::iterator it = policy_brokers_.begin();
- it != policy_brokers_.end(); ++it) {
- if (it->second->account_id() == account_id) {
- DCHECK(!it->second->extension_loader()->IsCacheRunning());
- it->second->extension_loader()->StartCache(extension_cache_task_runner_);
- return true;
- }
- }
- return false;
-}
-
-void DeviceLocalAccountPolicyService::OnOrphanedExtensionCachesDeleted() {
- DCHECK_EQ(IN_PROGRESS, orphan_cache_deletion_state_);
-
- orphan_cache_deletion_state_ = DONE;
- StartExtensionCachesIfPossible();
-}
-
-void DeviceLocalAccountPolicyService::OnObsoleteExtensionCacheShutdown(
- const std::string& account_id) {
- DCHECK_NE(NOT_STARTED, orphan_cache_deletion_state_);
- DCHECK(IsExtensionCacheDirectoryBusy(account_id));
-
- // The account with |account_id| was deleted and the broker for it has shut
- // down completely.
-
- if (StartExtensionCacheForAccountIfPresent(account_id)) {
- // If another account with the same ID was created in the meantime, its
- // extension cache is started, reusing the cache directory. The directory no
- // longer needs to be marked as busy in this case.
- busy_extension_cache_directories_.erase(account_id);
- return;
- }
-
- // If no account with |account_id| exists anymore, the cache directory should
- // be removed. The directory must stay marked as busy while the removal is in
- // progress.
- extension_cache_task_runner_->PostTaskAndReply(
- FROM_HERE,
- base::Bind(&DeleteObsoleteExtensionCache, account_id),
- base::Bind(&DeviceLocalAccountPolicyService::
- OnObsoleteExtensionCacheDeleted,
- weak_factory_.GetWeakPtr(),
- account_id));
-}
-
-void DeviceLocalAccountPolicyService::OnObsoleteExtensionCacheDeleted(
- const std::string& account_id) {
- DCHECK_EQ(DONE, orphan_cache_deletion_state_);
- DCHECK(IsExtensionCacheDirectoryBusy(account_id));
-
- // The cache directory for |account_id| has been deleted. The directory no
- // longer needs to be marked as busy.
- busy_extension_cache_directories_.erase(account_id);
-
- // If another account with the same ID was created in the meantime, start its
- // extension cache, creating a new cache directory.
- StartExtensionCacheForAccountIfPresent(account_id);
-}
-
void DeviceLocalAccountPolicyService::UpdateAccountListIfNonePending() {
// Avoid unnecessary calls to UpdateAccountList(): If an earlier call is still
// pending (because the |cros_settings_| are not trusted yet), the updated
// account list will be processed by that call when it eventually runs.
- if (!waiting_for_cros_settings_)
+ if (!cros_settings_callback_factory_.HasWeakPtrs())
UpdateAccountList();
}
void DeviceLocalAccountPolicyService::UpdateAccountList() {
- chromeos::CrosSettingsProvider::TrustedStatus status =
- cros_settings_->PrepareTrustedValues(
- base::Bind(&DeviceLocalAccountPolicyService::UpdateAccountList,
- weak_factory_.GetWeakPtr()));
- switch (status) {
- case chromeos::CrosSettingsProvider::TRUSTED:
- waiting_for_cros_settings_ = false;
- break;
- case chromeos::CrosSettingsProvider::TEMPORARILY_UNTRUSTED:
- waiting_for_cros_settings_ = true;
- return;
- case chromeos::CrosSettingsProvider::PERMANENTLY_UNTRUSTED:
- waiting_for_cros_settings_ = false;
- return;
+ if (chromeos::CrosSettingsProvider::TRUSTED !=
+ cros_settings_->PrepareTrustedValues(
+ base::Bind(&DeviceLocalAccountPolicyService::UpdateAccountList,
+ cros_settings_callback_factory_.GetWeakPtr()))) {
+ return;
}
// Update |policy_brokers_|, keeping existing entries.
PolicyBrokerMap old_policy_brokers;
policy_brokers_.swap(old_policy_brokers);
- std::set<std::string> subdirectories_to_keep;
const std::vector<DeviceLocalAccount> device_local_accounts =
GetDeviceLocalAccounts(cros_settings_);
for (std::vector<DeviceLocalAccount>::const_iterator it =
device_local_accounts.begin();
it != device_local_accounts.end(); ++it) {
- PolicyBrokerMap::iterator broker_it = old_policy_brokers.find(it->user_id);
+ PolicyBrokerWrapper& wrapper = policy_brokers_[it->user_id];
+ wrapper.user_id = it->user_id;
+ wrapper.account_id = it->account_id;
+ wrapper.parent = this;
- scoped_ptr<DeviceLocalAccountPolicyBroker> broker;
- bool broker_initialized = false;
- if (broker_it != old_policy_brokers.end()) {
- // Reuse the existing broker if present.
- broker.reset(broker_it->second);
- old_policy_brokers.erase(broker_it);
- broker_initialized = true;
- } else {
- scoped_ptr<DeviceLocalAccountPolicyStore> store(
- new DeviceLocalAccountPolicyStore(it->account_id,
- session_manager_client_,
- device_settings_service_,
- store_background_task_runner_));
- store->AddObserver(this);
- broker.reset(new DeviceLocalAccountPolicyBroker(
- *it,
- store.Pass(),
- base::MessageLoopProxy::current()));
- }
+ // Reuse the existing broker if present.
+ PolicyBrokerWrapper& existing_wrapper = old_policy_brokers[it->user_id];
+ wrapper.broker = existing_wrapper.broker;
+ existing_wrapper.broker = NULL;
// Fire up the cloud connection for fetching policy for the account from
// the cloud if this is an enterprise-managed device.
- broker->ConnectIfPossible(device_settings_service_,
- device_management_service_);
-
- policy_brokers_[it->user_id] = broker.release();
- if (!broker_initialized) {
- // The broker must be initialized after it has been added to
- // |policy_brokers_|.
- policy_brokers_[it->user_id]->Initialize();
- }
-
- if (orphan_cache_deletion_state_ == NOT_STARTED) {
- subdirectories_to_keep.insert(
- GetCacheSubdirectoryForAccountID(it->account_id));
- }
+ wrapper.ConnectIfPossible();
}
+ DeleteBrokers(&old_policy_brokers);
- std::set<std::string> obsolete_account_ids;
- for (PolicyBrokerMap::const_iterator it = old_policy_brokers.begin();
- it != old_policy_brokers.end(); ++it) {
- obsolete_account_ids.insert(it->second->account_id());
- }
-
- if (orphan_cache_deletion_state_ == NOT_STARTED) {
- DCHECK(old_policy_brokers.empty());
- DCHECK(busy_extension_cache_directories_.empty());
-
- // If this method is running for the first time, no extension caches have
- // been started yet. Take this opportunity to do a clean-up by removing
- // orphaned cache directories not found in |subdirectories_to_keep| from the
- // cache directory.
- orphan_cache_deletion_state_ = IN_PROGRESS;
- extension_cache_task_runner_->PostTaskAndReply(
- FROM_HERE,
- base::Bind(&DeleteOrphanedExtensionCaches, subdirectories_to_keep),
- base::Bind(&DeviceLocalAccountPolicyService::
- OnOrphanedExtensionCachesDeleted,
- weak_factory_.GetWeakPtr()));
-
- // Start the extension caches for all brokers. These belong to accounts in
- // |account_ids| and are not affected by the clean-up.
- StartExtensionCachesIfPossible();
- } else {
- // If this method has run before, obsolete brokers may exist. Shut down
- // their extension caches and delete the brokers.
- DeleteBrokers(&old_policy_brokers);
-
- if (orphan_cache_deletion_state_ == DONE) {
- // If the initial clean-up of orphaned cache directories has been
- // complete, start any extension caches that are not running yet but can
- // be started now because their cache directories are not busy.
- StartExtensionCachesIfPossible();
- }
- }
-
FOR_EACH_OBSERVER(Observer, observers_, OnDeviceLocalAccountsChanged());
}
void DeviceLocalAccountPolicyService::DeleteBrokers(PolicyBrokerMap* map) {
- for (PolicyBrokerMap::iterator it = map->begin(); it != map->end(); ++it) {
- it->second->core()->store()->RemoveObserver(this);
- scoped_refptr<chromeos::DeviceLocalAccountExternalPolicyLoader>
- extension_loader = it->second->extension_loader();
- if (extension_loader->IsCacheRunning()) {
- DCHECK(!IsExtensionCacheDirectoryBusy(it->second->account_id()));
- busy_extension_cache_directories_.insert(it->second->account_id());
- extension_loader->StopCache(base::Bind(
- &DeviceLocalAccountPolicyService::OnObsoleteExtensionCacheShutdown,
- weak_factory_.GetWeakPtr(),
- it->second->account_id()));
- }
- delete it->second;
- }
+ for (PolicyBrokerMap::iterator it = map->begin(); it != map->end(); ++it)
+ it->second.DeleteBroker();
map->clear();
}
@@ -475,8 +283,8 @@
CloudPolicyStore* store) {
for (PolicyBrokerMap::iterator it(policy_brokers_.begin());
it != policy_brokers_.end(); ++it) {
- if (it->second->core()->store() == store)
- return it->second;
+ if (it->second.broker && it->second.broker->core()->store() == store)
+ return it->second.broker;
}
return NULL;
}

Powered by Google App Engine
This is Rietveld 408576698