Remote assistance on Chrome OS Part IV - It2MeHost

remoting/host/it2me/it2me_native_messaging_host.cc

 // Copyright 2013 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/it2me/it2me_native_messaging_host.h"
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
+#include "base/message_loop/message_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringize_macros.h"
 #include "base/threading/thread.h"
 #include "base/values.h"
 #include "net/base/net_util.h"
 #include "net/url_request/url_fetcher.h"
 #include "remoting/base/auth_token_util.h"
 #include "remoting/base/service_urls.h"
 #include "remoting/host/chromoting_host_context.h"
 #include "remoting/host/host_exit_codes.h"
 #include "remoting/protocol/name_value_map.h"
 
 namespace remoting {
 
 namespace {
 
 const remoting::protocol::NameMapElement<It2MeHostState> kIt2MeHostStates[] = {
     {kDisconnected, "DISCONNECTED"},
     {kStarting, "STARTING"},
     {kRequestedAccessCode, "REQUESTED_ACCESS_CODE"},
     {kReceivedAccessCode, "RECEIVED_ACCESS_CODE"},
     {kConnected, "CONNECTED"},
     {kDisconnecting, "DISCONNECTING"},
     {kError, "ERROR"},
     {kInvalidDomainError, "INVALID_DOMAIN_ERROR"}, };
 
 }  // namespace
 
+// static
+scoped_ptr<extensions::NativeMessageHost> It2MeNativeMessagingHost::Create() {
+  scoped_ptr<It2MeHostFactory> factory(new remoting::It2MeHostFactory());
+  scoped_refptr<ChromotingHostContext> context =
+      ChromotingHostContext::Create(new AutoThreadTaskRunner(
+          base::MessageLoopProxy::current(), base::Bind(&base::DoNothing)));
+  scoped_ptr<extensions::NativeMessageHost> host(new It2MeNativeMessagingHost(
+  // Remoting uses AutoThread as its threading primitive, which will
+  // exit itself (by posting a Quit message) when there are no more references
+  // to the task runner.
+  // On ChromeOS, the It2MeNativeMessagingHost is created on the UI
+  // thread of the browser process.  Since we do not want to quit the browser
+  // UI thread just because there are no more tasks in the task_runner,
+  // base::DoNothing is passed in as the quit closure.

[Wez, 2014/10/17 17:57:59]

This comment is in the wrong place, and misses the point.

AutoThreadTaskRunner is a TaskRunner with the special property that it will
continue to process tasks until no references remain, at least. The QuitClosure
we usually pass does the simple thing of stopping the underlying TaskRunner, but
in general the point is that the underlying TaskRunner _must not stop processing
tasks_ until that callback is invoked. So by passing DoNothing you're violating
that contract, and you may find that things break at browser shutdown.

[kelvinp, 2014/10/20 00:21:15]

I can update the comment.

Yes, I agree that this is a hack, but this is the best solution that I can come
across so far to reconcile the two contraditing threading ownership philosophies
in Chrome and remoting.  Chrome has an explicit ownership model in the sense
that those task runners should only be destroyed by the browser thread.  By
definition, the assumptions of the AutoThreadTaskRunner won't hold.

I have tested the shutdown scenarios extensively and I have found no crashes. 
There is only a leak in the url_request_context_getter when the entire chrome
process goes away, which I think it is ok.

+      context,
+      factory.Pass()));
+  return host.Pass();
+}
+
 It2MeNativeMessagingHost::It2MeNativeMessagingHost(
-    scoped_refptr<AutoThreadTaskRunner> task_runner,
+    scoped_refptr<ChromotingHostContext> context,
     scoped_ptr<It2MeHostFactory> factory)
     : client_(NULL),
       factory_(factory.Pass()),
       weak_factory_(this) {
   weak_ptr_ = weak_factory_.GetWeakPtr();
 
-  // Initialize the host context to manage the threads for the it2me host.
-  // The native messaging host, rather than the It2MeHost object, owns and
-  // maintains the lifetime of the host context.
-
-  host_context_.reset(ChromotingHostContext::Create(task_runner).release());
+  host_context_ = context;
+  task_runner_ = host_context_->ui_task_runner();
 
   const ServiceUrls* service_urls = ServiceUrls::GetInstance();
   const bool xmpp_server_valid =
       net::ParseHostAndPort(service_urls->xmpp_server_address(),
                             &xmpp_server_config_.host,
                             &xmpp_server_config_.port);
   DCHECK(xmpp_server_valid);
 
   xmpp_server_config_.use_tls = service_urls->xmpp_server_use_tls();
   directory_bot_jid_ = service_urls->directory_bot_jid();
@@ skipping 131 matching lines @@
   }
 
   if (!message.GetString("directoryBotJid", &directory_bot_jid_)) {
     SendErrorAndExit(response.Pass(),
                      "'directoryBotJid' not found in request.");
     return;
   }
 #endif  // !defined(NDEBUG)
 
   // Create the It2Me host and start connecting.
-  it2me_host_ = factory_->CreateIt2MeHost(host_context_.get(),
+  it2me_host_ = factory_->CreateIt2MeHost(host_context_,
                                           host_context_->ui_task_runner(),
                                           weak_ptr_,
                                           xmpp_config,
                                           directory_bot_jid_);
   it2me_host_->Connect();
 
   SendMessageToClient(response.Pass());
 }
 
 void It2MeNativeMessagingHost::ProcessDisconnect(
@@ skipping 90 matching lines @@
 }
 
 /* static */
 std::string It2MeNativeMessagingHost::HostStateToString(
     It2MeHostState host_state) {
   return ValueToName(kIt2MeHostStates, host_state);
 }
 
 }  // namespace remoting