Initial implementation of an IPC adapter to expose Chrome IPC to Native Client.

Initial implementation of an IPC adapter to expose Chrome IPC to Native Client.

This provides an implementation of sendmsg and recvmsg approxinately to what we think NaCl will expose to Chrome. Since NaCl isn't ready yet in this regard, it's still a bit speculative. And there is no support for sending handles across which will be the tricky part.

TEST=included unit test
BUG=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=129981

[brettw, 2012-03-26 21:45:53]

Jam -> IPC
Bill -> everything else

[bbudge, 2012-03-26 22:52:30]

http://codereview.chromium.org/9863005/diff/1/chrome/nacl/nacl_ipc_adapter.cc
File chrome/nacl/nacl_ipc_adapter.cc (right):

http://codereview.chromium.org/9863005/diff/1/chrome/nacl/nacl_ipc_adapter.cc...
chrome/nacl/nacl_ipc_adapter.cc:166: // Nothing accumulated, so see if the input
data contains a full message.
Something seems wrong here. The above 'if' block will drop down here after
adding the data.

http://codereview.chromium.org/9863005/diff/1/chrome/nacl/nacl_ipc_adapter.cc...
chrome/nacl/nacl_ipc_adapter.cc:178: locked_data_.to_be_sent_.append(input_data,
input_data_len);
Seems like data could get added twice.

http://codereview.chromium.org/9863005/diff/1/chrome/nacl/nacl_ipc_adapter.cc...
chrome/nacl/nacl_ipc_adapter.cc:224: // leak memory to the untrusted plugin, so
zero everything out first.
Maybe memory -> data so it doesn't sound like a memory leak?

http://codereview.chromium.org/9863005/diff/1/chrome/nacl/nacl_ipc_adapter.cc...
chrome/nacl/nacl_ipc_adapter.cc:296: // sometimes out buffer will point to the
to_be_sent_ string which is
s/out/our

[brettw, 2012-03-27 17:40:27]

New snap up with comments addressed.

http://codereview.chromium.org/9863005/diff/1/chrome/nacl/nacl_ipc_adapter.cc
File chrome/nacl/nacl_ipc_adapter.cc (right):

http://codereview.chromium.org/9863005/diff/1/chrome/nacl/nacl_ipc_adapter.cc...
chrome/nacl/nacl_ipc_adapter.cc:166: // Nothing accumulated, so see if the input
data contains a full message.
The comment was misleading but I think the code was correct. I moved things
around a bit and fixed the comments, hopefully it's more clear now.

http://codereview.chromium.org/9863005/diff/1/chrome/nacl/nacl_ipc_adapter.cc...
chrome/nacl/nacl_ipc_adapter.cc:178: locked_data_.to_be_sent_.append(input_data,
input_data_len);
Whoops! Nice catch. I modified the test to send in 3 chunks to catch this.

[bbudge, 2012-03-27 18:38:36]

More comments. Mostly readability.

http://codereview.chromium.org/9863005/diff/2001/chrome/nacl/nacl_ipc_adapter.cc
File chrome/nacl/nacl_ipc_adapter.cc (right):

http://codereview.chromium.org/9863005/diff/2001/chrome/nacl/nacl_ipc_adapter...
chrome/nacl/nacl_ipc_adapter.cc:34: BUFFER_UNDERFLOW
This name seems counterintuitive to me. Perhaps something like
MESSAGE_DATA_OVERFLOW or MESSAGE_CONTAINS_EXTRA_DATA?

http://codereview.chromium.org/9863005/diff/2001/chrome/nacl/nacl_ipc_adapter...
chrome/nacl/nacl_ipc_adapter.cc:278: 
Extra line?

http://codereview.chromium.org/9863005/diff/2001/chrome/nacl/nacl_ipc_manager.h
File chrome/nacl/nacl_ipc_manager.h (right):

http://codereview.chromium.org/9863005/diff/2001/chrome/nacl/nacl_ipc_manager...
chrome/nacl/nacl_ipc_manager.h:19: // map from handle to our IPC adapters.
It seems like it would be better if this comment explained about the use of this
class rather than the implementation. It wasn't obvious to me at first that
NaCLIPCAdapters weren't used directly.

[brettw, 2012-03-28 18:08:03]

New snap up

[bbudge, 2012-03-28 18:16:07]

LGTM