diff options
author | mpcomplete@google.com <mpcomplete@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-27 18:40:50 +0000 |
---|---|---|
committer | mpcomplete@google.com <mpcomplete@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-27 18:40:50 +0000 |
commit | 19b5c90babcde9f48ed94a90bc390b6f750f245a (patch) | |
tree | 699b01e6b6ac1b405890d85085eaed2a07b08a36 /mojo/system | |
parent | c532e56ac387b94c75cd76099c2fabe12afa5b20 (diff) | |
download | chromium_src-19b5c90babcde9f48ed94a90bc390b6f750f245a.zip chromium_src-19b5c90babcde9f48ed94a90bc390b6f750f245a.tar.gz chromium_src-19b5c90babcde9f48ed94a90bc390b6f750f245a.tar.bz2 |
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
Diffstat (limited to 'mojo/system')
-rw-r--r-- | mojo/system/dispatcher.cc | 67 | ||||
-rw-r--r-- | mojo/system/dispatcher.h | 47 | ||||
-rw-r--r-- | mojo/system/message_in_transit.cc | 64 | ||||
-rw-r--r-- | mojo/system/message_in_transit.h | 42 |
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_); } |