The me2me host is now configurable from the web UI on Windows.

remoting/host/plugin/daemon_controller_win.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/plugin/daemon_controller.h"
 
+#include <objbase.h>
+
 #include "base/basictypes.h"
+#include "base/bind.h"
 #include "base/compiler_specific.h"
+#include "base/file_path.h"
+#include "base/file_util.h"
+#include "base/json/json_reader.h"
+#include "base/json/json_writer.h"
 #include "base/logging.h"
+#include "base/synchronization/lock.h"
+#include "base/threading/thread.h"
+#include "base/utf_string_conversions.h"
 #include "base/values.h"
+#include "remoting/base/scoped_sc_handle_win.h"
+#include "remoting/host/branding.h"
+
+// MIDL-generated declarations and definitions.
+#include <elevated_controller.h>

[Sergey Ulanov, 2012/03/30 07:42:35]

nit: I think this should be in quotes, it's not a system header.

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Done.

+#include <elevated_controller_i.c>
 
 namespace remoting {
 
 namespace {
 
+// The COM elevation moniker for the elevated controller.
+const char kElevationMoniker[] = "Elevation:Administrator!new:"
+    "{430a9403-8176-4733-afdc-0b325a8fda84}";
+
+// Name of the worker thread.
+const char kWorkerThreadName[] = "Daemon Controller thread";
+
+// A simple wrapper arounf base::Thread making sure that COM is initialized on

[Jamie, 2012/03/30 01:11:47]

Nit: s/arounf/around/

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Done.

+// the owner thread.
+class CoThread: public base::Thread {

[Jamie, 2012/03/30 01:11:47]

Nit: spacing around colon.

[Sergey Ulanov, 2012/03/30 07:42:35]

should it be called ComThread?

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Done.

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Done.

+ public:
+  explicit CoThread(const char* name);
+
+  // Activates an elevated instance of elevated controller and returns
+  // the pointer to the control interface in |control_out|. This routine keeps
+  // the ownership of the pointer so the caller should not call call AddRef() or
+  // Release().
+  HRESULT ActivateElevatedController(IDaemonControl** control_out);
+
+ protected:
+  virtual void Init() OVERRIDE;
+  virtual void CleanUp() OVERRIDE;
+
+  IDaemonControl* control_;
+
+  DISALLOW_COPY_AND_ASSIGN(CoThread);
+};
+
 class DaemonControllerWin : public remoting::DaemonController {
  public:
   DaemonControllerWin();
+  virtual ~DaemonControllerWin();
 
   virtual State GetState() 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:
+  // Activates an elevated instance of elevated controller and returns
+  // the pointer to the control interface in |control_out|.
+  HRESULT ActivateElevatedController(IDaemonControl** control_out);
+
+  // Opens the controlled service handle.
+  DWORD OpenService(ScopedScHandle* service_out);
+
+  // Worker functions called in the context of the worker thread.
+  void DoGetConfig(const GetConfigCallback& callback);
+  void DoSetConfigAndStart(scoped_ptr<base::DictionaryValue> config);
+  void DoStop();
+
+  // Converts SERVICE_XXX contants representing the service state (i.e.
+  // SERVICE_RUNNING, SERVICE_STOPPED) to the daemon state.
+  static State ConvertToDaemonState(DWORD service_state);
+
+  // Service status change notification callback.
+  static VOID CALLBACK OnServiceStatusChange(PVOID context);
+
+  // The worker thread used for servicing long running operations.
+  CoThread worker_thread_;
+
+  // The lock protecting access to all data members below.
+  base::Lock lock_;
+
+  // The error occurred during the last transition.
+  HRESULT last_error_;
+
+  // Cached daemon state.
+  State state_;
+
+  // The state that should never be reported to JS unless there is an error.
+  // For instance, when Start() is called, the state of the service doesn't
+  // switch to "starting" immediately. This could lead to JS interpreting
+  // "stopped" as a failure to start the service.
+  State forbidden_state_;

[Jamie, 2012/03/30 01:11:47]

This is a confusing name. If it's doing what I think it is, this is a
"termination" state that signifies an error (ie, once we reach this state,
something has gone wrong, and it's not going to recover). If so, perhaps it
should be called error_termination_state_?

[Sergey Ulanov, 2012/03/30 07:42:35]

Alternatively you can have flags like start_pending_ and stop_pending_ to
indicated when start/stop task has been posted to CoThread, but hasn't been
processed. And then you adjust the value returned from GetState() depending on
these flags.
But that's optional for this CL - we will need to cleanup this interface anyway.

[Sergey Ulanov, 2012/03/30 07:42:35]

It's not really termination state. The problem is that the webapp expects
GetState() to return STARTING after calling Start(), but that is hard to satisfy
when the service state machine is controlled by the OS, and we need to call the
OS on a separate thread.
We should add completion callbacks to Start() and Stop() methods to avoid hacks
like this (and that way we won't need state polling in the webapp).

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Exactly. I added a TODO.

+
   DISALLOW_COPY_AND_ASSIGN(DaemonControllerWin);
 };
 
-DaemonControllerWin::DaemonControllerWin() {
+CoThread::CoThread(const char* name) : base::Thread(name), control_(NULL) {
 }
 
-DaemonController::State DaemonControllerWin::GetState() {
-  return DaemonController::STATE_NOT_IMPLEMENTED;
+void CoThread::Init() {
+  CoInitialize(NULL);
+}
+
+void CoThread::CleanUp() {
+  if (control_ != NULL) {
+    control_->Release();
+  }
+
+  CoUninitialize();
+}
+
+HRESULT CoThread::ActivateElevatedController(
+    IDaemonControl** control_out) {
+  HRESULT hr = E_FAIL;

[Jamie, 2012/03/30 01:11:47]

Couldn't you initialize this to S_OK and skip the else clause below?

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Done.

+
+  if (control_ == NULL) {
+    BIND_OPTS3 bind_options;
+    memset(&bind_options, 0, sizeof(bind_options));
+    bind_options.cbStruct = sizeof(bind_options);
+    bind_options.hwnd = NULL;
+    bind_options.dwClassContext  = CLSCTX_LOCAL_SERVER;
+
+    IDaemonControl* control = NULL;
+    hr = ::CoGetObject(ASCIIToUTF16(kElevationMoniker).c_str(),

[Sergey Ulanov, 2012/03/30 07:42:35]

Are you sure that the result of ASCIIToUTF16() is not destroyed before
CoGetObject() is called? If not maybe better to allocate variable for that
string.

[alexeypa (please no reviews), 2012/03/30 16:30:55]

I'm pretty sure. It is guaranteed by the C++ standard.

+                       &bind_options,
+                       IID_IDaemonControl,
+                       (void**)&control_);

[Sergey Ulanov, 2012/03/30 07:42:35]

C style casts are not allowed by code style:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Castin...

use reinterpret_cast<>

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Done.

+  } else {
+    hr = S_OK;
+  }
+
+  if (SUCCEEDED(hr)) {
+    *control_out = control_;
+  } else {
+    LOG(ERROR) << "Failed to create the elevated controller (error: 0x"
+        << std::hex << hr << std::dec << ").";

[Jamie, 2012/03/30 01:11:47]

Is the explicit reversion to std::dec necessary?

[Sergey Ulanov, 2012/03/30 07:42:35]

May be cleaner to use StringPrintf() for format hex numbers - iostream interface
is ugly. Feel free to disagree.

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Yes.

[alexeypa (please no reviews), 2012/03/30 16:30:55]

I tried using StringPrintf. The mixture of streams and printf formatting looks
weird and it is not a bit shorter. I'll keep this code as it is. There are other
examples of using manipulators in the code.

+  }
+
+  return hr;
+}
+
+DaemonControllerWin::DaemonControllerWin()
+    : last_error_(S_OK),
+      state_(STATE_UNKNOWN),
+      forbidden_state_(STATE_UNKNOWN),
+      worker_thread_(kWorkerThreadName) {
+

[Sergey Ulanov, 2012/03/30 07:42:35]

nit: don't need this empty line.

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Without the empty line it looks cluttered. I added a comment explaining why UI
message loop is needed instead of the empty line. :-)

+  base::Thread::Options thread_options(MessageLoop::TYPE_UI, 0);
+  if (!worker_thread_.StartWithOptions(thread_options)) {
+    // N.B. StartWithOptions() does not report the error code returned by
+    // the system.
+    last_error_ = E_FAIL;
+  }
+}
+
+DaemonControllerWin::~DaemonControllerWin() {
+  worker_thread_.Stop();
+}
+
+remoting::DaemonController::State DaemonControllerWin::GetState() {
+  // TODO(alexeypa): convert polling to async callbacks once there is a thread
+  // that can receive APC callbacks.
+  ScopedScHandle service;
+  DWORD error = OpenService(&service);
+
+  if (error == ERROR_SUCCESS) {
+    SERVICE_STATUS status;
+    if (::QueryServiceStatus(service, &status)) {
+      State new_state = ConvertToDaemonState(status.dwCurrentState);
+
+      base::AutoLock lock(lock_);
+      if (forbidden_state_ != new_state || FAILED(last_error_)) {

[Jamie, 2012/03/30 01:11:47]

Why the second part of the OR? It's overruled by the next line. Also, the first
part of the OR feels like a test for success, (we're not in the forbidden state)
but the second feels like a test for failure. I can't parse what's going on
here.

[alexeypa (please no reviews), 2012/03/30 16:30:55]

There are two hacks in there. First we don't report the state we started an
operation in (i.e. "stop" for starting and "running" for stopping) unless an
error occurs. In the latter case there will be no further processing even if the
state didn't change. I.e. for starting: stop -> stop (with error).

The 2nd hack is because we don't have a good way to report an error. We have
start_failed state but what is there is an error when stopping the daemon?

I prefer to keep two hack separate and remove them one by one as we are
refactoring the JS interface.

+        state_ = new_state;
+      }
+
+      // TODO(alexeypa): remove this hack once JS nicely reports errors.
+      if (FAILED(last_error_)) {
+        state_ = STATE_START_FAILED;
+      }
+
+      return state_;
+    } else {
+      error = GetLastError();
+      LOG_GETLASTERROR(ERROR)
+          << "Failed to query the state of the '" << kWindowsServiceName
+          << "' service";
+    }
+  }
+
+  base::AutoLock lock(lock_);
+  if (error == ERROR_SERVICE_DOES_NOT_EXIST) {
+    state_ = STATE_NOT_INSTALLED;

[Jamie, 2012/03/30 01:11:47]

Should return NOT_IMPLEMENTED for now, because we can't install, but the JS
doesn't know that.

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Done.

+  } else {
+    last_error_ = HRESULT_FROM_WIN32(error);
+    state_ = STATE_UNKNOWN;
+  }
+
+  return state_;
 }
 
 void DaemonControllerWin::GetConfig(const GetConfigCallback& callback) {
-  NOTIMPLEMENTED();
+  worker_thread_.message_loop_proxy()->PostTask(
+      FROM_HERE,
+      base::Bind(&DaemonControllerWin::DoGetConfig,
+                 base::Unretained(this), callback));
 }
 
 void DaemonControllerWin::SetConfigAndStart(
     scoped_ptr<base::DictionaryValue> config) {
-  NOTIMPLEMENTED();
+  base::AutoLock lock(lock_);
+
+  // TODO(alexeypa): implement on-demand installation.
+  if (state_ == STATE_STOPPED) {
+    last_error_ = S_OK;
+    forbidden_state_ = STATE_STOPPED;
+    state_ = STATE_STARTING;
+    worker_thread_.message_loop_proxy()->PostTask(
+        FROM_HERE,
+        base::Bind(&DaemonControllerWin::DoSetConfigAndStart,
+                   base::Unretained(this), base::Passed(&config)));
+  }
 }
 
 void DaemonControllerWin::SetPin(const std::string& pin) {
   NOTIMPLEMENTED();
 }
 
 void DaemonControllerWin::Stop() {
-  NOTIMPLEMENTED();
+  base::AutoLock lock(lock_);
+
+  if (state_ == STATE_STARTING ||
+      state_ == STATE_STARTED ||
+      state_ == STATE_STOPPING) {
+
+    last_error_ = S_OK;
+    forbidden_state_ = STATE_STARTED;
+    state_ = STATE_STOPPING;
+    worker_thread_.message_loop_proxy()->PostTask(
+        FROM_HERE,
+        base::Bind(&DaemonControllerWin::DoStop, base::Unretained(this)));
+  }
+}
+
+DWORD DaemonControllerWin::OpenService(ScopedScHandle* service_out) {
+  // Open the service and query its current state.
+  ScopedScHandle scmanager(
+      ::OpenSCManagerW(NULL, SERVICES_ACTIVE_DATABASE,
+                       SC_MANAGER_CONNECT | SC_MANAGER_ENUMERATE_SERVICE));
+  if (!scmanager.IsValid()) {
+    DWORD error = GetLastError();
+    LOG_GETLASTERROR(ERROR)
+        << "Failed to connect to the service control manager";
+    return error;
+  }
+
+  ScopedScHandle service(
+      ::OpenServiceW(scmanager, ASCIIToUTF16(kWindowsServiceName).c_str(),
+                     SERVICE_QUERY_STATUS));
+  if (!service.IsValid()) {
+    DWORD error = GetLastError();
+    if (error != ERROR_SERVICE_DOES_NOT_EXIST) {
+      LOG_GETLASTERROR(ERROR)
+          << "Failed to open to the '" << kWindowsServiceName << "' service";
+    }
+    return error;
+  }
+
+  service_out->Set(service.Take());
+  return ERROR_SUCCESS;
+}
+
+void DaemonControllerWin::DoGetConfig(const GetConfigCallback& callback) {
+  IDaemonControl* control = NULL;
+  HRESULT hr = worker_thread_.ActivateElevatedController(&control);
+  if (FAILED(hr)) {
+    callback.Run(scoped_ptr<base::DictionaryValue>());
+    return;
+  }
+
+  // Get the host configuration.
+  BSTR host_config = NULL;
+  hr = control->GetConfig(&host_config);
+  if (FAILED(hr)) {
+    callback.Run(scoped_ptr<base::DictionaryValue>());
+    return;
+  }
+
+  string16 file_content(static_cast<char16*>(host_config),
+                        ::SysStringLen(host_config));
+  SysFreeString(host_config);
+
+  // Parse the string into a dictionary.
+  scoped_ptr<base::Value> config(
+      base::JSONReader::Read(UTF16ToUTF8(file_content), true));

[Sergey Ulanov, 2012/03/30 07:42:35]

We should probably change this interface to pass the config as string. Doesn't
look like DictionaryValue is useful here - HostScriptObject serializes it back
to string anyway...

[alexeypa (please no reviews), 2012/03/30 16:30:55]

It would definitely make my life easier.

+
+  if (config.get() == NULL || !config->IsType(base::Value::TYPE_DICTIONARY)) {

[Sergey Ulanov, 2012/03/30 07:42:35]

It would be cleaner to use GetAsDictionary() method instead of IsType() and
static_cast below.

[alexeypa (please no reviews), 2012/03/30 16:30:55]

Done.

+    callback.Run(scoped_ptr<base::DictionaryValue>());
+    return;
+  }
+
+  callback.Run(scoped_ptr<base::DictionaryValue>(
+      static_cast<base::DictionaryValue*>(config.release())));
+}
+
+void DaemonControllerWin::DoSetConfigAndStart(
+    scoped_ptr<base::DictionaryValue> config) {
+
+  IDaemonControl* control = NULL;
+  HRESULT hr = worker_thread_.ActivateElevatedController(&control);
+  if (FAILED(hr)) {
+    base::AutoLock lock(lock_);
+    last_error_ = hr;
+    return;
+  }
+
+  // Set the configuration file
+  std::string file_content;
+  base::JSONWriter::Write(config.get(), &file_content);
+
+  BSTR host_config = ::SysAllocString(UTF8ToUTF16(file_content).c_str());
+  if (host_config == NULL) {
+    base::AutoLock lock(lock_);
+    last_error_ = E_OUTOFMEMORY;
+    return;
+  }
+
+  hr = control->SetConfig(host_config);
+  ::SysFreeString(host_config);
+
+  if (FAILED(hr)) {
+    base::AutoLock lock(lock_);
+    last_error_ = hr;
+    return;
+  }
+
+  // Start daemon.
+  hr = control->StartDaemon();
+
+  if (FAILED(hr)) {
+    base::AutoLock lock(lock_);
+    last_error_ = hr;
+  }
+}
+
+void DaemonControllerWin::DoStop() {
+  IDaemonControl* control = NULL;
+  HRESULT hr = worker_thread_.ActivateElevatedController(&control);
+  if (FAILED(hr)) {
+    base::AutoLock lock(lock_);
+    last_error_ = hr;
+    return;
+  }
+
+  hr = control->StopDaemon();
+
+  if (FAILED(hr)) {
+    base::AutoLock lock(lock_);
+    last_error_ = hr;
+  }
+}
+
+// static
+remoting::DaemonController::State DaemonControllerWin::ConvertToDaemonState(

[Sergey Ulanov, 2012/03/30 07:42:35]

nit: doesn't look like this need to be a class member. Maybe move make it a
static function.

[alexeypa (please no reviews), 2012/03/30 16:30:55]

It is a static member. It does not have to be a member but it an easy way to say
that the function knows something private to DaemonControllerWin.
DaemonControllerWin is not known outside of this module so we are not polluting
the headers either.

+    DWORD service_state) {
+
+  switch (service_state) {
+  case SERVICE_RUNNING:
+    return STATE_STARTED;
+
+  case SERVICE_CONTINUE_PENDING:
+  case SERVICE_START_PENDING:
+    return STATE_STARTING;
+    break;
+
+  case SERVICE_PAUSE_PENDING:
+  case SERVICE_STOP_PENDING:
+    return STATE_STOPPING;
+    break;
+
+  case SERVICE_PAUSED:
+  case SERVICE_STOPPED:
+    return STATE_STOPPED;
+    break;
+
+  default:
+    NOTREACHED();
+    return STATE_UNKNOWN;
+  }
 }
 
 }  // namespace
 
 DaemonController* remoting::DaemonController::Create() {
   return new DaemonControllerWin();
 }
 
 }  // namespace remoting