Refactored launching of nexes to separate file.

Refactored launching of nexes to separate file.

This CL should be useful for the translation steps, as each of

1) Translating
2) Linking, and
3) Launching the translated nexe

will require a new nexe launching step. Ideally the communication
between the content handler and any necessary child nexes can occur
through the MojoHandle.

BUG=https://github.com/domokit/mojo/issues/396
R=mseaborn@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/9b9b2a663c77dcbf70d4194f0beeb2e6dcf38dd4

[Sean Klein, 2015-10-01 18:18:27]

Patchset #1 (id:1) has been deleted

[Sean Klein, 2015-10-01 22:21:05]

Patchset #1 (id:20001) has been deleted

[Sean Klein, 2015-10-01 22:42:24]

Patchset #1 (id:40001) has been deleted

[Sean Klein, 2015-10-01 22:52:18]

smklein@chromium.org changed reviewers:
+ phosek@chromium.org

[Petr Hosek, 2015-10-01 23:03:57]

https://codereview.chromium.org/1371413003/diff/60001/mojo/nacl/nexe_launcher...
File mojo/nacl/nexe_launcher_nonsfi.h (right):

https://codereview.chromium.org/1371413003/diff/60001/mojo/nacl/nexe_launcher...
mojo/nacl/nexe_launcher_nonsfi.h:16: void launch_nexe_nonsfi(int fd, MojoHandle
handle);
Since this is C++, shouldn't this live in nacl namespace? We should also follow
Mojo convetions so this function should probably be named as
MojoLaunchNexeNonsfi (or something along those lines).

https://codereview.chromium.org/1371413003/diff/60001/mojo/nacl/nexe_launcher...
mojo/nacl/nexe_launcher_nonsfi.h:18: #endif // MOJO_NACL_NEXE_LAUNCHER_NONSFI_H_
Two spaces between `endif` and `//` (it's weird but that's what Google/Chromium
code style says).

[Sean Klein, 2015-10-01 23:24:05]

https://codereview.chromium.org/1371413003/diff/60001/mojo/nacl/nexe_launcher...
File mojo/nacl/nexe_launcher_nonsfi.h (right):

https://codereview.chromium.org/1371413003/diff/60001/mojo/nacl/nexe_launcher...
mojo/nacl/nexe_launcher_nonsfi.h:16: void launch_nexe_nonsfi(int fd, MojoHandle
handle);
On 2015/10/01 23:03:57, Petr Hosek wrote:
> Since this is C++, shouldn't this live in nacl namespace? We should also
follow
> Mojo convetions so this function should probably be named as
> MojoLaunchNexeNonsfi (or something along those lines).

Done.

https://codereview.chromium.org/1371413003/diff/60001/mojo/nacl/nexe_launcher...
mojo/nacl/nexe_launcher_nonsfi.h:18: #endif // MOJO_NACL_NEXE_LAUNCHER_NONSFI_H_
On 2015/10/01 23:03:57, Petr Hosek wrote:
> Two spaces between `endif` and `//` (it's weird but that's what
Google/Chromium
> code style says).

Done.

[Sean Klein, 2015-10-02 20:30:31]

smklein@chromium.org changed reviewers:
+ mseaborn@chromium.org

[Mark Seaborn, 2015-10-02 22:31:57]

LGTM

https://codereview.chromium.org/1371413003/diff/80001/mojo/nacl/nexe_launcher...
File mojo/nacl/nexe_launcher_nonsfi.h (right):

https://codereview.chromium.org/1371413003/diff/80001/mojo/nacl/nexe_launcher...
mojo/nacl/nexe_launcher_nonsfi.h:9: #include
"native_client/src/public/irt_core.h"
Nit: These two NaCl headers aren't needed by this .h file.

https://codereview.chromium.org/1371413003/diff/80001/mojo/nacl/nexe_launcher...
mojo/nacl/nexe_launcher_nonsfi.h:18: void MojoLaunchNexeNonsfi(int fd,
MojoHandle handle);
Suggestion: Could make this more self-documenting by naming the args "nexe_fd"
and "initial_handle".

https://codereview.chromium.org/1371413003/diff/80001/services/nacl/content_h...
File services/nacl/content_handler_main_nonsfi.cc (right):

https://codereview.chromium.org/1371413003/diff/80001/services/nacl/content_h...
services/nacl/content_handler_main_nonsfi.cc:60: MojoHandle h =
application_request.PassMessagePipe().release().value();
Nit: might want to avoid single-char variable names and call this "handle" or
"initial_handle".

[Sean Klein, 2015-10-02 22:41:11]

https://codereview.chromium.org/1371413003/diff/80001/mojo/nacl/nexe_launcher...
File mojo/nacl/nexe_launcher_nonsfi.h (right):

https://codereview.chromium.org/1371413003/diff/80001/mojo/nacl/nexe_launcher...
mojo/nacl/nexe_launcher_nonsfi.h:9: #include
"native_client/src/public/irt_core.h"
On 2015/10/02 22:31:57, Mark Seaborn wrote:
> Nit: These two NaCl headers aren't needed by this .h file.

Done.

https://codereview.chromium.org/1371413003/diff/80001/mojo/nacl/nexe_launcher...
mojo/nacl/nexe_launcher_nonsfi.h:18: void MojoLaunchNexeNonsfi(int fd,
MojoHandle handle);
On 2015/10/02 22:31:57, Mark Seaborn wrote:
> Suggestion: Could make this more self-documenting by naming the args "nexe_fd"
> and "initial_handle".

Done.

https://codereview.chromium.org/1371413003/diff/80001/services/nacl/content_h...
File services/nacl/content_handler_main_nonsfi.cc (right):

https://codereview.chromium.org/1371413003/diff/80001/services/nacl/content_h...
services/nacl/content_handler_main_nonsfi.cc:60: MojoHandle h =
application_request.PassMessagePipe().release().value();
On 2015/10/02 22:31:57, Mark Seaborn wrote:
> Nit: might want to avoid single-char variable names and call this "handle" or
> "initial_handle".

Done.

[Sean Klein, 2015-10-02 22:49:23]

Committed patchset #3 (id:100001) manually as
9b9b2a663c77dcbf70d4194f0beeb2e6dcf38dd4 (presubmit successful).