Refactor the daemon controller so that the callbacks are called on the caller thread.

remoting/host/setup/daemon_controller_delegate_linux.cc

Index: remoting/host/setup/daemon_controller_delegate_linux.cc
diff --git a/remoting/host/setup/daemon_controller_linux.cc b/remoting/host/setup/daemon_controller_delegate_linux.cc
similarity index 55%
rename from remoting/host/setup/daemon_controller_linux.cc
rename to remoting/host/setup/daemon_controller_delegate_linux.cc
index 3d4588599c582f06f1352689da8d428262d05f2b..e1e982822a053ac21136173bb281e2aae679b5d8 100644
--- a/remoting/host/setup/daemon_controller_linux.cc
+++ b/remoting/host/setup/daemon_controller_delegate_linux.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "remoting/host/setup/daemon_controller.h"
+#include "remoting/host/setup/daemon_controller_delegate_linux.h"
 
 #include <unistd.h>
 
@@ -22,7 +22,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
-#include "base/threading/thread.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "net/base/net_util.h"
 #include "remoting/host/host_config.h"
@@ -44,6 +44,8 @@ const int64 kDaemonTimeoutMs = 60000;
 // Timeout for commands that require password prompt - 5 minutes.
 const int64 kSudoTimeoutSeconds = 5 * 60;
 
+void Dummy() {}

[Sergey Ulanov, 2013/09/09 18:12:31]

not used anywhere

[alexeypa (please no reviews), 2013/09/09 19:30:57]

Done.

+
 std::string GetMd5(const std::string& value) {
   base::MD5Context ctx;
   base::MD5Init(&ctx);
@@ -53,46 +55,13 @@ std::string GetMd5(const std::string& value) {
   return StringToLowerASCII(base::HexEncode(digest.a, sizeof(digest.a)));
 }
 
-class DaemonControllerLinux : public remoting::DaemonController {
- public:
-  DaemonControllerLinux();
-
-  virtual State GetState() OVERRIDE;
-  virtual void GetConfig(const GetConfigCallback& callback) OVERRIDE;
-  virtual void SetConfigAndStart(
-      scoped_ptr<base::DictionaryValue> config,
-      bool consent,
-      const CompletionCallback& done) OVERRIDE;
-  virtual void UpdateConfig(scoped_ptr<base::DictionaryValue> config,
-                            const CompletionCallback& done_callback) OVERRIDE;
-  virtual void Stop(const CompletionCallback& done_callback) OVERRIDE;
-  virtual void SetWindow(void* window_handle) OVERRIDE;
-  virtual void GetVersion(const GetVersionCallback& done_callback) OVERRIDE;
-  virtual void GetUsageStatsConsent(
-      const GetUsageStatsConsentCallback& done) OVERRIDE;
-
- private:
-  base::FilePath GetConfigPath();
-
-  void DoGetConfig(const GetConfigCallback& callback);
-  void DoSetConfigAndStart(scoped_ptr<base::DictionaryValue> config,
-                           const CompletionCallback& done);
-  void DoUpdateConfig(scoped_ptr<base::DictionaryValue> config,
-                      const CompletionCallback& done_callback);
-  void DoStop(const CompletionCallback& done_callback);
-  void DoGetVersion(const GetVersionCallback& done_callback);
-
-  base::Thread file_io_thread_;
-
-  DISALLOW_COPY_AND_ASSIGN(DaemonControllerLinux);
-};
-
-DaemonControllerLinux::DaemonControllerLinux()
-    : file_io_thread_("DaemonControllerFileIO") {
-  file_io_thread_.Start();
+base::FilePath GetConfigPath() {
+  std::string filename = "host#" + GetMd5(net::GetHostName()) + ".json";
+  return file_util::GetHomeDir().
+      Append(".config/chrome-remote-desktop").Append(filename);
 }
 
-static bool GetScriptPath(base::FilePath* result) {
+bool GetScriptPath(base::FilePath* result) {
   base::FilePath candidate_exe(kDaemonScript);
   if (access(candidate_exe.value().c_str(), X_OK) == 0) {
     *result = candidate_exe;
@@ -101,7 +70,7 @@ static bool GetScriptPath(base::FilePath* result) {
   return false;
 }
 
-static bool RunHostScriptWithTimeout(
+bool RunHostScriptWithTimeout(
     const std::vector<std::string>& args,
     base::TimeDelta timeout,
     int* exit_code) {
@@ -147,13 +116,21 @@ static bool RunHostScriptWithTimeout(
   return true;
 }
 
-static bool RunHostScript(const std::vector<std::string>& args,
+bool RunHostScript(const std::vector<std::string>& args,
                           int* exit_code) {
   return RunHostScriptWithTimeout(
       args, base::TimeDelta::FromMilliseconds(kDaemonTimeoutMs), exit_code);
 }
 
-remoting::DaemonController::State DaemonControllerLinux::GetState() {
+}  // namespace
+
+DaemonControllerDelegateLinux::DaemonControllerDelegateLinux() {
+}
+
+DaemonControllerDelegateLinux::~DaemonControllerDelegateLinux() {
+}
+
+DaemonController::State DaemonControllerDelegateLinux::GetState() {
   std::vector<std::string> args;
   args.push_back("--check-running");
   int exit_code = 0;
@@ -161,74 +138,20 @@ remoting::DaemonController::State DaemonControllerLinux::GetState() {
     // TODO(jamiewalch): When we have a good story for installing, return
     // NOT_INSTALLED rather than NOT_IMPLEMENTED (the former suppresses
     // the relevant UI in the web-app).
-    return remoting::DaemonController::STATE_NOT_IMPLEMENTED;
+    return DaemonController::STATE_NOT_IMPLEMENTED;
   }
 
   if (exit_code == 0) {
-    return remoting::DaemonController::STATE_STARTED;
+    return DaemonController::STATE_STARTED;
   } else {
-    return remoting::DaemonController::STATE_STOPPED;
+    return DaemonController::STATE_STOPPED;
   }
 }
 
-void DaemonControllerLinux::GetConfig(const GetConfigCallback& callback) {
-  // base::Unretained() is safe because we control lifetime of the thread.
-  file_io_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(
-      &DaemonControllerLinux::DoGetConfig, base::Unretained(this), callback));
-}
-
-void DaemonControllerLinux::GetUsageStatsConsent(
-    const GetUsageStatsConsentCallback& done) {
-  // Crash dump collection is not implemented on Linux yet.
-  // http://crbug.com/130678.
-  done.Run(false, false, false);
-}
-
-void DaemonControllerLinux::SetConfigAndStart(
-    scoped_ptr<base::DictionaryValue> config,
-    bool /* consent */,
-    const CompletionCallback& done) {
-  // base::Unretained() is safe because we control lifetime of the thread.
-  file_io_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(
-      &DaemonControllerLinux::DoSetConfigAndStart, base::Unretained(this),
-      base::Passed(&config), done));
-}
-
-void DaemonControllerLinux::UpdateConfig(
-    scoped_ptr<base::DictionaryValue> config,
-    const CompletionCallback& done_callback) {
-  file_io_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(
-      &DaemonControllerLinux::DoUpdateConfig, base::Unretained(this),
-      base::Passed(&config), done_callback));
-}
-
-void DaemonControllerLinux::Stop(const CompletionCallback& done_callback) {
-  file_io_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(
-      &DaemonControllerLinux::DoStop, base::Unretained(this),
-      done_callback));
-}
-
-void DaemonControllerLinux::SetWindow(void* window_handle) {
-  // noop
-}
-
-void DaemonControllerLinux::GetVersion(
-    const GetVersionCallback& done_callback) {
-  file_io_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(
-      &DaemonControllerLinux::DoGetVersion, base::Unretained(this),
-      done_callback));
-}
-
-base::FilePath DaemonControllerLinux::GetConfigPath() {
-  std::string filename = "host#" + GetMd5(net::GetHostName()) + ".json";
-  return file_util::GetHomeDir().
-      Append(".config/chrome-remote-desktop").Append(filename);
-}
-
-void DaemonControllerLinux::DoGetConfig(const GetConfigCallback& callback) {
+scoped_ptr<base::DictionaryValue> DaemonControllerDelegateLinux::GetConfig() {
   scoped_ptr<base::DictionaryValue> result(new base::DictionaryValue());
 
-  if (GetState() != remoting::DaemonController::STATE_NOT_IMPLEMENTED) {
+  if (GetState() != DaemonController::STATE_NOT_IMPLEMENTED) {
     JsonHostConfig config(GetConfigPath());
     if (config.Read()) {
       std::string value;
@@ -243,13 +166,13 @@ void DaemonControllerLinux::DoGetConfig(const GetConfigCallback& callback) {
     }
   }
 
-  callback.Run(result.Pass());
+  return result.Pass();
 }
 
-void DaemonControllerLinux::DoSetConfigAndStart(
+void DaemonControllerDelegateLinux::SetConfigAndStart(
     scoped_ptr<base::DictionaryValue> config,
-    const CompletionCallback& done_callback) {
-
+    bool consent,
+    const DaemonController::CompletionCallback& done) {
   // Add the user to chrome-remote-desktop group first.
   std::vector<std::string> args;
   args.push_back("--add-user");
@@ -259,7 +182,7 @@ void DaemonControllerLinux::DoSetConfigAndStart(
           &exit_code) ||
       exit_code != 0) {
     LOG(ERROR) << "Failed to add user to chrome-remote-desktop group.";
-    done_callback.Run(RESULT_FAILED);
+    done.Run(DaemonController::RESULT_FAILED);
     return;
   }
 
@@ -268,7 +191,7 @@ void DaemonControllerLinux::DoSetConfigAndStart(
   if (!base::DirectoryExists(config_dir) &&
       !file_util::CreateDirectory(config_dir)) {
     LOG(ERROR) << "Failed to create config directory " << config_dir.value();
-    done_callback.Run(RESULT_FAILED);
+    done.Run(DaemonController::RESULT_FAILED);
     return;
   }
 
@@ -277,66 +200,62 @@ void DaemonControllerLinux::DoSetConfigAndStart(
   if (!config_file.CopyFrom(config.get()) ||
       !config_file.Save()) {
     LOG(ERROR) << "Failed to update config file.";
-    done_callback.Run(RESULT_FAILED);
+    done.Run(DaemonController::RESULT_FAILED);
     return;
   }
 
   // Finally start the host.
   args.clear();
   args.push_back("--start");
-  AsyncResult result;
-  if (RunHostScript(args, &exit_code)) {
-    result = (exit_code == 0) ? RESULT_OK : RESULT_FAILED;
-  } else {
-    result = RESULT_FAILED;
-  }
-  done_callback.Run(result);
+  DaemonController::AsyncResult result = DaemonController::RESULT_FAILED;
+  if (RunHostScript(args, &exit_code) && (exit_code == 0))
+    result = DaemonController::RESULT_OK;
+
+  done.Run(result);
 }
 
-void DaemonControllerLinux::DoUpdateConfig(
+void DaemonControllerDelegateLinux::UpdateConfig(
     scoped_ptr<base::DictionaryValue> config,
-    const CompletionCallback& done_callback) {
+    const DaemonController::CompletionCallback& done) {
   JsonHostConfig config_file(GetConfigPath());
   if (!config_file.Read() ||
       !config_file.CopyFrom(config.get()) ||
       !config_file.Save()) {
     LOG(ERROR) << "Failed to update config file.";
-    done_callback.Run(RESULT_FAILED);
+    done.Run(DaemonController::RESULT_FAILED);
     return;
   }
 
   std::vector<std::string> args;
   args.push_back("--reload");
-  AsyncResult result;
-  int exit_code;
-  if (RunHostScript(args, &exit_code)) {
-    result = (exit_code == 0) ? RESULT_OK : RESULT_FAILED;
-  } else {
-    result = RESULT_FAILED;
-  }
+  int exit_code = 0;
+  DaemonController::AsyncResult result = DaemonController::RESULT_FAILED;
+  if (RunHostScript(args, &exit_code) && (exit_code == 0))
+    result = DaemonController::RESULT_OK;
 
-  done_callback.Run(result);
+  done.Run(result);
 }
 
-void DaemonControllerLinux::DoStop(const CompletionCallback& done_callback) {
+void DaemonControllerDelegateLinux::Stop(
+    const DaemonController::CompletionCallback& done) {
   std::vector<std::string> args;
   args.push_back("--stop");
   int exit_code = 0;
-  AsyncResult result;
-  if (RunHostScript(args, &exit_code)) {
-    result = (exit_code == 0) ? RESULT_OK : RESULT_FAILED;
-  } else {
-    result = RESULT_FAILED;
-  }
-  done_callback.Run(result);
+  DaemonController::AsyncResult result = DaemonController::RESULT_FAILED;
+  if (RunHostScript(args, &exit_code) && (exit_code == 0))
+    result = DaemonController::RESULT_OK;
+
+  done.Run(result);
 }
 
-void DaemonControllerLinux::DoGetVersion(
-    const GetVersionCallback& done_callback) {
+void DaemonControllerDelegateLinux::SetWindow(void* window_handle) {
+  // noop
+}
+
+std::string DaemonControllerDelegateLinux::GetVersion() {
   base::FilePath script_path;
   if (!GetScriptPath(&script_path)) {
-    done_callback.Run(std::string());
-    return;
+    return std::string();
   }
   CommandLine command_line(script_path);
   command_line.AppendArg("--host-version");
@@ -348,24 +267,33 @@ void DaemonControllerLinux::DoGetVersion(
   if (!result || exit_code != 0) {
     LOG(ERROR) << "Failed to run \"" << command_line.GetCommandLineString()
                << "\". Exit code: " << exit_code;
-    done_callback.Run(std::string());
-    return;
+    return std::string();
   }
 
   TrimWhitespaceASCII(version, TRIM_ALL, &version);
   if (!ContainsOnlyChars(version, "0123456789.")) {
     LOG(ERROR) << "Received invalid host version number: " << version;
-    done_callback.Run(std::string());
-    return;
+    return std::string();
   }
 
-  done_callback.Run(version);
+  return version;
 }
 
-}  // namespace
+DaemonController::UsageStatsConsent
+DaemonControllerDelegateLinux::GetUsageStatsConsent() {
+  // Crash dump collection is not implemented on Linux yet.
+  // http://crbug.com/130678.
+  DaemonController::UsageStatsConsent consent;
+  consent.supported = false;
+  consent.allowed = false;
+  consent.set_by_policy = false;
+  return consent;
+}
 
-scoped_ptr<DaemonController> remoting::DaemonController::Create() {
-  return scoped_ptr<DaemonController>(new DaemonControllerLinux());
+scoped_refptr<DaemonController> DaemonController::Create() {
+  scoped_ptr<DaemonController::Delegate> delegate(
+      new DaemonControllerDelegateLinux());
+  return new DaemonController(delegate.Pass());
 }
 
 }  // namespace remoting