Reporting of policy errors via host-offline-reason: part 1

remoting/host/remoting_me2me_host.cc

Index: remoting/host/remoting_me2me_host.cc
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 6aa6b1e27eefdfead7a33b6ab89920cf4f6ba5cc..464055aec82de9bfcedf5b9b5f507d6d4c5dbc56 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -234,6 +234,7 @@ class HostProcess
 
   // Handles policy updates, by calling On*PolicyUpdate methods.
   void OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies);
+  void OnPolicyError();
   void ApplyHostDomainPolicy();
   void ApplyUsernamePolicy();
   bool OnHostDomainPolicyUpdate(base::DictionaryValue* policies);
@@ -527,7 +528,8 @@ void HostProcess::OnConfigUpdated(
     policy_watcher_.reset(
         policy_hack::PolicyWatcher::Create(context_->file_task_runner()));
     policy_watcher_->StartWatching(
-        base::Bind(&HostProcess::OnPolicyUpdate, base::Unretained(this)));
+        base::Bind(&HostProcess::OnPolicyUpdate, base::Unretained(this)),
+        base::Bind(&HostProcess::OnPolicyError, base::Unretained(this)));
   } else {
     // Reapply policies that could be affected by a new config.
     ApplyHostDomainPolicy();
@@ -623,6 +625,10 @@ void HostProcess::CreateAuthenticatorFactory() {
     // TODO(rmsousa): If the policy is bad the host should not go online. It
     // should keep running, but not connected, until the policies are fixed.
     // Having it show up as online and then reject all clients is misleading.
+    // TODO(lukasza): code review please: the new ShutdownHost call below seems

[Lambros, 2014/11/13 00:30:27]

Please remove this. When requesting a code review, you can add your own inline
comments using the Rietveld web UI, and reviewers will see these comments in the
request.

[Łukasz Anforowicz, 2014/11/13 17:48:09]

Done.

+    //                to match the TODO from rmsousa above (at least in spirit)
+    //                If this makes sense, then I can add this call in the next
+    //                check-in.
     LOG(ERROR) << "One of the third-party token URLs is empty or invalid. "
                << "Host will reject all clients until policies are corrected. "
                << "TokenUrl: " << third_party_auth_config_.token_url << ", "
@@ -932,6 +938,15 @@ void HostProcess::OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies) {
   }
 }
 
+void HostProcess::OnPolicyError() {
+  context_->network_task_runner()->PostTask(
+      FROM_HERE,
+      base::Bind(

[Lambros, 2014/11/13 00:30:27]

Formatting is a bit odd - see examples elsewhere, or use clang-format. Lines
below should be +4 indented, not +2. And some stuff could be collapsed onto the
previous line.

[Łukasz Anforowicz, 2014/11/13 17:48:09]

Ok to +4 rather than +2.  Let's discuss the other things - I think single value
or name-of-called-method per line is very readable, but if there is some kind of
policy for keeping things short, I can collapse into less lines.

+        &HostProcess::ShutdownHost,
+        this,
+        kInvalidHostConfigurationExitCode));
+}
+
 void HostProcess::ApplyHostDomainPolicy() {
   HOST_LOG << "Policy sets host domain: " << host_domain_;