Adding nonsfi content handler

services/nacl/nonsfi_content_handler_main.cc

Index: services/nacl/nonsfi_content_handler_main.cc
diff --git a/services/nacl/nonsfi_content_handler_main.cc b/services/nacl/nonsfi_content_handler_main.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6c985225dc15ae2bca785a8def670a34ba199085
--- /dev/null
+++ b/services/nacl/nonsfi_content_handler_main.cc
@@ -0,0 +1,166 @@
+// Copyright 2015 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 <fcntl.h>
+
+#include "base/files/file_util.h"
+#include "mojo/application/application_runner_chromium.h"
+#include "mojo/application/content_handler_factory.h"
+#include "mojo/data_pipe_utils/data_pipe_utils.h"
+#include "mojo/message_pump/message_pump_mojo.h"
+#include "mojo/public/c/system/main.h"
+#include "mojo/public/cpp/application/application_impl.h"
+#include "mojo/public/platform/nacl/mojo_irt.h"
+#include "nacl_bindings/monacl_sel_main.h"

[Mark Seaborn, 2015/09/01 15:55:45]

Shouldn't be needed: this refers to SFI NaCl.

[Sean Klein, 2015/09/01 20:24:47]

Done.

+#include "native_client/src/public/irt_core.h"
+#include "native_client/src/public/nacl_desc.h"
+#include "native_client/src/public/nonsfi/elf_loader.h"
+
+namespace nacl {
+namespace content_handler {
+
+MojoHandle g_mojo_handle = MOJO_HANDLE_INVALID;

[Mark Seaborn, 2015/09/01 15:55:45]

This can go inside the anon namespace.

[Sean Klein, 2015/09/01 20:24:48]

Done -- sort of. Moved into anon namespace of the interface file.

+
+namespace {
+
+MojoResult _MojoGetInitialHandle(MojoHandle* handle) {
+  // TODO(smklein): Test this handle
+  *handle = g_mojo_handle;
+  return MOJO_RESULT_OK;
+}
+
+const struct nacl_irt_mojo kIrtMojo = {

[Mark Seaborn, 2015/09/01 15:55:45]

This is duplicated from monacl_shell_nonsfi.cc.  Let's not duplicate code. :-) 
Can you split this and mojo_irt_nonsfi_query() into a separate file in order to
share it?

[Sean Klein, 2015/09/01 20:24:47]

Done.

+  MojoCreateSharedBuffer,
+  MojoDuplicateBufferHandle,
+  MojoMapBuffer,
+  MojoUnmapBuffer,
+  MojoCreateDataPipe,
+  MojoWriteData,
+  MojoBeginWriteData,
+  MojoEndWriteData,
+  MojoReadData,
+  MojoBeginReadData,
+  MojoEndReadData,
+  MojoGetTimeTicksNow,
+  MojoClose,
+  MojoWait,
+  MojoWaitMany,
+  MojoCreateMessagePipe,
+  MojoWriteMessage,
+  MojoReadMessage,
+  _MojoGetInitialHandle,
+};
+
+const struct nacl_irt_interface kIrtInterfaces[] = {
+  { NACL_IRT_MOJO_v0_1, &kIrtMojo, sizeof(kIrtMojo), nullptr }
+};
+
+size_t mojo_irt_nonsfi_query(const char* interface_ident,
+                             void* table, size_t tablesize) {
+  size_t result = nacl_irt_query_list(interface_ident,
+                                      table,
+                                      tablesize,
+                                      kIrtInterfaces,
+                                      sizeof(kIrtInterfaces));
+  if (result != 0)
+    return result;
+  return nacl_irt_query_core(interface_ident, table, tablesize);
+}
+
+// Copies response (input) into new temporary file at file_path (output).
+bool URLResponseToTempFile(mojo::URLResponsePtr& response,
+                           base::FilePath* file_path) {
+  if (!base::CreateTemporaryFile(file_path)) {
+    return false;
+  }
+
+  if (!mojo::common::BlockingCopyToFile(response->body.Pass(), *file_path)) {

[Mark Seaborn, 2015/09/01 15:55:45]

Could you make this operate on an FD rather than a filename?  Then we can
unlink() the temp file immediately after open()ing it, which would address the
TODO below.

You could do that as a separate change, as a cleanup to the existing code in
content_handler_main.cc.

[Sean Klein, 2015/09/01 20:24:48]

I will do it in this change for the nonsfi version -- I'll add a separate change
for the SFI version.

+    base::DeleteFile(*file_path, false);
+    return false;
+  }
+
+  // TODO(ncbray): can we ensure temp file deletion even if we crash?
+  return true;
+}
+
+// Opens file from filepath, returns descriptor.
+int FileDescFromPath(base::FilePath& path, char* pathname) {
+  strncpy(pathname, path.value().c_str(), PATH_MAX);

[Mark Seaborn, 2015/09/01 15:55:45]

Can you avoid using fixed length buffers?  Why not pass path.value().c_str() to
open()?

[Sean Klein, 2015/09/01 20:24:48]

Done.

+  int fd = open(pathname, O_RDONLY);
+  if (fd < 0) {
+    LOG(FATAL) << "Failed to open " << pathname << ": " << strerror(errno)
+               << "\n";
+  }
+  return fd;
+}
+}  // namespace
+
+class NaClContentHandler : public mojo::ApplicationDelegate,
+                           public mojo::ContentHandlerFactory::Delegate {
+ public:
+  NaClContentHandler() : content_handler_factory_(this) {}
+
+ private:
+  // Overridden from ApplicationDelegate:
+  void Initialize(mojo::ApplicationImpl* app) override {}
+
+  // Overridden from ApplicationDelegate:
+  bool ConfigureIncomingConnection(
+      mojo::ApplicationConnection* connection) override {
+    connection->AddService(&content_handler_factory_);
+    return true;
+  }
+
+  // Overridden from ContentHandlerFactory::ManagedDelegate:
+  void RunApplication(
+      mojo::InterfaceRequest<mojo::Application> application_request,
+      mojo::URLResponsePtr response) override {
+    // Needed to use Mojo interfaces on this thread.
+    base::MessageLoop loop(mojo::common::MessagePumpMojo::Create());
+    // Aquire the nexe.

[Mark Seaborn, 2015/09/01 15:55:45]

"Acquire".  Same below.

[Sean Klein, 2015/09/01 20:24:47]

Whoops. Fixed.

+    base::FilePath nexe_path;
+    if (!URLResponseToTempFile(response, &nexe_path)) {
+      LOG(FATAL) << "could not redirect nexe to temp file";
+    }
+    // Aquire the IRT.
+    //mojo::URLLoaderPtr url_loader = url_loader_.Pass();

[Mark Seaborn, 2015/09/01 15:55:45]

Remove commented-out code.

[Sean Klein, 2015/09/01 20:24:47]

Done.

+    //LoadIRT(url_loader);
+
+    // Run.
+    char argv[PATH_MAX + 1];
+    int fd = FileDescFromPath(nexe_path, argv);
+    uintptr_t entry = NaClLoadElfFile(fd);
+
+    g_mojo_handle = application_request.PassMessagePipe().release().value();
+    int argc = 1;
+    char* argvp = &argv[0];

[Mark Seaborn, 2015/09/01 15:55:45]

You're using the temp filename as the program's argv[0], but it would be better
not to expose the temp filename.  You could pass a fixed argv[0] string here. 
For comparison, sel_main.c uses "NaClMain".

[Sean Klein, 2015/09/01 20:24:47]

Done.

+    int exit_code =
+        nacl_irt_nonsfi_entry(argc, &argvp, environ,

[Mark Seaborn, 2015/09/01 15:55:45]

This function is not supposed to return.  You can call abort() after it.

[Sean Klein, 2015/09/01 20:24:47]

Done.

+                              reinterpret_cast<nacl_entry_func_t>(entry),
+                              mojo_irt_nonsfi_query);
+    // TODO(ncbray): are the file handles actually closed at this point?

[Mark Seaborn, 2015/09/01 15:55:45]

Yes, |fd| gets closed by NaClLoadElfFile().  See
native_client/src/nonsfi/loader/elf_loader.c.

[Sean Klein, 2015/09/01 20:24:48]

Added comment noting this.

+
+    // Clean up.
+    if (!base::DeleteFile(nexe_path, false)) {

[Mark Seaborn, 2015/09/01 15:55:45]

Since control won't reach here, this cleanup is not going to happen.

You could do this cleanup immediately after we open() the file.

[Sean Klein, 2015/09/01 20:24:48]

Done.

+      LOG(FATAL) << "Failed to remove nexe temp file " << nexe_path.value();
+    }
+
+    // Exits the process cleanly, does not return.
+    mojo::NaClExit(exit_code);
+    NOTREACHED();
+  }
+
+  mojo::ContentHandlerFactory content_handler_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(NaClContentHandler);
+};
+
+}  // namespace content_handler
+}  // namespace nacl
+
+MojoResult MojoMain(MojoHandle application_request) {
+  mojo::ApplicationRunnerChromium runner(
+      new nacl::content_handler::NaClContentHandler());
+  return runner.Run(application_request);
+}