Using PolicyServiceWatcher instead of PolicyWatcherLinux/Win/Mac.

remoting/host/policy_hack/policy_watcher.h

Index: remoting/host/policy_hack/policy_watcher.h
diff --git a/remoting/host/policy_hack/policy_watcher.h b/remoting/host/policy_hack/policy_watcher.h
index dbbfd2310fe8104d7541da7b41b61750343ad4a1..78b4e043abb96d43ea355e246b5da82c149ce6ec 100644
--- a/remoting/host/policy_hack/policy_watcher.h
+++ b/remoting/host/policy_hack/policy_watcher.h
@@ -8,7 +8,6 @@
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "base/values.h"
-#include "components/policy/core/common/policy_service.h"
 
 namespace base {
 class SingleThreadTaskRunner;
@@ -16,6 +15,10 @@ class TimeDelta;
 class WaitableEvent;
 }  // namespace base
 
+namespace policy {
+class PolicyService;
+}  // namespace policy
+
 namespace remoting {
 namespace policy_hack {
 
@@ -28,11 +31,17 @@ class PolicyWatcher {
   typedef base::Callback<void(scoped_ptr<base::DictionaryValue>)>
       PolicyUpdatedCallback;
 
+  // TODO(lukasza): PolicyErrorCallback never gets called by
+  // PolicyServiceWatcher.  Need to either 1) remove error-handling from
+  // PolicyWatcher or 2) add error-handling around PolicyService
+  // 2a) Add policy name/type validation via policy::Schema::Normalize.
+  // 2b) Consider exposing parsing errors from policy::ConfigDirPolicyLoader.
+
   // Called after detecting malformed policies.
   typedef base::Callback<void()> PolicyErrorCallback;
 
   explicit PolicyWatcher(
-      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);

[Mattias Nissler (ping if slow), 2015/01/06 09:06:12]

Again, it would make sense to document and/or rename |task_runner| to make clear
what it's used for.

[Łukasz Anforowicz, 2015/01/07 17:54:15]

I tried to document this via comments.  I renamed |task_runner| to
|policy_service_task_runner| inside policy_service_watcher.cc, but I wasn't sure
if it makes sense here (please read through the new comments in policy_watcher.h
- the meaning of task_runner varies depending on usage context [which is
probably bad, but I am just preserving/documenting the existing behavior;  otoh,
we can discuss fixing this in a separate changelist]). 

   virtual ~PolicyWatcher();
 
   // This guarantees that the |policy_updated_callback| is called at least once
@@ -59,12 +68,9 @@ class PolicyWatcher {
   virtual void StopWatching(const base::Closure& stopped_callback);
 
   // Implemented by each platform.  |task_runner| should be an IO message loop.
-  // |policy_service| is currently only used on ChromeOS.  The caller must
-  // ensure that |policy_service| remains valid for the lifetime of
-  // PolicyWatcher.
   static scoped_ptr<PolicyWatcher> Create(
       policy::PolicyService* policy_service,
-      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
 
   // The name of the NAT traversal policy.
   static const char kNatPolicyName[];
@@ -127,11 +133,6 @@ class PolicyWatcher {
   // The counter is reset whenever policy has been successfully read.
   void SignalTransientPolicyError();
 
-  // Used for time-based reloads in case something goes wrong with the
-  // notification system.
-  void ScheduleFallbackReloadTask();
-  void ScheduleReloadTask(const base::TimeDelta& delay);
-
   // Returns a DictionaryValue containing the default values for each policy.
   const base::DictionaryValue& Defaults() const;