 Chromium Code Reviews
 Chromium Code Reviews Issue 2867223003:
  Send a message to the client if bad It2Me policies are read.  (Closed)
    
  
    Issue 2867223003:
  Send a message to the client if bad It2Me policies are read.  (Closed) 
  | Index: remoting/host/it2me/it2me_native_messaging_host.cc | 
| diff --git a/remoting/host/it2me/it2me_native_messaging_host.cc b/remoting/host/it2me/it2me_native_messaging_host.cc | 
| index 043795f568c04b943fe7781fd25df34d59c23025..dfb2a1cd4200e68beb1efe03c9b836fb67046731 100644 | 
| --- a/remoting/host/it2me/it2me_native_messaging_host.cc | 
| +++ b/remoting/host/it2me/it2me_native_messaging_host.cc | 
| @@ -61,22 +61,22 @@ 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 | 
| @@ -94,13 +94,14 @@ It2MeNativeMessagingHost::It2MeNativeMessagingHost( | 
| 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() { | 
| @@ -314,11 +315,22 @@ void It2MeNativeMessagingHost::ProcessConnect( | 
| } | 
| #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)); | 
| @@ -385,6 +397,15 @@ void It2MeNativeMessagingHost::SendErrorAndExit( | 
| 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) { | 
| @@ -498,6 +519,25 @@ void It2MeNativeMessagingHost::OnPolicyUpdate( | 
| } | 
| } | 
| +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( |