From 761cc1ae378ccc5c459cf9e3c52e4540c193ccd0 Mon Sep 17 00:00:00 2001
From: "lambroslambrou@chromium.org"
 <lambroslambrou@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 28 Aug 2013 00:24:58 +0000
Subject: Fix false "network error" from start-host tool.

Also log process-launching failures when running start-host (or using
web-app host setup flow).

BUG=274817

Review URL: https://chromiumcodereview.appspot.com/22849020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@219870 0039d316-1c4b-4281-b951-d872f2087c98
---
 remoting/host/setup/daemon_controller_linux.cc |  6 ++++++
 remoting/host/setup/host_starter.cc            | 20 +++++++++++++++-----
 remoting/host/setup/host_starter.h             |  5 +++++
 remoting/host/setup/service_client.cc          |  5 ++++-
 4 files changed, 30 insertions(+), 6 deletions(-)

(limited to 'remoting')

diff --git a/remoting/host/setup/daemon_controller_linux.cc b/remoting/host/setup/daemon_controller_linux.cc
index e2d6566..3d45885 100644
--- a/remoting/host/setup/daemon_controller_linux.cc
+++ b/remoting/host/setup/daemon_controller_linux.cc
@@ -110,10 +110,12 @@ static bool RunHostScriptWithTimeout(
   // As long as we're relying on running an external binary from the
   // PATH, don't do it as root.
   if (getuid() == 0) {
+    LOG(ERROR) << "Refusing to run script as root.";
     return false;
   }
   base::FilePath script_path;
   if (!GetScriptPath(&script_path)) {
+    LOG(ERROR) << "GetScriptPath() failed.";
     return false;
   }
   CommandLine command_line(script_path);
@@ -130,11 +132,15 @@ static bool RunHostScriptWithTimeout(
   base::LaunchOptions options;
   options.fds_to_remap = &fds_to_remap;
   if (!base::LaunchProcess(command_line, options, &process_handle)) {
+    LOG(ERROR) << "Failed to run command: "
+               << command_line.GetCommandLineString();
     return false;
   }
 
   if (!base::WaitForExitCodeWithTimeout(process_handle, exit_code, timeout)) {
     base::KillProcess(process_handle, 0, false);
+    LOG(ERROR) << "Timeout exceeded for command: "
+               << command_line.GetCommandLineString();
     return false;
   }
 
diff --git a/remoting/host/setup/host_starter.cc b/remoting/host/setup/host_starter.cc
index 3a0646b..6758f58 100644
--- a/remoting/host/setup/host_starter.cc
+++ b/remoting/host/setup/host_starter.cc
@@ -25,6 +25,7 @@ HostStarter::HostStarter(
       service_client_(service_client.Pass()),
       daemon_controller_(daemon_controller.Pass()),
       consent_to_data_collection_(false),
+      unregistering_host_(false),
       weak_ptr_factory_(this),
       weak_ptr_(weak_ptr_factory_.GetWeakPtr()) {
   main_task_runner_ = base::ThreadTaskRunnerHandle::Get();
@@ -178,14 +179,13 @@ void HostStarter::OnHostStarted(DaemonController::AsyncResult result) {
     return;
   }
   if (result != DaemonController::RESULT_OK) {
+    unregistering_host_ = true;
     service_client_->UnregisterHost(host_id_, access_token_, this);
     return;
   }
-  Result done_result = (result == DaemonController::RESULT_OK) ?
-      START_COMPLETE : START_ERROR;
   CompletionCallback cb = on_done_;
   on_done_.Reset();
-  cb.Run(done_result);
+  cb.Run(START_COMPLETE);
 }
 
 void HostStarter::OnOAuthError() {
@@ -196,7 +196,12 @@ void HostStarter::OnOAuthError() {
   }
   CompletionCallback cb = on_done_;
   on_done_.Reset();
-  cb.Run(OAUTH_ERROR);
+  if (unregistering_host_) {
+    LOG(ERROR) << "OAuth error occurred when unregistering host.";
+    cb.Run(START_ERROR);
+  } else {
+    cb.Run(OAUTH_ERROR);
+  }
 }
 
 void HostStarter::OnNetworkError(int response_code) {
@@ -207,7 +212,12 @@ void HostStarter::OnNetworkError(int response_code) {
   }
   CompletionCallback cb = on_done_;
   on_done_.Reset();
-  cb.Run(NETWORK_ERROR);
+  if (unregistering_host_) {
+    LOG(ERROR) << "Network error occurred when unregistering host.";
+    cb.Run(START_ERROR);
+  } else {
+    cb.Run(NETWORK_ERROR);
+  }
 }
 
 void HostStarter::OnHostUnregistered() {
diff --git a/remoting/host/setup/host_starter.h b/remoting/host/setup/host_starter.h
index 3bb2e3e..ef97d21 100644
--- a/remoting/host/setup/host_starter.h
+++ b/remoting/host/setup/host_starter.h
@@ -91,6 +91,11 @@ class HostStarter : public gaia::GaiaOAuthClient::Delegate,
   std::string host_id_;
   bool use_service_account_;
 
+  // True if the host was not started and unregistration was requested. If this
+  // is set and a network/OAuth error occurs during unregistration, this will
+  // be logged, but the error will still be reported as START_ERROR.
+  bool unregistering_host_;
+
   base::WeakPtrFactory<HostStarter> weak_ptr_factory_;
   base::WeakPtr<HostStarter> weak_ptr_;
 
diff --git a/remoting/host/setup/service_client.cc b/remoting/host/setup/service_client.cc
index 215b04e..43f24ca 100644
--- a/remoting/host/setup/service_client.cc
+++ b/remoting/host/setup/service_client.cc
@@ -134,7 +134,10 @@ void ServiceClient::Core::HandleResponse(const net::URLFetcher* source) {
     delegate_->OnOAuthError();
     return;
   }
-  if (source->GetResponseCode() == net::HTTP_OK) {
+
+  // Treat codes 2xx as successful; for example, HTTP_NO_CONTENT (204) can be
+  // returned from a DELETE_REQUEST.
+  if (source->GetResponseCode() / 100 == 2) {
     switch (old_type) {
       case PENDING_REQUEST_NONE:
         break;
-- 
cgit v1.1