Send a message to the client if bad It2Me policies are read.

remoting/host/it2me/it2me_native_messaging_host.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/host/it2me/it2me_native_messaging_host.h"
 
 #include <memory>
 #include <string>
 #include <utility>
 
@@ skipping 43 matching lines @@
     {kConnecting, "CONNECTING"},
 };
 
 #if defined(OS_WIN)
 const base::FilePath::CharType kBaseHostBinaryName[] =
     FILE_PATH_LITERAL("remote_assistance_host.exe");
 const base::FilePath::CharType kElevatedHostBinaryName[] =
     FILE_PATH_LITERAL("remote_assistance_host_uiaccess.exe");
 #endif  // defined(OS_WIN)
 
-// Helper function to run |callback| on the correct thread using |task_runner|.
+// Helper functions to run |callback| asynchronously on the correct thread
+// using |task_runner|.
 void PolicyUpdateCallback(
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
     remoting::PolicyWatcher::PolicyUpdatedCallback callback,
     std::unique_ptr<base::DictionaryValue> policies) {
   DCHECK(!callback.is_null());
-
-  // Always post the task so the execution is consistent (always asynchronous).
   task_runner->PostTask(FROM_HERE,
                         base::Bind(callback, base::Passed(&policies)));
 }
 
-// Called when malformed policies are detected.
-void OnPolicyError() {
-  // TODO(joedow): Report the policy error to the user.  crbug.com/433009
-  NOTIMPLEMENTED();
+void PolicyErrorCallback(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    remoting::PolicyWatcher::PolicyErrorCallback callback) {
+  DCHECK(!callback.is_null());

[joedow, 2017/05/09 18:09:47]

nit: DCHECK(callback);

[Jamie, 2017/05/09 18:19:33]

Done, here and above.

+  task_runner->PostTask(FROM_HERE, callback);
 }
 
 }  // namespace
 
 It2MeNativeMessagingHost::It2MeNativeMessagingHost(
     bool needs_elevation,
     std::unique_ptr<PolicyWatcher> policy_watcher,
     std::unique_ptr<ChromotingHostContext> context,
     std::unique_ptr<It2MeHostFactory> factory)
     : needs_elevation_(needs_elevation),
       host_context_(std::move(context)),
       factory_(std::move(factory)),
       policy_watcher_(std::move(policy_watcher)),
       weak_factory_(this) {
   weak_ptr_ = weak_factory_.GetWeakPtr();
 
   // The policy watcher runs on the |file_task_runner| but we want to run the
-  // update code on |task_runner| so we use a shim to post the callback to the
-  // preferred task runner.
+  // callbacks on |task_runner| so we use a shim to post them to it.
   PolicyWatcher::PolicyUpdatedCallback update_callback =
       base::Bind(&It2MeNativeMessagingHost::OnPolicyUpdate, weak_ptr_);
+  PolicyWatcher::PolicyErrorCallback error_callback =
+      base::Bind(&It2MeNativeMessagingHost::OnPolicyError, weak_ptr_);
   policy_watcher_->StartWatching(
       base::Bind(&PolicyUpdateCallback, task_runner(), update_callback),
-      base::Bind(&OnPolicyError));
+      base::Bind(&PolicyErrorCallback, task_runner(), error_callback));
 }
 
 It2MeNativeMessagingHost::~It2MeNativeMessagingHost() {
   DCHECK(task_runner()->BelongsToCurrentThread());
 
   if (it2me_host_.get()) {
     it2me_host_->Disconnect();
     it2me_host_ = nullptr;
   }
 }
@@ skipping 193 matching lines @@
   std::string directory_bot_jid = service_urls->directory_bot_jid();
 
 #if !defined(NDEBUG)
   if (!message->GetString("directoryBotJid", &directory_bot_jid)) {
     SendErrorAndExit(std::move(response),
                      "'directoryBotJid' not found in request.");
     return;
   }
 #endif  // !defined(NDEBUG)
 
+  std::unique_ptr<base::DictionaryValue> policies =
+      policy_watcher_->GetCurrentPolicies();
+  if (policies->size() == 0) {
+    // At this point policies have been read, so if there are none set then
+    // it indicates an error. Since this can be fixed by end users it has a
+    // dedicated message type rather than the generic "error" so that the
+    // right error message can be displayed.
+    SendPolicyErrorAndExit();
+    return;
+  }
+
   // Create the It2Me host and start connecting.
   it2me_host_ = factory_->CreateIt2MeHost(host_context_->Copy(), weak_ptr_,
                                           std::move(signal_strategy), username,
                                           directory_bot_jid);
-  it2me_host_->OnPolicyUpdate(policy_watcher_->GetCurrentPolicies());
+  it2me_host_->OnPolicyUpdate(std::move(policies));
   it2me_host_->Connect();
 
   SendMessageToClient(std::move(response));
 }
 
 void It2MeNativeMessagingHost::ProcessDisconnect(
     std::unique_ptr<base::DictionaryValue> message,
     std::unique_ptr<base::DictionaryValue> response) {
   DCHECK(task_runner()->BelongsToCurrentThread());
   DCHECK(policy_received_);
@@ skipping 46 matching lines @@
   LOG(ERROR) << description;
 
   response->SetString("type", "error");
   response->SetString("description", description);
   SendMessageToClient(std::move(response));
 
   // Trigger a host shutdown by sending an empty message.
   client_->CloseChannel(std::string());
 }
 
+void It2MeNativeMessagingHost::SendPolicyErrorAndExit() const {
+  DCHECK(task_runner()->BelongsToCurrentThread());
+
+  auto message = base::MakeUnique<base::DictionaryValue>();
+  message->SetString("type", "policyError");
+  SendMessageToClient(std::move(message));
+  client_->CloseChannel(std::string());
+}
+
 void It2MeNativeMessagingHost::OnStateChanged(
     It2MeHostState state,
     const std::string& error_message) {
   DCHECK(task_runner()->BelongsToCurrentThread());
 
   state_ = state;
 
   std::unique_ptr<base::DictionaryValue> message(new base::DictionaryValue());
 
   message->SetString("type", "hostStateChanged");
@@ skipping 93 matching lines @@
     if (pending_connect_) {
       base::ResetAndReturn(&pending_connect_).Run();
     }
   }
 
   if (it2me_host_.get()) {
     it2me_host_->OnPolicyUpdate(std::move(policies));
   }
 }
 
+void It2MeNativeMessagingHost::OnPolicyError() {
+  LOG(ERROR) << "Malformed policies detected.";
+  policy_received_ = true;
+
+  if (it2me_host_) {
+    // If there is already a connection, close it and notify the webapp.
+    it2me_host_->Disconnect();
+    it2me_host_ = nullptr;
+    SendPolicyErrorAndExit();
+

[joedow, 2017/05/09 18:09:47]

nit: remove newline

[Jamie, 2017/05/09 18:19:33]

Done.

+  } else if (pending_connect_) {
+    // If there is no connection, run the pending connection callback if there
+    // is one, but otherwise do nothing. The policy error will be sent when a
+    // connection is made; doing so beforehand would break assumptions made by
+    // the Chrome app.
+    base::ResetAndReturn(&pending_connect_).Run();
+  }
+}
+
 #if defined(OS_WIN)
 
 bool It2MeNativeMessagingHost::DelegateToElevatedHost(
     std::unique_ptr<base::DictionaryValue> message) {
   DCHECK(task_runner()->BelongsToCurrentThread());
   DCHECK(needs_elevation_);
 
   if (!elevated_host_) {
     base::FilePath binary_path =
         base::CommandLine::ForCurrentProcess()->GetProgram();
@@ skipping 21 matching lines @@
 
 bool It2MeNativeMessagingHost::DelegateToElevatedHost(
     std::unique_ptr<base::DictionaryValue> message) {
   NOTREACHED();
   return false;
 }
 
 #endif  // !defined(OS_WIN)
 
 }  // namespace remoting