[mojo] Use base::Pickle for Message storage.

mojo/public/cpp/bindings/lib/pickle_buffer.h

+// 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.
+
+#ifndef MOJO_PUBLIC_CPP_BINDINGS_LIB_PICKLE_BUFFER_H_
+#define MOJO_PUBLIC_CPP_BINDINGS_LIB_PICKLE_BUFFER_H_
+
+#include "base/macros.h"
+#include "base/pickle.h"
+#include "mojo/public/cpp/bindings/lib/buffer.h"
+
+namespace mojo {
+namespace internal {
+
+// An implementation of Buffer which uses base::Pickle for its backing. Note
+// that this does not use Pickle's header structure at all, instead storing
+// the complete Message (including header) in the Pickle's payload.
+class PickleBuffer : public base::Pickle, public Buffer {

[yzshen1, 2015/12/15 19:07:02]

Does it make sense to let PickleBuffer has a Pickle member instead?
IMO unless there are compelling reasons, it is nicer to avoid implementation
inheritance. Moreover, this class hides the data() method of base::Pickle.

+ public:
+  PickleBuffer();
+  ~PickleBuffer() override;
+
+  const void* data() const {
+    return reinterpret_cast<const void*>(

[yzshen1, 2015/12/15 19:07:02]

nit: I think both reinterpret_cast can be static_cast.

+        reinterpret_cast<const char*>(base::Pickle::data())
+            + sizeof(PaddedHeader));
+  }
+
+  void* data() {
+    return const_cast<void*>(static_cast<const PickleBuffer*>(this)->data());
+  }
+
+  size_t data_num_bytes() const { return payload_size(); }
+
+  // Resizes and zeroes the underlying Pickle's capacity.
+  void AllocData(uint32_t num_bytes);
+
+  // Resizes the underlying Pickle's capacity without zeroing.
+  void AllocUninitializedData(uint32_t num_bytes);
+
+  // Buffer:
+  PickleBuffer* AsPickleBuffer() override;
+
+ private:
+  // Buffer implementation. Note that this may grow the Pickle's capacity if
+  // necessary, but you can avoid this by calling AllocData to preallocate when

[yzshen1, 2015/12/15 19:07:02]

I noticed that there is a DCHECK in the code, but maybe we should DCHECK before
making the change and fail the allocation if it needs to grow the Pickle's
capacity?

Because it violates the interface definition of Buffer: "Allocations remain
valid for the lifetime of the Buffer."

+  // the final buffer size is known in advance.
+  //
+  // This guarantees that the returned data is aligned on an 8-byte boundary.
+  void* Allocate(size_t num_bytes) override;
+
+  // TODO(rockot): Stop wasting 8 bytes per buffer.
+  //
+  // We don't use Pickle's header at all, but its base header type consumes 4
+  // bytes. We waste another 4 bytes to keep our actual buffer aligned to an
+  // 8-byte boundary.
+  //
+  // The reason we don't use base::Pickle's header (yet) is that it stores
+  // payload length in the first 4 bytes. Mojo Messages are packed like structs,
+  // where the first 4 bytes are header size rather than payload size.
+  //
+  // We can adopt Pickle's header format for Mojo messages (or do any number of

[yzshen1, 2015/12/15 19:07:02]

I personally prefer not to change our wire format to fit Pickle definition.
Pickle seems to be something internal and (hopefully) transitional. If there is
one thing that needs to be changed, I think it should be Pickle.

+  // other things to clean this up), but wasting 8 bytes is for now the most
+  // efficient path toward blending existing IPC traits with Mojo bindings.
+  struct PaddedHeader : public base::Pickle::Header {
+    uint32_t padding;
+  };
+
+  static_assert(sizeof(PaddedHeader) % 8 == 0,
+      "PickleBuffer requires a Pickle header size with 8-byte alignment.");
+
+  // The index into the message payload where the next Buffer::Allocate should
+  // claim memory.
+  size_t allocation_cursor_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(PickleBuffer);
+};
+
+}  // namespace internal
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_BINDINGS_LIB_PICKLE_BUFFER_H_