Implement DaemonController Start(), Stop() and GetStatus() for Mac.

remoting/host/plugin/daemon_controller_mac.cc

Index: remoting/host/plugin/daemon_controller_mac.cc
diff --git a/remoting/host/plugin/daemon_controller_mac.cc b/remoting/host/plugin/daemon_controller_mac.cc
index 1a8a2e0f8d6815f49146d2e012f416a00b0a0055..a130fdef4353c9da23c8e85c774d49765284fb55 100644
--- a/remoting/host/plugin/daemon_controller_mac.cc
+++ b/remoting/host/plugin/daemon_controller_mac.cc
@@ -4,17 +4,40 @@
 
 #include "remoting/host/plugin/daemon_controller.h"
 
+#include <launch.h>
+#include <ServiceManagement/ServiceManagement.h>
+
 #include "base/basictypes.h"
+#include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/mac/authorization_util.h"
+#include "base/mac/foundation_util.h"
+#include "base/mac/mac_logging.h"
+#include "base/mac/scoped_authorizationref.h"
+#include "base/mac/scoped_cftyperef.h"
+#include "base/threading/thread.h"
 
 namespace remoting {
 
 namespace {
 
+// The name of the Remoting Host service that is registered with launchd.
+#define kServiceName "com.google.chrome_remote_desktop"

[Jamie, 2012/03/23 03:24:05]

Any reason not to make this a const char[]?

[Lambros, 2012/03/26 19:15:26]

The CFSTR() macro seems to need a literal string.  The same situation occurs
elsewhere in Chrome, for example, chrome/common/service_process_util_mac.mm so I
guess #define is OK.

+
+// This helper script is executed as root.  It is passed a command-line option
+// (--enable or --disable), which causes it to create or remove a trigger file.
+// The trigger file (defined in the service's plist file) informs launchd
+// whether the Host service should be running.  Creating the trigger file causes
+// launchd to immediately start the service.  Deleting the trigger file has no
+// immediate effect, but it prevents the service from being restarted if it
+// becomes stopped.
+const char kStartStopTool[] = "/Library/PrivilegedHelperTools/me2me.sh";

[garykac, 2012/03/23 16:28:29]

Since this is a system directory that includes files from other apps/services,
we should add our 'com.google.chrome_remote_desktop." prefix to this file (and
any other file we add here).

I've already done this with the files used in the installer.

[Lambros, 2012/03/26 19:15:26]

Done.

+
 class DaemonControllerMac : public remoting::DaemonController {
  public:
   DaemonControllerMac();
+  virtual ~DaemonControllerMac();
 
   virtual State GetState() OVERRIDE;
   virtual bool SetPin(const std::string& pin) OVERRIDE;
@@ -22,14 +45,53 @@ class DaemonControllerMac : public remoting::DaemonController {
   virtual bool Stop() OVERRIDE;
 
  private:
+  void DoStart();
+  void DoStop();
+
+  bool RunToolScriptAsRoot(const char* command);
+  bool StopService();
+
+  // The API for gaining root privileges is blocking (it prompts the user for
+  // a password).  Since Start() and Stop() must not block the main thread, they
+  // need to post their tasks to a separate thread.
+  base::Thread root_thread_;

[Jamie, 2012/03/23 03:24:05]

I think auth_thread might be a better name, since it doesn't actually run as
root.

[Sergey Ulanov, 2012/03/23 17:56:48]

+1

[Lambros, 2012/03/26 19:15:26]

Done.

+
+  // This distinguishes between STATE_START_FAILED and STATE_STOPPED in
+  // GetState().
+  bool start_failed_;
+
   DISALLOW_COPY_AND_ASSIGN(DaemonControllerMac);
 };
 
-DaemonControllerMac::DaemonControllerMac() {
+DaemonControllerMac::DaemonControllerMac()
+    : root_thread_("Root thread"),
+      start_failed_(false) {
+  root_thread_.Start();
+}
+
+DaemonControllerMac::~DaemonControllerMac() {
+  // This will block if the thread is waiting on a root password prompt.  There
+  // doesn't seem to be an easy solution for this, other than to spawn a
+  // separate process to do the root elevation.
+
+  // TODO(lambroslambrou): Improve this, either by finding a way to terminate
+  // the thread, or by moving to a separate process.
+  root_thread_.Stop();
 }
 
 remoting::DaemonController::State DaemonControllerMac::GetState() {
-  return remoting::DaemonController::STATE_NOT_IMPLEMENTED;
+  base::mac::ScopedCFTypeRef<CFDictionaryRef> job(
+      SMJobCopyDictionary(kSMDomainUserLaunchd, CFSTR(kServiceName)));
+  if (!job)
+    return remoting::DaemonController::STATE_NOT_INSTALLED;

[Jamie, 2012/03/23 03:24:05]

Until we have an installation story, I think you should return NOT_IMPLEMENTED
here, with a TODO to fix it up later. Otherwise the web-app will display the
"start" UI but you won't be able to do anything with it.

[Lambros, 2012/03/26 19:15:26]

Done.

+
+  if (CFDictionaryGetValue(job.get(), CFSTR("PID"))) {
+    return remoting::DaemonController::STATE_STARTED;
+  } else {
+    // Service is stopped, or a start attempt failed.
+    return start_failed_ ? remoting::DaemonController::STATE_START_FAILED :
+        remoting::DaemonController::STATE_STOPPED;
+  }
 }
 
 bool DaemonControllerMac::SetPin(const std::string& pin) {
@@ -38,13 +100,93 @@ bool DaemonControllerMac::SetPin(const std::string& pin) {
 }
 
 bool DaemonControllerMac::Start() {
-  NOTIMPLEMENTED();
-  return false;
+  start_failed_ = false;
+  root_thread_.message_loop_proxy()->PostTask(
+      FROM_HERE,
+      base::Bind(&DaemonControllerMac::DoStart, base::Unretained(this)));

[Sergey Ulanov, 2012/03/23 17:56:48]

Add comment that it is safe to use base::Unretained() because this object owns
the thread, and so it always outlives it.

[Lambros, 2012/03/26 19:15:26]

Done.

+  return true;
 }
 
 bool DaemonControllerMac::Stop() {
-  NOTIMPLEMENTED();
-  return false;
+  root_thread_.message_loop_proxy()->PostTask(
+      FROM_HERE,
+      base::Bind(&DaemonControllerMac::DoStop, base::Unretained(this)));
+  return true;
+}
+
+void DaemonControllerMac::DoStart() {
+  // Creating the trigger file causes launchd to start the service, so the
+  // extra step performed in DoStop() is not necessary here.
+  if (!RunToolScriptAsRoot("--enable"))
+    start_failed_ = true;
+}
+
+void DaemonControllerMac::DoStop() {
+  if (!RunToolScriptAsRoot("--disable"))
+    return;
+
+  // Deleting the trigger file does not cause launchd to stop the service.
+  // Since the service is running for the local user's desktop (not as root),
+  // it has to be stopped for that user.  This cannot easily be done in the
+  // shell-script running as root, so it is done here instead.
+  StopService();
+}
+
+bool DaemonControllerMac::RunToolScriptAsRoot(const char* command) {
+  base::mac::ScopedAuthorizationRef authorization(
+      base::mac::AuthorizationCreateToRunAsRoot(CFSTR("")));
+  if (!authorization) {
+    LOG(ERROR) << "Failed to get root privileges.";
+    return false;
+  }
+
+  const char* arguments[] = {command, NULL};

[Sergey Ulanov, 2012/03/23 17:56:48]

nit: add spaces after { and before }.

[Lambros, 2012/03/26 19:15:26]

Done.

+  int exit_status;
+  OSStatus status = base::mac::ExecuteWithPrivilegesAndWait(
+      authorization.get(),
+      kStartStopTool,
+      kAuthorizationFlagDefaults,
+      arguments,
+      NULL,
+      &exit_status);
+  if (status != errAuthorizationSuccess) {
+    OSSTATUS_LOG(ERROR, status) << "AuthorizationExecuteWithPrivileges";
+    return false;
+  }
+  if (exit_status != 0) {
+    LOG(ERROR) << kStartStopTool << " failed with exit status " << exit_status;
+    return false;
+  }
+
+  return true;
+}
+
+bool DaemonControllerMac::StopService() {
+  launch_data_t message = launch_data_alloc(LAUNCH_DATA_DICTIONARY);

[Sergey Ulanov, 2012/03/23 17:56:48]

Maybe move ScopedLaunchData from chrome/browser/mac to base/mac and use it here?
(shouldn't block this CL).

+  launch_data_dict_insert(message, launch_data_new_string(kServiceName),
+                          LAUNCH_KEY_STOPJOB);
+  launch_data_t response = launch_msg(message);
+  launch_data_free(message);
+
+  if (!response) {
+    LOG(ERROR) << "Failed to send message to launchd";
+    return false;
+  }
+
+  bool success = false;
+  if (launch_data_get_type(response) == LAUNCH_DATA_ERRNO) {
+    int error = launch_data_get_errno(response);
+    if (error) {
+      LOG(ERROR) << "launchd returned error " << error;
+    } else {
+      success = true;
+    }
+  } else {
+    LOG(ERROR) << "launchd returned unexpected response";
+  }
+
+  launch_data_free(response);
+  return success;
 }
 
 }  // namespace