[Chromoting] Refactor the host policy watcher so that policies can easily be added.

remoting/host/policy_hack/nat_policy.cc

Index: remoting/host/policy_hack/nat_policy.cc
diff --git a/remoting/host/policy_hack/nat_policy.cc b/remoting/host/policy_hack/nat_policy.cc
index 77f376a7da5da3704791cfc1eeb6629d4b8535ae..f6909da99110887fad5f59e1821a12b92d9bf7c2 100644
--- a/remoting/host/policy_hack/nat_policy.cc
+++ b/remoting/host/policy_hack/nat_policy.cc
@@ -24,87 +24,138 @@ namespace {
 // delegate never reports a change to the ReloadObserver.
 const int kFallbackReloadDelayMinutes = 15;
 
+// Gets a boolean from a dictionary, or returns a default value if the boolean
+// couldn't be read.
+bool GetBooleanOrDefault(const base::DictionaryValue* dict, const char* key,
+                         bool default_if_value_missing,
+                         bool default_if_value_not_boolean) {
+  if (!dict->HasKey(key)) {
+    return default_if_value_missing;
+  }
+  base::Value* value;
+  if (dict->Get(key, &value) && value->IsType(base::Value::TYPE_BOOLEAN)) {
+    bool bool_value;
+    CHECK(value->GetAsBoolean(&bool_value));
+    return bool_value;
+  }
+  return default_if_value_not_boolean;
+}
+
+// Copies a boolean from one dictionary to another, using a default value
+// if the boolean couldn't be read from the first dictionary.
+void CopyBooleanOrDefault(base::DictionaryValue* to,
+                          const base::DictionaryValue* from, const char* key,
+                          bool default_if_value_missing,
+                          bool default_if_value_not_boolean) {
+  to->Set(key, base::Value::CreateBooleanValue(
+      GetBooleanOrDefault(from, key, default_if_value_missing,
+                          default_if_value_not_boolean)));
+}
+
+// Copies all policy values from one dictionary to another, using default values
+// when necessary.
+scoped_ptr<base::DictionaryValue> AddDefaultValuesWhenNecessary(
+    const base::DictionaryValue* from) {
+  scoped_ptr<base::DictionaryValue> to(new base::DictionaryValue());
+  CopyBooleanOrDefault(to.get(), from,
+                       PolicyWatcher::kNatPolicyName, true, false);
+  return to.Pass();
+}
+
 }  // namespace
 
-const char NatPolicy::kNatPolicyName[] = "RemoteAccessHostFirewallTraversal";
+const char PolicyWatcher::kNatPolicyName[] =
+    "RemoteAccessHostFirewallTraversal";
+
+const char* const PolicyWatcher::kBooleanPolicyNames[] =

[garykac, 2012/07/23 17:29:26]

We'll have boolean, string and int policy types that the host needs to read. You
may want to consider making this a struct of {policy_name, policy_type}s
(although that can be a follow-up).

[simonmorris, 2012/07/23 19:34:55]

The next CL will have a different policy type, so we'll address this question
then.

+    { PolicyWatcher::kNatPolicyName };
 
-NatPolicy::NatPolicy(scoped_refptr<base::SingleThreadTaskRunner> task_runner)
+const int PolicyWatcher::kBooleanPolicyNamesNum =
+    arraysize(kBooleanPolicyNames);
+
+PolicyWatcher::PolicyWatcher(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
     : task_runner_(task_runner),
-      current_nat_enabled_state_(false),
-      first_state_published_(false),
+      old_policies_(new base::DictionaryValue()),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
 }
 
-NatPolicy::~NatPolicy() {
+PolicyWatcher::~PolicyWatcher() {
 }
 
-void NatPolicy::StartWatching(const NatEnabledCallback& nat_enabled_cb) {
-  if (!OnPolicyThread()) {
+void PolicyWatcher::StartWatching(const PolicyCallback& policy_callback) {
+  if (!OnPolicyWatcherThread()) {
     task_runner_->PostTask(FROM_HERE,
-                           base::Bind(&NatPolicy::StartWatching,
+                           base::Bind(&PolicyWatcher::StartWatching,
                                       base::Unretained(this),
-                                      nat_enabled_cb));
+                                      policy_callback));
     return;
   }
 
-  nat_enabled_cb_ = nat_enabled_cb;
+  policy_callback_ = policy_callback;
   StartWatchingInternal();
 }
 
-void NatPolicy::StopWatching(base::WaitableEvent* done) {
-  if (!OnPolicyThread()) {
+void PolicyWatcher::StopWatching(base::WaitableEvent* done) {
+  if (!OnPolicyWatcherThread()) {
     task_runner_->PostTask(FROM_HERE,
-                           base::Bind(&NatPolicy::StopWatching,
+                           base::Bind(&PolicyWatcher::StopWatching,
                                       base::Unretained(this), done));
     return;
   }
 
   StopWatchingInternal();
   weak_factory_.InvalidateWeakPtrs();
-  nat_enabled_cb_.Reset();
+  policy_callback_.Reset();
 
   done->Signal();
 }
 
-void NatPolicy::ScheduleFallbackReloadTask() {
-  DCHECK(OnPolicyThread());
+void PolicyWatcher::ScheduleFallbackReloadTask() {
+  DCHECK(OnPolicyWatcherThread());
   ScheduleReloadTask(
       base::TimeDelta::FromMinutes(kFallbackReloadDelayMinutes));
 }
 
-void NatPolicy::ScheduleReloadTask(const base::TimeDelta& delay) {
-  DCHECK(OnPolicyThread());
+void PolicyWatcher::ScheduleReloadTask(const base::TimeDelta& delay) {
+  DCHECK(OnPolicyWatcherThread());
   task_runner_->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&NatPolicy::Reload, weak_factory_.GetWeakPtr()),
+      base::Bind(&PolicyWatcher::Reload, weak_factory_.GetWeakPtr()),
       delay);
 }
 
-bool NatPolicy::OnPolicyThread() const {
+bool PolicyWatcher::OnPolicyWatcherThread() const {
   return task_runner_->BelongsToCurrentThread();
 }
 
-void NatPolicy::UpdateNatPolicy(base::DictionaryValue* new_policy) {
-  DCHECK(OnPolicyThread());
-  bool new_nat_enabled_state = false;
-  if (!new_policy->HasKey(kNatPolicyName)) {
-    // If unspecified, the default value of this policy is true.
-    new_nat_enabled_state = true;
-  } else {
-    // Otherwise, try to parse the value and only change from false if we get
-    // a successful read.
-    base::Value* value;
-    if (new_policy->Get(kNatPolicyName, &value) &&
-        value->IsType(base::Value::TYPE_BOOLEAN)) {
-      CHECK(value->GetAsBoolean(&new_nat_enabled_state));
+void PolicyWatcher::UpdatePolicies(
+    const base::DictionaryValue* new_policies_raw) {
+  DCHECK(OnPolicyWatcherThread());
+
+  // Use default values for any missing policies.
+  scoped_ptr<base::DictionaryValue> new_policies =
+      AddDefaultValuesWhenNecessary(new_policies_raw);
+
+  // Find the changed policies.
+  scoped_ptr<base::DictionaryValue> changed_policies(
+      new base::DictionaryValue());
+  base::DictionaryValue::Iterator iter(*new_policies);
+  while (iter.HasNext()) {
+    base::Value* old_policy;
+    if (!(old_policies_->Get(iter.key(), &old_policy) &&
+          old_policy->Equals(&iter.value()))) {
+      changed_policies->Set(iter.key(), iter.value().DeepCopy());
     }
+    iter.Advance();
   }
 
-  if (!first_state_published_ ||
-      (new_nat_enabled_state != current_nat_enabled_state_)) {
-    first_state_published_ = true;
-    current_nat_enabled_state_ = new_nat_enabled_state;
-    nat_enabled_cb_.Run(current_nat_enabled_state_);
+  // Save the new policies.
+  old_policies_.swap(new_policies);
+
+  // Notify our client of the changed policies.
+  if (!changed_policies->empty()) {
+    policy_callback_.Run(changed_policies.Pass());
   }
 }