Revert 253869 "Mojo: Add hooks for dispatchers to serialize them..."

> Mojo: Add hooks for dispatchers to serialize themselves.
> 
> R=yzshen@chromium.org
> 
> Review URL: https://codereview.chromium.org/182163002

TBR=viettrungluu@chromium.org

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

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

4 files changed, 42 insertions, 178 deletions

diff --git a/mojo/system/dispatcher.cc b/mojo/system/dispatcher.cc
index 3d970d0..ac02b83 100644
--- a/mojo/system/dispatcher.cc
+++ b/mojo/system/dispatcher.cc
@@ -28,23 +28,6 @@ DispatcherTransport Dispatcher::CoreImplAccess::TryStartTransport(
   return DispatcherTransport(dispatcher);
 }
 
-// static
-size_t Dispatcher::MessageInTransitAccess::GetMaximumSerializedSize(
-    const Dispatcher* dispatcher,
-    const Channel* channel) {
-  DCHECK(dispatcher);
-  return dispatcher->GetMaximumSerializedSize(channel);
-}
-
-// static
-size_t Dispatcher::MessageInTransitAccess::SerializeAndClose(
-    Dispatcher* dispatcher,
-    void* destination,
-    Channel* channel) {
-  DCHECK(dispatcher);
-  return dispatcher->SerializeAndClose(destination, channel);
-}
-
 MojoResult Dispatcher::Close() {
   base::AutoLock locker(lock_);
   if (is_closed_)
@@ -314,23 +297,6 @@ void Dispatcher::RemoveWaiterImplNoLock(Waiter* /*waiter*/) {
   // will do something nontrivial.
 }
 
-size_t Dispatcher::GetMaximumSerializedSizeImplNoLock(
-      const Channel* /*channel*/) const {
-  lock_.AssertAcquired();
-  DCHECK(!is_closed_);
-  // By default, serializing isn't supported.
-  return 0;
-}
-
-size_t Dispatcher::SerializeAndCloseImplNoLock(void* /*destination*/,
-                                               Channel* /*channel*/) {
-  lock_.AssertAcquired();
-  DCHECK(is_closed_);
-  // By default, serializing isn't supported, so just close.
-  CloseImplNoLock();
-  return 0;
-}
-
 bool Dispatcher::IsBusyNoLock() const {
   lock_.AssertAcquired();
   DCHECK(!is_closed_);
@@ -358,39 +324,6 @@ Dispatcher::CreateEquivalentDispatcherAndCloseNoLock() {
   return CreateEquivalentDispatcherAndCloseImplNoLock();
 }
 
-size_t Dispatcher::GetMaximumSerializedSize(const Channel* channel) const {
-  DCHECK(channel);
-  DCHECK(HasOneRef());
-
-  base::AutoLock locker(lock_);
-  DCHECK(!is_closed_);
-  return GetMaximumSerializedSizeImplNoLock(channel);
-}
-
-size_t Dispatcher::SerializeAndClose(void* destination, Channel* channel) {
-  DCHECK(destination);
-  DCHECK(channel);
-  DCHECK(HasOneRef());
-
-  base::AutoLock locker(lock_);
-  DCHECK(!is_closed_);
-
-  // We have to call |GetMaximumSerializedSizeImplNoLock()| first, because we
-  // leave it to |SerializeAndCloseImplNoLock()| to close the thing.
-  size_t max_size = DCHECK_IS_ON() ?
-      GetMaximumSerializedSizeImplNoLock(channel) : static_cast<size_t>(-1);
-
-  // Like other |...Close()| methods, we mark ourselves as closed before calling
-  // the impl.
-  is_closed_ = true;
-  // No need to cancel waiters: we shouldn't have any (and shouldn't be in
-  // |Core|'s handle table.
-
-  size_t size = SerializeAndCloseImplNoLock(destination, channel);
-  DCHECK_LE(size, max_size);
-  return size;
-}
-
 // DispatcherTransport ---------------------------------------------------------
 
 void DispatcherTransport::End() {
diff --git a/mojo/system/dispatcher.h b/mojo/system/dispatcher.h
index 73bf77b..59e536a 100644
--- a/mojo/system/dispatcher.h
+++ b/mojo/system/dispatcher.h
@@ -19,12 +19,10 @@
 namespace mojo {
 namespace system {
 
-class Channel;
 class CoreImpl;
 class Dispatcher;
 class DispatcherTransport;
 class LocalMessagePipeEndpoint;
-class MessageInTransit;
 class ProxyMessagePipeEndpoint;
 class Waiter;
 
@@ -141,24 +139,6 @@ class MOJO_SYSTEM_IMPL_EXPORT Dispatcher :
     static DispatcherTransport TryStartTransport(Dispatcher* dispatcher);
   };
 
-  // A |MessageInTransit| may serialize dispatchers that are attached to it to a
-  // given |Channel|. These may only be called on such dispatchers. See the
-  // |Dispatcher| methods of the same names for more details.
-  // TODO(vtl): Consider making another wrapper similar to |DispatcherTransport|
-  // (but with an owning, unique reference), and having
-  // |CreateEquivalentDispatcherAndCloseImplNoLock()| return that wrapper (and
-  // |MessageInTransit| only holding on to such wrappers).
-  class MessageInTransitAccess {
-   private:
-    friend class MessageInTransit;
-
-    static size_t GetMaximumSerializedSize(const Dispatcher* dispatcher,
-                                           const Channel* channel);
-    static size_t SerializeAndClose(Dispatcher* dispatcher,
-                                    void* destination,
-                                    Channel* channel);
-  };
-
  protected:
   Dispatcher();
 
@@ -214,17 +194,6 @@ class MOJO_SYSTEM_IMPL_EXPORT Dispatcher :
                                          MojoResult wake_result);
   virtual void RemoveWaiterImplNoLock(Waiter* waiter);
 
-  // These implement the API used to serialize dispatchers to a |Channel|
-  // (described below). They will only be called on a dispatcher that's attached
-  // to and "owned" by a |MessageInTransit|. See the non-"impl" versions for
-  // more information.
-  // TODO(vtl): Consider making these pure virtual once most things support
-  // being passed over a message pipe.
-  virtual size_t GetMaximumSerializedSizeImplNoLock(
-      const Channel* channel) const;
-  virtual size_t SerializeAndCloseImplNoLock(void* destination,
-                                             Channel* channel);
-
   // Available to subclasses. (Note: Returns a non-const reference, just like
   // |base::AutoLock|'s constructor takes a non-const reference.)
   base::Lock& lock() const { return lock_; }
@@ -250,22 +219,6 @@ class MOJO_SYSTEM_IMPL_EXPORT Dispatcher :
   // under the dispatcher's lock.
   scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndCloseNoLock();
 
-  // API to serialize dispatchers to a |Channel|, exposed to only
-  // |MessageInTransit| (via |MessageInTransitAccess|). They may only be called
-  // on a dispatcher attached to a |MessageInTransit| (and in particular not in
-  // |CoreImpl|'s handle table).
-  // Gets the maximum amount of space that'll be needed to serialize this
-  // dispatcher to the given |Channel|. Returns zero to indicate that this
-  // dispatcher cannot be serialized (to the given |Channel|).
-  size_t GetMaximumSerializedSize(const Channel* channel) const;
-  // Serializes this dispatcher to the given |Channel| by writing to
-  // |destination| and then closes this dispatcher. It may write no more than
-  // was indicated by |GetMaximumSerializedSize()|. (WARNING: Beware of races,
-  // e.g., if something can be mutated between the two calls!) This may return
-  // zero to indicate failure to serialize (but this dispatcher will still be
-  // closed).
-  size_t SerializeAndClose(void* destination, Channel* channel);
-
   // This protects the following members as well as any state added by
   // subclasses.
   mutable base::Lock lock_;
diff --git a/mojo/system/message_in_transit.cc b/mojo/system/message_in_transit.cc
index 49f1f6c..3a0abe2 100644
--- a/mojo/system/message_in_transit.cc
+++ b/mojo/system/message_in_transit.cc
@@ -20,16 +20,12 @@ struct MessageInTransit::PrivateStructForCompileAsserts {
   // The size of |Header| must be appropriate to maintain alignment of the
   // following data.
   COMPILE_ASSERT(sizeof(Header) % kMessageAlignment == 0,
-                 sizeof_MessageInTransit_Header_invalid);
+                 sizeof_MessageInTransit_Header_not_a_multiple_of_alignment);
   // Avoid dangerous situations, but making sure that the size of the "header" +
   // the size of the data fits into a 31-bit number.
   COMPILE_ASSERT(static_cast<uint64_t>(sizeof(Header)) + kMaxMessageNumBytes <=
                      0x7fffffffULL,
                  kMaxMessageNumBytes_too_big);
-
-  // The size of |HandleTableEntry| must be appropriate to maintain alignment.
-  COMPILE_ASSERT(sizeof(HandleTableEntry) % kMessageAlignment == 0,
-                 sizeof_MessageInTransit_HandleTableEntry_invalid);
 };
 
 STATIC_CONST_MEMBER_DEFINITION const MessageInTransit::Type
@@ -196,48 +192,36 @@ void MessageInTransit::SerializeAndCloseDispatchers(Channel* channel) {
   if (!num_handles())
     return;
 
-  size_t handle_table_size = num_handles() * sizeof(HandleTableEntry);
-  // The size of the secondary buffer. We'll start with the size of the handle
-  // table, and add to it as we go along.
-  size_t size = handle_table_size;
+  // The size of the secondary buffer. We'll start with the size of the entry
+  // size table (which will contain the size of the data for each handle), and
+  // add to it as we go along.
+  size_t size = RoundUpMessageAlignment(num_handles() * sizeof(uint32_t));
+  // The maximum size that we'll need for the secondary buffer. We'll allocate
+  // this much.
+  size_t max_size = size;
+  // TODO(vtl): Iterate through dispatchers and query their maximum size (and
+  // add each, rounded up, to |max_size|).
+
+  secondary_buffer_ = base::AlignedAlloc(max_size, kMessageAlignment);
+  // TODO(vtl): I wonder if it's faster to clear everything once, or to only
+  // clear padding as needed.
+  memset(secondary_buffer_, 0, max_size);
+
+  uint32_t* entry_size_table = static_cast<uint32_t*>(secondary_buffer_);
   for (size_t i = 0; i < dispatchers_->size(); i++) {
+    // The entry size table entry is already zero by default.
     if (!(*dispatchers_)[i])
       continue;
 
-    size += RoundUpMessageAlignment(
-        Dispatcher::MessageInTransitAccess::GetMaximumSerializedSize(
-            (*dispatchers_)[i].get(), channel));
-    // TODO(vtl): Check for overflow?
-  }
-
-  secondary_buffer_ = base::AlignedAlloc(size, kMessageAlignment);
-  // TODO(vtl): Check for overflow?
-  secondary_buffer_size_ = static_cast<uint32_t>(size);
-  // Entirely clear out the secondary buffer, since then we won't have to worry
-  // about clearing padding or unused space (e.g., if a dispatcher fails to
-  // serialize).
-  memset(secondary_buffer_, 0, size);
-
-  HandleTableEntry* handle_table =
-      static_cast<HandleTableEntry*>(secondary_buffer_);
-  size_t current_offset = handle_table_size;
-  for (size_t i = 0; i < dispatchers_->size(); i++) {
-    if (!(*dispatchers_)[i]) {
-      // The |handle_table[i].size| is already zero, designating this entry as
-      // invalid.
-      continue;
-    }
+    // TODO(vtl): Serialize this dispatcher (getting its actual size, and adding
+    // that (rounded up) to |size|.
+    entry_size_table[i] = 0;
 
-    handle_table[i].offset = static_cast<uint32_t>(current_offset);
-    handle_table[i].size =
-        Dispatcher::MessageInTransitAccess::SerializeAndClose(
-            (*dispatchers_)[i].get(),
-            static_cast<char*>(secondary_buffer_) + current_offset,
-            channel);
-    current_offset += RoundUpMessageAlignment(handle_table[i].size);
-    DCHECK_LE(current_offset, size);
+    DCHECK((*dispatchers_)[i]->HasOneRef());
+    (*dispatchers_)[i]->Close();
   }
 
+  secondary_buffer_size_ = static_cast<uint32_t>(size);
   UpdateTotalSize();
 }
 
diff --git a/mojo/system/message_in_transit.h b/mojo/system/message_in_transit.h
index 1503e44..60d5c25 100644
--- a/mojo/system/message_in_transit.h
+++ b/mojo/system/message_in_transit.h
@@ -20,25 +20,6 @@ namespace system {
 class Channel;
 
 // This class is used to represent data in transit. It is thread-unsafe.
-//
-// |MessageInTransit| buffers:
-//
-// A |MessageInTransit| can be serialized by writing the main buffer and then,
-// if it has one, the secondary buffer. Both buffers are
-// |kMessageAlignment|-byte aligned and a multiple of |kMessageAlignment| bytes
-// in size.
-//
-// The main buffer consists of the header (of type |Header|, which is an
-// internal detail of this class) followed immediately by the message data
-// (accessed by |bytes()| and of size |num_bytes()|, and also
-// |kMessageAlignment|-byte aligned), and then any padding needed to make the
-// main buffer a multiple of |kMessageAlignment| bytes in size.
-//
-// The secondary buffer consists first of a table of |HandleTableEntry|s (each
-// of which is already a multiple of |kMessageAlignment| in size), followed by
-// data needed for the |HandleTableEntry|s: A |HandleTableEntry| consists of an
-// offset (in bytes, relative to the start of the secondary buffer; we guarantee
-// that it's a multiple of |kMessageAlignment|), and a size (in bytes).
 class MOJO_SYSTEM_IMPL_EXPORT MessageInTransit {
  public:
   typedef uint16_t Type;
@@ -119,6 +100,24 @@ class MOJO_SYSTEM_IMPL_EXPORT MessageInTransit {
   // stays alive through the call.
   void SerializeAndCloseDispatchers(Channel* channel);
 
+  // |MessageInTransit| buffers: A |MessageInTransit| can be serialized by
+  // writing the main buffer and then, if it has one, the secondary buffer. Both
+  // buffers are |kMessageAlignment|-byte aligned and a multiple of
+  // |kMessageAlignment| bytes in size.
+  //
+  // The main buffer consists of the header (of type |Header|, which is an
+  // internal detail of this class) followed immediately by the message data
+  // (accessed by |bytes()| and of size |num_bytes()|, and also
+  // |kMessageAlignment|-byte aligned), and then any padding needed to make the
+  // main buffer a multiple of |kMessageAlignment| bytes in size.
+  //
+  // The secondary buffer consists first of a table of |uint32_t|s with
+  // |num_handles()| entries; each entry in the table is the (unpadded) size of
+  // the data for a handle (a size of 0 indicates in invalid handle/null
+  // dispatcher). The table is followed by padding, then the first entry and
+  // padding, the second entry and padding, etc. (padding as required to
+  // maintain |kMessageAlignment|-byte alignment).
+
   // Gets the main buffer and its size (in number of bytes), respectively.
   const void* main_buffer() const { return main_buffer_; }
   size_t main_buffer_size() const { return main_buffer_size_; }
@@ -189,11 +188,6 @@ class MOJO_SYSTEM_IMPL_EXPORT MessageInTransit {
     uint32_t reserved1;
   };
 
-  struct HandleTableEntry {
-    uint32_t offset;
-    uint32_t size;  // (Not including any padding.) A size of 0 means "invalid".
-  };
-
   const Header* header() const {
     return static_cast<const Header*>(main_buffer_);
   }