NaCl cleanup: Stop connecting to the SRPC trusted command channel

components/nacl/loader/nacl_trusted_listener.cc

Index: components/nacl/loader/nacl_trusted_listener.cc
diff --git a/components/nacl/loader/nacl_trusted_listener.cc b/components/nacl/loader/nacl_trusted_listener.cc
index 4c67a09f490644cb55857f4ac307e779919ddea7..0bbd82fc53af25583e9410b78965e53f3e35b094 100644
--- a/components/nacl/loader/nacl_trusted_listener.cc
+++ b/components/nacl/loader/nacl_trusted_listener.cc
@@ -5,6 +5,32 @@
 #include "components/nacl/loader/nacl_trusted_listener.h"
 
 #include "base/single_thread_task_runner.h"
+#include "ipc/message_filter.h"
+#include "native_client/src/public/chrome_main.h"
+
+namespace {
+
+// The OnChannelError() event must be processed in a MessageFilter so it can
+// be handled on the IO thread.  The main thread used by NaClListener is busy
+// in NaClChromeMainAppStart(), so it can't be used for servicing messages.
+class EOFMessageFilter : public IPC::MessageFilter {
+ public:
+  void OnChannelError() override {

[jvoung (off chromium), 2015/06/09 15:43:44]

It wasn't too clear if this should also override OnChannelClosing, but there is
a lot of instances of one or the other and not many instances of both from code
search.

[Mark Seaborn, 2015/06/10 00:31:04]

From reading ipc/, it looks like OnChannelClosing() is only called as a result
of someone calling channel->Close().  So:
 * Close() -> OnChannelClosing(): means our channel endpoint is closed.
 * OnChannelError(): means the channel's other endpoint was closed.

channel->Close() should never happen on this channel.  There is a
channel_->Close() call in NaClTrustedListener::OnChannelError() below, but:

 * It's probably unnecessary (just boilerplate?)
 * NaClTrustedListener::OnChannelError() never actually gets called.  A call to
it gets queued on the main thread, but the main thread gets blocked in
NaClChromeMainAppStart().

I could remove NaClTrustedListener::OnChannelError(), if you think it would make
the code clearer?

[jvoung (off chromium), 2015/06/10 16:33:01]

Okay removing NaClTrustedListener::OnChannelError() seems clearer to me, if it's
not actually being called / can't be called.

[Mark Seaborn, 2015/06/10 17:05:21]

OK, done.

+    // The renderer process dropped its connection to this process (the NaCl
+    // loader process), either because the <embed> element was removed
+    // (perhaps implicitly if the tab was closed) or because the renderer
+    // crashed.  The NaCl loader process should therefore exit.
+    //
+    // For SFI NaCl, trusted code does this exit voluntarily, but untrusted
+    // code cannot disable it.  However, for Non-SFI NaCl, the following exit
+    // call could be disabled by untrusted code.
+    NaClExit(0);
+  }
+ private:
+  ~EOFMessageFilter() override {}
+};
+
+}
 
 NaClTrustedListener::NaClTrustedListener(
     const IPC::ChannelHandle& handle,
@@ -17,6 +43,7 @@ NaClTrustedListener::NaClTrustedListener(
                                       ipc_task_runner,
                                       true,  /* create_channel_now */
                                       shutdown_event).Pass();
+  channel_->AddFilter(new EOFMessageFilter());
 }
 
 NaClTrustedListener::~NaClTrustedListener() {