Use shared memory for moving data over BlobChannel Mojo interface.

blimp/engine/mojo/blob_channel_service.cc

Index: blimp/engine/mojo/blob_channel_service.cc
diff --git a/blimp/engine/mojo/blob_channel_service.cc b/blimp/engine/mojo/blob_channel_service.cc
index 0234df60698b3a2c2628537b7e66cfd9ae44bfd9..79a95f6c87d08de7c3fb4b98da8ca3fe7a0dcee7 100644
--- a/blimp/engine/mojo/blob_channel_service.cc
+++ b/blimp/engine/mojo/blob_channel_service.cc
@@ -5,6 +5,7 @@
 #include "blimp/engine/mojo/blob_channel_service.h"
 
 #include "blimp/net/blob_channel/blob_channel_sender.h"
+#include "mojo/public/cpp/system/buffer.h"
 
 namespace blimp {
 namespace engine {
@@ -19,8 +20,34 @@ BlobChannelService::BlobChannelService(BlobChannelSender* blob_channel_sender,
 BlobChannelService::~BlobChannelService() {}
 
 void BlobChannelService::PutBlob(const mojo::String& id,
-                                 const mojo::String& data) {
-  blob_channel_sender_->PutBlob(id, new BlobData(data));
+                                 mojo::ScopedSharedBufferHandle data,
+                                 uint32_t size,
+                                 const PutBlobCallback& callback) {
+  // Map |data| into memory.
+  char* blob_buf = nullptr;

[Wez, 2016/06/11 00:16:37]

nit: Should this be const char*? We don't really want the receiver modifying the
Blob content, right?

[Kevin M, 2016/06/16 21:00:38]

MapBuffer() takes a non-const void*. I'd have to do some nasty const_cast'ing to
get this to work.

+  MojoResult result =
+      mojo::MapBuffer(data.get(), 0, size, reinterpret_cast<void**>(&blob_buf),
+                      MOJO_MAP_BUFFER_FLAG_NONE);
+  if (result != MOJO_RESULT_OK) {
+    LOG(ERROR) << "Couldn't map buffer for blob ID: " << id;

[Wez, 2016/06/11 00:16:37]

It's best not to log unbounded ERRORs - if we fail to map a blob then we'll
continue on, rather than exiting, and we're likely to fail to map the next one,
so we'll get cascades of errors.

One way to provide the signal without such detailed logging would be to add UMA
stats in this code, for example.

[maniscalco, 2016/06/13 15:23:08]

I asked for logging here because I'm operating under the assumption failure to
map/unmap a blob is a serious issue we cannot recover from and that logging the
result (we should log the result here like we do below) would help us find out
what happened.

The DLOG(FATAL) on the next line will terminate the program if we're built with
debug checks on, right?

[Wez, 2016/06/13 23:37:50]

TBH I'd expect that both map & unmap should only fail if we pass them invalid
args (e.g invalid memory handle, or already-unmapped address), so crashing is
probably the cleanest thing to do.  I think map may also fail if the
address-space is too fragmented but that seems incredibly unlikely (and would
probably indicate some other coding error anyway...)
Correct. I'd prefer that if we want to log something here then we just use
LOG(FATAL), so that we'll log _and_ crash in all builds.

[maniscalco, 2016/06/16 15:31:34]

LOG(FATAL) SGTM

[Kevin M, 2016/06/16 21:00:38]

Done.

+    DLOG(FATAL) << "Exiting.";
+    return;
+  }
+  DCHECK(blob_buf);

[Wez, 2016/06/11 00:16:37]

nit: You're about to copy data from |blob_buf|, so you'll get a nice clean crash
stack anyway if it's null here.

[Kevin M, 2016/06/16 21:00:38]

Done.

+
+  scoped_refptr<BlobData> new_blob(new BlobData);
+  new_blob->data.assign(blob_buf, size);
+  blob_channel_sender_->PutBlob(id, std::move(new_blob));

[Wez, 2016/06/11 00:16:37]

nit: Use a blank line here to separate the put-blob block from the unmap-memory
block?

[Kevin M, 2016/06/16 21:00:38]

Done.

+  result = mojo::UnmapBuffer(blob_buf);
+  if (result != MOJO_RESULT_OK) {
+    LOG(ERROR) << "Couldn't unmap buffer for blob ID: " << id
+               << " (result code: " << result << ")";
+    DLOG(FATAL) << "Exiting.";
+    return;

[Wez, 2016/06/11 00:16:37]

This feels like a use-case for CHECK_EQ() - we really shouldn't fail to un-map
memory, surely?

[Kevin M, 2016/06/16 21:00:38]

Done.

+  }
+
+  // Signals the renderer side that we are done with the shared memory |data|.
+  callback.Run();
 }
 
 void BlobChannelService::DeliverBlob(const mojo::String& id) {