Remove policy watching from It2MeHost.

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 302 matching lines @@
     SendErrorAndExit(std::move(response),
                      "'directoryBotJid' not found in request.");
     return;
   }
 #endif  // !defined(NDEBUG)
 
   // Create the It2Me host and start connecting.
   it2me_host_ = factory_->CreateIt2MeHost(
       host_context_->Copy(), policy_service_, weak_ptr_,
       std::move(signal_strategy), username, directory_bot_jid);
+  it2me_host_->OnPolicyUpdate(policy_watcher_->GetCurrentPolicies());
   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 129 matching lines @@
 }
 
 /* static */
 std::string It2MeNativeMessagingHost::HostStateToString(
     It2MeHostState host_state) {
   return ValueToName(kIt2MeHostStates, host_state);
 }
 
 void It2MeNativeMessagingHost::OnPolicyUpdate(
     std::unique_ptr<base::DictionaryValue> policies) {
-  if (policy_received_) {
-    // Don't dynamically change how the host operates since we don't have a good
-    // way to communicate changes to the user.
-    return;
+  // Don't dynamically change the elevation status since we don't have a good
+  // way to communicate changes to the user.

[rkjnsn, 2017/05/03 21:31:47]

Out of curiosity, in most places we try to enforce policy changes immediately,
even if it requires disconnecting the sessions and restarting the host. What
makes this one different?

[Jamie, 2017/05/03 23:41:10]

I'm not sure (Joe might be able to shed some light on it). I can always fix that
up as part of the CL that implements policy error handling.

[joedow, 2017/05/04 17:24:22]

Not all policy changes force a restart (for instance the Me2Me host will restart
for auth related policies but not others (OnUsernamePolicyUpdate for example)).

IIRC my thought was that this was something that would get enabled so the likely
transition would be from non-UiAccess to UiAccess.  Going the other way would
mean the remote user has UiAccess for the rest of the session, but that isn't a
big concern (in the future I would like to enable this regardless of policy).

[rkjnsn, 2017/05/04 20:45:45]

Isn't the native messaging host potentially long-running, though? That is, won't
the old policy potentially be used for multiple it2me sessions after having been
changed?

Either way, I'm not concerned about it for this CL, since the behavior is
unchanged.

+  if (!policy_received_) {
+    bool allow_elevated_host = false;
+    if (!policies->GetBoolean(
+            policy::key::kRemoteAccessHostAllowUiAccessForRemoteAssistance,
+            &allow_elevated_host)) {
+      LOG(WARNING) << "Failed to retrieve elevated host policy value.";
+    }
+#if defined(OS_WIN)
+    LOG(INFO) << "Allow UiAccess for Remote Assistance: "
+              << allow_elevated_host;
+#endif  // defined(OS_WIN)
+
+    policy_received_ = true;
+
+    // If |allow_elevated_host| is false, then we will fall back to using a host
+    // running in the current context regardless of the elevation request.  This
+    // may not be ideal, but is still functional.
+    needs_elevation_ = needs_elevation_ && allow_elevated_host;
+    if (!pending_connect_.is_null()) {
+      base::ResetAndReturn(&pending_connect_).Run();
+    }
   }
 
-  bool allow_elevated_host = false;
-  if (!policies->GetBoolean(
-          policy::key::kRemoteAccessHostAllowUiAccessForRemoteAssistance,
-          &allow_elevated_host)) {
-    LOG(WARNING) << "Failed to retrieve elevated host policy value.";
-  }
-#if defined(OS_WIN)
-  LOG(INFO) << "Allow UiAccess for Remote Assistance: " << allow_elevated_host;
-#endif  // defined(OS_WIN)
-
-  policy_received_ = true;
-
-  // If |allow_elevated_host| is false, then we will fall back to using a host
-  // running in the current context regardless of the elevation request.  This
-  // may not be ideal, but is still functional.
-  needs_elevation_ = needs_elevation_ && allow_elevated_host;
-  if (!pending_connect_.is_null()) {
-    base::ResetAndReturn(&pending_connect_).Run();
+  if (it2me_host_.get()) {
+    it2me_host_->OnPolicyUpdate(std::move(policies));
   }
 }
 
 #if defined(OS_WIN)
 
 bool It2MeNativeMessagingHost::DelegateToElevatedHost(
     std::unique_ptr<base::DictionaryValue> message) {
   DCHECK(task_runner()->BelongsToCurrentThread());
   DCHECK(needs_elevation_);
 
@@ skipping 24 matching lines @@
 
 bool It2MeNativeMessagingHost::DelegateToElevatedHost(
     std::unique_ptr<base::DictionaryValue> message) {
   NOTREACHED();
   return false;
 }
 
 #endif  // !defined(OS_WIN)
 
 }  // namespace remoting