diff options
author | erikchen <erikchen@chromium.org> | 2015-09-16 10:35:04 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-09-16 17:36:13 +0000 |
commit | 1fa7dad91bc711eb1e481aa467a2f38c77cdae54 (patch) | |
tree | 8a49f6ba7e937c8bb35b04c890ce0bf82aa42e56 /ipc | |
parent | 152f7f2870e2745de968e9dc21b1ee34099b967c (diff) | |
download | chromium_src-1fa7dad91bc711eb1e481aa467a2f38c77cdae54.zip chromium_src-1fa7dad91bc711eb1e481aa467a2f38c77cdae54.tar.gz chromium_src-1fa7dad91bc711eb1e481aa467a2f38c77cdae54.tar.bz2 |
Revert of Reland #2 "ipc: Add a new field num_brokered_attachments to the message header." (patchset #5 id:80001 of https://codereview.chromium.org/1334593002/ )
Reason for revert:
CL was only ever intended to be in 1 canary release.
Original issue's description:
> Reland #2 "ipc: Add a new field num_brokered_attachments to the message header."
>
> This original version of this CL is causing an unusual crash in Canary. This CL
> adds a message verifier to the ipc code so that it can dynamically verify the
> contents of this message that is being corrupted. This CL also verifies the
> message at several different points in the dispatch process. This will help
> narrow down the range of code that is corrupting the message.
>
> I expect this CL to cause ~100 crashes in the next Chrome Canary. I intend to
> revert this CL after a single Canary release.
>
> BUG=527588
>
> Committed: https://crrev.com/a2e71be46dc4bdcbb544db479680f65a390ae8f3
> Cr-Commit-Position: refs/heads/master@{#349056}
TBR=avi@chromium.org,tsepez@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=527588
Review URL: https://codereview.chromium.org/1350823002
Cr-Commit-Position: refs/heads/master@{#349152}
Diffstat (limited to 'ipc')
-rw-r--r-- | ipc/ipc_channel.h | 5 | ||||
-rw-r--r-- | ipc/ipc_channel_common.cc | 12 | ||||
-rw-r--r-- | ipc/ipc_channel_reader.cc | 7 | ||||
-rw-r--r-- | ipc/ipc_channel_win.cc | 11 | ||||
-rw-r--r-- | ipc/ipc_message.cc | 16 | ||||
-rw-r--r-- | ipc/ipc_message.h | 7 |
6 files changed, 2 insertions, 56 deletions
diff --git a/ipc/ipc_channel.h b/ipc/ipc_channel.h index 06555d3..4784a5c 100644 --- a/ipc/ipc_channel.h +++ b/ipc/ipc_channel.h @@ -169,11 +169,6 @@ class IPC_EXPORT Channel : public Endpoint { ~Channel() override; - // TODO(erikchen): Temporary code to help track http://crbug.com/527588. - using MessageVerifier = void (*)(const Message*); - static void SetMessageVerifier(MessageVerifier verifier); - static MessageVerifier GetMessageVerifier(); - // Connect the pipe. On the server side, this will initiate // waiting for connections. On the client, it attempts to // connect to a pre-existing pipe. Note, calling Connect() diff --git a/ipc/ipc_channel_common.cc b/ipc/ipc_channel_common.cc index 1a227ad..5438c66 100644 --- a/ipc/ipc_channel_common.cc +++ b/ipc/ipc_channel_common.cc @@ -6,8 +6,6 @@ namespace IPC { -static Channel::MessageVerifier g_message_verifier = nullptr; - // static scoped_ptr<Channel> Channel::CreateClient( const IPC::ChannelHandle& channel_handle, @@ -58,16 +56,6 @@ scoped_ptr<Channel> Channel::CreateServer( Channel::~Channel() { } -// static -void Channel::SetMessageVerifier(MessageVerifier verifier) { - g_message_verifier = verifier; -} - -// static -Channel::MessageVerifier Channel::GetMessageVerifier() { - return g_message_verifier; -} - bool Channel::IsSendThreadSafe() const { return false; } diff --git a/ipc/ipc_channel_reader.cc b/ipc/ipc_channel_reader.cc index 8ec6694..031417b 100644 --- a/ipc/ipc_channel_reader.cc +++ b/ipc/ipc_channel_reader.cc @@ -11,7 +11,6 @@ #include "ipc/ipc_message.h" #include "ipc/ipc_message_attachment_set.h" #include "ipc/ipc_message_macros.h" -#include "ipc/ipc_message_start.h" namespace IPC { namespace internal { @@ -190,12 +189,6 @@ void ChannelReader::DispatchMessage(Message* m) { handled = GetAttachmentBroker()->OnMessageReceived(*m); } #endif // USE_ATTACHMENT_BROKER - - // TODO(erikchen): Temporary code to help track http://crbug.com/527588. - Channel::MessageVerifier verifier = Channel::GetMessageVerifier(); - if (verifier) - verifier(m); - if (!handled) listener_->OnMessageReceived(*m); if (m->dispatch_error()) diff --git a/ipc/ipc_channel_win.cc b/ipc/ipc_channel_win.cc index 56dc4b7..79b0001 100644 --- a/ipc/ipc_channel_win.cc +++ b/ipc/ipc_channel_win.cc @@ -474,17 +474,6 @@ bool ChannelWin::ProcessOutgoingMessages( // Write to pipe... OutputElement* element = output_queue_.front(); - - // TODO(erikchen): Temporary code to help track http://crbug.com/527588. - { - const Message* m = element->get_message(); - if (m) { - Channel::MessageVerifier verifier = Channel::GetMessageVerifier(); - if (verifier) - verifier(m); - } - } - DCHECK(element->size() <= INT_MAX); BOOL ok = WriteFile(pipe_.Get(), element->data(), diff --git a/ipc/ipc_message.cc b/ipc/ipc_message.cc index 75f628e..99868a3 100644 --- a/ipc/ipc_message.cc +++ b/ipc/ipc_message.cc @@ -49,9 +49,6 @@ Message::~Message() { Message::Message() : base::Pickle(sizeof(Header)) { header()->routing = header()->type = 0; header()->flags = GetRefNumUpper24(); -#if USE_ATTACHMENT_BROKER - header()->num_brokered_attachments = 0; -#endif #if defined(OS_POSIX) header()->num_fds = 0; header()->pad = 0; @@ -65,9 +62,6 @@ Message::Message(int32_t routing_id, uint32_t type, PriorityValue priority) header()->type = type; DCHECK((priority & 0xffffff00) == 0); header()->flags = priority | GetRefNumUpper24(); -#if USE_ATTACHMENT_BROKER - header()->num_brokered_attachments = 0; -#endif #if defined(OS_POSIX) header()->num_fds = 0; header()->pad = 0; @@ -155,7 +149,7 @@ void Message::FindNext(const char* range_start, // The data is not copied. size_t pickle_len = static_cast<size_t>(pickle_end - range_start); Message message(range_start, static_cast<int>(pickle_len)); - size_t num_attachments = message.header()->num_brokered_attachments; + int num_attachments = message.header()->num_brokered_attachments; // Check for possible overflows. size_t max_size_t = std::numeric_limits<size_t>::max(); @@ -171,7 +165,7 @@ void Message::FindNext(const char* range_start, if (buffer_length < attachment_length + pickle_len) return; - for (size_t i = 0; i < num_attachments; ++i) { + for (int i = 0; i < num_attachments; ++i) { const char* attachment_start = pickle_end + i * BrokerableAttachment::kNonceSize; BrokerableAttachment::AttachmentId id(attachment_start, @@ -198,12 +192,6 @@ bool Message::WriteAttachment(scoped_refptr<MessageAttachment> attachment) { // We write the index of the descriptor so that we don't have to // keep the current descriptor as extra decoding state when deserialising. WriteInt(attachment_set()->size()); - -#if USE_ATTACHMENT_BROKER - if (attachment->GetType() == MessageAttachment::TYPE_BROKERABLE_ATTACHMENT) - header()->num_brokered_attachments += 1; -#endif - return attachment_set()->AddAttachment(attachment); } diff --git a/ipc/ipc_message.h b/ipc/ipc_message.h index af102d7..8210880 100644 --- a/ipc/ipc_message.h +++ b/ipc/ipc_message.h @@ -12,7 +12,6 @@ #include "base/memory/ref_counted.h" #include "base/pickle.h" #include "base/trace_event/trace_event.h" -#include "ipc/attachment_broker.h" #include "ipc/brokerable_attachment.h" #include "ipc/ipc_export.h" @@ -252,12 +251,6 @@ class IPC_EXPORT Message : public base::Pickle { int32_t routing; // ID of the view that this message is destined for uint32_t type; // specifies the user-defined message type uint32_t flags; // specifies control flags for the message -#if USE_ATTACHMENT_BROKER - // The number of brokered attachments included with this message. The - // ids of the brokered attachment ids are sent immediately after the pickled - // message, before the next pickled message is sent. - uint32_t num_brokered_attachments; -#endif #if defined(OS_POSIX) uint16_t num_fds; // the number of descriptors included with this message uint16_t pad; // explicitly initialize this to appease valgrind |