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

remoting/host/plugin/host_script_object.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/host_script_object.h"
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 627 matching lines @@
 
 HostNPScriptObject::HostNPScriptObject(
     NPP plugin,
     NPObject* parent,
     scoped_refptr<AutoThreadTaskRunner> plugin_task_runner)
     : plugin_(plugin),
       parent_(parent),
       plugin_task_runner_(plugin_task_runner),
       am_currently_logging_(false),
       state_(kDisconnected),
-      daemon_controller_(DaemonController::Create()),
       weak_factory_(this),
       weak_ptr_(weak_factory_.GetWeakPtr()) {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
   // Set the thread task runner for the plugin thread so that timers and other
   // code using |base::ThreadTaskRunnerHandle| could be used on the plugin
   // thread.
   //
   // If component build is used, Chrome and the plugin may end up sharing base
   // binary. This means that the instance of |base::ThreadTaskRunnerHandle|
   // created by Chrome for the current thread is shared as well. This routinely
   // happens in the development setting so the below check for
   // |!base::ThreadTaskRunnerHandle::IsSet()| is a hack/workaround allowing this
   // configuration to work. It lets the plugin to access Chrome's message loop
   // directly via |base::ThreadTaskRunnerHandle|. This is safe as long as both
   // Chrome and the plugin are built from the same version of the sources.
   if (!base::ThreadTaskRunnerHandle::IsSet()) {
     plugin_task_runner_handle_.reset(
         new base::ThreadTaskRunnerHandle(plugin_task_runner_));
   }
 
+  daemon_controller_ = DaemonController::Create();
+
   ServiceUrls* service_urls = ServiceUrls::GetInstance();
   bool xmpp_server_valid = net::ParseHostAndPort(
       service_urls->xmpp_server_address(),
       &xmpp_server_config_.host, &xmpp_server_config_.port);
   // For the plugin, this is always the default address, which must be valid.
   DCHECK(xmpp_server_valid);
   xmpp_server_config_.use_tls = service_urls->xmpp_server_use_tls();
   directory_bot_jid_ = service_urls->directory_bot_jid();
 
   // Create worker thread for encryption key generation and loading the paired
@@ skipping 568 matching lines @@
     return false;
   }
 
   if (config_dict->HasKey(kHostIdConfigPath) ||
       config_dict->HasKey(kXmppLoginConfigPath)) {
     SetException("updateDaemonConfig: trying to update immutable config "
                  "parameters");
     return false;
   }
 
-  // TODO(wez): Pass a static method here, that will post the result
-  // back to us on the right thread (crbug.com/156257).
   daemon_controller_->UpdateConfig(
       config_dict.Pass(),
       base::Bind(&HostNPScriptObject::InvokeAsyncResultCallback,
                  base::Unretained(this), callback_obj));
   return true;
 }
 
 bool HostNPScriptObject::GetDaemonConfig(const NPVariant* args,
                                          uint32_t arg_count,
                                          NPVariant* result) {
   if (arg_count != 1) {
     SetException("getDaemonConfig: bad number of arguments");
     return false;
   }
 
   ScopedRefNPObject callback_obj(ObjectFromNPVariant(args[0]));
   if (!callback_obj.get()) {
     SetException("getDaemonConfig: invalid callback parameter");
     return false;
   }
 
-  // TODO(wez): Pass a static method here, that will post the result
-  // back to us on the right thread (crbug.com/156257).
   daemon_controller_->GetConfig(
       base::Bind(&HostNPScriptObject::InvokeGetDaemonConfigCallback,
                  base::Unretained(this), callback_obj));
 
   return true;
 }
 
 bool HostNPScriptObject::GetDaemonVersion(const NPVariant* args,
                                           uint32_t arg_count,
                                           NPVariant* result) {
   if (arg_count != 1) {
     SetException("getDaemonVersion: bad number of arguments");
     return false;
   }
 
   ScopedRefNPObject callback_obj(ObjectFromNPVariant(args[0]));
   if (!callback_obj.get()) {
     SetException("getDaemonVersion: invalid callback parameter");
     return false;
   }
 
-  // TODO(wez): Pass a static method here, that will post the result
-  // back to us on the right thread (crbug.com/156257).
   daemon_controller_->GetVersion(
       base::Bind(&HostNPScriptObject::InvokeGetDaemonVersionCallback,
                  base::Unretained(this), callback_obj));
 
   return true;
 }
 
 bool HostNPScriptObject::GetPairedClients(const NPVariant* args,
                                           uint32_t arg_count,
                                           NPVariant* result) {
@@ skipping 26 matching lines @@
     SetException("getUsageStatsConsent: bad number of arguments");
     return false;
   }
 
   ScopedRefNPObject callback_obj(ObjectFromNPVariant(args[0]));
   if (!callback_obj.get()) {
     SetException("getUsageStatsConsent: invalid callback parameter");
     return false;
   }
 
-  // TODO(wez): Pass a static method here, that will post the result
-  // back to us on the right thread (crbug.com/156257).
   daemon_controller_->GetUsageStatsConsent(
       base::Bind(&HostNPScriptObject::InvokeGetUsageStatsConsentCallback,
                  base::Unretained(this), callback_obj));
   return true;
 }
 
 bool HostNPScriptObject::StartDaemon(const NPVariant* args,
                                      uint32_t arg_count,
                                      NPVariant* result) {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
@@ skipping 18 matching lines @@
     SetException("startDaemon: invalid consent parameter");
     return false;
   }
 
   ScopedRefNPObject callback_obj(ObjectFromNPVariant(args[2]));
   if (!callback_obj.get()) {
     SetException("startDaemon: invalid callback parameter");
     return false;
   }
 
-  // TODO(wez): Pass a static method here, that will post the result
-  // back to us on the right thread (crbug.com/156257).
   daemon_controller_->SetConfigAndStart(
       config_dict.Pass(),
       NPVARIANT_TO_BOOLEAN(args[1]),
       base::Bind(&HostNPScriptObject::InvokeAsyncResultCallback,
                  base::Unretained(this), callback_obj));
   return true;
 }
 
 bool HostNPScriptObject::StopDaemon(const NPVariant* args,
                                     uint32_t arg_count,
                                     NPVariant* result) {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
   if (arg_count != 1) {
     SetException("stopDaemon: bad number of arguments");
     return false;
   }
 
   ScopedRefNPObject callback_obj(ObjectFromNPVariant(args[0]));
   if (!callback_obj.get()) {
     SetException("stopDaemon: invalid callback parameter");
     return false;
   }
 
-  // TODO(wez): Pass a static method here, that will post the result
-  // back to us on the right thread (crbug.com/156257).
   daemon_controller_->Stop(
       base::Bind(&HostNPScriptObject::InvokeAsyncResultCallback,
                  base::Unretained(this), callback_obj));
   return true;
 }
 
 void HostNPScriptObject::NotifyStateChanged(State state) {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
   state_ = state;
@@ skipping 121 matching lines @@
   params[0] = NPVariantFromString(private_key);
   params[1] = NPVariantFromString(public_key);
   InvokeAndIgnoreResult(callback.get(), params, arraysize(params));
   g_npnetscape_funcs->releasevariantvalue(&(params[0]));
   g_npnetscape_funcs->releasevariantvalue(&(params[1]));
 }
 
 void HostNPScriptObject::InvokeAsyncResultCallback(
     const ScopedRefNPObject& callback,
     DaemonController::AsyncResult result) {
-  if (!plugin_task_runner_->BelongsToCurrentThread()) {
-    plugin_task_runner_->PostTask(
-        FROM_HERE, base::Bind(
-            &HostNPScriptObject::InvokeAsyncResultCallback,
-            weak_ptr_, callback, result));
-    return;
-  }
+  DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
   NPVariant result_var;
   INT32_TO_NPVARIANT(static_cast<int32>(result), result_var);
   InvokeAndIgnoreResult(callback.get(), &result_var, 1);
   g_npnetscape_funcs->releasevariantvalue(&result_var);
 }
 
 void HostNPScriptObject::InvokeBooleanCallback(
     const ScopedRefNPObject& callback, bool result) {
   if (!plugin_task_runner_->BelongsToCurrentThread()) {

[Wez, 2013/09/10 09:05:37]

nit: This can be replaced by
DCHECK(plugin_task_runner_->BelongsToCurrentThread()), I think, since
PairingRegistry always calls back on the caller thread.

[alexeypa (please no reviews), 2013/09/10 16:59:22]

Done.

     plugin_task_runner_->PostTask(
         FROM_HERE, base::Bind(
             &HostNPScriptObject::InvokeBooleanCallback,
             weak_ptr_, callback, result));
     return;
   }
 
   NPVariant result_var;
   BOOLEAN_TO_NPVARIANT(result, result_var);
   InvokeAndIgnoreResult(callback.get(), &result_var, 1);
   g_npnetscape_funcs->releasevariantvalue(&result_var);
 }
 
 void HostNPScriptObject::InvokeGetDaemonConfigCallback(
     const ScopedRefNPObject& callback,
     scoped_ptr<base::DictionaryValue> config) {
-  if (!plugin_task_runner_->BelongsToCurrentThread()) {
-    plugin_task_runner_->PostTask(
-        FROM_HERE, base::Bind(
-            &HostNPScriptObject::InvokeGetDaemonConfigCallback,
-            weak_ptr_, callback, base::Passed(&config)));
-    return;
-  }
+  DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
   // There is no easy way to create a dictionary from an NPAPI plugin
   // so we have to serialize the dictionary to pass it to JavaScript.
   std::string config_str;
   if (config.get())
     base::JSONWriter::Write(config.get(), &config_str);
 
   NPVariant config_val = NPVariantFromString(config_str);
   InvokeAndIgnoreResult(callback.get(), &config_val, 1);
   g_npnetscape_funcs->releasevariantvalue(&config_val);
 }
 
 void HostNPScriptObject::InvokeGetDaemonVersionCallback(
     const ScopedRefNPObject& callback, const std::string& version) {
-  if (!plugin_task_runner_->BelongsToCurrentThread()) {
-    plugin_task_runner_->PostTask(
-        FROM_HERE, base::Bind(
-            &HostNPScriptObject::InvokeGetDaemonVersionCallback,
-            weak_ptr_, callback, version));
-    return;
-  }
+  DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
   NPVariant version_val = NPVariantFromString(version);
   InvokeAndIgnoreResult(callback.get(), &version_val, 1);
   g_npnetscape_funcs->releasevariantvalue(&version_val);
 }
 
 void HostNPScriptObject::InvokeGetPairedClientsCallback(
     const ScopedRefNPObject& callback,
     scoped_ptr<base::ListValue> paired_clients) {
   if (!plugin_task_runner_->BelongsToCurrentThread()) {
     plugin_task_runner_->PostTask(
         FROM_HERE, base::Bind(
             &HostNPScriptObject::InvokeGetPairedClientsCallback,
             weak_ptr_, callback, base::Passed(&paired_clients)));
     return;
   }
 
   std::string paired_clients_json;
   base::JSONWriter::Write(paired_clients.get(), &paired_clients_json);
 
   NPVariant paired_clients_val = NPVariantFromString(paired_clients_json);
   InvokeAndIgnoreResult(callback.get(), &paired_clients_val, 1);
   g_npnetscape_funcs->releasevariantvalue(&paired_clients_val);
 }
 
 void HostNPScriptObject::InvokeGetUsageStatsConsentCallback(
     const ScopedRefNPObject& callback,
-    bool supported,
-    bool allowed,
-    bool set_by_policy) {
-  if (!plugin_task_runner_->BelongsToCurrentThread()) {
-    plugin_task_runner_->PostTask(
-        FROM_HERE, base::Bind(
-            &HostNPScriptObject::InvokeGetUsageStatsConsentCallback,
-            weak_ptr_, callback, supported, allowed,
-            set_by_policy));
-    return;
-  }
+    const DaemonController::UsageStatsConsent& consent) {
+  DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
   NPVariant params[3];
-  BOOLEAN_TO_NPVARIANT(supported, params[0]);
-  BOOLEAN_TO_NPVARIANT(allowed, params[1]);
-  BOOLEAN_TO_NPVARIANT(set_by_policy, params[2]);
+  BOOLEAN_TO_NPVARIANT(consent.supported, params[0]);
+  BOOLEAN_TO_NPVARIANT(consent.allowed, params[1]);
+  BOOLEAN_TO_NPVARIANT(consent.set_by_policy, params[2]);
   InvokeAndIgnoreResult(callback.get(), params, arraysize(params));
   g_npnetscape_funcs->releasevariantvalue(&(params[0]));
   g_npnetscape_funcs->releasevariantvalue(&(params[1]));
   g_npnetscape_funcs->releasevariantvalue(&(params[2]));
 }
 
 void HostNPScriptObject::LogDebugInfo(const std::string& message) {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
   if (log_debug_info_func_.get()) {
@@ skipping 24 matching lines @@
 }
 
 void HostNPScriptObject::SetException(const std::string& exception_string) {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
   g_npnetscape_funcs->setexception(parent_, exception_string.c_str());
   LOG(INFO) << exception_string;
 }
 
 }  // namespace remoting