Update DaemonController interface to support changing host configuration.

Also cleaned up Start()/Stop() methods so that they don't return any
value - it will not be possible to return result synchronously anyway.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@129328 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 101 insertions, 63 deletions

diff --git a/remoting/host/plugin/daemon_controller.h b/remoting/host/plugin/daemon_controller.h
index b1d46e7..9144204 100644
--- a/remoting/host/plugin/daemon_controller.h
+++ b/remoting/host/plugin/daemon_controller.h
@@ -7,6 +7,13 @@
 
 #include <string>
 
+#include "base/callback_forward.h"
+#include "base/memory/scoped_ptr.h"
+
+namespace base {
+class DictionaryValue;
+}  // namespace base
+
 namespace remoting {
 
 class DaemonController {
@@ -38,29 +45,47 @@ class DaemonController {
     STATE_UNKNOWN = 4
   };
 
+  // The callback for GetConfig(). |config| is set to NULL in case of
+  // an error. Otherwise it is a dictionary that contains the
+  // following values: host_id and xmpp_login, which may be empty if
+  // the host is not initialized yet. The config must not contain any
+  // security sensitive information, such as authentication tokens and
+  // private keys.
+  typedef base::Callback<void (scoped_ptr<base::DictionaryValue> config)>
+      GetConfigCallback;
+
   virtual ~DaemonController() {}
 
   // Return the "installed/running" state of the daemon process.
+  //
+  // TODO(sergeyu): This method is called synchronously from the
+  // webapp. In most cases it requires IO operations, so it may block
+  // the user interface. Replace it with asynchronous notifications,
+  // e.g. with StartStateNotifications()/StopStateNotifications() methods.
   virtual State GetState() = 0;
 
-  // Set the PIN for accessing this host, which should be expressed as a
-  // UTF8-encoded string. It is permitted to call SetPin when the daemon
-  // is already running. Returns true if successful, or false if the PIN
-  // does not satisfy complexity requirements.
-  //
-  // TODO(jamiewalch): More state-setting methods needed here. Sufficient
-  // state must be set prior to calling any other DaemonController method;
-  // this should be documented for each method.
-  virtual bool SetPin(const std::string& pin) = 0;
+  // Queries current host configuration. The |callback| is called
+  // after configuration is read.
+  virtual void GetConfig(const GetConfigCallback& callback) = 0;
 
   // Start the daemon process. Since this may require that the daemon be
   // downloaded and installed, Start may return before the daemon process
   // is actually running--poll GetState until the state is STATE_STARTED
   // or STATE_START_FAILED.
   //
-  // Start fails synchronously and returns false if sufficient state has
-  // not been set, including a valid PIN.
-  virtual bool Start() = 0;
+  // TODO(sergeyu): This method writes config and starts the host -
+  // these two steps are merged for simplicity. Consider splitting it
+  // into SetConfig() and Start() once we have basic host setup flow
+  // working.
+  virtual void SetConfigAndStart(scoped_ptr<base::DictionaryValue> config) = 0;
+
+  // Set the PIN for accessing this host, which should be expressed as a
+  // UTF8-encoded string. It is permitted to call SetPin when the daemon
+  // is already running. Returns true if successful, or false if the PIN
+  // does not satisfy complexity requirements.
+  //
+  // TODO(sergeyu): Add callback to be called after PIN is updated.
+  virtual void SetPin(const std::string& pin) = 0;
 
   // Stop the daemon process. It is permitted to call Stop while the daemon
   // process is being installed, in which case the installation should be
@@ -68,10 +93,7 @@ class DaemonController {
   // daemon process is not started automatically upon successful installation.
   // As with Start, Stop may return before the operation is complete--poll
   // GetState until the state is STATE_STOPPED.
-  //
-  // Stop fails synchronously and returns false if sufficient state has not
-  // been set.
-  virtual bool Stop() = 0;
+  virtual void Stop() = 0;
 
   static DaemonController* Create();
 };
diff --git a/remoting/host/plugin/daemon_controller_linux.cc b/remoting/host/plugin/daemon_controller_linux.cc
index 28299ec..88d634e 100644
--- a/remoting/host/plugin/daemon_controller_linux.cc
+++ b/remoting/host/plugin/daemon_controller_linux.cc
@@ -27,9 +27,11 @@ class DaemonControllerLinux : public remoting::DaemonController {
   DaemonControllerLinux();
 
   virtual State GetState() OVERRIDE;
-  virtual bool SetPin(const std::string& pin) OVERRIDE;
-  virtual bool Start() OVERRIDE;
-  virtual bool Stop() OVERRIDE;
+  virtual void GetConfig(const GetConfigCallback& callback) OVERRIDE;
+  virtual void SetConfigAndStart(
+      scoped_ptr<base::DictionaryValue> config) OVERRIDE;
+  virtual void SetPin(const std::string& pin) OVERRIDE;
+  virtual void Stop() OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DaemonControllerLinux);
@@ -105,29 +107,36 @@ remoting::DaemonController::State DaemonControllerLinux::GetState() {
   }
 }
 
-bool DaemonControllerLinux::SetPin(const std::string& pin) {
+void DaemonControllerLinux::GetConfig(const GetConfigCallback& callback) {
+  NOTIMPLEMENTED();
+}
+
+void DaemonControllerLinux::SetConfigAndStart(
+    scoped_ptr<base::DictionaryValue> config) {
+  // TODO(sergeyu): Save the |config|.
+  // TODO(sergeyu): Set state to START_FAILED if RunScript() fails.
+  std::vector<std::string> no_args;
+  RunScript(no_args, NULL);
+}
+
+void DaemonControllerLinux::SetPin(const std::string& pin) {
   std::vector<std::string> args;
   args.push_back("--explicit-pin");
   args.push_back(pin);
   int exit_code = 0;
-  return RunScript(args, &exit_code) && exit_code == 0;
-}
-
-bool DaemonControllerLinux::Start() {
-  std::vector<std::string> no_args;
-  return RunScript(no_args, NULL);
+  RunScript(args, &exit_code);
 }
 
 // TODO(jamiewalch): If Stop is called after Start but before GetState returns
 // STARTED, then it should prevent the daemon from starting; currently it will
 // just fail silently.
-bool DaemonControllerLinux::Stop() {
+void DaemonControllerLinux::Stop() {
   std::vector<std::string> args;
   args.push_back("--stop");
   // TODO(jamiewalch): Make Stop asynchronous like start once there's UI in the
   // web-app to support it.
   int exit_code = 0;
-  return RunScript(args, &exit_code) && exit_code == 0;
+  RunScript(args, &exit_code);
 }
 
 }  // namespace
diff --git a/remoting/host/plugin/daemon_controller_mac.cc b/remoting/host/plugin/daemon_controller_mac.cc
index 1a8a2e0..0aa7f95 100644
--- a/remoting/host/plugin/daemon_controller_mac.cc
+++ b/remoting/host/plugin/daemon_controller_mac.cc
@@ -17,9 +17,11 @@ class DaemonControllerMac : public remoting::DaemonController {
   DaemonControllerMac();
 
   virtual State GetState() OVERRIDE;
-  virtual bool SetPin(const std::string& pin) OVERRIDE;
-  virtual bool Start() OVERRIDE;
-  virtual bool Stop() OVERRIDE;
+  virtual void GetConfig(const GetConfigCallback& callback) OVERRIDE;
+  virtual void SetConfigAndStart(
+      scoped_ptr<base::DictionaryValue> config) OVERRIDE;
+  virtual void SetPin(const std::string& pin) OVERRIDE;
+  virtual void Stop() OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DaemonControllerMac);
@@ -28,23 +30,25 @@ class DaemonControllerMac : public remoting::DaemonController {
 DaemonControllerMac::DaemonControllerMac() {
 }
 
-remoting::DaemonController::State DaemonControllerMac::GetState() {
-  return remoting::DaemonController::STATE_NOT_IMPLEMENTED;
+DaemonController::State DaemonControllerMac::GetState() {
+  return DaemonController::STATE_NOT_IMPLEMENTED;
 }
 
-bool DaemonControllerMac::SetPin(const std::string& pin) {
+void DaemonControllerMac::GetConfig(const GetConfigCallback& callback) {
   NOTIMPLEMENTED();
-  return false;
 }
 
-bool DaemonControllerMac::Start() {
+void DaemonControllerMac::SetConfigAndStart(
+    scoped_ptr<base::DictionaryValue> config) {
   NOTIMPLEMENTED();
-  return false;
 }
 
-bool DaemonControllerMac::Stop() {
+void DaemonControllerMac::SetPin(const std::string& pin) {
+  NOTIMPLEMENTED();
+}
+
+void DaemonControllerMac::Stop() {
   NOTIMPLEMENTED();
-  return false;
 }
 
 }  // namespace
diff --git a/remoting/host/plugin/daemon_controller_win.cc b/remoting/host/plugin/daemon_controller_win.cc
index 3c03c2a..901fe95 100644
--- a/remoting/host/plugin/daemon_controller_win.cc
+++ b/remoting/host/plugin/daemon_controller_win.cc
@@ -7,6 +7,7 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/values.h"
 
 namespace remoting {
 
@@ -17,9 +18,11 @@ class DaemonControllerWin : public remoting::DaemonController {
   DaemonControllerWin();
 
   virtual State GetState() OVERRIDE;
-  virtual bool SetPin(const std::string& pin) OVERRIDE;
-  virtual bool Start() OVERRIDE;
-  virtual bool Stop() OVERRIDE;
+  virtual void GetConfig(const GetConfigCallback& callback) OVERRIDE;
+  virtual void SetConfigAndStart(
+      scoped_ptr<base::DictionaryValue> config) OVERRIDE;
+  virtual void SetPin(const std::string& pin) OVERRIDE;
+  virtual void Stop() OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DaemonControllerWin);
@@ -28,23 +31,25 @@ class DaemonControllerWin : public remoting::DaemonController {
 DaemonControllerWin::DaemonControllerWin() {
 }
 
-remoting::DaemonController::State DaemonControllerWin::GetState() {
-  return remoting::DaemonController::STATE_NOT_IMPLEMENTED;
+DaemonController::State DaemonControllerWin::GetState() {
+  return DaemonController::STATE_NOT_IMPLEMENTED;
 }
 
-bool DaemonControllerWin::SetPin(const std::string& pin) {
+void DaemonControllerWin::GetConfig(const GetConfigCallback& callback) {
   NOTIMPLEMENTED();
-  return false;
 }
 
-bool DaemonControllerWin::Start() {
+void DaemonControllerWin::SetConfigAndStart(
+    scoped_ptr<base::DictionaryValue> config) {
   NOTIMPLEMENTED();
-  return false;
 }
 
-bool DaemonControllerWin::Stop() {
+void DaemonControllerWin::SetPin(const std::string& pin) {
+  NOTIMPLEMENTED();
+}
+
+void DaemonControllerWin::Stop() {
   NOTIMPLEMENTED();
-  return false;
 }
 
 }  // namespace
diff --git a/remoting/host/plugin/host_script_object.cc b/remoting/host/plugin/host_script_object.cc
index 817e5a7..6c94143 100644
--- a/remoting/host/plugin/host_script_object.cc
+++ b/remoting/host/plugin/host_script_object.cc
@@ -577,9 +577,7 @@ bool HostNPScriptObject::SetDaemonPin(const NPVariant* args,
     return false;
   }
   if (NPVARIANT_IS_STRING(args[0])) {
-    bool set_pin_result =
-        daemon_controller_->SetPin(StringFromNPVariant(args[0]));
-    BOOLEAN_TO_NPVARIANT(set_pin_result, *result);
+    daemon_controller_->SetPin(StringFromNPVariant(args[0]));
     return true;
   } else {
     SetException("startDaemon: unexpected type for argument 1");
@@ -594,8 +592,9 @@ bool HostNPScriptObject::StartDaemon(const NPVariant* args,
     SetException("startDaemon: bad number of arguments");
     return false;
   }
-  bool start_result = daemon_controller_->Start();
-  BOOLEAN_TO_NPVARIANT(start_result, *result);
+  // TODO(sergeyu): Receive |config| parameters.
+  scoped_ptr<base::DictionaryValue> config(new base::DictionaryValue());
+  daemon_controller_->SetConfigAndStart(config.Pass());
   return true;
 }
 
@@ -606,8 +605,7 @@ bool HostNPScriptObject::StopDaemon(const NPVariant* args,
     SetException("startDaemon: bad number of arguments");
     return false;
   }
-  bool stop_result = daemon_controller_->Stop();
-  BOOLEAN_TO_NPVARIANT(stop_result, *result);
+  daemon_controller_->Stop();
   return true;
 }
 
diff --git a/remoting/webapp/host_plugin_proto.js b/remoting/webapp/host_plugin_proto.js
index 74a24af..8adf49d 100644
--- a/remoting/webapp/host_plugin_proto.js
+++ b/remoting/webapp/host_plugin_proto.js
@@ -26,14 +26,14 @@ remoting.HostPlugin.prototype.disconnect = function() {};
 remoting.HostPlugin.prototype.localize = function(callback) {};
 
 /** @param {string} pin The new PIN.
- *  @return {boolean} True if the PIN was changed successfully. */
-remoting.HostPlugin.prototype.setDaemonPin = function(pin) { return false; };
+ *  @return {void} Nothing. */
+remoting.HostPlugin.prototype.setDaemonPin = function(pin) {};
 
-/** @return {boolean} True if successful (poll daemonState for completion). */
-remoting.HostPlugin.prototype.startDaemon = function() { return false; };
+/** @return {void} Nothing. */
+remoting.HostPlugin.prototype.startDaemon = function() {};
 
-/** @return {boolean} True if successful. */
-remoting.HostPlugin.prototype.stopDaemon = function() { return false; };
+/** @return {void} Nothing. */
+remoting.HostPlugin.prototype.stopDaemon = function() {};
 
 /** @type {number} */ remoting.HostPlugin.prototype.state;