Remote Assistance on Chrome OS Part III - NativeMessageHost

chrome/browser/extensions/api/messaging/native_message_port.cc

Index: chrome/browser/extensions/api/messaging/native_message_port.cc
diff --git a/chrome/browser/extensions/api/messaging/native_message_port.cc b/chrome/browser/extensions/api/messaging/native_message_port.cc
index a0a47d561bbb77cad1e495fcaad3a1a5bb46da41..22e1f7bbb7619b3b72a223c28f80946f820a874c 100644
--- a/chrome/browser/extensions/api/messaging/native_message_port.cc
+++ b/chrome/browser/extensions/api/messaging/native_message_port.cc
@@ -5,27 +5,57 @@
 #include "chrome/browser/extensions/api/messaging/native_message_port.h"
 
 #include "base/bind.h"
+#include "base/single_thread_task_runner.h"
 #include "chrome/browser/extensions/api/messaging/native_message_process_host.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace extensions {
 
-NativeMessagePort::NativeMessagePort(NativeMessageProcessHost* native_process)
-    : native_process_(native_process) {
+NativeMessagePort::NativeMessagePort(
+    base::WeakPtr<MessageService> message_service,
+    int port_id,
+    scoped_ptr<NativeMessageHost> native_message_host)
+    : weak_message_service_(message_service),
+      port_id_(port_id),
+      native_message_host_(native_message_host.Pass()),
+      host_task_runner_(native_message_host_->task_runner()),
+      weak_factory_(this) {
+  native_message_host_->set_client(weak_factory_.GetWeakPtr());

[Sergey Ulanov, 2014/09/30 20:33:39]

See my comment in native_message_process_host.cc. This class should be
responsible for jumping from UI to IO thread and _back_. Ideally
NativeMessageHost implementations should call the Client  interface on the same
thread it does everything else. This means that you want to call set_client() on
the IO thread. Take a look at remoting/host/host_window_proxy.h - that's a good
example of a similar object that forwards calls between two threads. Internally
it has Core object internally that works on the target thread and is destroyed
on the target thread. I think you want something similar here:
 - NativeMessagePort::Core would own native_message_host and call it on the
host_task_runner_.
 - NativeMessagePort owns instance of the Core and posts tasks to
host_task_runner_.
 - Core handles Client callbacks from NativeMessageHost and posts tasks on UI
thread for NativeMessagePort.

The goal here is to isolate threading logic in this class instead of having it
sprinkled all around (e.g. in NativeMessageProcessHost). 

[kelvinp, 2014/10/01 06:08:02]

Done.

 }
 
 NativeMessagePort::~NativeMessagePort() {
-  content::BrowserThread::DeleteSoon(
-      content::BrowserThread::IO, FROM_HERE, native_process_);
+  if (!host_task_runner_->BelongsToCurrentThread()) {
+    host_task_runner_->DeleteSoon(FROM_HERE, native_message_host_.release());
+  }
 }
 
 void NativeMessagePort::DispatchOnMessage(
     const Message& message,
     int target_port_id) {
-  content::BrowserThread::PostTask(
-      content::BrowserThread::IO, FROM_HERE,
-      base::Bind(&NativeMessageProcessHost::Send,
-                 base::Unretained(native_process_), message.data));
+  if (!host_task_runner_->BelongsToCurrentThread()) {
+    host_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&NativeMessagePort::DispatchOnMessage,
+                   base::Unretained(this),
+                   message,
+                   target_port_id));
+    return;
+  }
+  native_message_host_->OnMessage(message.data);
+}
+
+void NativeMessagePort::PostMessageFromNativeHost(const std::string& message) {
+  if (weak_message_service_) {
+    weak_message_service_->PostMessage(
+        port_id_, Message(message, false /* user_gesture */));
+  }
+}
+
+void NativeMessagePort::CloseChannel(const std::string& error_message) {
+  if (weak_message_service_) {
+    weak_message_service_->CloseChannel(port_id_, error_message);
+  }
 }
 
 }  // namespace extensions
+