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

Unified Diff: remoting/host/policy_watcher.cc

Issue 2710023003: Refactor PolicyWatcher to allow mocking (Closed)
Patch Set: Created 3 years, 10 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: remoting/host/policy_watcher.cc
diff --git a/remoting/host/policy_watcher.cc b/remoting/host/policy_watcher.cc
index b21da1bac6a43659e31ebaa26eb0564c17c72f15..da7a16334128103bee2e236540bd4180bd1626e4 100644
--- a/remoting/host/policy_watcher.cc
+++ b/remoting/host/policy_watcher.cc
@@ -47,6 +47,70 @@ namespace key = ::policy::key;
namespace {
+// Watches for changes to the managed remote access host policies.
+class PolicyWatcherImpl : public PolicyWatcher,
+ private policy::PolicyService::Observer {
+ public:
+ // |policy_service_task_runner| is the task runner where it is safe
+ // to call |policy_service_| methods and where we expect to get callbacks
+ // from |policy_service_|.
+ PolicyWatcherImpl(
+ policy::PolicyService* policy_service,
+ std::unique_ptr<policy::PolicyService> owned_policy_service,
+ std::unique_ptr<policy::ConfigurationPolicyProvider>
+ owned_policy_provider,
+ std::unique_ptr<policy::SchemaRegistry> owned_schema_registry);
+
+ ~PolicyWatcherImpl() override;
+
+ void StartWatching(const PolicyUpdatedCallback& policy_updated_callback,
+ const PolicyErrorCallback& policy_error_callback) override;
+
+ private:
+ friend class PolicyWatcherTest;
+
+ const policy::Schema* GetPolicySchema() const override;
+ const base::DictionaryValue* GetDefaultValues() const override;
+
+ // Simplifying wrapper around Schema::Normalize.
+ // - Returns false if |dict| is invalid (i.e. contains mistyped policy
+ // values).
+ // - Returns true if |dict| was valid or got normalized.
+ bool NormalizePolicies(base::DictionaryValue* dict);
+
+ // Stores |new_policies| into |old_policies_|. Returns dictionary with items
+ // from |new_policies| that are different from the old |old_policies_|.
+ std::unique_ptr<base::DictionaryValue> StoreNewAndReturnChangedPolicies(
+ std::unique_ptr<base::DictionaryValue> new_policies);
+
+ // Signals policy error to the registered |PolicyErrorCallback|.
+ void SignalPolicyError();
+
+ // PolicyService::Observer interface.
+ void OnPolicyUpdated(const policy::PolicyNamespace& ns,
+ const policy::PolicyMap& previous,
+ const policy::PolicyMap& current) override;
+ void OnPolicyServiceInitialized(policy::PolicyDomain domain) override;
+
+ PolicyUpdatedCallback policy_updated_callback_;
+ PolicyErrorCallback policy_error_callback_;
+
+ std::unique_ptr<base::DictionaryValue> old_policies_;
+ std::unique_ptr<base::DictionaryValue> default_values_;
+
+ policy::PolicyService* policy_service_;
+
+ // Order of fields below is important to ensure destruction takes object
+ // dependencies into account:
+ // - |owned_policy_service_| uses |owned_policy_provider_|
+ // - |owned_policy_provider_| uses |owned_schema_registry_|
+ std::unique_ptr<policy::SchemaRegistry> owned_schema_registry_;
+ std::unique_ptr<policy::ConfigurationPolicyProvider> owned_policy_provider_;
+ std::unique_ptr<policy::PolicyService> owned_policy_service_;
+
+ DISALLOW_COPY_AND_ASSIGN(PolicyWatcherImpl);
+};
+
// Copies all policy values from one dictionary to another, using values from
// |default_values| if they are not set in |from|.
std::unique_ptr<base::DictionaryValue> CopyValuesAndAddDefaults(
@@ -136,9 +200,16 @@ bool VerifyWellformedness(const base::DictionaryValue& changed_policies) {
return true;
}
-} // namespace
+void CopyDictionaryValue(const base::DictionaryValue& from,
+ base::DictionaryValue& to,
+ std::string key) {
+ const base::Value* value;
+ if (from.Get(key, &value)) {
+ to.Set(key, value->CreateDeepCopy());
+ }
+}
-void PolicyWatcher::StartWatching(
+void PolicyWatcherImpl::StartWatching(
const PolicyUpdatedCallback& policy_updated_callback,
const PolicyErrorCallback& policy_error_callback) {
DCHECK(CalledOnValidThread());
@@ -158,12 +229,12 @@ void PolicyWatcher::StartWatching(
}
}
-void PolicyWatcher::SignalPolicyError() {
+void PolicyWatcherImpl::SignalPolicyError() {
old_policies_->Clear();
policy_error_callback_.Run();
}
-PolicyWatcher::PolicyWatcher(
+PolicyWatcherImpl::PolicyWatcherImpl(
policy::PolicyService* policy_service,
std::unique_ptr<policy::PolicyService> owned_policy_service,
std::unique_ptr<policy::ConfigurationPolicyProvider> owned_policy_provider,
@@ -199,7 +270,7 @@ PolicyWatcher::PolicyWatcher(
key::kRemoteAccessHostAllowUiAccessForRemoteAssistance, false);
}
-PolicyWatcher::~PolicyWatcher() {
+PolicyWatcherImpl::~PolicyWatcherImpl() {
// Stop observing |policy_service_| if StartWatching() has been called.
if (!policy_updated_callback_.is_null()) {
policy_service_->RemoveObserver(policy::POLICY_DOMAIN_CHROME, this);
@@ -210,11 +281,15 @@ PolicyWatcher::~PolicyWatcher() {
}
}
-const policy::Schema* PolicyWatcher::GetPolicySchema() const {
+const policy::Schema* PolicyWatcherImpl::GetPolicySchema() const {
return owned_schema_registry_->schema_map()->GetSchema(GetPolicyNamespace());
}
-bool PolicyWatcher::NormalizePolicies(base::DictionaryValue* policy_dict) {
+const base::DictionaryValue* PolicyWatcherImpl::GetDefaultValues() const {
+ return default_values_.get();
+}
+
+bool PolicyWatcherImpl::NormalizePolicies(base::DictionaryValue* policy_dict) {
// Allowing unrecognized policy names allows presence of
// 1) comments (i.e. JSON of the form: { "_comment": "blah", ... }),
// 2) policies intended for future/newer versions of the host,
@@ -239,19 +314,8 @@ bool PolicyWatcher::NormalizePolicies(base::DictionaryValue* policy_dict) {
}
}
-namespace {
-void CopyDictionaryValue(const base::DictionaryValue& from,
- base::DictionaryValue& to,
- std::string key) {
- const base::Value* value;
- if (from.Get(key, &value)) {
- to.Set(key, value->CreateDeepCopy());
- }
-}
-} // namespace
-
std::unique_ptr<base::DictionaryValue>
-PolicyWatcher::StoreNewAndReturnChangedPolicies(
+PolicyWatcherImpl::StoreNewAndReturnChangedPolicies(
std::unique_ptr<base::DictionaryValue> new_policies) {
// Find the changed policies.
std::unique_ptr<base::DictionaryValue> changed_policies(
@@ -285,9 +349,9 @@ PolicyWatcher::StoreNewAndReturnChangedPolicies(
return changed_policies;
}
-void PolicyWatcher::OnPolicyUpdated(const policy::PolicyNamespace& ns,
- const policy::PolicyMap& previous,
- const policy::PolicyMap& current) {
+void PolicyWatcherImpl::OnPolicyUpdated(const policy::PolicyNamespace& ns,
+ const policy::PolicyMap& previous,
+ const policy::PolicyMap& current) {
std::unique_ptr<base::DictionaryValue> new_policies =
CopyChromotingPoliciesIntoDictionary(current);
@@ -318,12 +382,18 @@ void PolicyWatcher::OnPolicyUpdated(const policy::PolicyNamespace& ns,
policy_updated_callback_.Run(std::move(changed_policies));
}
-void PolicyWatcher::OnPolicyServiceInitialized(policy::PolicyDomain domain) {
+void PolicyWatcherImpl::OnPolicyServiceInitialized(
+ policy::PolicyDomain domain) {
policy::PolicyNamespace ns = GetPolicyNamespace();
const policy::PolicyMap& current = policy_service_->GetPolicies(ns);
OnPolicyUpdated(ns, current, current);
}
+} // namespace
+
+PolicyWatcher::PolicyWatcher() {}
+PolicyWatcher::~PolicyWatcher() {}
+
std::unique_ptr<PolicyWatcher> PolicyWatcher::CreateFromPolicyLoader(
std::unique_ptr<policy::AsyncPolicyLoader> async_policy_loader) {
std::unique_ptr<policy::SchemaRegistry> schema_registry =
@@ -339,7 +409,7 @@ std::unique_ptr<PolicyWatcher> PolicyWatcher::CreateFromPolicyLoader(
new policy::PolicyServiceImpl(providers));
policy::PolicyService* borrowed_policy_service = policy_service.get();
- return base::WrapUnique(new PolicyWatcher(
+ return base::WrapUnique(new PolicyWatcherImpl(
borrowed_policy_service, std::move(policy_service),
std::move(policy_provider), std::move(schema_registry)));
}

Powered by Google App Engine
This is Rietveld 408576698