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

remoting/host/setup/daemon_controller_delegate_mac.mm

Index: remoting/host/setup/daemon_controller_delegate_mac.mm
diff --git a/remoting/host/setup/daemon_controller_mac.mm b/remoting/host/setup/daemon_controller_delegate_mac.mm
similarity index 49%
rename from remoting/host/setup/daemon_controller_mac.mm
rename to remoting/host/setup/daemon_controller_delegate_mac.mm
index aaf69fbf2d8d206eef2a70b63f4292493ad02174..749a6ab6a31667f0e914fadaad52ec1825d22d72 100644
--- a/remoting/host/setup/daemon_controller_mac.mm
+++ b/remoting/host/setup/daemon_controller_delegate_mac.mm
@@ -1,8 +1,10 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2013 The Chromium Authors. All rights reserved.

[Sergey Ulanov, 2013/09/09 20:19:51]

I think this counts as renamed file, so you should keep 2012 here.

[alexeypa (please no reviews), 2013/09/09 20:39:59]

Done.

 // 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 <CoreFoundation/CoreFoundation.h>
+
+#include "remoting/host/setup/daemon_controller_delegate_mac.h"
 
 #include <launch.h>
 #include <stdio.h>
@@ -20,7 +22,6 @@
 #include "base/mac/mac_logging.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_launch_data.h"
-#include "base/threading/thread.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "remoting/host/constants_mac.h"
@@ -29,81 +30,14 @@
 
 namespace remoting {
 
-namespace {
-
-class DaemonControllerMac : public remoting::DaemonController {
- public:
-  DaemonControllerMac();
-  virtual ~DaemonControllerMac();
-
-  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& callback) OVERRIDE;
-
- private:
-  void DoGetConfig(const GetConfigCallback& callback);
-  void DoGetVersion(const GetVersionCallback& callback);
-  void DoSetConfigAndStart(scoped_ptr<base::DictionaryValue> config,
-                           bool consent,
-                           const CompletionCallback& done);
-  void DoUpdateConfig(scoped_ptr<base::DictionaryValue> config,
-                      const CompletionCallback& done_callback);
-  void DoStop(const CompletionCallback& done_callback);
-  void DoGetUsageStatsConsent(const GetUsageStatsConsentCallback& callback);
-
-  void ShowPreferencePane(const std::string& config_data,
-                          const CompletionCallback& done_callback);
-  void RegisterForPreferencePaneNotifications(
-      const CompletionCallback &done_callback);
-  void DeregisterForPreferencePaneNotifications();
-  void PreferencePaneCallbackDelegate(CFStringRef name);
-  static bool DoShowPreferencePane(const std::string& config_data);
-  static void PreferencePaneCallback(CFNotificationCenterRef center,
-                                     void* observer,
-                                     CFStringRef name,
-                                     const void* object,
-                                     CFDictionaryRef user_info);
-
-  base::Thread auth_thread_;
-  CompletionCallback current_callback_;
-
-  DISALLOW_COPY_AND_ASSIGN(DaemonControllerMac);
-};
-
-DaemonControllerMac::DaemonControllerMac()
-    : auth_thread_("Auth thread") {
-  auth_thread_.Start();
+DaemonControllerDelegateMac::DaemonControllerDelegateMac() {
 }
 
-DaemonControllerMac::~DaemonControllerMac() {
-  auth_thread_.Stop();
+DaemonControllerDelegateMac::~DaemonControllerDelegateMac() {
   DeregisterForPreferencePaneNotifications();
 }
 
-void DaemonControllerMac::DeregisterForPreferencePaneNotifications() {
-  CFNotificationCenterRemoveObserver(
-      CFNotificationCenterGetDistributedCenter(),
-      this,
-      CFSTR(UPDATE_SUCCEEDED_NOTIFICATION_NAME),
-      NULL);
-  CFNotificationCenterRemoveObserver(
-      CFNotificationCenterGetDistributedCenter(),
-      this,
-      CFSTR(UPDATE_FAILED_NOTIFICATION_NAME),
-      NULL);
-}
-
-DaemonController::State DaemonControllerMac::GetState() {
+DaemonController::State DaemonControllerDelegateMac::GetState() {
   pid_t job_pid = base::mac::PIDForJob(kServiceName);
   if (job_pid < 0) {
     return DaemonController::STATE_NOT_INSTALLED;
@@ -115,59 +49,7 @@ DaemonController::State DaemonControllerMac::GetState() {
   }
 }
 
-void DaemonControllerMac::GetConfig(const GetConfigCallback& callback) {
-  // base::Unretained() is safe, since this object owns the thread and therefore
-  // outlives it.
-  auth_thread_.message_loop_proxy()->PostTask(
-      FROM_HERE,
-      base::Bind(&DaemonControllerMac::DoGetConfig, base::Unretained(this),
-                 callback));
-}
-
-void DaemonControllerMac::SetConfigAndStart(
-    scoped_ptr<base::DictionaryValue> config,
-    bool consent,
-    const CompletionCallback& done) {
-  auth_thread_.message_loop_proxy()->PostTask(
-      FROM_HERE, base::Bind(
-          &DaemonControllerMac::DoSetConfigAndStart, base::Unretained(this),
-          base::Passed(&config), consent, done));
-}
-
-void DaemonControllerMac::UpdateConfig(
-    scoped_ptr<base::DictionaryValue> config,
-    const CompletionCallback& done_callback) {
-  auth_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(
-      &DaemonControllerMac::DoUpdateConfig, base::Unretained(this),
-      base::Passed(&config), done_callback));
-}
-
-void DaemonControllerMac::Stop(const CompletionCallback& done_callback) {
-  auth_thread_.message_loop_proxy()->PostTask(
-      FROM_HERE, base::Bind(
-          &DaemonControllerMac::DoStop, base::Unretained(this), done_callback));
-}
-
-void DaemonControllerMac::SetWindow(void* window_handle) {
-  // noop
-}
-
-void DaemonControllerMac::GetVersion(const GetVersionCallback& callback) {
-  auth_thread_.message_loop_proxy()->PostTask(
-      FROM_HERE,
-      base::Bind(&DaemonControllerMac::DoGetVersion, base::Unretained(this),
-                 callback));
-}
-
-void DaemonControllerMac::GetUsageStatsConsent(
-    const GetUsageStatsConsentCallback& callback) {
-  auth_thread_.message_loop_proxy()->PostTask(
-      FROM_HERE,
-      base::Bind(&DaemonControllerMac::DoGetUsageStatsConsent,
-                 base::Unretained(this), callback));
-}
-
-void DaemonControllerMac::DoGetConfig(const GetConfigCallback& callback) {
+scoped_ptr<base::DictionaryValue> DaemonControllerDelegateMac::GetConfig() {
   base::FilePath config_path(kHostConfigFilePath);
   JsonHostConfig host_config(config_path);
   scoped_ptr<base::DictionaryValue> config;
@@ -181,10 +63,48 @@ void DaemonControllerMac::DoGetConfig(const GetConfigCallback& callback) {
       config.get()->SetString(kXmppLoginConfigPath, value);
   }
 
-  callback.Run(config.Pass());
+  return config.Pass();
+}
+
+void DaemonControllerDelegateMac::SetConfigAndStart(
+    scoped_ptr<base::DictionaryValue> config,
+    bool consent,
+    const DaemonController::CompletionCallback& done) {
+  config->SetBoolean(kUsageStatsConsentConfigPath, consent);
+  std::string config_data;
+  base::JSONWriter::Write(config.get(), &config_data);
+  ShowPreferencePane(config_data, done);
+}
+
+void DaemonControllerDelegateMac::UpdateConfig(
+    scoped_ptr<base::DictionaryValue> config,
+    const DaemonController::CompletionCallback& done) {
+  base::FilePath config_file_path(kHostConfigFilePath);
+  JsonHostConfig config_file(config_file_path);
+  if (!config_file.Read()) {
+    done.Run(DaemonController::RESULT_FAILED);
+    return;
+  }
+  if (!config_file.CopyFrom(config.get())) {
+    LOG(ERROR) << "Failed to update configuration.";
+    done.Run(DaemonController::RESULT_FAILED);
+    return;
+  }
+
+  std::string config_data = config_file.GetSerializedData();
+  ShowPreferencePane(config_data, done);
+}
+
+void DaemonControllerDelegateMac::Stop(
+    const DaemonController::CompletionCallback& done) {
+  ShowPreferencePane("", done);
+}
+
+void DaemonControllerDelegateMac::SetWindow(void* window_handle) {
+  // noop
 }
 
-void DaemonControllerMac::DoGetVersion(const GetVersionCallback& callback) {
+std::string DaemonControllerDelegateMac::GetVersion() {
   std::string version = "";
   std::string command_line = remoting::kHostHelperScriptPath;
   command_line += " --host-version";
@@ -205,60 +125,103 @@ void DaemonControllerMac::DoGetVersion(const GetVersionCallback& callback) {
       version = result;
     }
   }
-  callback.Run(version);
-}
 
-void DaemonControllerMac::DoSetConfigAndStart(
-    scoped_ptr<base::DictionaryValue> config,
-    bool consent,
-    const CompletionCallback& done) {
-  config->SetBoolean(kUsageStatsConsentConfigPath, consent);
-  std::string config_data;
-  base::JSONWriter::Write(config.get(), &config_data);
-  ShowPreferencePane(config_data, done);
+  return version;
 }
 
-void DaemonControllerMac::DoUpdateConfig(
-    scoped_ptr<base::DictionaryValue> config,
-    const CompletionCallback& done_callback) {
-  base::FilePath config_file_path(kHostConfigFilePath);
-  JsonHostConfig config_file(config_file_path);
-  if (!config_file.Read()) {
-    done_callback.Run(RESULT_FAILED);
-    return;
-  }
-  if (!config_file.CopyFrom(config.get())) {
-    LOG(ERROR) << "Failed to update configuration.";
-    done_callback.Run(RESULT_FAILED);
-    return;
-  }
-
-  std::string config_data = config_file.GetSerializedData();
-  ShowPreferencePane(config_data, done_callback);
-}
+DaemonController::UsageStatsConsent
+DaemonControllerDelegateMac::GetUsageStatsConsent() {
+  DaemonController::UsageStatsConsent consent;
+  consent.supported = true;
+  consent.allowed = false;
+  // set_by_policy is not yet supported.
+  consent.set_by_policy = false;
 
-void DaemonControllerMac::DoGetUsageStatsConsent(
-    const GetUsageStatsConsentCallback& callback) {
-  bool allowed = false;
   base::FilePath config_file_path(kHostConfigFilePath);
   JsonHostConfig host_config(config_file_path);
   if (host_config.Read()) {
-    host_config.GetBoolean(kUsageStatsConsentConfigPath, &allowed);
+    host_config.GetBoolean(kUsageStatsConsentConfigPath, &consent.allowed);
   }
-  // set_by_policy is not yet supported.
-  callback.Run(true, allowed, false /* set_by_policy */);
+
+  return consent;
 }
 
-void DaemonControllerMac::ShowPreferencePane(
-    const std::string& config_data, const CompletionCallback& done_callback) {
+void DaemonControllerDelegateMac::ShowPreferencePane(
+    const std::string& config_data,
+    const DaemonController::CompletionCallback& done) {
   if (DoShowPreferencePane(config_data)) {
-    RegisterForPreferencePaneNotifications(done_callback);
+    RegisterForPreferencePaneNotifications(done);
+  } else {
+    done.Run(DaemonController::RESULT_FAILED);
+  }
+}
+
+// CFNotificationCenterAddObserver ties the thread on which distributed
+// notifications are received to the one on which it is first called.
+// This is safe because HostNPScriptObject::InvokeAsyncResultCallback
+// bounces the invocation to the correct thread, so it doesn't matter
+// which thread CompletionCallbacks are called on.
+void DaemonControllerDelegateMac::RegisterForPreferencePaneNotifications(
+    const DaemonController::CompletionCallback& done) {
+  // We can only have one callback registered at a time. This is enforced by the
+  // UX flow of the web-app.
+  DCHECK(current_callback_.is_null());
+  current_callback_ = done;
+
+  CFNotificationCenterAddObserver(
+      CFNotificationCenterGetDistributedCenter(),
+      this,
+      &DaemonControllerDelegateMac::PreferencePaneCallback,
+      CFSTR(UPDATE_SUCCEEDED_NOTIFICATION_NAME),
+      NULL,
+      CFNotificationSuspensionBehaviorDeliverImmediately);
+  CFNotificationCenterAddObserver(
+      CFNotificationCenterGetDistributedCenter(),
+      this,
+      &DaemonControllerDelegateMac::PreferencePaneCallback,
+      CFSTR(UPDATE_FAILED_NOTIFICATION_NAME),
+      NULL,
+      CFNotificationSuspensionBehaviorDeliverImmediately);
+}
+
+void DaemonControllerDelegateMac::DeregisterForPreferencePaneNotifications() {
+  CFNotificationCenterRemoveObserver(
+      CFNotificationCenterGetDistributedCenter(),
+      this,
+      CFSTR(UPDATE_SUCCEEDED_NOTIFICATION_NAME),
+      NULL);
+  CFNotificationCenterRemoveObserver(
+      CFNotificationCenterGetDistributedCenter(),
+      this,
+      CFSTR(UPDATE_FAILED_NOTIFICATION_NAME),
+      NULL);
+}
+
+void DaemonControllerDelegateMac::PreferencePaneCallbackDelegate(
+    CFStringRef name) {
+  DaemonController::AsyncResult result = DaemonController::RESULT_FAILED;
+  if (CFStringCompare(name, CFSTR(UPDATE_SUCCEEDED_NOTIFICATION_NAME), 0) ==
+          kCFCompareEqualTo) {
+    result = DaemonController::RESULT_OK;
+  } else if (CFStringCompare(name, CFSTR(UPDATE_FAILED_NOTIFICATION_NAME), 0) ==
+          kCFCompareEqualTo) {
+    result = DaemonController::RESULT_FAILED;
   } else {
-    done_callback.Run(RESULT_FAILED);
+    LOG(WARNING) << "Ignoring unexpected notification: " << name;
+    return;
   }
+
+  DCHECK(!current_callback_.is_null());
+  DaemonController::CompletionCallback done = current_callback_;
+  current_callback_.Reset();
+  done.Run(result);
+
+  DeregisterForPreferencePaneNotifications();
 }
 
-bool DaemonControllerMac::DoShowPreferencePane(const std::string& config_data) {
+// static
+bool DaemonControllerDelegateMac::DoShowPreferencePane(
+    const std::string& config_data) {
   if (!config_data.empty()) {
     base::FilePath config_path;
     if (!file_util::GetTempDir(&config_path)) {
@@ -306,73 +269,27 @@ bool DaemonControllerMac::DoShowPreferencePane(const std::string& config_data) {
   return true;
 }
 
-void DaemonControllerMac::DoStop(const CompletionCallback& done_callback) {
-  ShowPreferencePane("", done_callback);
-}
-
-// CFNotificationCenterAddObserver ties the thread on which distributed
-// notifications are received to the one on which it is first called.
-// This is safe because HostNPScriptObject::InvokeAsyncResultCallback
-// bounces the invocation to the correct thread, so it doesn't matter
-// which thread CompletionCallbacks are called on.
-void DaemonControllerMac::RegisterForPreferencePaneNotifications(
-    const CompletionCallback& done_callback) {
-  // We can only have one callback registered at a time. This is enforced by the
-  // UX flow of the web-app.
-  DCHECK(current_callback_.is_null());
-  current_callback_ = done_callback;
-
-  CFNotificationCenterAddObserver(
-      CFNotificationCenterGetDistributedCenter(),
-      this,
-      &DaemonControllerMac::PreferencePaneCallback,
-      CFSTR(UPDATE_SUCCEEDED_NOTIFICATION_NAME),
-      NULL,
-      CFNotificationSuspensionBehaviorDeliverImmediately);
-  CFNotificationCenterAddObserver(
-      CFNotificationCenterGetDistributedCenter(),
-      this,
-      &DaemonControllerMac::PreferencePaneCallback,
-      CFSTR(UPDATE_FAILED_NOTIFICATION_NAME),
-      NULL,
-      CFNotificationSuspensionBehaviorDeliverImmediately);
-}
-
-void DaemonControllerMac::PreferencePaneCallbackDelegate(CFStringRef name) {
-  AsyncResult result = RESULT_FAILED;
-  if (CFStringCompare(name, CFSTR(UPDATE_SUCCEEDED_NOTIFICATION_NAME), 0) ==
-          kCFCompareEqualTo) {
-    result = RESULT_OK;
-  } else if (CFStringCompare(name, CFSTR(UPDATE_FAILED_NOTIFICATION_NAME), 0) ==
-          kCFCompareEqualTo) {
-    result = RESULT_FAILED;
-  } else {
-    LOG(WARNING) << "Ignoring unexpected notification: " << name;
+// static
+void DaemonControllerDelegateMac::PreferencePaneCallback(
+    CFNotificationCenterRef center,
+    void* observer,
+    CFStringRef name,
+    const void* object,
+    CFDictionaryRef user_info) {
+  DaemonControllerDelegateMac* self =
+      reinterpret_cast<DaemonControllerDelegateMac*>(observer);
+  if (!self) {
+    LOG(WARNING) << "Ignoring notification with NULL observer: " << name;
     return;
   }
-  DCHECK(!current_callback_.is_null());
-  current_callback_.Run(result);
-  current_callback_.Reset();
-  DeregisterForPreferencePaneNotifications();
-}
 
-void DaemonControllerMac::PreferencePaneCallback(CFNotificationCenterRef center,
-                                                 void* observer,
-                                                 CFStringRef name,
-                                                 const void* object,
-                                                 CFDictionaryRef user_info) {
-  DaemonControllerMac* self = reinterpret_cast<DaemonControllerMac*>(observer);
-  if (self) {
-    self->PreferencePaneCallbackDelegate(name);
-  } else {
-    LOG(WARNING) << "Ignoring notification with NULL observer: " << name;
-  }
+  self->PreferencePaneCallbackDelegate(name);
 }
 
-}  // namespace
-
-scoped_ptr<DaemonController> remoting::DaemonController::Create() {
-  return scoped_ptr<DaemonController>(new DaemonControllerMac());
+scoped_refptr<DaemonController> DaemonController::Create() {
+  scoped_ptr<DaemonController::Delegate> delegate(
+      new DaemonControllerDelegateMac());
+  return new DaemonController(delegate.Pass());
 }
 
 }  // namespace remoting