Mojo: Make MessageInTransit more opaque still.

R=yzshen@chromium.org

Review URL: https://codereview.chromium.org/173333002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@252272 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 79 insertions, 22 deletions

diff --git a/mojo/system/message_in_transit.cc b/mojo/system/message_in_transit.cc
index b7c536e..2eb9e26 100644
--- a/mojo/system/message_in_transit.cc
+++ b/mojo/system/message_in_transit.cc
@@ -77,6 +77,33 @@ void MessageInTransit::Destroy() {
   base::AlignedFree(this);
 }
 
+// static
+bool MessageInTransit::GetNextMessageSize(const void* buffer,
+                                          size_t buffer_size,
+                                          size_t* next_message_size) {
+  DCHECK(buffer);
+  DCHECK_EQ(reinterpret_cast<uintptr_t>(buffer) %
+                MessageInTransit::kMessageAlignment, 0u);
+  DCHECK(next_message_size);
+
+  if (buffer_size < sizeof(Header))
+    return false;
+
+  const Header* header = static_cast<const Header*>(buffer);
+  *next_message_size =
+      RoundUpMessageAlignment(sizeof(MessageInTransit) + header->data_size);
+  return true;
+}
+
+// static
+const MessageInTransit* MessageInTransit::CreateReadOnlyFromBuffer(
+    const void* buffer) {
+  DCHECK(buffer);
+  DCHECK_EQ(reinterpret_cast<uintptr_t>(buffer) %
+                MessageInTransit::kMessageAlignment, 0u);
+  return static_cast<const MessageInTransit*>(buffer);
+}
+
 MessageInTransit::MessageInTransit(uint32_t data_size,
                                    Type type,
                                    Subtype subtype,
diff --git a/mojo/system/message_in_transit.h b/mojo/system/message_in_transit.h
index ecc4808..10229e8 100644
--- a/mojo/system/message_in_transit.h
+++ b/mojo/system/message_in_transit.h
@@ -53,6 +53,28 @@ class MOJO_SYSTEM_IMPL_EXPORT MessageInTransit {
   // Destroys a |MessageInTransit| created using |Create()| or |Clone()|.
   void Destroy();
 
+  // Gets the size of the next message from |buffer|, which has |buffer_size|
+  // bytes currently available, returning true and setting |*next_message_size|
+  // on success. |buffer| should be aligned on a |kMessageAlignment| boundary
+  // (and on success, |*next_message_size| will be a multiple of
+  // |kMessageAlignment|).
+  // TODO(vtl): In |RawChannelPosix|, the alignment requirements are currently
+  // satisified on a faith-based basis.
+  static bool GetNextMessageSize(const void* buffer,
+                                 size_t buffer_size,
+                                 size_t* next_message_size);
+
+  // Creates a read-only |MessageInTransit| from |buffer|, which must have
+  // enough data (as indicated by |GetNextMessageSize()|). |buffer| has the same
+  // alignment requirements as in |GetNextMessageSize()|.
+  //
+  // The returned message should not be destroyed using |Destroy()|, and the
+  // underlying buffer must remain alive and unmodified as long as the returned
+  // message is in use.
+  // TODO(vtl): Change these odd semantics (once I make |MessageInTransit|s have
+  // pointers to the buffers, instead of being POD).
+  static const MessageInTransit* CreateReadOnlyFromBuffer(const void* buffer);
+
   // Gets the "main" buffer for a |MessageInTransit|. A |MessageInTransit| can
   // be serialized by writing the main buffer. The returned pointer will be
   // aligned to a multiple of |kMessageAlignment| bytes, and the size of the
diff --git a/mojo/system/raw_channel_posix.cc b/mojo/system/raw_channel_posix.cc
index b395a59..9dc4f7c 100644
--- a/mojo/system/raw_channel_posix.cc
+++ b/mojo/system/raw_channel_posix.cc
@@ -251,15 +251,22 @@ void RawChannelPosix::OnFileCanReadWithoutBlocking(int fd) {
     read_buffer_num_valid_bytes_ += static_cast<size_t>(bytes_read);
 
     // Dispatch all the messages that we can.
-    while (read_buffer_num_valid_bytes_ >= sizeof(MessageInTransit)) {
+    size_t message_size;
+    // Note that we rely on short-circuit evaluation here:
+    //   - |read_buffer_start| may be an invalid index into |read_buffer_| if
+    //     |read_buffer_num_valid_bytes_| is zero.
+    //   - |message_size| is only valid if |GetNextMessageSize()| returns true.
+    // TODO(vtl): Use |message_size| more intelligently (e.g., to request the
+    // next read).
+    while (read_buffer_num_valid_bytes_ > 0 &&
+           MessageInTransit::GetNextMessageSize(
+               &read_buffer_[read_buffer_start], read_buffer_num_valid_bytes_,
+               &message_size) &&
+           read_buffer_num_valid_bytes_ >= message_size) {
       const MessageInTransit* message =
-          reinterpret_cast<const MessageInTransit*>(
+          MessageInTransit::CreateReadOnlyFromBuffer(
               &read_buffer_[read_buffer_start]);
-      DCHECK_EQ(reinterpret_cast<size_t>(message) %
-                    MessageInTransit::kMessageAlignment, 0u);
-      // If we have the header, not the whole message....
-      if (read_buffer_num_valid_bytes_ < message->main_buffer_size())
-        break;
+      DCHECK_EQ(message->main_buffer_size(), message_size);
 
       // Dispatch the message.
       delegate()->OnReadMessage(*message);
@@ -271,8 +278,8 @@ void RawChannelPosix::OnFileCanReadWithoutBlocking(int fd) {
       did_dispatch_message = true;
 
       // Update our state.
-      read_buffer_start += message->main_buffer_size();
-      read_buffer_num_valid_bytes_ -= message->main_buffer_size();
+      read_buffer_start += message_size;
+      read_buffer_num_valid_bytes_ -= message_size;
     }
 
     // If we dispatched any messages, stop reading for now (and let the message
diff --git a/mojo/system/raw_channel_posix_unittest.cc b/mojo/system/raw_channel_posix_unittest.cc
index 7547021..ab80487c 100644
--- a/mojo/system/raw_channel_posix_unittest.cc
+++ b/mojo/system/raw_channel_posix_unittest.cc
@@ -145,20 +145,21 @@ class TestMessageReaderAndChecker {
       bytes_.insert(bytes_.end(), buffer, buffer + read_size);
 
       // If we have the header....
-      if (bytes_.size() >= sizeof(MessageInTransit)) {
-        const MessageInTransit* message =
-            reinterpret_cast<const MessageInTransit*>(bytes_.data());
-        CHECK_EQ(reinterpret_cast<size_t>(message) %
-                     MessageInTransit::kMessageAlignment, 0u);
-
-        if (message->data_size() != expected_size) {
-          LOG(ERROR) << "Wrong size: " << message->data_size() << " instead of "
-                     << expected_size << " bytes.";
-          return false;
-        }
-
+      size_t message_size;
+      if (MessageInTransit::GetNextMessageSize(bytes_.data(), bytes_.size(),
+                                               &message_size)) {
         // If we've read the whole message....
-        if (bytes_.size() >= message->main_buffer_size()) {
+        if (bytes_.size() >= message_size) {
+          const MessageInTransit* message =
+              MessageInTransit::CreateReadOnlyFromBuffer(bytes_.data());
+          CHECK_EQ(message->main_buffer_size(), message_size);
+
+          if (message->data_size() != expected_size) {
+            LOG(ERROR) << "Wrong size: " << message_size << " instead of "
+                       << expected_size << " bytes.";
+            return false;
+          }
+
           if (!CheckMessageData(message->data(), message->data_size())) {
             LOG(ERROR) << "Incorrect message data.";
             return false;