diff options
31 files changed, 181 insertions, 650 deletions
diff --git a/ipc/BUILD.gn b/ipc/BUILD.gn index 85cc29a..3fda6e4 100644 --- a/ipc/BUILD.gn +++ b/ipc/BUILD.gn @@ -82,8 +82,6 @@ component("ipc") { "param_traits_macros.h", "param_traits_read_macros.h", "param_traits_write_macros.h", - "placeholder_brokerable_attachment.cc", - "placeholder_brokerable_attachment.h", "struct_constructor_macros.h", "struct_destructor_macros.h", "unix_domain_socket_util.cc", @@ -143,9 +141,6 @@ if (!is_android) { "ipc_sync_channel_unittest.cc", "ipc_sync_message_unittest.cc", "ipc_sync_message_unittest.h", - "ipc_test_message_generator.cc", - "ipc_test_message_generator.h", - "ipc_test_messages.h", "sync_socket_unittest.cc", "unix_domain_socket_util_unittest.cc", ] diff --git a/ipc/attachment_broker_privileged_win.cc b/ipc/attachment_broker_privileged_win.cc index 20eb4f3..c8cc1d0 100644 --- a/ipc/attachment_broker_privileged_win.cc +++ b/ipc/attachment_broker_privileged_win.cc @@ -22,7 +22,7 @@ bool AttachmentBrokerPrivilegedWin::SendAttachmentToProcess( const BrokerableAttachment* attachment, base::ProcessId destination_process) { switch (attachment->GetBrokerableType()) { - case BrokerableAttachment::WIN_HANDLE: { + case BrokerableAttachment::WIN_HANDLE: const internal::HandleAttachmentWin* handle_attachment = static_cast<const internal::HandleAttachmentWin*>(attachment); HandleWireFormat wire_format = @@ -33,10 +33,6 @@ bool AttachmentBrokerPrivilegedWin::SendAttachmentToProcess( return false; RouteDuplicatedHandle(new_wire_format); return true; - } - case BrokerableAttachment::PLACEHOLDER: - NOTREACHED(); - return false; } return false; } diff --git a/ipc/attachment_broker_privileged_win_unittest.cc b/ipc/attachment_broker_privileged_win_unittest.cc index 610921e..6e8f881 100644 --- a/ipc/attachment_broker_privileged_win_unittest.cc +++ b/ipc/attachment_broker_privileged_win_unittest.cc @@ -17,7 +17,6 @@ #include "ipc/ipc_listener.h" #include "ipc/ipc_message.h" #include "ipc/ipc_test_base.h" -#include "ipc/ipc_test_messages.h" namespace { @@ -36,83 +35,25 @@ std::string ReadFromFile(HANDLE h) { HANDLE GetHandleFromBrokeredAttachment( const scoped_refptr<IPC::BrokerableAttachment>& attachment) { if (attachment->GetType() != - IPC::BrokerableAttachment::TYPE_BROKERABLE_ATTACHMENT) { - LOG(INFO) << "Attachment type not TYPE_BROKERABLE_ATTACHMENT."; + IPC::BrokerableAttachment::TYPE_BROKERABLE_ATTACHMENT) return nullptr; - } - - if (attachment->GetBrokerableType() != - IPC::BrokerableAttachment::WIN_HANDLE) { - LOG(INFO) << "Brokerable type not WIN_HANDLE."; + if (attachment->GetBrokerableType() != IPC::BrokerableAttachment::WIN_HANDLE) return nullptr; - } - IPC::internal::HandleAttachmentWin* received_handle_attachment = static_cast<IPC::internal::HandleAttachmentWin*>(attachment.get()); return received_handle_attachment->get_handle(); } -// |message| must be deserializable as a TestHandleWinMsg. Returns the HANDLE, -// or nullptr if deserialization failed. -HANDLE GetHandleFromTestHandleWinMsg(const IPC::Message& message) { - // Expect a message with a brokered attachment. - if (!message.HasBrokerableAttachments()) { - LOG(INFO) << "Message missing brokerable attachment."; - return nullptr; - } - - TestHandleWinMsg::Schema::Param p; - bool success = TestHandleWinMsg::Read(&message, &p); - if (!success) { - LOG(INFO) << "Failed to deserialize message."; - return nullptr; - } - - IPC::HandleWin handle_win = base::get<1>(p); - return handle_win.get_handle(); -} - -// |message| must be deserializable as a TestTwoHandleWinMsg. Returns the -// HANDLE, or nullptr if deserialization failed. -HANDLE GetHandleFromTestTwoHandleWinMsg(const IPC::Message& message, - int index) { - // Expect a message with a brokered attachment. - if (!message.HasBrokerableAttachments()) { - LOG(INFO) << "Message missing brokerable attachment."; - return nullptr; - } - - TestTwoHandleWinMsg::Schema::Param p; - bool success = TestTwoHandleWinMsg::Read(&message, &p); - if (!success) { - LOG(INFO) << "Failed to deserialize message."; - return nullptr; - } - - IPC::HandleWin handle_win; - if (index == 0) - handle_win = base::get<0>(p); - else if (index == 1) - handle_win = base::get<1>(p); - return handle_win.get_handle(); -} - -// |message| must be deserializable as a TestHandleWinMsg. Returns true if the -// attached file HANDLE has contents |kDataBuffer|. -bool CheckContentsOfTestMessage(const IPC::Message& message) { - HANDLE h = GetHandleFromTestHandleWinMsg(message); - if (h == nullptr) { - LOG(INFO) << "Failed to get handle from TestHandleWinMsg."; +// Returns true if |attachment| is a file HANDLE whose contents is +// |kDataBuffer|. +bool CheckContentsOfBrokeredAttachment( + const scoped_refptr<IPC::BrokerableAttachment>& attachment) { + HANDLE h = GetHandleFromBrokeredAttachment(attachment); + if (h == nullptr) return false; - } std::string contents = ReadFromFile(h); - bool success = (contents == std::string(kDataBuffer)); - if (!success) { - LOG(INFO) << "Expected contents: " << std::string(kDataBuffer); - LOG(INFO) << "Read contents: " << contents; - } - return success; + return contents == std::string(kDataBuffer); } enum TestResult { @@ -147,37 +88,30 @@ class MockObserver : public IPC::AttachmentBroker::Observer { // message is received, or the channel has an error. class ProxyListener : public IPC::Listener { public: - ProxyListener() : listener_(nullptr), reason_(MESSAGE_RECEIVED) {} + ProxyListener() : reason_(MESSAGE_RECEIVED) {} ~ProxyListener() override {} // The reason for exiting the message loop. enum Reason { MESSAGE_RECEIVED, CHANNEL_ERROR }; bool OnMessageReceived(const IPC::Message& message) override { - bool result = false; - if (listener_) - result = listener_->OnMessageReceived(message); + bool result = listener_->OnMessageReceived(message); reason_ = MESSAGE_RECEIVED; - messages_.push_back(message); - base::MessageLoop::current()->QuitNow(); + base::MessageLoop::current()->Quit(); return result; } void OnChannelError() override { reason_ = CHANNEL_ERROR; - base::MessageLoop::current()->QuitNow(); + base::MessageLoop::current()->Quit(); } void set_listener(IPC::Listener* listener) { listener_ = listener; } Reason get_reason() { return reason_; } - IPC::Message get_first_message() { return messages_[0]; } - void pop_first_message() { messages_.erase(messages_.begin()); } - bool has_message() { return !messages_.empty(); } private: IPC::Listener* listener_; Reason reason_; - std::vector<IPC::Message> messages_; }; // Waits for a result to be sent over the channel. Quits the message loop @@ -206,7 +140,7 @@ class ResultListener : public IPC::Listener { // as the privileged process. class IPCAttachmentBrokerPrivilegedWinTest : public IPCTestBase { public: - IPCAttachmentBrokerPrivilegedWinTest() {} + IPCAttachmentBrokerPrivilegedWinTest() : message_index_(0) {} ~IPCAttachmentBrokerPrivilegedWinTest() override {} void SetUp() override { @@ -254,8 +188,12 @@ class IPCAttachmentBrokerPrivilegedWinTest : public IPCTestBase { } void SendMessageWithAttachment(HANDLE h) { - IPC::HandleWin handle_win(h, IPC::HandleWin::FILE_READ_WRITE); - IPC::Message* message = new TestHandleWinMsg(100, handle_win, 200); + IPC::Message* message = + new IPC::Message(0, 2, IPC::Message::PRIORITY_NORMAL); + message->WriteInt(message_index_++); + scoped_refptr<IPC::internal::HandleAttachmentWin> attachment( + new IPC::internal::HandleAttachmentWin(h, IPC::HandleWin::DUPLICATE)); + ASSERT_TRUE(message->WriteAttachment(attachment)); sender()->Send(message); } @@ -266,6 +204,7 @@ class IPCAttachmentBrokerPrivilegedWinTest : public IPCTestBase { private: base::ScopedTempDir temp_dir_; base::FilePath temp_path_; + int message_index_; ProxyListener proxy_listener_; scoped_ptr<IPC::AttachmentBrokerUnprivilegedWin> broker_; MockObserver observer_; @@ -321,9 +260,7 @@ TEST_F(IPCAttachmentBrokerPrivilegedWinTest, SendHandleWithoutPermissions) { BOOL result = ::DuplicateHandle(GetCurrentProcess(), h, GetCurrentProcess(), &h2, 0, FALSE, DUPLICATE_CLOSE_SOURCE); ASSERT_TRUE(result); - IPC::HandleWin handle_win(h2, IPC::HandleWin::DUPLICATE); - IPC::Message* message = new TestHandleWinMsg(100, handle_win, 200); - sender()->Send(message); + SendMessageWithAttachment(h2); base::MessageLoop::current()->Run(); // Check the result. @@ -369,18 +306,19 @@ TEST_F(IPCAttachmentBrokerPrivilegedWinTest, SendHandleToSelf) { CommonTearDown(); } -// Similar to SendHandle, but sends a message with the same handle twice. -TEST_F(IPCAttachmentBrokerPrivilegedWinTest, SendTwoHandles) { - Init("SendTwoHandles"); +// Similar to SendHandle, except this test uses the HandleWin class. +TEST_F(IPCAttachmentBrokerPrivilegedWinTest, SendHandleWin) { + Init("SendHandleWin"); CommonSetUp(); ResultListener result_listener; get_proxy_listener()->set_listener(&result_listener); HANDLE h = CreateTempFile(); - IPC::HandleWin handle_win1(h, IPC::HandleWin::FILE_READ_WRITE); - IPC::HandleWin handle_win2(h, IPC::HandleWin::FILE_READ_WRITE); - IPC::Message* message = new TestTwoHandleWinMsg(handle_win1, handle_win2); + IPC::HandleWin handle_win(h, IPC::HandleWin::FILE_READ_WRITE); + IPC::Message* message = new IPC::Message(0, 2, IPC::Message::PRIORITY_NORMAL); + message->WriteInt(0); + IPC::ParamTraits<IPC::HandleWin>::Write(message, handle_win); sender()->Send(message); base::MessageLoop::current()->Run(); @@ -392,64 +330,49 @@ TEST_F(IPCAttachmentBrokerPrivilegedWinTest, SendTwoHandles) { CommonTearDown(); } -// Similar to SendHandle, but sends the same message twice. -TEST_F(IPCAttachmentBrokerPrivilegedWinTest, SendHandleTwice) { - Init("SendHandleTwice"); - - CommonSetUp(); - ResultListener result_listener; - get_proxy_listener()->set_listener(&result_listener); - - HANDLE h = CreateTempFile(); - SendMessageWithAttachment(h); - SendMessageWithAttachment(h); - base::MessageLoop::current()->Run(); - - // Check the result. - ASSERT_EQ(ProxyListener::MESSAGE_RECEIVED, - get_proxy_listener()->get_reason()); - ASSERT_EQ(result_listener.get_result(), RESULT_SUCCESS); - - CommonTearDown(); -} - -using OnMessageReceivedCallback = void (*)(IPC::Sender* sender, - const IPC::Message& message); +using OnMessageReceivedCallback = + void (*)(MockObserver* observer, + IPC::AttachmentBrokerPrivilegedWin* broker, + IPC::Sender* sender); int CommonPrivilegedProcessMain(OnMessageReceivedCallback callback, const char* channel_name) { - LOG(INFO) << "Privileged process start."; base::MessageLoopForIO main_message_loop; ProxyListener listener; // Set up IPC channel. IPC::AttachmentBrokerPrivilegedWin broker; + listener.set_listener(&broker); scoped_ptr<IPC::Channel> channel(IPC::Channel::CreateClient( IPCTestBase::GetChannelName(channel_name), &listener, &broker)); broker.RegisterCommunicationChannel(channel.get()); CHECK(channel->Connect()); + MockObserver observer; + broker.AddObserver(&observer); + while (true) { - LOG(INFO) << "Privileged process spinning run loop."; base::MessageLoop::current()->Run(); ProxyListener::Reason reason = listener.get_reason(); if (reason == ProxyListener::CHANNEL_ERROR) break; - while (listener.has_message()) { - LOG(INFO) << "Privileged process running callback."; - callback(channel.get(), listener.get_first_message()); - LOG(INFO) << "Privileged process finishing callback."; - listener.pop_first_message(); - } + callback(&observer, &broker, channel.get()); } - LOG(INFO) << "Privileged process end."; return 0; } -void SendHandleCallback(IPC::Sender* sender, const IPC::Message& message) { - bool success = CheckContentsOfTestMessage(message); +void SendHandleCallback(MockObserver* observer, + IPC::AttachmentBrokerPrivilegedWin* broker, + IPC::Sender* sender) { + IPC::BrokerableAttachment::AttachmentId* id = observer->get_id(); + scoped_refptr<IPC::BrokerableAttachment> received_attachment; + broker->GetAttachmentWithId(*id, &received_attachment); + + // Check that it's the expected handle. + bool success = CheckContentsOfBrokeredAttachment(received_attachment); + SendControlMessage(sender, success); } @@ -457,9 +380,16 @@ MULTIPROCESS_IPC_TEST_CLIENT_MAIN(SendHandle) { return CommonPrivilegedProcessMain(&SendHandleCallback, "SendHandle"); } -void SendHandleWithoutPermissionsCallback(IPC::Sender* sender, - const IPC::Message& message) { - HANDLE h = GetHandleFromTestHandleWinMsg(message); +void SendHandleWithoutPermissionsCallback( + MockObserver* observer, + IPC::AttachmentBrokerPrivilegedWin* broker, + IPC::Sender* sender) { + IPC::BrokerableAttachment::AttachmentId* id = observer->get_id(); + scoped_refptr<IPC::BrokerableAttachment> received_attachment; + broker->GetAttachmentWithId(*id, &received_attachment); + + // Check that it's the expected handle. + HANDLE h = GetHandleFromBrokeredAttachment(received_attachment); if (h != nullptr) { SetFilePointer(h, 0, nullptr, FILE_BEGIN); @@ -482,7 +412,9 @@ MULTIPROCESS_IPC_TEST_CLIENT_MAIN(SendHandleWithoutPermissions) { "SendHandleWithoutPermissions"); } -void SendHandleToSelfCallback(IPC::Sender* sender, const IPC::Message&) { +void SendHandleToSelfCallback(MockObserver* observer, + IPC::AttachmentBrokerPrivilegedWin* broker, + IPC::Sender* sender) { // Do nothing special. The default behavior already runs the // AttachmentBrokerPrivilegedWin. } @@ -492,69 +424,8 @@ MULTIPROCESS_IPC_TEST_CLIENT_MAIN(SendHandleToSelf) { "SendHandleToSelf"); } -void SendTwoHandlesCallback(IPC::Sender* sender, const IPC::Message& message) { - // Check for two handles. - HANDLE h1 = GetHandleFromTestTwoHandleWinMsg(message, 0); - HANDLE h2 = GetHandleFromTestTwoHandleWinMsg(message, 1); - if (h1 == nullptr || h2 == nullptr) { - SendControlMessage(sender, false); - return; - } - - // Check that their contents are correct. - std::string contents1 = ReadFromFile(h1); - std::string contents2 = ReadFromFile(h2); - if (contents1 != std::string(kDataBuffer) || - contents2 != std::string(kDataBuffer)) { - SendControlMessage(sender, false); - return; - } - - // Check that the handles point to the same file. - const char text[] = "katy perry"; - DWORD bytes_written = 0; - SetFilePointer(h1, 0, nullptr, FILE_BEGIN); - BOOL success = ::WriteFile(h1, text, static_cast<DWORD>(strlen(text)), - &bytes_written, nullptr); - if (!success) { - SendControlMessage(sender, false); - return; - } - - SetFilePointer(h2, 0, nullptr, FILE_BEGIN); - char buffer[100]; - DWORD bytes_read; - success = ::ReadFile(h2, buffer, static_cast<DWORD>(strlen(text)), - &bytes_read, nullptr); - if (!success) { - SendControlMessage(sender, false); - return; - } - success = std::string(buffer, bytes_read) == std::string(text); - SendControlMessage(sender, success); -} - -MULTIPROCESS_IPC_TEST_CLIENT_MAIN(SendTwoHandles) { - return CommonPrivilegedProcessMain(&SendTwoHandlesCallback, "SendTwoHandles"); -} - -void SendHandleTwiceCallback(IPC::Sender* sender, const IPC::Message& message) { - // We expect the same message twice. - static int i = 0; - static bool success = true; - success &= CheckContentsOfTestMessage(message); - if (i == 0) { - LOG(INFO) << "Received first message."; - ++i; - } else { - LOG(INFO) << "Received second message."; - SendControlMessage(sender, success); - } -} - -MULTIPROCESS_IPC_TEST_CLIENT_MAIN(SendHandleTwice) { - return CommonPrivilegedProcessMain(&SendHandleTwiceCallback, - "SendHandleTwice"); +MULTIPROCESS_IPC_TEST_CLIENT_MAIN(SendHandleWin) { + return CommonPrivilegedProcessMain(&SendHandleCallback, "SendHandleWin"); } } // namespace diff --git a/ipc/attachment_broker_unprivileged_win.cc b/ipc/attachment_broker_unprivileged_win.cc index f5a6e04..3d3e845 100644 --- a/ipc/attachment_broker_unprivileged_win.cc +++ b/ipc/attachment_broker_unprivileged_win.cc @@ -20,17 +20,13 @@ bool AttachmentBrokerUnprivilegedWin::SendAttachmentToProcess( const BrokerableAttachment* attachment, base::ProcessId destination_process) { switch (attachment->GetBrokerableType()) { - case BrokerableAttachment::WIN_HANDLE: { + case BrokerableAttachment::WIN_HANDLE: const internal::HandleAttachmentWin* handle_attachment = static_cast<const internal::HandleAttachmentWin*>(attachment); internal::HandleAttachmentWin::WireFormat format = handle_attachment->GetWireFormat(destination_process); return get_sender()->Send( new AttachmentBrokerMsg_DuplicateWinHandle(format)); - } - case BrokerableAttachment::PLACEHOLDER: - NOTREACHED(); - return false; } return false; } diff --git a/ipc/brokerable_attachment.cc b/ipc/brokerable_attachment.cc index 4a1becc..f1cc9b2 100644 --- a/ipc/brokerable_attachment.cc +++ b/ipc/brokerable_attachment.cc @@ -4,65 +4,47 @@ #include "ipc/brokerable_attachment.h" -#include "ipc/attachment_broker.h" - -#if USE_ATTACHMENT_BROKER #include "crypto/random.h" -#endif namespace IPC { -#if USE_ATTACHMENT_BROKER -BrokerableAttachment::AttachmentId::AttachmentId() { - // In order to prevent mutually untrusted processes from stealing resources - // from one another, the nonce must be secret. This generates a 128-bit, - // cryptographicaly-strong random number. - crypto::RandBytes(nonce, BrokerableAttachment::kNonceSize); -} -#else -BrokerableAttachment::AttachmentId::AttachmentId() { - CHECK(false) << "Not allowed to construct an attachment id if the platform " - "does not support attachment brokering."; -} -#endif +namespace { -BrokerableAttachment::AttachmentId::AttachmentId(const char* start_address, - size_t size) { - DCHECK(size == BrokerableAttachment::kNonceSize); - for (size_t i = 0; i < BrokerableAttachment::kNonceSize; ++i) - nonce[i] = start_address[i]; +// In order to prevent mutually untrusted processes from stealing resources from +// one another, the nonce must be secret. This generates a 128-bit, +// cryptographicaly-strong random number. +BrokerableAttachment::AttachmentId GetRandomId() { + BrokerableAttachment::AttachmentId id; + crypto::RandBytes(id.nonce, BrokerableAttachment::kNonceSize); + return id; } -void BrokerableAttachment::AttachmentId::SerializeToBuffer(char* start_address, - size_t size) { - DCHECK(size == BrokerableAttachment::kNonceSize); - for (size_t i = 0; i < BrokerableAttachment::kNonceSize; ++i) - start_address[i] = nonce[i]; -} +} // namespace -BrokerableAttachment::BrokerableAttachment() {} +BrokerableAttachment::BrokerableAttachment() + : id_(GetRandomId()), needs_brokering_(false) {} -BrokerableAttachment::BrokerableAttachment(const AttachmentId& id) : id_(id) {} +BrokerableAttachment::BrokerableAttachment(const AttachmentId& id, + bool needs_brokering) + : id_(id), needs_brokering_(needs_brokering) {} -BrokerableAttachment::~BrokerableAttachment() {} +BrokerableAttachment::~BrokerableAttachment() { +} BrokerableAttachment::AttachmentId BrokerableAttachment::GetIdentifier() const { return id_; } bool BrokerableAttachment::NeedsBrokering() const { - return GetBrokerableType() == PLACEHOLDER; + return needs_brokering_; } -BrokerableAttachment::Type BrokerableAttachment::GetType() const { - return TYPE_BROKERABLE_ATTACHMENT; +void BrokerableAttachment::SetNeedsBrokering(bool needs_brokering) { + needs_brokering_ = needs_brokering; } -#if defined(OS_POSIX) -base::PlatformFile BrokerableAttachment::TakePlatformFile() { - NOTREACHED(); - return base::PlatformFile(); +BrokerableAttachment::Type BrokerableAttachment::GetType() const { + return TYPE_BROKERABLE_ATTACHMENT; } -#endif // OS_POSIX } // namespace IPC diff --git a/ipc/brokerable_attachment.h b/ipc/brokerable_attachment.h index cad6f4d..452d129 100644 --- a/ipc/brokerable_attachment.h +++ b/ipc/brokerable_attachment.h @@ -22,15 +22,6 @@ class IPC_EXPORT BrokerableAttachment : public MessageAttachment { struct IPC_EXPORT AttachmentId { uint8_t nonce[kNonceSize]; - // Default constructor returns a random nonce. - AttachmentId(); - - // Constructs an AttachmentId from a buffer. - AttachmentId(const char* start_address, size_t size); - - // Writes the nonce into a buffer. - void SerializeToBuffer(char* start_address, size_t size); - bool operator==(const AttachmentId& rhs) const { for (size_t i = 0; i < kNonceSize; ++i) { if (nonce[i] != rhs.nonce[i]) @@ -51,7 +42,6 @@ class IPC_EXPORT BrokerableAttachment : public MessageAttachment { }; enum BrokerableType { - PLACEHOLDER, WIN_HANDLE, }; @@ -62,26 +52,31 @@ class IPC_EXPORT BrokerableAttachment : public MessageAttachment { // can be used. bool NeedsBrokering() const; + // Fills in the data of this instance with the data from |attachment|. + // This instance must require brokering, |attachment| must be brokered, and + // both instances must have the same identifier. + virtual void PopulateWithAttachment( + const BrokerableAttachment* attachment) = 0; + // Returns TYPE_BROKERABLE_ATTACHMENT Type GetType() const override; virtual BrokerableType GetBrokerableType() const = 0; -// MessageAttachment override. -#if defined(OS_POSIX) - base::PlatformFile TakePlatformFile() override; -#endif // OS_POSIX - protected: BrokerableAttachment(); - BrokerableAttachment(const AttachmentId& id); + BrokerableAttachment(const AttachmentId& id, bool needs_brokering); ~BrokerableAttachment() override; + void SetNeedsBrokering(bool needs_brokering); + private: // This member uniquely identifies a BrokerableAttachment across all Chrome // processes. const AttachmentId id_; + // Whether the attachment still needs to be filled in by an AttachmentBroker. + bool needs_brokering_; DISALLOW_COPY_AND_ASSIGN(BrokerableAttachment); }; diff --git a/ipc/handle_attachment_win.cc b/ipc/handle_attachment_win.cc index b7f4373..50e3e6d 100644 --- a/ipc/handle_attachment_win.cc +++ b/ipc/handle_attachment_win.cc @@ -14,13 +14,13 @@ HandleAttachmentWin::HandleAttachmentWin(const HANDLE& handle, : handle_(handle), permissions_(permissions) {} HandleAttachmentWin::HandleAttachmentWin(const WireFormat& wire_format) - : BrokerableAttachment(wire_format.attachment_id), + : BrokerableAttachment(wire_format.attachment_id, false), handle_(LongToHandle(wire_format.handle)), permissions_(wire_format.permissions) {} HandleAttachmentWin::HandleAttachmentWin( const BrokerableAttachment::AttachmentId& id) - : BrokerableAttachment(id), + : BrokerableAttachment(id, true), handle_(INVALID_HANDLE_VALUE), permissions_(HandleWin::INVALID) {} @@ -32,6 +32,19 @@ HandleAttachmentWin::BrokerableType HandleAttachmentWin::GetBrokerableType() return WIN_HANDLE; } +void HandleAttachmentWin::PopulateWithAttachment( + const BrokerableAttachment* attachment) { + DCHECK(NeedsBrokering()); + DCHECK(!attachment->NeedsBrokering()); + DCHECK(GetIdentifier() == attachment->GetIdentifier()); + DCHECK_EQ(GetBrokerableType(), attachment->GetBrokerableType()); + + const HandleAttachmentWin* handle_attachment = + static_cast<const HandleAttachmentWin*>(attachment); + handle_ = handle_attachment->handle_; + SetNeedsBrokering(false); +} + HandleAttachmentWin::WireFormat HandleAttachmentWin::GetWireFormat( const base::ProcessId& destination) const { WireFormat format; diff --git a/ipc/handle_attachment_win.h b/ipc/handle_attachment_win.h index cd086b9..127cd57 100644 --- a/ipc/handle_attachment_win.h +++ b/ipc/handle_attachment_win.h @@ -40,6 +40,7 @@ class IPC_EXPORT HandleAttachmentWin : public BrokerableAttachment { explicit HandleAttachmentWin(const BrokerableAttachment::AttachmentId& id); BrokerableType GetBrokerableType() const override; + void PopulateWithAttachment(const BrokerableAttachment* attachment) override; // Returns the wire format of this attachment. WireFormat GetWireFormat(const base::ProcessId& destination) const; diff --git a/ipc/handle_win.cc b/ipc/handle_win.cc index 5d25905..ea268bd 100644 --- a/ipc/handle_win.cc +++ b/ipc/handle_win.cc @@ -11,8 +11,6 @@ namespace IPC { -HandleWin::HandleWin() : handle_(nullptr), permissions_(INVALID) {} - HandleWin::HandleWin(const HANDLE& handle, Permissions permissions) : handle_(handle), permissions_(permissions) {} diff --git a/ipc/handle_win.h b/ipc/handle_win.h index a8f6978..f5f0263 100644 --- a/ipc/handle_win.h +++ b/ipc/handle_win.h @@ -28,8 +28,6 @@ class IPC_EXPORT HandleWin { MAX_PERMISSIONS = FILE_READ_WRITE }; - // Default constructor makes an invalid HANDLE. - HandleWin(); HandleWin(const HANDLE& handle, Permissions permissions); HANDLE get_handle() const { return handle_; } diff --git a/ipc/ipc.gyp b/ipc/ipc.gyp index 00f58b4..fb64c32 100644 --- a/ipc/ipc.gyp +++ b/ipc/ipc.gyp @@ -59,9 +59,6 @@ 'ipc_sync_channel_unittest.cc', 'ipc_sync_message_unittest.cc', 'ipc_sync_message_unittest.h', - 'ipc_test_messages.h', - 'ipc_test_message_generator.cc', - 'ipc_test_message_generator.h', 'run_all_unittests.cc', 'sync_socket_unittest.cc', 'unix_domain_socket_util_unittest.cc', diff --git a/ipc/ipc.gypi b/ipc/ipc.gypi index dbcdec2..beb0a6a 100644 --- a/ipc/ipc.gypi +++ b/ipc/ipc.gypi @@ -87,8 +87,6 @@ 'param_traits_macros.h', 'param_traits_read_macros.h', 'param_traits_write_macros.h', - 'placeholder_brokerable_attachment.cc', - 'placeholder_brokerable_attachment.h', 'struct_constructor_macros.h', 'struct_destructor_macros.h', 'unix_domain_socket_util.cc', diff --git a/ipc/ipc_channel.cc b/ipc/ipc_channel.cc index 89505cb..ac09c5a 100644 --- a/ipc/ipc_channel.cc +++ b/ipc/ipc_channel.cc @@ -41,26 +41,4 @@ std::string Channel::GenerateUniqueRandomChannelID() { base::RandInt(0, std::numeric_limits<int32>::max())); } -Channel::OutputElement::OutputElement(Message* message) - : message_(message), buffer_(nullptr), length_(0) {} - -Channel::OutputElement::OutputElement(void* buffer, size_t length) - : message_(nullptr), buffer_(buffer), length_(length) {} - -Channel::OutputElement::~OutputElement() { - free(buffer_); -} - -size_t Channel::OutputElement::size() const { - if (message_) - return message_->size(); - return length_; -} - -const void* Channel::OutputElement::data() const { - if (message_) - return message_->data(); - return buffer_; -} - } // namespace IPC diff --git a/ipc/ipc_channel.h b/ipc/ipc_channel.h index 9021ab4..c74d084 100644 --- a/ipc/ipc_channel.h +++ b/ipc/ipc_channel.h @@ -52,7 +52,7 @@ class IPC_EXPORT Channel : public Endpoint { MODE_CLIENT_FLAG = 0x2, MODE_NAMED_FLAG = 0x4, #if defined(OS_POSIX) - MODE_OPEN_ACCESS_FLAG = 0x8, // Don't restrict access based on client UID. + MODE_OPEN_ACCESS_FLAG = 0x8, // Don't restrict access based on client UID. #endif }; @@ -66,8 +66,8 @@ class IPC_EXPORT Channel : public Endpoint { MODE_NAMED_SERVER = MODE_SERVER_FLAG | MODE_NAMED_FLAG, MODE_NAMED_CLIENT = MODE_CLIENT_FLAG | MODE_NAMED_FLAG, #if defined(OS_POSIX) - MODE_OPEN_NAMED_SERVER = - MODE_OPEN_ACCESS_FLAG | MODE_SERVER_FLAG | MODE_NAMED_FLAG + MODE_OPEN_NAMED_SERVER = MODE_OPEN_ACCESS_FLAG | MODE_SERVER_FLAG | + MODE_NAMED_FLAG #endif }; @@ -195,9 +195,9 @@ class IPC_EXPORT Channel : public Endpoint { // threads. This is constant for the lifetime of the |Channel|. virtual bool IsSendThreadSafe() const; -// NaCl in Non-SFI mode runs on Linux directly, and the following functions -// compiled on Linux are also needed. Please see also comments in -// components/nacl_nonsfi.gyp for more details. + // NaCl in Non-SFI mode runs on Linux directly, and the following functions + // compiled on Linux are also needed. Please see also comments in + // components/nacl_nonsfi.gyp for more details. #if defined(OS_POSIX) && !defined(OS_NACL_SFI) // On POSIX an IPC::Channel wraps a socketpair(), this method returns the // FD # for the client end of the socket. @@ -240,26 +240,6 @@ class IPC_EXPORT Channel : public Endpoint { // process such that it acts similar to if it was exec'd, for tests. static void NotifyProcessForkedForTesting(); #endif - - protected: - // An OutputElement is a wrapper around a Message or raw buffer while it is - // waiting to be passed to the system's underlying IPC mechanism. - class OutputElement { - public: - // Takes ownership of message. - OutputElement(Message* message); - // Takes ownership of the buffer. - OutputElement(void* buffer, size_t length); - ~OutputElement(); - size_t size() const; - const void* data() const; - const Message* get_message() const { return message_.get(); } - - private: - scoped_ptr<const Message> message_; - void* buffer_; - size_t length_; - }; }; #if defined(OS_POSIX) diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc index 977db22..b5e2332 100644 --- a/ipc/ipc_channel_posix.cc +++ b/ipc/ipc_channel_posix.cc @@ -205,7 +205,6 @@ ChannelPosix::ChannelPosix(const IPC::ChannelHandle& channel_handle, ChannelPosix::~ChannelPosix() { in_dtor_ = true; - CleanUp(); Close(); } diff --git a/ipc/ipc_channel_reader.cc b/ipc/ipc_channel_reader.cc index c47d2bc..a76b7e6 100644 --- a/ipc/ipc_channel_reader.cc +++ b/ipc/ipc_channel_reader.cc @@ -20,7 +20,8 @@ ChannelReader::ChannelReader(Listener* listener) : listener_(listener) { } ChannelReader::~ChannelReader() { - DCHECK(blocked_ids_.empty()); + if (!blocked_ids_.empty()) + StopObservingAttachmentBroker(); } ChannelReader::DispatchState ChannelReader::ProcessIncomingMessages() { @@ -84,14 +85,11 @@ bool ChannelReader::TranslateInputData(const char* input_data, // Dispatch all complete messages in the data buffer. while (p < end) { - Message::NextMessageInfo info = Message::FindNext(p, end); - if (info.message_found) { - int pickle_len = static_cast<int>(info.pickle_end - p); - Message translated_message(p, pickle_len); - - for (const auto& id : info.attachment_ids) - translated_message.AddPlaceholderBrokerableAttachmentWithId(id); + const char* message_tail = Message::FindNext(p, end); + if (message_tail) { + int len = static_cast<int>(message_tail - p); + Message translated_message(p, len); if (!GetNonBrokeredAttachments(&translated_message)) return false; @@ -105,7 +103,7 @@ bool ChannelReader::TranslateInputData(const char* input_data, if (blocked_ids.empty()) { // Dispatch the message and continue the loop. DispatchMessage(&translated_message); - p = info.message_end; + p = message_tail; continue; } @@ -116,7 +114,7 @@ bool ChannelReader::TranslateInputData(const char* input_data, // Make a deep copy of |translated_message| to add to the queue. scoped_ptr<Message> m(new Message(translated_message)); queued_messages_.push_back(m.release()); - p = info.message_end; + p = message_tail; } else { // Last message is partial. break; @@ -151,13 +149,6 @@ ChannelReader::DispatchState ChannelReader::DispatchMessages() { return DISPATCH_FINISHED; } -void ChannelReader::CleanUp() { - if (!blocked_ids_.empty()) { - StopObservingAttachmentBroker(); - blocked_ids_.clear(); - } -} - void ChannelReader::DispatchMessage(Message* m) { m->set_sender_pid(GetSenderPID()); @@ -200,9 +191,8 @@ ChannelReader::AttachmentIdSet ChannelReader::GetBrokeredAttachments( #if USE_ATTACHMENT_BROKER MessageAttachmentSet* set = msg->attachment_set(); - std::vector<const BrokerableAttachment*> brokerable_attachments_copy = - set->PeekBrokerableAttachments(); - for (const BrokerableAttachment* attachment : brokerable_attachments_copy) { + for (const scoped_refptr<BrokerableAttachment>& attachment : + set->GetBrokerableAttachmentsForUpdating()) { if (attachment->NeedsBrokering()) { AttachmentBroker* broker = GetAttachmentBroker(); scoped_refptr<BrokerableAttachment> brokered_attachment; @@ -213,7 +203,7 @@ ChannelReader::AttachmentIdSet ChannelReader::GetBrokeredAttachments( continue; } - set->ReplacePlaceholderWithAttachment(brokered_attachment); + attachment->PopulateWithAttachment(brokered_attachment.get()); } } #endif // USE_ATTACHMENT_BROKER diff --git a/ipc/ipc_channel_reader.h b/ipc/ipc_channel_reader.h index 97f6747..6f4b870 100644 --- a/ipc/ipc_channel_reader.h +++ b/ipc/ipc_channel_reader.h @@ -75,10 +75,6 @@ class IPC_EXPORT ChannelReader : public SupportsAttachmentBrokering, Listener* listener() const { return listener_; } - // Subclasses should call this method in their destructor to give this class a - // chance to clean up state that might be dependent on subclass members. - void CleanUp(); - // Populates the given buffer with data from the pipe. // // Returns the state of the read. On READ_SUCCESS, the number of bytes diff --git a/ipc/ipc_channel_reader_unittest.cc b/ipc/ipc_channel_reader_unittest.cc index 0058980..cbcd5dc 100644 --- a/ipc/ipc_channel_reader_unittest.cc +++ b/ipc/ipc_channel_reader_unittest.cc @@ -9,7 +9,6 @@ #include "ipc/attachment_broker.h" #include "ipc/brokerable_attachment.h" #include "ipc/ipc_channel_reader.h" -#include "ipc/placeholder_brokerable_attachment.h" #include "testing/gtest/include/gtest/gtest.h" #if USE_ATTACHMENT_BROKER @@ -20,9 +19,15 @@ namespace { class MockAttachment : public BrokerableAttachment { public: - MockAttachment() {} + MockAttachment(int internal_state) : internal_state_(internal_state) {} MockAttachment(BrokerableAttachment::AttachmentId id) - : BrokerableAttachment(id) {} + : BrokerableAttachment(id, true), internal_state_(-1) {} + + void PopulateWithAttachment(const BrokerableAttachment* attachment) override { + const MockAttachment* mock_attachment = + static_cast<const MockAttachment*>(attachment); + internal_state_ = mock_attachment->internal_state_; + } #if defined(OS_POSIX) base::PlatformFile TakePlatformFile() override { @@ -34,6 +39,8 @@ class MockAttachment : public BrokerableAttachment { private: ~MockAttachment() override {} + // Internal state differentiates MockAttachments. + int internal_state_; }; class MockAttachmentBroker : public AttachmentBroker { @@ -98,12 +105,12 @@ TEST(ChannelReaderTest, AttachmentAlreadyBrokered) { MockAttachmentBroker broker; MockChannelReader reader; reader.set_broker(&broker); - scoped_refptr<MockAttachment> attachment(new MockAttachment); + scoped_refptr<MockAttachment> attachment(new MockAttachment(5)); broker.AddAttachment(attachment); Message* m = new Message; - PlaceholderBrokerableAttachment* needs_brokering_attachment = - new PlaceholderBrokerableAttachment(attachment->GetIdentifier()); + MockAttachment* needs_brokering_attachment = + new MockAttachment(attachment->GetIdentifier()); EXPECT_TRUE(m->WriteAttachment(needs_brokering_attachment)); reader.AddMessageForDispatch(m); EXPECT_EQ(ChannelReader::DISPATCH_FINISHED, reader.DispatchMessages()); @@ -114,11 +121,11 @@ TEST(ChannelReaderTest, AttachmentNotYetBrokered) { MockAttachmentBroker broker; MockChannelReader reader; reader.set_broker(&broker); - scoped_refptr<MockAttachment> attachment(new MockAttachment); + scoped_refptr<MockAttachment> attachment(new MockAttachment(5)); Message* m = new Message; - PlaceholderBrokerableAttachment* needs_brokering_attachment = - new PlaceholderBrokerableAttachment(attachment->GetIdentifier()); + MockAttachment* needs_brokering_attachment = + new MockAttachment(attachment->GetIdentifier()); EXPECT_TRUE(m->WriteAttachment(needs_brokering_attachment)); reader.AddMessageForDispatch(m); EXPECT_EQ(ChannelReader::DISPATCH_WAITING_ON_BROKER, diff --git a/ipc/ipc_channel_win.cc b/ipc/ipc_channel_win.cc index f89e946..ad7ffb5 100644 --- a/ipc/ipc_channel_win.cc +++ b/ipc/ipc_channel_win.cc @@ -53,7 +53,6 @@ ChannelWin::ChannelWin(const IPC::ChannelHandle& channel_handle, } ChannelWin::~ChannelWin() { - CleanUp(); Close(); } @@ -75,9 +74,9 @@ void ChannelWin::Close() { } while (!output_queue_.empty()) { - OutputElement* element = output_queue_.front(); + Message* m = output_queue_.front(); output_queue_.pop(); - delete element; + delete m; } } @@ -127,19 +126,7 @@ bool ChannelWin::ProcessMessageForDelivery(Message* message) { TRACE_EVENT_FLAG_FLOW_OUT); // |output_queue_| takes ownership of |message|. - OutputElement* element = new OutputElement(message); - output_queue_.push(element); - -#if USE_ATTACHMENT_BROKER - if (message->HasBrokerableAttachments()) { - // |output_queue_| takes ownership of |ids.buffer|. - Message::SerializedAttachmentIds ids = - message->SerializedIdsOfBrokerableAttachments(); - OutputElement* new_element = new OutputElement(ids.buffer, ids.size); - output_queue_.push(new_element); - } -#endif - + output_queue_.push(message); // ensure waiting to write if (!waiting_connect_) { if (!output_state_.is_pending) { @@ -376,8 +363,7 @@ bool ChannelWin::CreatePipe(const IPC::ChannelHandle &channel_handle, return false; } - OutputElement* element = new OutputElement(m.release()); - output_queue_.push(element); + output_queue_.push(m.release()); return true; } @@ -467,9 +453,9 @@ bool ChannelWin::ProcessOutgoingMessages( } // Message was sent. CHECK(!output_queue_.empty()); - OutputElement* element = output_queue_.front(); + Message* m = output_queue_.front(); output_queue_.pop(); - delete element; + delete m; } if (output_queue_.empty()) @@ -479,21 +465,20 @@ bool ChannelWin::ProcessOutgoingMessages( return false; // Write to pipe... - OutputElement* element = output_queue_.front(); - DCHECK(element->size() <= INT_MAX); - BOOL ok = WriteFile(pipe_.Get(), element->data(), - static_cast<uint32>(element->size()), NULL, + Message* m = output_queue_.front(); + DCHECK(m->size() <= INT_MAX); + BOOL ok = WriteFile(pipe_.Get(), + m->data(), + static_cast<uint32>(m->size()), + NULL, &output_state_.context.overlapped); if (!ok) { DWORD write_error = GetLastError(); if (write_error == ERROR_IO_PENDING) { output_state_.is_pending = true; - const Message* m = element->get_message(); - if (m) { - DVLOG(2) << "sent pending message @" << m << " on channel @" << this - << " with type " << m->type(); - } + DVLOG(2) << "sent pending message @" << m << " on channel @" << this + << " with type " << m->type(); return true; } @@ -501,11 +486,8 @@ bool ChannelWin::ProcessOutgoingMessages( return false; } - const Message* m = element->get_message(); - if (m) { - DVLOG(2) << "sent message @" << m << " on channel @" << this - << " with type " << m->type(); - } + DVLOG(2) << "sent message @" << m << " on channel @" << this + << " with type " << m->type(); output_state_.is_pending = true; return true; diff --git a/ipc/ipc_channel_win.h b/ipc/ipc_channel_win.h index 9d561b0..d905297 100644 --- a/ipc/ipc_channel_win.h +++ b/ipc/ipc_channel_win.h @@ -105,7 +105,7 @@ class ChannelWin : public Channel, std::queue<Message*> prelim_queue_; // Messages to be sent are queued here. - std::queue<OutputElement*> output_queue_; + std::queue<Message*> output_queue_; // In server-mode, we have to wait for the client to connect before we // can begin reading. We make use of the input_state_ when performing diff --git a/ipc/ipc_message.cc b/ipc/ipc_message.cc index 6b55d27..07d5d25 100644 --- a/ipc/ipc_message.cc +++ b/ipc/ipc_message.cc @@ -9,7 +9,6 @@ #include "build/build_config.h" #include "ipc/ipc_message_attachment.h" #include "ipc/ipc_message_attachment_set.h" -#include "ipc/placeholder_brokerable_attachment.h" #if defined(OS_POSIX) #include "base/file_descriptor_posix.h" @@ -47,9 +46,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; @@ -63,9 +59,6 @@ Message::Message(int32 routing_id, uint32 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; @@ -134,91 +127,10 @@ void Message::set_received_time(int64 time) const { } #endif -Message::NextMessageInfo::NextMessageInfo() - : message_found(false), pickle_end(nullptr), message_end(nullptr) {} -Message::NextMessageInfo::~NextMessageInfo() {} - -// static -Message::NextMessageInfo Message::FindNext(const char* range_start, - const char* range_end) { - NextMessageInfo info; - const char* pickle_end = - base::Pickle::FindNext(sizeof(Header), range_start, range_end); - if (pickle_end == nullptr) - return info; - info.pickle_end = pickle_end; - -#if USE_ATTACHMENT_BROKER - // 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)); - int num_attachments = message.header()->num_brokered_attachments; - - // Each brokered attachment adds kNonceSize bytes of data to the message. We - // want to avoid overflows in our computations, so we limit the number of - // brokerable attachments to 100. - if (num_attachments > 100) - return info; - - // Check whether the range includes the attachments. - size_t attachment_length = num_attachments * BrokerableAttachment::kNonceSize; - size_t buffer_length = static_cast<size_t>(range_end - range_start); - if (buffer_length < attachment_length + pickle_len) - return info; - - for (int i = 0; i < num_attachments; ++i) { - const char* attachment_start = - pickle_end + i * (BrokerableAttachment::kNonceSize); - BrokerableAttachment::AttachmentId id(attachment_start, - BrokerableAttachment::kNonceSize); - info.attachment_ids.push_back(id); - } - info.message_end = - pickle_end + num_attachments * BrokerableAttachment::kNonceSize; -#else - info.message_end = pickle_end; -#endif // USE_ATTACHMENT_BROKER - - info.message_found = true; - return info; -} - -Message::SerializedAttachmentIds -Message::SerializedIdsOfBrokerableAttachments() { - DCHECK(HasBrokerableAttachments()); - std::vector<const BrokerableAttachment*> attachments = - attachment_set_->PeekBrokerableAttachments(); - size_t size = attachments.size() * BrokerableAttachment::kNonceSize; - char* buffer = static_cast<char*>(malloc(size)); - for (size_t i = 0; i < attachments.size(); ++i) { - const BrokerableAttachment* attachment = attachments[i]; - char* start_range = buffer + i * BrokerableAttachment::kNonceSize; - BrokerableAttachment::AttachmentId id = attachment->GetIdentifier(); - id.SerializeToBuffer(start_range, BrokerableAttachment::kNonceSize); - } - SerializedAttachmentIds ids; - ids.buffer = buffer; - ids.size = size; - return ids; -} - -bool Message::AddPlaceholderBrokerableAttachmentWithId( - BrokerableAttachment::AttachmentId id) { - scoped_refptr<PlaceholderBrokerableAttachment> attachment( - new PlaceholderBrokerableAttachment(id)); - return attachment_set()->AddAttachment(attachment); -} - 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 e49ce9f..d12f6a3 100644 --- a/ipc/ipc_message.h +++ b/ipc/ipc_message.h @@ -11,8 +11,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" #if !defined(NDEBUG) @@ -166,46 +164,11 @@ class IPC_EXPORT Message : public base::Pickle { static void Log(std::string* name, const Message* msg, std::string* l) { } - // The static method FindNext() returns several pieces of information, which - // are aggregated into an instance of this struct. - struct NextMessageInfo { - NextMessageInfo(); - ~NextMessageInfo(); - // Whether an entire message was found in the given memory range. If this is - // false, the other fields are left uninitialized. - bool message_found; - // The start address is passed into FindNext() by the caller, so isn't - // repeated in this struct. The end address of the pickle should be used to - // construct a base::Pickle. - const char* pickle_end; - // The end address of the message should be used to determine the start - // address of the next message. - const char* message_end; - // If the message has brokerable attachments, this vector will contain the - // ids of the brokerable attachments. The caller of FindNext() is - // responsible for adding the attachments to the message. - std::vector<BrokerableAttachment::AttachmentId> attachment_ids; - }; - - static NextMessageInfo FindNext(const char* range_start, - const char* range_end); - - struct SerializedAttachmentIds { - void* buffer; - size_t size; - }; - // Creates a buffer that contains a serialization of the ids of the brokerable - // attachments of the message. This buffer is intended to be sent over the IPC - // channel immediately after the pickled message. The caller takes ownership - // of the buffer. - // This method should only be called if the message has brokerable - // attachments. - SerializedAttachmentIds SerializedIdsOfBrokerableAttachments(); - - // Adds a placeholder brokerable attachment that must be replaced before the - // message can be dispatched. - bool AddPlaceholderBrokerableAttachmentWithId( - BrokerableAttachment::AttachmentId id); + // Find the end of the message data that starts at range_start. Returns NULL + // if the entire message is not found in the given data range. + static const char* FindNext(const char* range_start, const char* range_end) { + return base::Pickle::FindNext(sizeof(Header), range_start, range_end); + } // WriteAttachment appends |attachment| to the end of the set. It returns // false iff the set is full. @@ -259,12 +222,6 @@ class IPC_EXPORT Message : public base::Pickle { int32 routing; // ID of the view that this message is destined for uint32 type; // specifies the user-defined message type uint32 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 num_brokered_attachments; -#endif #if defined(OS_POSIX) uint16 num_fds; // the number of descriptors included with this message uint16 pad; // explicitly initialize this to appease valgrind diff --git a/ipc/ipc_message_attachment_set.cc b/ipc/ipc_message_attachment_set.cc index 6a8024f..1a4969f 100644 --- a/ipc/ipc_message_attachment_set.cc +++ b/ipc/ipc_message_attachment_set.cc @@ -143,25 +143,17 @@ MessageAttachmentSet::PeekBrokerableAttachments() const { return output; } -void MessageAttachmentSet::ReplacePlaceholderWithAttachment( - const scoped_refptr<BrokerableAttachment>& attachment) { - for (auto it = attachments_.begin(); it != attachments_.end(); ++it) { - if ((*it)->GetType() != MessageAttachment::TYPE_BROKERABLE_ATTACHMENT) - continue; - BrokerableAttachment* brokerable_attachment = - static_cast<BrokerableAttachment*>(it->get()); - - if (brokerable_attachment->GetBrokerableType() == - BrokerableAttachment::PLACEHOLDER && - brokerable_attachment->GetIdentifier() == attachment->GetIdentifier()) { - *it = attachment; - return; +std::vector<scoped_refptr<BrokerableAttachment>> +MessageAttachmentSet::GetBrokerableAttachmentsForUpdating() { + std::vector<scoped_refptr<BrokerableAttachment>> output; + for (const scoped_refptr<MessageAttachment>& attachment : attachments_) { + if (attachment->GetType() == + MessageAttachment::TYPE_BROKERABLE_ATTACHMENT) { + output.push_back(scoped_refptr<BrokerableAttachment>( + static_cast<BrokerableAttachment*>(attachment.get()))); } } - - // This function should only be called if there is a placeholder ready to be - // replaced. - NOTREACHED(); + return output; } #if defined(OS_POSIX) diff --git a/ipc/ipc_message_attachment_set.h b/ipc/ipc_message_attachment_set.h index 893f58c..5e4b195 100644 --- a/ipc/ipc_message_attachment_set.h +++ b/ipc/ipc_message_attachment_set.h @@ -60,11 +60,8 @@ class IPC_EXPORT MessageAttachmentSet // Returns a vector of all brokerable attachments. std::vector<const BrokerableAttachment*> PeekBrokerableAttachments() const; - - // Replaces a placeholder brokerable attachment with |attachment|, matching - // them by their id. - void ReplacePlaceholderWithAttachment( - const scoped_refptr<BrokerableAttachment>& attachment); + std::vector<scoped_refptr<BrokerableAttachment>> + GetBrokerableAttachmentsForUpdating(); #if defined(OS_POSIX) // This is the maximum number of descriptors per message. We need to know this diff --git a/ipc/ipc_message_start.h b/ipc/ipc_message_start.h index d26f57c..f5538a9 100644 --- a/ipc/ipc_message_start.h +++ b/ipc/ipc_message_start.h @@ -124,7 +124,6 @@ enum IPCMessageStart { ChromeAppBannerMsgStart, SafeJsonParserMsgStart, AttachmentBrokerMsgStart, - IPCTestMsgStart, LastIPCMsgStart // Must come last. }; diff --git a/ipc/ipc_message_utils.cc b/ipc/ipc_message_utils.cc index 3670497..dbb2572 100644 --- a/ipc/ipc_message_utils.cc +++ b/ipc/ipc_message_utils.cc @@ -452,7 +452,7 @@ void ParamTraits<std::vector<bool> >::Log(const param_type& p, std::string* l) { void ParamTraits<BrokerableAttachment::AttachmentId>::Write( Message* m, const param_type& p) { - m->WriteBytes(p.nonce, static_cast<int>(BrokerableAttachment::kNonceSize)); + m->WriteBytes(p.nonce, BrokerableAttachment::kNonceSize); } bool ParamTraits<BrokerableAttachment::AttachmentId>::Read( @@ -460,10 +460,8 @@ bool ParamTraits<BrokerableAttachment::AttachmentId>::Read( base::PickleIterator* iter, param_type* r) { const char* data; - if (!iter->ReadBytes(&data, - static_cast<int>(BrokerableAttachment::kNonceSize))) { + if (!iter->ReadBytes(&data, BrokerableAttachment::kNonceSize)) return false; - } memcpy(r->nonce, data, BrokerableAttachment::kNonceSize); return true; } diff --git a/ipc/ipc_test_message_generator.cc b/ipc/ipc_test_message_generator.cc deleted file mode 100644 index c0e9c03..0000000 --- a/ipc/ipc_test_message_generator.cc +++ /dev/null @@ -1,33 +0,0 @@ -// 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. - -// Get basic type definitions. -#define IPC_MESSAGE_IMPL -#include "ipc/ipc_test_message_generator.h" - -// Generate constructors. -#include "ipc/struct_constructor_macros.h" -#include "ipc/ipc_test_message_generator.h" - -// Generate destructors. -#include "ipc/struct_destructor_macros.h" -#include "ipc/ipc_test_message_generator.h" - -// Generate param traits write methods. -#include "ipc/param_traits_write_macros.h" -namespace IPC { -#include "ipc/ipc_test_message_generator.h" -} // namespace IPC - -// Generate param traits read methods. -#include "ipc/param_traits_read_macros.h" -namespace IPC { -#include "ipc/ipc_test_message_generator.h" -} // namespace IPC - -// Generate param traits log methods. -#include "ipc/param_traits_log_macros.h" -namespace IPC { -#include "ipc/ipc_test_message_generator.h" -} // namespace IPC diff --git a/ipc/ipc_test_message_generator.h b/ipc/ipc_test_message_generator.h deleted file mode 100644 index 169a85e..0000000 --- a/ipc/ipc_test_message_generator.h +++ /dev/null @@ -1,7 +0,0 @@ -// 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. - -// Multiply-included file, hence no include guard. - -#include "ipc/ipc_test_messages.h" diff --git a/ipc/ipc_test_messages.h b/ipc/ipc_test_messages.h deleted file mode 100644 index 4bf927e..0000000 --- a/ipc/ipc_test_messages.h +++ /dev/null @@ -1,14 +0,0 @@ -// 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. - -#include "build/build_config.h" - -#if defined(OS_WIN) -#include "ipc/handle_win.h" -#include "ipc/ipc_message_macros.h" -#define IPC_MESSAGE_START IPCTestMsgStart - -IPC_MESSAGE_CONTROL3(TestHandleWinMsg, int, IPC::HandleWin, int) -IPC_MESSAGE_CONTROL2(TestTwoHandleWinMsg, IPC::HandleWin, IPC::HandleWin) -#endif // defined(OS_WIN) diff --git a/ipc/placeholder_brokerable_attachment.cc b/ipc/placeholder_brokerable_attachment.cc deleted file mode 100644 index e64e398..0000000 --- a/ipc/placeholder_brokerable_attachment.cc +++ /dev/null @@ -1,14 +0,0 @@ -// 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. - -#include "ipc/placeholder_brokerable_attachment.h" - -namespace IPC { - -BrokerableAttachment::BrokerableType -PlaceholderBrokerableAttachment::GetBrokerableType() const { - return PLACEHOLDER; -} - -} // namespace IPC diff --git a/ipc/placeholder_brokerable_attachment.h b/ipc/placeholder_brokerable_attachment.h deleted file mode 100644 index 7a63c72..0000000 --- a/ipc/placeholder_brokerable_attachment.h +++ /dev/null @@ -1,28 +0,0 @@ -// 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 IPC_PLACEHOLDER_BROKERABLE_ATTACHMENT_H_ -#define IPC_PLACEHOLDER_BROKERABLE_ATTACHMENT_H_ - -#include "ipc/brokerable_attachment.h" -#include "ipc/ipc_export.h" - -namespace IPC { - -// This subclass of BrokerableAttachment has an AttachmentId, and nothing else. -// It is intended to be replaced by the attachment broker. -class IPC_EXPORT PlaceholderBrokerableAttachment : public BrokerableAttachment { - public: - PlaceholderBrokerableAttachment(const AttachmentId& id) - : BrokerableAttachment(id){}; - BrokerableType GetBrokerableType() const override; - - protected: - ~PlaceholderBrokerableAttachment() override{}; - DISALLOW_COPY_AND_ASSIGN(PlaceholderBrokerableAttachment); -}; - -} // namespace IPC - -#endif // IPC_PLACEHOLDER_BROKERABLE_ATTACHMENT_H_ |