[Chromoting] Add a simple Windows app that registers and starts a host.

remoting/host/setup/host_starter.cc

 // Copyright (c) 2012 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/setup/host_starter.h"
 
 #include "base/values.h"
 #include "crypto/random.h"
 #include "google_apis/google_api_keys.h"
 #include "remoting/host/pin_hash.h"
 #include "remoting/host/setup/oauth_helper.h"
 
 namespace {
 
 void AppendByte(std::string& s, uint8 byte) {
   const char alphabet[] = "0123456789abcdef";
   s.push_back(alphabet[byte / 16]);
   s.push_back(alphabet[byte & 15]);
 }
 
 std::string MakeHostId() {

[Sergey Ulanov, 2012/10/15 23:44:26]

I think I already asked about it in a previous CL: Why can't we reuse
base::GenerateGUID() here? If you don't wan't to rely on GUID generation on
windows, you can still reuse RandomDataToGUIDString() to format GUIDs.

[simonmorris, 2012/10/16 17:23:03]

I'll make such a change in a separate CL: it doesn't belong in this one.

   static const int random_byte_num = 16;
   static const unsigned int id_len = 36;
   scoped_array<uint8> random_bytes(new uint8[random_byte_num]);
   crypto::RandBytes(&random_bytes[0], random_byte_num);
   std::string id;
   id.reserve(id_len);
   int byte_count = 0;
   for (int i = 0; i < 4; i++) {
     AppendByte(id, random_bytes[byte_count++]);
   }
@@ skipping 54 matching lines @@
       new remoting::GaiaUserEmailFetcher(url_request_context_getter));
   scoped_ptr<remoting::ServiceClient> service_client(
       new remoting::ServiceClient(url_request_context_getter));
   scoped_ptr<remoting::DaemonController> daemon_controller(
       remoting::DaemonController::Create());
   return scoped_ptr<HostStarter>(
       new HostStarter(oauth_client.Pass(), user_email_fetcher.Pass(),
                       service_client.Pass(), daemon_controller.Pass()));
 }
 
-void HostStarter::StartHost(const std::string& host_name,
-                            const std::string& host_pin,
-                            bool consent_to_data_collection,
-                            const std::string& auth_code,
-                            CompletionCallback on_done) {
+void HostStarter::StartHost(
+    const std::string& host_name, const std::string& host_pin,

[Sergey Ulanov, 2012/10/15 23:44:26]

nit: One argument per line please
(http://dev.chromium.org/developers/coding-style)

[simonmorris, 2012/10/16 17:23:03]

Done.

+    bool consent_to_data_collection, const std::string& auth_code,
+    CompletionCallback on_done,
+    scoped_refptr<base::SingleThreadTaskRunner> on_done_runner) {
   if (in_progress_) {
     on_done.Run(START_IN_PROGRESS);
     return;
   }
   in_progress_ = true;
   host_name_ = host_name;
   host_pin_ = host_pin;
   consent_to_data_collection_ = consent_to_data_collection;
   on_done_ = on_done;
+  on_done_runner_ = on_done_runner;

[Sergey Ulanov, 2012/10/15 23:44:26]

Instead of passing on_done_runner here it would be better to remember the thread
we are called from (i.e. base::CurrentThreadRunnerHandle::Get()) so that the
called expects the callback on the same thread on which it calls StartHost().

Alternatively you can pass the main thread in the constructor, and then DCHECK
here that we are called on the right thread.

[simonmorris, 2012/10/16 17:23:03]

Done.

   // Map the authorization code to refresh and access tokens.
   oauth_client_->GetTokensFromAuthCode(oauth_client_info_, auth_code,
                                        kMaxGetTokensRetries, this);
 }
 
 void HostStarter::OnGetTokensResponse(
     const std::string& refresh_token,
     const std::string& access_token,
     int expires_in_seconds) {
   refresh_token_ = refresh_token;

[Sergey Ulanov, 2012/10/15 23:44:26]

Here and in all other callback methods: please add DCHECKs to verify that we are
on the right thread (i.e. main UI thread).

[simonmorris, 2012/10/16 17:23:03]

Many of the other callback methods are called from the network thread. I've
added code to post them onto the UI thread.

   access_token_ = access_token;
   // Get the email corresponding to the access token.
   user_email_fetcher_->GetUserEmail(access_token_, this);
 }
 
 void HostStarter::OnGetUserEmailResponse(const std::string& user_email) {
   user_email_ = user_email;
   // Register the host.
   host_id_ = MakeHostId();
   key_pair_.Generate();
@@ skipping 11 matching lines @@
   config->SetString("host_name", host_name_);
   config->SetString("private_key", key_pair_.GetAsString());
   config->SetString("host_secret_hash", host_secret_hash);
   daemon_controller_->SetConfigAndStart(
       config.Pass(), consent_to_data_collection_,
       base::Bind(&HostStarter::OnHostStarted,
                  base::Unretained(this)));
 }
 
 void HostStarter::OnHostStarted(DaemonController::AsyncResult result) {
-  on_done_.Run(
-      (result == DaemonController::RESULT_OK) ? START_COMPLETE : START_ERROR);
+  Result done_result = START_ERROR;
+  if (result == DaemonController::RESULT_OK)
+    done_result = START_COMPLETE;
+  on_done_runner_->PostTask(FROM_HERE, base::Bind(on_done_, done_result));
   // TODO(simonmorris): Unregister the host if we didn't start it.
   in_progress_ = false;

[Sergey Ulanov, 2012/10/15 23:44:26]

This method is called on the thread that daemon controller creates internally
and it is different from UI thread, so there is a potential race condition when
you mutate the object here. It would be better to jump back to the UI thread
first and only then do everything else.

[simonmorris, 2012/10/16 17:23:03]

Done.

 }
 
 void HostStarter::OnOAuthError() {
   on_done_.Run(OAUTH_ERROR);
   in_progress_ = false;
 }
 
 void HostStarter::OnNetworkError(int response_code) {
   on_done_.Run(NETWORK_ERROR);
   in_progress_ = false;
 }
 
 void HostStarter::OnRefreshTokenResponse(
     const std::string& access_token,
     int expires_in_seconds) {
   NOTREACHED();
   on_done_.Run(OAUTH_ERROR);
   in_progress_ = false;
 }
 
 }  // namespace remoting