diff options
| author | Allan Sandfeld Jensen <allan.jensen@theqtcompany.com> | 2015-06-18 14:10:49 +0200 |
|---|---|---|
| committer | Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com> | 2015-06-18 13:53:24 +0000 |
| commit | 813fbf95af77a531c57a8c497345ad2c61d475b3 (patch) | |
| tree | 821b2c8de8365f21b6c9ba17a236fb3006a1d506 /chromium/ipc | |
| parent | af6588f8d723931a298c995fa97259bb7f7deb55 (diff) | |
| download | qtwebengine-chromium-813fbf95af77a531c57a8c497345ad2c61d475b3.tar.gz | |
BASELINE: Update chromium to 44.0.2403.47
Change-Id: Ie056fedba95cf5e5c76b30c4b2c80fca4764aa2f
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>
Diffstat (limited to 'chromium/ipc')
80 files changed, 3410 insertions, 1610 deletions
diff --git a/chromium/ipc/BUILD.gn b/chromium/ipc/BUILD.gn index e7e08b5aadd..b756921e7dd 100644 --- a/chromium/ipc/BUILD.gn +++ b/chromium/ipc/BUILD.gn @@ -2,15 +2,15 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import("//testing/test.gni") + component("ipc") { sources = [ - "file_descriptor_set_posix.cc", - "file_descriptor_set_posix.h", "ipc_channel.cc", "ipc_channel.h", + "ipc_channel_common.cc", "ipc_channel_factory.cc", "ipc_channel_factory.h", - "ipc_channel_common.cc", "ipc_channel_handle.h", "ipc_channel_nacl.cc", "ipc_channel_nacl.h", @@ -31,6 +31,10 @@ component("ipc") { "ipc_logging.h", "ipc_message.cc", "ipc_message.h", + "ipc_message_attachment.cc", + "ipc_message_attachment.h", + "ipc_message_attachment_set.cc", + "ipc_message_attachment_set.h", "ipc_message_macros.h", "ipc_message_start.h", "ipc_message_utils.cc", @@ -38,6 +42,8 @@ component("ipc") { "ipc_param_traits.h", "ipc_platform_file.cc", "ipc_platform_file.h", + "ipc_platform_file_attachment_posix.cc", + "ipc_platform_file_attachment_posix.h", "ipc_sender.h", "ipc_switches.cc", "ipc_switches.h", @@ -61,7 +67,13 @@ component("ipc") { "unix_domain_socket_util.h", ] - if (!is_nacl) { + if (is_nacl) { + sources -= [ + "ipc_channel.cc", + "ipc_channel_posix.cc", + "unix_domain_socket_util.cc", + ] + } else { sources -= [ "ipc_channel_nacl.cc", "ipc_channel_nacl.h", @@ -69,28 +81,27 @@ component("ipc") { } if (is_win || is_ios) { - sources -= [ - "unix_domain_socket_util.cc", - ] + sources -= [ "unix_domain_socket_util.cc" ] } defines = [ "IPC_IMPLEMENTATION" ] deps = [ "//base", + # TODO(viettrungluu): Needed for base/lazy_instance.h, which is suspect. "//base/third_party/dynamic_annotations", ] } -# TODO(dpranke): crbug.com/360936. Get this to build and run on Android. +# TODO(GYP): crbug.com/360936. Get this to build and run on Android. if (!is_android) { test("ipc_tests") { sources = [ - "file_descriptor_set_posix_unittest.cc", "ipc_channel_posix_unittest.cc", "ipc_channel_unittest.cc", "ipc_fuzzing_tests.cc", + "ipc_message_attachment_set_posix_unittest.cc", "ipc_message_unittest.cc", "ipc_message_utils_unittest.cc", "ipc_send_fds_test.cc", @@ -144,7 +155,6 @@ if (!is_android) { # deps += "//base/allocator" # } #} - deps = [ ":ipc", ":test_support", @@ -164,17 +174,19 @@ static_library("test_support") { "ipc_multiprocess_test.h", "ipc_perftest_support.cc", "ipc_perftest_support.h", - "ipc_test_sink.cc", - "ipc_test_sink.h", + "ipc_security_test_util.cc", + "ipc_security_test_util.h", "ipc_test_base.cc", "ipc_test_base.h", - "ipc_test_channel_listener.h", "ipc_test_channel_listener.cc", + "ipc_test_channel_listener.h", + "ipc_test_sink.cc", + "ipc_test_sink.h", ] deps = [ ":ipc", "//base", + "//base/test:test_support", "//testing/gtest", ] } - diff --git a/chromium/ipc/file_descriptor_set_posix.cc b/chromium/ipc/file_descriptor_set_posix.cc deleted file mode 100644 index 568fee3323c..00000000000 --- a/chromium/ipc/file_descriptor_set_posix.cc +++ /dev/null @@ -1,162 +0,0 @@ -// Copyright (c) 2011 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/file_descriptor_set_posix.h" - -#include <sys/types.h> -#include <sys/stat.h> -#include <unistd.h> - -#include "base/logging.h" -#include "base/posix/eintr_wrapper.h" - -FileDescriptorSet::FileDescriptorSet() - : consumed_descriptor_highwater_(0) { -} - -FileDescriptorSet::~FileDescriptorSet() { - if (consumed_descriptor_highwater_ == size()) - return; - - // We close all the owning descriptors. If this message should have - // been transmitted, then closing those with close flags set mirrors - // the expected behaviour. - // - // If this message was received with more descriptors than expected - // (which could a DOS against the browser by a rogue renderer) then all - // the descriptors have their close flag set and we free all the extra - // kernel resources. - LOG(WARNING) << "FileDescriptorSet destroyed with unconsumed descriptors: " - << consumed_descriptor_highwater_ << "/" << size(); -} - -bool FileDescriptorSet::AddToBorrow(base::PlatformFile fd) { - DCHECK_EQ(consumed_descriptor_highwater_, 0u); - - if (size() == kMaxDescriptorsPerMessage) { - DLOG(WARNING) << "Cannot add file descriptor. FileDescriptorSet full."; - return false; - } - - descriptors_.push_back(fd); - return true; -} - -bool FileDescriptorSet::AddToOwn(base::ScopedFD fd) { - DCHECK_EQ(consumed_descriptor_highwater_, 0u); - - if (size() == kMaxDescriptorsPerMessage) { - DLOG(WARNING) << "Cannot add file descriptor. FileDescriptorSet full."; - return false; - } - - descriptors_.push_back(fd.get()); - owned_descriptors_.push_back(new base::ScopedFD(fd.Pass())); - DCHECK(size() <= kMaxDescriptorsPerMessage); - return true; -} - -base::PlatformFile FileDescriptorSet::TakeDescriptorAt(unsigned index) { - if (index >= size()) { - DLOG(WARNING) << "Accessing out of bound index:" - << index << "/" << size(); - return -1; - } - - - // We should always walk the descriptors in order, so it's reasonable to - // enforce this. Consider the case where a compromised renderer sends us - // the following message: - // - // ExampleMsg: - // num_fds:2 msg:FD(index = 1) control:SCM_RIGHTS {n, m} - // - // Here the renderer sent us a message which should have a descriptor, but - // actually sent two in an attempt to fill our fd table and kill us. By - // setting the index of the descriptor in the message to 1 (it should be - // 0), we would record a highwater of 1 and then consider all the - // descriptors to have been used. - // - // So we can either track of the use of each descriptor in a bitset, or we - // can enforce that we walk the indexes strictly in order. - // - // There's one more wrinkle: When logging messages, we may reparse them. So - // we have an exception: When the consumed_descriptor_highwater_ is at the - // end of the array and index 0 is requested, we reset the highwater value. - // TODO(morrita): This is absurd. This "wringle" disallow to introduce clearer - // ownership model. Only client is NaclIPCAdapter. See crbug.com/415294 - if (index == 0 && consumed_descriptor_highwater_ == descriptors_.size()) - consumed_descriptor_highwater_ = 0; - - if (index != consumed_descriptor_highwater_) - return -1; - - consumed_descriptor_highwater_ = index + 1; - - base::PlatformFile file = descriptors_[index]; - - // TODO(morrita): In production, descriptors_.size() should be same as - // owned_descriptors_.size() as all read descriptors are owned by Message. - // We have to do this because unit test breaks this assumption. It should be - // changed to exercise with own-able descriptors. - for (ScopedVector<base::ScopedFD>::const_iterator i = - owned_descriptors_.begin(); - i != owned_descriptors_.end(); - ++i) { - if ((*i)->get() == file) { - ignore_result((*i)->release()); - break; - } - } - - return file; -} - -void FileDescriptorSet::PeekDescriptors(base::PlatformFile* buffer) const { - std::copy(descriptors_.begin(), descriptors_.end(), buffer); -} - -bool FileDescriptorSet::ContainsDirectoryDescriptor() const { - struct stat st; - - for (std::vector<base::PlatformFile>::const_iterator i = descriptors_.begin(); - i != descriptors_.end(); - ++i) { - if (fstat(*i, &st) == 0 && S_ISDIR(st.st_mode)) - return true; - } - - return false; -} - -void FileDescriptorSet::CommitAll() { - descriptors_.clear(); - owned_descriptors_.clear(); - consumed_descriptor_highwater_ = 0; -} - -void FileDescriptorSet::ReleaseFDsToClose( - std::vector<base::PlatformFile>* fds) { - for (ScopedVector<base::ScopedFD>::iterator i = owned_descriptors_.begin(); - i != owned_descriptors_.end(); - ++i) { - fds->push_back((*i)->release()); - } - - CommitAll(); -} - -void FileDescriptorSet::AddDescriptorsToOwn(const base::PlatformFile* buffer, - unsigned count) { - DCHECK(count <= kMaxDescriptorsPerMessage); - DCHECK_EQ(size(), 0u); - DCHECK_EQ(consumed_descriptor_highwater_, 0u); - - descriptors_.reserve(count); - owned_descriptors_.reserve(count); - for (unsigned i = 0; i < count; ++i) { - descriptors_.push_back(buffer[i]); - owned_descriptors_.push_back(new base::ScopedFD(buffer[i])); - } -} diff --git a/chromium/ipc/file_descriptor_set_posix_unittest.cc b/chromium/ipc/file_descriptor_set_posix_unittest.cc deleted file mode 100644 index 34ef465edc0..00000000000 --- a/chromium/ipc/file_descriptor_set_posix_unittest.cc +++ /dev/null @@ -1,183 +0,0 @@ -// Copyright (c) 2011 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. - -// This test is POSIX only. - -#include "ipc/file_descriptor_set_posix.h" - -#include <unistd.h> -#include <fcntl.h> - -#include "base/basictypes.h" -#include "base/posix/eintr_wrapper.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace { - -// Get a safe file descriptor for test purposes. -int GetSafeFd() { - return open("/dev/null", O_RDONLY); -} - -// Returns true if fd was already closed. Closes fd if not closed. -bool VerifyClosed(int fd) { - const int duped = dup(fd); - if (duped != -1) { - EXPECT_NE(IGNORE_EINTR(close(duped)), -1); - EXPECT_NE(IGNORE_EINTR(close(fd)), -1); - return false; - } - return true; -} - -// The FileDescriptorSet will try and close some of the descriptor numbers -// which we given it. This is the base descriptor value. It's great enough such -// that no real descriptor will accidently be closed. -static const int kFDBase = 50000; - -TEST(FileDescriptorSet, BasicAdd) { - scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet); - - ASSERT_EQ(set->size(), 0u); - ASSERT_TRUE(set->empty()); - ASSERT_TRUE(set->AddToBorrow(kFDBase)); - ASSERT_EQ(set->size(), 1u); - ASSERT_TRUE(!set->empty()); - - // Empties the set and stops a warning about deleting a set with unconsumed - // descriptors - set->CommitAll(); -} - -TEST(FileDescriptorSet, BasicAddAndClose) { - scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet); - - ASSERT_EQ(set->size(), 0u); - ASSERT_TRUE(set->empty()); - const int fd = GetSafeFd(); - ASSERT_TRUE(set->AddToOwn(base::ScopedFD(fd))); - ASSERT_EQ(set->size(), 1u); - ASSERT_TRUE(!set->empty()); - - set->CommitAll(); - - ASSERT_TRUE(VerifyClosed(fd)); -} -TEST(FileDescriptorSet, MaxSize) { - scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet); - - for (size_t i = 0; i < FileDescriptorSet::kMaxDescriptorsPerMessage; ++i) - ASSERT_TRUE(set->AddToBorrow(kFDBase + 1 + i)); - - ASSERT_TRUE(!set->AddToBorrow(kFDBase)); - - set->CommitAll(); -} - -TEST(FileDescriptorSet, SetDescriptors) { - scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet); - - ASSERT_TRUE(set->empty()); - set->AddDescriptorsToOwn(NULL, 0); - ASSERT_TRUE(set->empty()); - - const int fd = GetSafeFd(); - static const int fds[] = {fd}; - set->AddDescriptorsToOwn(fds, 1); - ASSERT_TRUE(!set->empty()); - ASSERT_EQ(set->size(), 1u); - - set->CommitAll(); - - ASSERT_TRUE(VerifyClosed(fd)); -} - -TEST(FileDescriptorSet, PeekDescriptors) { - scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet); - - set->PeekDescriptors(NULL); - ASSERT_TRUE(set->AddToBorrow(kFDBase)); - - int fds[1]; - fds[0] = 0; - set->PeekDescriptors(fds); - ASSERT_EQ(fds[0], kFDBase); - set->CommitAll(); - ASSERT_TRUE(set->empty()); -} - -TEST(FileDescriptorSet, WalkInOrder) { - scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet); - - // TODO(morrita): This test is wrong. TakeDescriptorAt() shouldn't be - // used to retrieve borrowed descriptors. That never happens in production. - ASSERT_TRUE(set->AddToBorrow(kFDBase)); - ASSERT_TRUE(set->AddToBorrow(kFDBase + 1)); - ASSERT_TRUE(set->AddToBorrow(kFDBase + 2)); - - ASSERT_EQ(set->TakeDescriptorAt(0), kFDBase); - ASSERT_EQ(set->TakeDescriptorAt(1), kFDBase + 1); - ASSERT_EQ(set->TakeDescriptorAt(2), kFDBase + 2); - - set->CommitAll(); -} - -TEST(FileDescriptorSet, WalkWrongOrder) { - scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet); - - // TODO(morrita): This test is wrong. TakeDescriptorAt() shouldn't be - // used to retrieve borrowed descriptors. That never happens in production. - ASSERT_TRUE(set->AddToBorrow(kFDBase)); - ASSERT_TRUE(set->AddToBorrow(kFDBase + 1)); - ASSERT_TRUE(set->AddToBorrow(kFDBase + 2)); - - ASSERT_EQ(set->TakeDescriptorAt(0), kFDBase); - ASSERT_EQ(set->TakeDescriptorAt(2), -1); - - set->CommitAll(); -} - -TEST(FileDescriptorSet, WalkCycle) { - scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet); - - // TODO(morrita): This test is wrong. TakeDescriptorAt() shouldn't be - // used to retrieve borrowed descriptors. That never happens in production. - ASSERT_TRUE(set->AddToBorrow(kFDBase)); - ASSERT_TRUE(set->AddToBorrow(kFDBase + 1)); - ASSERT_TRUE(set->AddToBorrow(kFDBase + 2)); - - ASSERT_EQ(set->TakeDescriptorAt(0), kFDBase); - ASSERT_EQ(set->TakeDescriptorAt(1), kFDBase + 1); - ASSERT_EQ(set->TakeDescriptorAt(2), kFDBase + 2); - ASSERT_EQ(set->TakeDescriptorAt(0), kFDBase); - ASSERT_EQ(set->TakeDescriptorAt(1), kFDBase + 1); - ASSERT_EQ(set->TakeDescriptorAt(2), kFDBase + 2); - ASSERT_EQ(set->TakeDescriptorAt(0), kFDBase); - ASSERT_EQ(set->TakeDescriptorAt(1), kFDBase + 1); - ASSERT_EQ(set->TakeDescriptorAt(2), kFDBase + 2); - - set->CommitAll(); -} - -TEST(FileDescriptorSet, DontClose) { - scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet); - - const int fd = GetSafeFd(); - ASSERT_TRUE(set->AddToBorrow(fd)); - set->CommitAll(); - - ASSERT_FALSE(VerifyClosed(fd)); -} - -TEST(FileDescriptorSet, DoClose) { - scoped_refptr<FileDescriptorSet> set(new FileDescriptorSet); - - const int fd = GetSafeFd(); - ASSERT_TRUE(set->AddToOwn(base::ScopedFD(fd))); - set->CommitAll(); - - ASSERT_TRUE(VerifyClosed(fd)); -} - -} // namespace diff --git a/chromium/ipc/ipc.gyp b/chromium/ipc/ipc.gyp index 28c112b0ba6..1a28fc26c9a 100644 --- a/chromium/ipc/ipc.gyp +++ b/chromium/ipc/ipc.gyp @@ -44,11 +44,11 @@ '..' ], 'sources': [ - 'file_descriptor_set_posix_unittest.cc', 'ipc_channel_posix_unittest.cc', 'ipc_channel_proxy_unittest.cc', 'ipc_channel_unittest.cc', 'ipc_fuzzing_tests.cc', + 'ipc_message_attachment_set_posix_unittest.cc', 'ipc_message_unittest.cc', 'ipc_message_utils_unittest.cc', 'ipc_send_fds_test.cc', @@ -132,6 +132,8 @@ 'ipc_multiprocess_test.h', 'ipc_perftest_support.cc', 'ipc_perftest_support.h', + 'ipc_security_test_util.cc', + 'ipc_security_test_util.h', 'ipc_test_base.cc', 'ipc_test_base.h', 'ipc_test_channel_listener.cc', diff --git a/chromium/ipc/ipc.gypi b/chromium/ipc/ipc.gypi index 5f6875701ad..a10790ed103 100644 --- a/chromium/ipc/ipc.gypi +++ b/chromium/ipc/ipc.gypi @@ -11,8 +11,6 @@ # This part is shared between the targets defined below. ['ipc_target==1', { 'sources': [ - 'file_descriptor_set_posix.cc', - 'file_descriptor_set_posix.h', 'ipc_channel.cc', 'ipc_channel.h', 'ipc_channel_factory.cc', @@ -38,6 +36,10 @@ 'ipc_logging.h', 'ipc_message.cc', 'ipc_message.h', + 'ipc_message_attachment.cc', + 'ipc_message_attachment.h', + 'ipc_message_attachment_set.cc', + 'ipc_message_attachment_set.h', 'ipc_message_macros.h', 'ipc_message_start.h', 'ipc_message_utils.cc', @@ -45,6 +47,8 @@ 'ipc_param_traits.h', 'ipc_platform_file.cc', 'ipc_platform_file.h', + 'ipc_platform_file_attachment_posix.cc', + 'ipc_platform_file_attachment_posix.h', 'ipc_sender.h', 'ipc_switches.cc', 'ipc_switches.h', diff --git a/chromium/ipc/ipc_channel.cc b/chromium/ipc/ipc_channel.cc index 4a4e40ddc1f..ac09c5ab1ef 100644 --- a/chromium/ipc/ipc_channel.cc +++ b/chromium/ipc/ipc_channel.cc @@ -28,8 +28,13 @@ std::string Channel::GenerateUniqueRandomChannelID() { // the creator, an identifier for the child instance, and a strong random // component. The strong random component prevents other processes from // hijacking or squatting on predictable channel names. - +#if defined(OS_NACL_NONSFI) + // The seccomp sandbox disallows use of getpid(), so we provide a + // dummy PID. + int process_id = -1; +#else int process_id = base::GetCurrentProcId(); +#endif return base::StringPrintf("%d.%u.%d", process_id, g_last_id.GetNext(), diff --git a/chromium/ipc/ipc_channel.h b/chromium/ipc/ipc_channel.h index 2fc52cb5875..65c678b530d 100644 --- a/chromium/ipc/ipc_channel.h +++ b/chromium/ipc/ipc_channel.h @@ -178,7 +178,11 @@ class IPC_EXPORT Channel : public Sender { // // |message| must be allocated using operator new. This object will be // deleted once the contents of the Message have been sent. - virtual bool Send(Message* message) override = 0; + bool Send(Message* message) override = 0; + + // IsSendThreadSafe returns true iff it's safe to call |Send| from non-IO + // 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 diff --git a/chromium/ipc/ipc_channel_common.cc b/chromium/ipc/ipc_channel_common.cc index d7347cc7a1b..23b85e2834d 100644 --- a/chromium/ipc/ipc_channel_common.cc +++ b/chromium/ipc/ipc_channel_common.cc @@ -43,5 +43,9 @@ scoped_ptr<Channel> Channel::CreateServer( Channel::~Channel() { } +bool Channel::IsSendThreadSafe() const { + return false; +} + } // namespace IPC diff --git a/chromium/ipc/ipc_channel_nacl.cc b/chromium/ipc/ipc_channel_nacl.cc index 783ee262060..f5b33cea41a 100644 --- a/chromium/ipc/ipc_channel_nacl.cc +++ b/chromium/ipc/ipc_channel_nacl.cc @@ -12,13 +12,14 @@ #include "base/bind.h" #include "base/logging.h" -#include "base/message_loop/message_loop_proxy.h" +#include "base/single_thread_task_runner.h" #include "base/synchronization/lock.h" #include "base/task_runner_util.h" +#include "base/thread_task_runner_handle.h" #include "base/threading/simple_thread.h" -#include "ipc/file_descriptor_set_posix.h" #include "ipc/ipc_listener.h" #include "ipc/ipc_logging.h" +#include "ipc/ipc_message_attachment_set.h" #include "native_client/src/public/imc_syscalls.h" #include "native_client/src/public/imc_types.h" @@ -37,7 +38,7 @@ bool ReadDataOnReaderThread(int pipe, MessageContents* contents) { return false; contents->data.resize(Channel::kReadBufferSize); - contents->fds.resize(FileDescriptorSet::kMaxDescriptorsPerMessage); + contents->fds.resize(NACL_ABI_IMC_DESC_MAX); NaClAbiNaClImcMsgIoVec iov = { &contents->data[0], contents->data.size() }; NaClAbiNaClImcMsgHdr msg = { @@ -74,32 +75,32 @@ class ChannelNacl::ReaderThreadRunner // above callbacks. ReaderThreadRunner( int pipe, - base::Callback<void (scoped_ptr<MessageContents>)> data_read_callback, - base::Callback<void ()> failure_callback, - scoped_refptr<base::MessageLoopProxy> main_message_loop); + base::Callback<void(scoped_ptr<MessageContents>)> data_read_callback, + base::Callback<void()> failure_callback, + scoped_refptr<base::SingleThreadTaskRunner> main_task_runner); // DelegateSimpleThread implementation. Reads data from the pipe in a loop // until either we are told to quit or a read fails. - virtual void Run() override; + void Run() override; private: int pipe_; base::Callback<void (scoped_ptr<MessageContents>)> data_read_callback_; base::Callback<void ()> failure_callback_; - scoped_refptr<base::MessageLoopProxy> main_message_loop_; + scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_; DISALLOW_COPY_AND_ASSIGN(ReaderThreadRunner); }; ChannelNacl::ReaderThreadRunner::ReaderThreadRunner( int pipe, - base::Callback<void (scoped_ptr<MessageContents>)> data_read_callback, - base::Callback<void ()> failure_callback, - scoped_refptr<base::MessageLoopProxy> main_message_loop) + base::Callback<void(scoped_ptr<MessageContents>)> data_read_callback, + base::Callback<void()> failure_callback, + scoped_refptr<base::SingleThreadTaskRunner> main_task_runner) : pipe_(pipe), data_read_callback_(data_read_callback), failure_callback_(failure_callback), - main_message_loop_(main_message_loop) { + main_task_runner_(main_task_runner) { } void ChannelNacl::ReaderThreadRunner::Run() { @@ -107,10 +108,11 @@ void ChannelNacl::ReaderThreadRunner::Run() { scoped_ptr<MessageContents> msg_contents(new MessageContents); bool success = ReadDataOnReaderThread(pipe_, msg_contents.get()); if (success) { - main_message_loop_->PostTask(FROM_HERE, + main_task_runner_->PostTask( + FROM_HERE, base::Bind(data_read_callback_, base::Passed(&msg_contents))); } else { - main_message_loop_->PostTask(FROM_HERE, failure_callback_); + main_task_runner_->PostTask(FROM_HERE, failure_callback_); // Because the read failed, we know we're going to quit. Don't bother // trying to read again. return; @@ -159,15 +161,13 @@ bool ChannelNacl::Connect() { // where Channel::Send will be called, and the same thread that should receive // messages). The constructor might be invoked on another thread (see // ChannelProxy for an example of that). Therefore, we must wait until Connect - // is called to decide which MessageLoopProxy to pass to ReaderThreadRunner. - reader_thread_runner_.reset( - new ReaderThreadRunner( - pipe_, - base::Bind(&ChannelNacl::DidRecvMsg, - weak_ptr_factory_.GetWeakPtr()), - base::Bind(&ChannelNacl::ReadDidFail, - weak_ptr_factory_.GetWeakPtr()), - base::MessageLoopProxy::current())); + // is called to decide which SingleThreadTaskRunner to pass to + // ReaderThreadRunner. + reader_thread_runner_.reset(new ReaderThreadRunner( + pipe_, + base::Bind(&ChannelNacl::DidRecvMsg, weak_ptr_factory_.GetWeakPtr()), + base::Bind(&ChannelNacl::ReadDidFail, weak_ptr_factory_.GetWeakPtr()), + base::ThreadTaskRunnerHandle::Get())); reader_thread_.reset( new base::DelegateSimpleThread(reader_thread_runner_.get(), "ipc_channel_nacl reader thread")); @@ -175,9 +175,9 @@ bool ChannelNacl::Connect() { waiting_connect_ = false; // If there were any messages queued before connection, send them. ProcessOutgoingMessages(); - base::MessageLoopProxy::current()->PostTask(FROM_HERE, - base::Bind(&ChannelNacl::CallOnChannelConnected, - weak_ptr_factory_.GetWeakPtr())); + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::Bind(&ChannelNacl::CallOnChannelConnected, + weak_ptr_factory_.GetWeakPtr())); return true; } @@ -200,6 +200,7 @@ void ChannelNacl::Close() { } bool ChannelNacl::Send(Message* message) { + DCHECK(!message->HasAttachments()); DVLOG(2) << "sending message @" << message << " on channel @" << this << " with type " << message->type(); scoped_ptr<Message> message_ptr(message); @@ -275,10 +276,10 @@ bool ChannelNacl::ProcessOutgoingMessages() { linked_ptr<Message> msg = output_queue_.front(); output_queue_.pop_front(); - int fds[FileDescriptorSet::kMaxDescriptorsPerMessage]; - const size_t num_fds = msg->file_descriptor_set()->size(); - DCHECK(num_fds <= FileDescriptorSet::kMaxDescriptorsPerMessage); - msg->file_descriptor_set()->PeekDescriptors(fds); + int fds[MessageAttachmentSet::kMaxDescriptorsPerMessage]; + const size_t num_fds = msg->attachment_set()->size(); + DCHECK(num_fds <= MessageAttachmentSet::kMaxDescriptorsPerMessage); + msg->attachment_set()->PeekDescriptors(fds); NaClAbiNaClImcMsgIoVec iov = { const_cast<void*>(msg->data()), msg->size() @@ -298,7 +299,7 @@ bool ChannelNacl::ProcessOutgoingMessages() { << msg->size(); return false; } else { - msg->file_descriptor_set()->CommitAll(); + msg->attachment_set()->CommitAll(); } // Message sent OK! @@ -352,8 +353,7 @@ bool ChannelNacl::WillDispatchInputMessage(Message* msg) { // The shenaniganery below with &foo.front() requires input_fds_ to have // contiguous underlying storage (such as a simple array or a std::vector). // This is why the header warns not to make input_fds_ a deque<>. - msg->file_descriptor_set()->AddDescriptorsToOwn(&input_fds_.front(), - header_fds); + msg->attachment_set()->AddDescriptorsToOwn(&input_fds_.front(), header_fds); input_fds_.clear(); return true; } diff --git a/chromium/ipc/ipc_channel_nacl.h b/chromium/ipc/ipc_channel_nacl.h index 9c1e80fb10f..f0649b26022 100644 --- a/chromium/ipc/ipc_channel_nacl.h +++ b/chromium/ipc/ipc_channel_nacl.h @@ -38,14 +38,14 @@ class ChannelNacl : public Channel, ChannelNacl(const IPC::ChannelHandle& channel_handle, Mode mode, Listener* listener); - virtual ~ChannelNacl(); + ~ChannelNacl() override; // Channel implementation. - virtual base::ProcessId GetPeerPID() const override; - virtual base::ProcessId GetSelfPID() const override; - virtual bool Connect() override; - virtual void Close() override; - virtual bool Send(Message* message) override; + base::ProcessId GetPeerPID() const override; + base::ProcessId GetSelfPID() const override; + bool Connect() override; + void Close() override; + bool Send(Message* message) override; // Posted to the main thread by ReaderThreadRunner. void DidRecvMsg(scoped_ptr<MessageContents> contents); @@ -59,12 +59,12 @@ class ChannelNacl : public Channel, void CallOnChannelConnected(); // ChannelReader implementation. - virtual ReadState ReadData(char* buffer, - int buffer_len, - int* bytes_read) override; - virtual bool WillDispatchInputMessage(Message* msg) override; - virtual bool DidEmptyInputBuffers() override; - virtual void HandleInternalMessage(const Message& msg) override; + ReadState ReadData(char* buffer, + int buffer_len, + int* bytes_read) override; + bool WillDispatchInputMessage(Message* msg) override; + bool DidEmptyInputBuffers() override; + void HandleInternalMessage(const Message& msg) override; Mode mode_; bool waiting_connect_; diff --git a/chromium/ipc/ipc_channel_posix.cc b/chromium/ipc/ipc_channel_posix.cc index 08529932250..aac7e795257 100644 --- a/chromium/ipc/ipc_channel_posix.cc +++ b/chromium/ipc/ipc_channel_posix.cc @@ -37,11 +37,12 @@ #include "base/stl_util.h" #include "base/strings/string_util.h" #include "base/synchronization/lock.h" -#include "ipc/file_descriptor_set_posix.h" #include "ipc/ipc_descriptors.h" #include "ipc/ipc_listener.h" #include "ipc/ipc_logging.h" +#include "ipc/ipc_message_attachment_set.h" #include "ipc/ipc_message_utils.h" +#include "ipc/ipc_platform_file_attachment_posix.h" #include "ipc/ipc_switches.h" #include "ipc/unix_domain_socket_util.h" @@ -189,6 +190,7 @@ ChannelPosix::ChannelPosix(const IPC::ChannelHandle& channel_handle, waiting_connect_(true), message_send_bytes_written_(0), pipe_name_(channel_handle.name), + in_dtor_(false), must_unlink_(false) { memset(input_cmsg_buf_, 0, sizeof(input_cmsg_buf_)); if (!CreatePipe(channel_handle)) { @@ -200,6 +202,7 @@ ChannelPosix::ChannelPosix(const IPC::ChannelHandle& channel_handle, } ChannelPosix::~ChannelPosix() { + in_dtor_ = true; Close(); } @@ -395,13 +398,13 @@ void ChannelPosix::CloseFileDescriptors(Message* msg) { // descriptor. For more information, see: // http://crbug.com/298276 std::vector<int> to_close; - msg->file_descriptor_set()->ReleaseFDsToClose(&to_close); + msg->attachment_set()->ReleaseFDsToClose(&to_close); for (size_t i = 0; i < to_close.size(); i++) { fds_to_close_.insert(to_close[i]); QueueCloseFDMessage(to_close[i], 2); } #else - msg->file_descriptor_set()->CommitAll(); + msg->attachment_set()->CommitAll(); #endif } @@ -428,20 +431,19 @@ bool ChannelPosix::ProcessOutgoingMessages() { struct iovec iov = {const_cast<char*>(out_bytes), amt_to_write}; msgh.msg_iov = &iov; msgh.msg_iovlen = 1; - char buf[CMSG_SPACE( - sizeof(int) * FileDescriptorSet::kMaxDescriptorsPerMessage)]; + char buf[CMSG_SPACE(sizeof(int) * + MessageAttachmentSet::kMaxDescriptorsPerMessage)]; ssize_t bytes_written = 1; int fd_written = -1; - if (message_send_bytes_written_ == 0 && - !msg->file_descriptor_set()->empty()) { + if (message_send_bytes_written_ == 0 && !msg->attachment_set()->empty()) { // This is the first chunk of a message which has descriptors to send struct cmsghdr *cmsg; - const unsigned num_fds = msg->file_descriptor_set()->size(); + const unsigned num_fds = msg->attachment_set()->size(); - DCHECK(num_fds <= FileDescriptorSet::kMaxDescriptorsPerMessage); - if (msg->file_descriptor_set()->ContainsDirectoryDescriptor()) { + DCHECK(num_fds <= MessageAttachmentSet::kMaxDescriptorsPerMessage); + if (msg->attachment_set()->ContainsDirectoryDescriptor()) { LOG(FATAL) << "Panic: attempting to transport directory descriptor over" " IPC. Aborting to maintain sandbox isolation."; // If you have hit this then something tried to send a file descriptor @@ -457,7 +459,7 @@ bool ChannelPosix::ProcessOutgoingMessages() { cmsg->cmsg_level = SOL_SOCKET; cmsg->cmsg_type = SCM_RIGHTS; cmsg->cmsg_len = CMSG_LEN(sizeof(int) * num_fds); - msg->file_descriptor_set()->PeekDescriptors( + msg->attachment_set()->PeekDescriptors( reinterpret_cast<int*>(CMSG_DATA(cmsg))); msgh.msg_controllen = cmsg->cmsg_len; @@ -488,7 +490,7 @@ bool ChannelPosix::ProcessOutgoingMessages() { fd_written = pipe_.get(); #if defined(IPC_USES_READWRITE) if ((mode_ & MODE_CLIENT_FLAG) && IsHelloMessage(*msg)) { - DCHECK_EQ(msg->file_descriptor_set()->size(), 1U); + DCHECK_EQ(msg->attachment_set()->size(), 1U); } if (!msgh.msg_controllen) { bytes_written = @@ -554,6 +556,7 @@ bool ChannelPosix::ProcessOutgoingMessages() { } bool ChannelPosix::Send(Message* message) { + DCHECK(!message->HasMojoHandles()); DVLOG(2) << "sending message @" << message << " on channel @" << this << " with type " << message->type() << " (" << output_queue_.size() << " in queue)"; @@ -612,7 +615,7 @@ void ChannelPosix::ResetToAcceptingConnectionState() { // Unregister libevent for the unix domain socket and close it. read_watcher_.StopWatchingFileDescriptor(); write_watcher_.StopWatchingFileDescriptor(); - pipe_.reset(); + ResetSafely(&pipe_); #if defined(IPC_USES_READWRITE) fd_pipe_.reset(); remote_fd_pipe_.reset(); @@ -771,14 +774,20 @@ void ChannelPosix::ClosePipeOnError() { } int ChannelPosix::GetHelloMessageProcId() const { +#if defined(OS_NACL_NONSFI) + // In nacl_helper_nonsfi, getpid() invoked by GetCurrentProcId() is not + // allowed and would cause a SIGSYS crash because of the seccomp sandbox. + return -1; +#else int pid = base::GetCurrentProcId(); #if defined(OS_LINUX) // Our process may be in a sandbox with a separate PID namespace. if (global_pid_) { pid = global_pid_; } -#endif +#endif // defined(OS_LINUX) return pid; +#endif // defined(OS_NACL_NONSFI) } void ChannelPosix::QueueHelloMessage() { @@ -792,10 +801,11 @@ void ChannelPosix::QueueHelloMessage() { #if defined(IPC_USES_READWRITE) scoped_ptr<Message> hello; if (remote_fd_pipe_.is_valid()) { - if (!msg->WriteBorrowingFile(remote_fd_pipe_.get())) { + if (!msg->WriteAttachment( + new internal::PlatformFileAttachment(remote_fd_pipe_.get()))) { NOTREACHED() << "Unable to pickle hello message file descriptors"; } - DCHECK_EQ(msg->file_descriptor_set()->size(), 1U); + DCHECK_EQ(msg->attachment_set()->size(), 1U); } #endif // IPC_USES_READWRITE output_queue_.push(msg.release()); @@ -903,7 +913,7 @@ bool ChannelPosix::WillDispatchInputMessage(Message* msg) { error = "Message needs unreceived descriptors"; } - if (header_fds > FileDescriptorSet::kMaxDescriptorsPerMessage) + if (header_fds > MessageAttachmentSet::kMaxDescriptorsPerMessage) error = "Message requires an excessive number of descriptors"; if (error) { @@ -919,8 +929,7 @@ bool ChannelPosix::WillDispatchInputMessage(Message* msg) { // The shenaniganery below with &foo.front() requires input_fds_ to have // contiguous underlying storage (such as a simple array or a std::vector). // This is why the header warns not to make input_fds_ a deque<>. - msg->file_descriptor_set()->AddDescriptorsToOwn(&input_fds_.front(), - header_fds); + msg->attachment_set()->AddDescriptorsToOwn(&input_fds_.front(), header_fds); input_fds_.erase(input_fds_.begin(), input_fds_.begin() + header_fds); return true; } @@ -1005,7 +1014,7 @@ void ChannelPosix::HandleInternalMessage(const Message& msg) { case Channel::HELLO_MESSAGE_TYPE: int pid; - if (!msg.ReadInt(&iter, &pid)) + if (!iter.ReadInt(&pid)) NOTREACHED(); #if defined(IPC_USES_READWRITE) @@ -1013,12 +1022,12 @@ void ChannelPosix::HandleInternalMessage(const Message& msg) { // With IPC_USES_READWRITE, the Hello message from the client to the // server also contains the fd_pipe_, which will be used for all // subsequent file descriptor passing. - DCHECK_EQ(msg.file_descriptor_set()->size(), 1U); - base::ScopedFD descriptor; - if (!msg.ReadFile(&iter, &descriptor)) { + DCHECK_EQ(msg.attachment_set()->size(), 1U); + scoped_refptr<MessageAttachment> attachment; + if (!msg.ReadAttachment(&iter, &attachment)) { NOTREACHED(); } - fd_pipe_.reset(descriptor.release()); + fd_pipe_.reset(attachment->TakePlatformFile()); } #endif // IPC_USES_READWRITE peer_pid_ = pid; @@ -1028,9 +1037,9 @@ void ChannelPosix::HandleInternalMessage(const Message& msg) { #if defined(OS_MACOSX) case Channel::CLOSE_FD_MESSAGE_TYPE: int fd, hops; - if (!msg.ReadInt(&iter, &hops)) + if (!iter.ReadInt(&hops)) NOTREACHED(); - if (!msg.ReadInt(&iter, &fd)) + if (!iter.ReadInt(&fd)) NOTREACHED(); if (hops == 0) { if (fds_to_close_.erase(fd) > 0) { @@ -1080,6 +1089,25 @@ base::ProcessId ChannelPosix::GetSelfPID() const { return GetHelloMessageProcId(); } +void ChannelPosix::ResetSafely(base::ScopedFD* fd) { + if (!in_dtor_) { + fd->reset(); + return; + } + + // crbug.com/449233 + // The CL [1] tightened the error check for closing FDs, but it turned + // out that there are existing cases that hit the newly added check. + // ResetSafely() is the workaround for that crash, turning it from + // from PCHECK() to DPCHECK() so that it doesn't crash in production. + // [1] https://crrev.com/ce44fef5fd60dd2be5c587d4b084bdcd36adcee4 + int fd_to_close = fd->release(); + if (-1 != fd_to_close) { + int rv = IGNORE_EINTR(close(fd_to_close)); + DPCHECK(0 == rv); + } +} + //------------------------------------------------------------------------------ // Channel's methods diff --git a/chromium/ipc/ipc_channel_posix.h b/chromium/ipc/ipc_channel_posix.h index 1cb76102c39..a65283d3bea 100644 --- a/chromium/ipc/ipc_channel_posix.h +++ b/chromium/ipc/ipc_channel_posix.h @@ -17,8 +17,8 @@ #include "base/files/scoped_file.h" #include "base/message_loop/message_loop.h" #include "base/process/process.h" -#include "ipc/file_descriptor_set_posix.h" #include "ipc/ipc_channel_reader.h" +#include "ipc/ipc_message_attachment_set.h" #if !defined(OS_MACOSX) // On Linux, the seccomp sandbox makes it very expensive to call @@ -178,7 +178,7 @@ class IPC_EXPORT ChannelPosix : public Channel, // message has no payload and a full complement of descriptors. static const size_t kMaxReadFDs = (Channel::kReadBufferSize / sizeof(IPC::Message::Header)) * - FileDescriptorSet::kMaxDescriptorsPerMessage; + MessageAttachmentSet::kMaxDescriptorsPerMessage; // Buffer size for file descriptors used for recvmsg. On Mac the CMSG macros // don't seem to be constant so we have to pick a "large enough" value. @@ -202,6 +202,10 @@ class IPC_EXPORT ChannelPosix : public Channel, // implementation! std::vector<int> input_fds_; + + void ResetSafely(base::ScopedFD* fd); + bool in_dtor_; + #if defined(OS_MACOSX) // On OSX, sent FDs must not be closed until we get an ack. // Keep track of sent FDs here to make sure the remote is not diff --git a/chromium/ipc/ipc_channel_posix_unittest.cc b/chromium/ipc/ipc_channel_posix_unittest.cc index 786623b8062..aa545402f4b 100644 --- a/chromium/ipc/ipc_channel_posix_unittest.cc +++ b/chromium/ipc/ipc_channel_posix_unittest.cc @@ -14,11 +14,12 @@ #include "base/basictypes.h" #include "base/files/file_path.h" #include "base/files/file_util.h" +#include "base/location.h" #include "base/memory/scoped_ptr.h" -#include "base/message_loop/message_loop.h" #include "base/path_service.h" #include "base/posix/eintr_wrapper.h" -#include "base/process/kill.h" +#include "base/process/process.h" +#include "base/single_thread_task_runner.h" #include "base/test/multiprocess_test.h" #include "base/test/test_timeouts.h" #include "ipc/ipc_listener.h" @@ -88,7 +89,7 @@ class IPCChannelPosixTestListener : public IPC::Listener { loop->QuitNow(); } else { // Die as soon as Run is called. - loop->PostTask(FROM_HERE, loop->QuitClosure()); + loop->task_runner()->PostTask(FROM_HERE, loop->QuitClosure()); } } @@ -109,21 +110,17 @@ class IPCChannelPosixTest : public base::MultiProcessTest { static const std::string GetChannelDirName(); protected: - virtual void SetUp(); - virtual void TearDown(); + void SetUp() override; + void TearDown() override; private: scoped_ptr<base::MessageLoopForIO> message_loop_; }; const std::string IPCChannelPosixTest::GetChannelDirName() { -#if defined(OS_ANDROID) base::FilePath tmp_dir; - PathService::Get(base::DIR_CACHE, &tmp_dir); + PathService::Get(base::DIR_TEMP, &tmp_dir); return tmp_dir.value(); -#else - return "/var/tmp"; -#endif } const std::string IPCChannelPosixTest::GetConnectionSocketName() { @@ -192,7 +189,7 @@ void IPCChannelPosixTest::SpinRunLoop(base::TimeDelta delay) { // in the case of a bad test. Usually, the run loop will quit sooner than // that because all tests use a IPCChannelPosixTestListener which quits the // current run loop on any channel activity. - loop->PostDelayedTask(FROM_HERE, loop->QuitClosure(), delay); + loop->task_runner()->PostDelayedTask(FROM_HERE, loop->QuitClosure(), delay); loop->Run(); } @@ -211,6 +208,7 @@ TEST_F(IPCChannelPosixTest, BasicListen) { ASSERT_FALSE(channel->HasAcceptedConnection()); channel->ResetToAcceptingConnectionState(); ASSERT_FALSE(channel->HasAcceptedConnection()); + unlink(handle.name.c_str()); } TEST_F(IPCChannelPosixTest, BasicConnected) { @@ -293,8 +291,8 @@ TEST_F(IPCChannelPosixTest, AdvancedConnected) { ASSERT_TRUE(channel->AcceptsConnections()); ASSERT_FALSE(channel->HasAcceptedConnection()); - base::ProcessHandle handle = SpawnChild("IPCChannelPosixTestConnectionProc"); - ASSERT_TRUE(handle); + base::Process process = SpawnChild("IPCChannelPosixTestConnectionProc"); + ASSERT_TRUE(process.IsValid()); SpinRunLoop(TestTimeouts::action_max_timeout()); ASSERT_EQ(IPCChannelPosixTestListener::CONNECTED, listener.status()); ASSERT_TRUE(channel->HasAcceptedConnection()); @@ -304,10 +302,11 @@ TEST_F(IPCChannelPosixTest, AdvancedConnected) { channel->Send(message); SpinRunLoop(TestTimeouts::action_timeout()); int exit_code = 0; - EXPECT_TRUE(base::WaitForExitCode(handle, &exit_code)); + EXPECT_TRUE(process.WaitForExit(&exit_code)); EXPECT_EQ(0, exit_code); ASSERT_EQ(IPCChannelPosixTestListener::CHANNEL_ERROR, listener.status()); ASSERT_FALSE(channel->HasAcceptedConnection()); + unlink(chan_handle.name.c_str()); } TEST_F(IPCChannelPosixTest, ResetState) { @@ -323,16 +322,16 @@ TEST_F(IPCChannelPosixTest, ResetState) { ASSERT_TRUE(channel->AcceptsConnections()); ASSERT_FALSE(channel->HasAcceptedConnection()); - base::ProcessHandle handle = SpawnChild("IPCChannelPosixTestConnectionProc"); - ASSERT_TRUE(handle); + base::Process process = SpawnChild("IPCChannelPosixTestConnectionProc"); + ASSERT_TRUE(process.IsValid()); SpinRunLoop(TestTimeouts::action_max_timeout()); ASSERT_EQ(IPCChannelPosixTestListener::CONNECTED, listener.status()); ASSERT_TRUE(channel->HasAcceptedConnection()); channel->ResetToAcceptingConnectionState(); ASSERT_FALSE(channel->HasAcceptedConnection()); - base::ProcessHandle handle2 = SpawnChild("IPCChannelPosixTestConnectionProc"); - ASSERT_TRUE(handle2); + base::Process process2 = SpawnChild("IPCChannelPosixTestConnectionProc"); + ASSERT_TRUE(process2.IsValid()); SpinRunLoop(TestTimeouts::action_max_timeout()); ASSERT_EQ(IPCChannelPosixTestListener::CONNECTED, listener.status()); ASSERT_TRUE(channel->HasAcceptedConnection()); @@ -341,12 +340,13 @@ TEST_F(IPCChannelPosixTest, ResetState) { IPC::Message::PRIORITY_NORMAL); channel->Send(message); SpinRunLoop(TestTimeouts::action_timeout()); - EXPECT_TRUE(base::KillProcess(handle, 0, false)); + EXPECT_TRUE(process.Terminate(0, false)); int exit_code = 0; - EXPECT_TRUE(base::WaitForExitCode(handle2, &exit_code)); + EXPECT_TRUE(process2.WaitForExit(&exit_code)); EXPECT_EQ(0, exit_code); ASSERT_EQ(IPCChannelPosixTestListener::CHANNEL_ERROR, listener.status()); ASSERT_FALSE(channel->HasAcceptedConnection()); + unlink(chan_handle.name.c_str()); } TEST_F(IPCChannelPosixTest, BadChannelName) { @@ -383,16 +383,16 @@ TEST_F(IPCChannelPosixTest, MultiConnection) { ASSERT_TRUE(channel->AcceptsConnections()); ASSERT_FALSE(channel->HasAcceptedConnection()); - base::ProcessHandle handle = SpawnChild("IPCChannelPosixTestConnectionProc"); - ASSERT_TRUE(handle); + base::Process process = SpawnChild("IPCChannelPosixTestConnectionProc"); + ASSERT_TRUE(process.IsValid()); SpinRunLoop(TestTimeouts::action_max_timeout()); ASSERT_EQ(IPCChannelPosixTestListener::CONNECTED, listener.status()); ASSERT_TRUE(channel->HasAcceptedConnection()); - base::ProcessHandle handle2 = SpawnChild("IPCChannelPosixFailConnectionProc"); - ASSERT_TRUE(handle2); + base::Process process2 = SpawnChild("IPCChannelPosixFailConnectionProc"); + ASSERT_TRUE(process2.IsValid()); SpinRunLoop(TestTimeouts::action_max_timeout()); int exit_code = 0; - EXPECT_TRUE(base::WaitForExitCode(handle2, &exit_code)); + EXPECT_TRUE(process2.WaitForExit(&exit_code)); EXPECT_EQ(exit_code, 0); ASSERT_EQ(IPCChannelPosixTestListener::DENIED, listener.status()); ASSERT_TRUE(channel->HasAcceptedConnection()); @@ -401,10 +401,11 @@ TEST_F(IPCChannelPosixTest, MultiConnection) { IPC::Message::PRIORITY_NORMAL); channel->Send(message); SpinRunLoop(TestTimeouts::action_timeout()); - EXPECT_TRUE(base::WaitForExitCode(handle, &exit_code)); + EXPECT_TRUE(process.WaitForExit(&exit_code)); EXPECT_EQ(exit_code, 0); ASSERT_EQ(IPCChannelPosixTestListener::CHANNEL_ERROR, listener.status()); ASSERT_FALSE(channel->HasAcceptedConnection()); + unlink(chan_handle.name.c_str()); } TEST_F(IPCChannelPosixTest, DoubleServer) { @@ -443,6 +444,7 @@ TEST_F(IPCChannelPosixTest, IsNamedServerInitialized) { channel->Close(); ASSERT_FALSE(IPC::Channel::IsNamedServerInitialized( connection_socket_name)); + unlink(chan_handle.name.c_str()); } // A long running process that connects to us diff --git a/chromium/ipc/ipc_channel_proxy.cc b/chromium/ipc/ipc_channel_proxy.cc index 64fab8fca56..57676189b8d 100644 --- a/chromium/ipc/ipc_channel_proxy.cc +++ b/chromium/ipc/ipc_channel_proxy.cc @@ -9,6 +9,7 @@ #include "base/location.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "base/profiler/scoped_tracker.h" #include "base/single_thread_task_runner.h" #include "base/thread_task_runner_handle.h" #include "ipc/ipc_channel_factory.h" @@ -29,6 +30,7 @@ ChannelProxy::Context::Context( listener_(listener), ipc_task_runner_(ipc_task_runner), channel_connected_called_(false), + channel_send_thread_safe_(false), message_filter_router_(new MessageFilterRouter()), peer_pid_(base::kNullProcessId) { DCHECK(ipc_task_runner_.get()); @@ -50,10 +52,20 @@ void ChannelProxy::Context::ClearIPCTaskRunner() { ipc_task_runner_ = NULL; } +void ChannelProxy::Context::SetListenerTaskRunner( + scoped_refptr<base::SingleThreadTaskRunner> task_runner) { + DCHECK(ipc_task_runner_.get() != task_runner.get()); + DCHECK(listener_task_runner_->BelongsToCurrentThread()); + DCHECK(task_runner->BelongsToCurrentThread()); + listener_task_runner_ = task_runner; +} + void ChannelProxy::Context::CreateChannel(scoped_ptr<ChannelFactory> factory) { + base::AutoLock l(channel_lifetime_lock_); DCHECK(!channel_); channel_id_ = factory->GetName(); channel_ = factory->BuildChannel(this); + channel_send_thread_safe_ = channel_->IsSendThreadSafe(); } bool ChannelProxy::Context::TryFilters(const Message& message) { @@ -138,6 +150,10 @@ void ChannelProxy::Context::OnChannelOpened() { // Called on the IPC::Channel thread void ChannelProxy::Context::OnChannelClosed() { + // TODO(pkasting): Remove ScopedTracker below once crbug.com/477117 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "477117 ChannelProxy::Context::OnChannelClosed")); // It's okay for IPC::ChannelProxy::Close to be called more than once, which // would result in this branch being taken. if (!channel_) @@ -155,7 +171,7 @@ void ChannelProxy::Context::OnChannelClosed() { // access it any more. pending_filters_.clear(); - channel_.reset(); + ClearChannel(); // Balance with the reference taken during startup. This may result in // self-destruction. @@ -168,6 +184,10 @@ void ChannelProxy::Context::Clear() { // Called on the IPC::Channel thread void ChannelProxy::Context::OnSendMessage(scoped_ptr<Message> message) { + // TODO(pkasting): Remove ScopedTracker below once crbug.com/477117 is fixed. + tracked_objects::ScopedTracker tracking_profile( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "477117 ChannelProxy::Context::OnSendMessage")); if (!channel_) { OnChannelClosed(); return; @@ -244,7 +264,7 @@ void ChannelProxy::Context::AddFilter(MessageFilter* filter) { // Called on the listener's thread void ChannelProxy::Context::OnDispatchMessage(const Message& message) { -#ifdef IPC_MESSAGE_LOG_ENABLED +#if defined(IPC_MESSAGE_LOG_ENABLED) Logging* logger = Logging::GetInstance(); std::string name; logger->GetMessageText(message.type(), &name, &message, NULL); @@ -303,6 +323,31 @@ void ChannelProxy::Context::OnDispatchBadMessage(const Message& message) { listener_->OnBadMessageReceived(message); } +void ChannelProxy::Context::ClearChannel() { + base::AutoLock l(channel_lifetime_lock_); + channel_.reset(); +} + +void ChannelProxy::Context::SendFromThisThread(Message* message) { + base::AutoLock l(channel_lifetime_lock_); + if (!channel_) + return; + DCHECK(channel_->IsSendThreadSafe()); + channel_->Send(message); +} + +void ChannelProxy::Context::Send(Message* message) { + if (channel_send_thread_safe_) { + SendFromThisThread(message); + return; + } + + ipc_task_runner()->PostTask( + FROM_HERE, base::Bind(&ChannelProxy::Context::OnSendMessage, this, + base::Passed(scoped_ptr<Message>(message)))); +} + + //----------------------------------------------------------------------------- // static @@ -329,12 +374,18 @@ scoped_ptr<ChannelProxy> ChannelProxy::Create( ChannelProxy::ChannelProxy(Context* context) : context_(context), did_init_(false) { +#if defined(ENABLE_IPC_FUZZER) + outgoing_message_filter_ = NULL; +#endif } ChannelProxy::ChannelProxy( Listener* listener, const scoped_refptr<base::SingleThreadTaskRunner>& ipc_task_runner) : context_(new Context(listener, ipc_task_runner)), did_init_(false) { +#if defined(ENABLE_IPC_FUZZER) + outgoing_message_filter_ = NULL; +#endif } ChannelProxy::~ChannelProxy() { @@ -403,14 +454,19 @@ bool ChannelProxy::Send(Message* message) { // TODO(alexeypa): add DCHECK(CalledOnValidThread()) here. Currently there are // tests that call Send() from a wrong thread. See http://crbug.com/163523. +#ifdef ENABLE_IPC_FUZZER + // In IPC fuzzing builds, it is possible to define a filter to apply to + // outgoing messages. It will either rewrite the message and return a new + // one, freeing the original, or return the message unchanged. + if (outgoing_message_filter()) + message = outgoing_message_filter()->Rewrite(message); +#endif + #ifdef IPC_MESSAGE_LOG_ENABLED Logging::GetInstance()->OnSendMessage(message, context_->channel_id()); #endif - context_->ipc_task_runner()->PostTask( - FROM_HERE, - base::Bind(&ChannelProxy::Context::OnSendMessage, - context_, base::Passed(scoped_ptr<Message>(message)))); + context_->Send(message); return true; } @@ -428,6 +484,13 @@ void ChannelProxy::RemoveFilter(MessageFilter* filter) { make_scoped_refptr(filter))); } +void ChannelProxy::SetListenerTaskRunner( + scoped_refptr<base::SingleThreadTaskRunner> task_runner) { + DCHECK(CalledOnValidThread()); + + context()->SetListenerTaskRunner(task_runner); +} + void ChannelProxy::ClearIPCTaskRunner() { DCHECK(CalledOnValidThread()); diff --git a/chromium/ipc/ipc_channel_proxy.h b/chromium/ipc/ipc_channel_proxy.h index dda5fa5f41c..5d38006b5d5 100644 --- a/chromium/ipc/ipc_channel_proxy.h +++ b/chromium/ipc/ipc_channel_proxy.h @@ -55,8 +55,25 @@ class SendCallbackHelper; // The consumer of IPC::ChannelProxy is responsible for allocating the Thread // instance where the IPC::Channel will be created and operated. // +// Thread-safe send +// +// If a particular |Channel| implementation has a thread-safe |Send()| operation +// then ChannelProxy skips the inter-thread hop and calls |Send()| directly. In +// this case the |channel_| variable is touched by multiple threads so +// |channel_lifetime_lock_| is used to protect it. The locking overhead is only +// paid if the underlying channel supports thread-safe |Send|. +// class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe { public: +#if defined(ENABLE_IPC_FUZZER) + // Interface for a filter to be imposed on outgoing messages which can + // re-write the message. Used for testing. + class OutgoingMessageFilter { + public: + virtual Message* Rewrite(Message* message) = 0; + }; +#endif + // Initializes a channel proxy. The channel_handle and mode parameters are // passed directly to the underlying IPC::Channel. The listener is called on // the thread that creates the ChannelProxy. The filter's OnMessageReceived @@ -112,6 +129,18 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe { void AddFilter(MessageFilter* filter); void RemoveFilter(MessageFilter* filter); +#if defined(ENABLE_IPC_FUZZER) + void set_outgoing_message_filter(OutgoingMessageFilter* filter) { + outgoing_message_filter_ = filter; + } +#endif + + // Set the task runner on which dispatched messages are posted. Both the new + // task runner and the existing task runner must run on the same thread, and + // must belong to the calling thread. + void SetListenerTaskRunner( + scoped_refptr<base::SingleThreadTaskRunner> listener_task_runner); + // Called to clear the pointer to the IPC task runner when it's going away. void ClearIPCTaskRunner(); @@ -142,6 +171,8 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe { Context(Listener* listener, const scoped_refptr<base::SingleThreadTaskRunner>& ipc_thread); void ClearIPCTaskRunner(); + void SetListenerTaskRunner( + scoped_refptr<base::SingleThreadTaskRunner> listener_task_runner); base::SingleThreadTaskRunner* ipc_task_runner() const { return ipc_task_runner_.get(); } @@ -150,6 +181,9 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe { // Dispatches a message on the listener thread. void OnDispatchMessage(const Message& message); + // Sends |message| from appropriate thread. + void Send(Message* message); + protected: friend class base::RefCountedThreadSafe<Context>; ~Context() override; @@ -177,7 +211,7 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe { private: friend class ChannelProxy; - friend class SendCallbackHelper; + friend class IpcSecurityTestUtil; // Create the Channel void CreateChannel(scoped_ptr<ChannelFactory> factory); @@ -193,6 +227,9 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe { void OnDispatchError(); void OnDispatchBadMessage(const Message& message); + void SendFromThisThread(Message* message); + void ClearChannel(); + scoped_refptr<base::SingleThreadTaskRunner> listener_task_runner_; Listener* listener_; @@ -203,10 +240,18 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe { // Note, channel_ may be set on the Listener thread or the IPC thread. // But once it has been set, it must only be read or cleared on the IPC // thread. + // One exception is the thread-safe send. See the class comment. scoped_ptr<Channel> channel_; std::string channel_id_; bool channel_connected_called_; + // Lock for |channel_| value. This is only relevant in the context of + // thread-safe send. + base::Lock channel_lifetime_lock_; + // Indicates the thread-safe send availability. This is constant once + // |channel_| is set. + bool channel_send_thread_safe_; + // Routes a given message to a proper subset of |filters_|, depending // on which message classes a filter might support. scoped_ptr<MessageFilterRouter> message_filter_router_; @@ -224,8 +269,14 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe { Context* context() { return context_.get(); } +#if defined(ENABLE_IPC_FUZZER) + OutgoingMessageFilter* outgoing_message_filter() const { + return outgoing_message_filter_; + } +#endif + private: - friend class SendCallbackHelper; + friend class IpcSecurityTestUtil; // By maintaining this indirection (ref-counted) to our internal state, we // can safely be destroyed while the background thread continues to do stuff @@ -234,6 +285,10 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe { // Whether the channel has been initialized. bool did_init_; + +#if defined(ENABLE_IPC_FUZZER) + OutgoingMessageFilter* outgoing_message_filter_; +#endif }; } // namespace IPC diff --git a/chromium/ipc/ipc_channel_proxy_unittest.cc b/chromium/ipc/ipc_channel_proxy_unittest.cc index 848367ec782..4144a8adba1 100644 --- a/chromium/ipc/ipc_channel_proxy_unittest.cc +++ b/chromium/ipc/ipc_channel_proxy_unittest.cc @@ -4,7 +4,6 @@ #include "build/build_config.h" -#include "base/message_loop/message_loop.h" #include "base/pickle.h" #include "base/threading/thread.h" #include "ipc/ipc_message.h" @@ -47,7 +46,6 @@ namespace { class QuitListener : public IPC::Listener { public: QuitListener() : bad_message_received_(false) {} - ~QuitListener() override {} bool OnMessageReceived(const IPC::Message& message) override { IPC_BEGIN_MESSAGE_MAP(QuitListener, message) @@ -76,7 +74,6 @@ class QuitListener : public IPC::Listener { class ChannelReflectorListener : public IPC::Listener { public: ChannelReflectorListener() : channel_(NULL) {} - ~ChannelReflectorListener() override {} void Init(IPC::Channel* channel) { DCHECK(!channel_); @@ -232,9 +229,9 @@ class MessageCountFilter : public IPC::MessageFilter { class IPCChannelProxyTest : public IPCTestBase { public: IPCChannelProxyTest() {} - virtual ~IPCChannelProxyTest() {} + ~IPCChannelProxyTest() override {} - virtual void SetUp() override { + void SetUp() override { IPCTestBase::SetUp(); Init("ChannelProxyClient"); @@ -245,12 +242,12 @@ class IPCChannelProxyTest : public IPCTestBase { thread_->StartWithOptions(options); listener_.reset(new QuitListener()); - CreateChannelProxy(listener_.get(), thread_->message_loop_proxy().get()); + CreateChannelProxy(listener_.get(), thread_->task_runner().get()); ASSERT_TRUE(StartClient()); } - virtual void TearDown() { + void TearDown() override { DestroyChannelProxy(); thread_.reset(); listener_.reset(); @@ -380,10 +377,7 @@ TEST_F(IPCChannelProxyTest, BadMessageOnIPCThread) { class IPCChannelBadMessageTest : public IPCTestBase { public: - IPCChannelBadMessageTest() {} - virtual ~IPCChannelBadMessageTest() {} - - virtual void SetUp() override { + void SetUp() override { IPCTestBase::SetUp(); Init("ChannelProxyClient"); @@ -395,7 +389,7 @@ class IPCChannelBadMessageTest : public IPCTestBase { ASSERT_TRUE(StartClient()); } - virtual void TearDown() { + void TearDown() override { listener_.reset(); IPCTestBase::TearDown(); } diff --git a/chromium/ipc/ipc_channel_unittest.cc b/chromium/ipc/ipc_channel_unittest.cc index dd31e9e9c47..b6a02262613 100644 --- a/chromium/ipc/ipc_channel_unittest.cc +++ b/chromium/ipc/ipc_channel_unittest.cc @@ -10,8 +10,9 @@ #include <string> -#include "base/message_loop/message_loop.h" #include "base/pickle.h" +#include "base/strings/string16.h" +#include "base/strings/utf_string_conversions.h" #include "base/threading/thread.h" #include "ipc/ipc_message.h" #include "ipc/ipc_test_base.h" @@ -26,32 +27,32 @@ class IPCChannelTest : public IPCTestBase { TEST_F(IPCChannelTest, BasicMessageTest) { int v1 = 10; std::string v2("foobar"); - std::wstring v3(L"hello world"); + base::string16 v3(base::ASCIIToUTF16("hello world")); IPC::Message m(0, 1, IPC::Message::PRIORITY_NORMAL); EXPECT_TRUE(m.WriteInt(v1)); EXPECT_TRUE(m.WriteString(v2)); - EXPECT_TRUE(m.WriteWString(v3)); + EXPECT_TRUE(m.WriteString16(v3)); PickleIterator iter(m); int vi; std::string vs; - std::wstring vw; + base::string16 vs16; - EXPECT_TRUE(m.ReadInt(&iter, &vi)); + EXPECT_TRUE(iter.ReadInt(&vi)); EXPECT_EQ(v1, vi); - EXPECT_TRUE(m.ReadString(&iter, &vs)); + EXPECT_TRUE(iter.ReadString(&vs)); EXPECT_EQ(v2, vs); - EXPECT_TRUE(m.ReadWString(&iter, &vw)); - EXPECT_EQ(v3, vw); + EXPECT_TRUE(iter.ReadString16(&vs16)); + EXPECT_EQ(v3, vs16); // should fail - EXPECT_FALSE(m.ReadInt(&iter, &vi)); - EXPECT_FALSE(m.ReadString(&iter, &vs)); - EXPECT_FALSE(m.ReadWString(&iter, &vw)); + EXPECT_FALSE(iter.ReadInt(&vi)); + EXPECT_FALSE(iter.ReadString(&vs)); + EXPECT_FALSE(iter.ReadString16(&vs16)); } TEST_F(IPCChannelTest, ChannelTest) { @@ -126,7 +127,7 @@ TEST_F(IPCChannelTest, ChannelProxyTest) { // Set up IPC channel proxy. IPC::TestChannelListener listener; - CreateChannelProxy(&listener, thread.message_loop_proxy().get()); + CreateChannelProxy(&listener, thread.task_runner().get()); listener.Init(sender()); ASSERT_TRUE(StartClient()); diff --git a/chromium/ipc/ipc_channel_win.cc b/chromium/ipc/ipc_channel_win.cc index db8c96116e3..27043730695 100644 --- a/chromium/ipc/ipc_channel_win.cc +++ b/chromium/ipc/ipc_channel_win.cc @@ -21,27 +21,6 @@ #include "ipc/ipc_logging.h" #include "ipc/ipc_message_utils.h" -namespace { - -enum DebugFlags { - INIT_DONE = 1 << 0, - CALLED_CONNECT = 1 << 1, - PENDING_CONNECT = 1 << 2, - CONNECT_COMPLETED = 1 << 3, - PIPE_CONNECTED = 1 << 4, - WRITE_MSG = 1 << 5, - READ_MSG = 1 << 6, - WRITE_COMPLETED = 1 << 7, - READ_COMPLETED = 1 << 8, - CLOSED = 1 << 9, - WAIT_FOR_READ = 1 << 10, - WAIT_FOR_WRITE = 1 << 11, - WAIT_FOR_READ_COMPLETE = 1 << 12, - WAIT_FOR_WRITE_COMPLETE = 1 << 13 -}; - -} // namespace - namespace IPC { ChannelWin::State::State(ChannelWin* channel) : is_pending(false) { @@ -50,8 +29,9 @@ ChannelWin::State::State(ChannelWin* channel) : is_pending(false) { } ChannelWin::State::~State() { - COMPILE_ASSERT(!offsetof(ChannelWin::State, context), - starts_with_io_context); + static_assert(offsetof(ChannelWin::State, context) == 0, + "ChannelWin::State should have context as its first data" + "member."); } ChannelWin::ChannelWin(const IPC::ChannelHandle &channel_handle, @@ -63,11 +43,6 @@ ChannelWin::ChannelWin(const IPC::ChannelHandle &channel_handle, waiting_connect_(mode & MODE_SERVER_FLAG), processing_incoming_(false), validate_client_(false), - writing_(false), - debug_flags_(0), - write_error_(0), - last_write_error_(0), - write_size_(0), client_secret_(0), weak_factory_(this) { CreatePipe(channel_handle, mode); @@ -78,10 +53,8 @@ ChannelWin::~ChannelWin() { } void ChannelWin::Close() { - if (thread_check_.get()) { + if (thread_check_.get()) DCHECK(thread_check_->CalledOnValidThread()); - } - debug_flags_ |= CLOSED; if (input_state_.is_pending || output_state_.is_pending) CancelIo(pipe_.Get()); @@ -91,12 +64,6 @@ void ChannelWin::Close() { if (pipe_.IsValid()) pipe_.Close(); - if (input_state_.is_pending) - debug_flags_ |= WAIT_FOR_READ; - - if (output_state_.is_pending) - debug_flags_ |= WAIT_FOR_WRITE; - // Make sure all IO has completed. base::Time start = base::Time::Now(); while (input_state_.is_pending || output_state_.is_pending) { @@ -111,6 +78,7 @@ void ChannelWin::Close() { } bool ChannelWin::Send(Message* message) { + DCHECK(!message->HasAttachments()); DCHECK(thread_check_->CalledOnValidThread()); DVLOG(2) << "sending message @" << message << " on channel @" << this << " with type " << message->type() @@ -158,7 +126,6 @@ ChannelWin::ReadState ChannelWin::ReadData( if (!pipe_.IsValid()) return READ_FAILED; - debug_flags_ |= READ_MSG; DWORD bytes_read = 0; BOOL ok = ReadFile(pipe_.Get(), buffer, buffer_len, &bytes_read, &input_state_.context.overlapped); @@ -297,7 +264,7 @@ bool ChannelWin::CreatePipe(const IPC::ChannelHandle &channel_handle, 0, NULL, OPEN_EXISTING, - SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION | + SECURITY_SQOS_PRESENT | SECURITY_ANONYMOUS | FILE_FLAG_OVERLAPPED, NULL)); } else { @@ -325,8 +292,6 @@ bool ChannelWin::CreatePipe(const IPC::ChannelHandle &channel_handle, return false; } - debug_flags_ |= INIT_DONE; - output_queue_.push(m.release()); return true; } @@ -374,8 +339,6 @@ bool ChannelWin::ProcessConnection() { return false; BOOL ok = ConnectNamedPipe(pipe_.Get(), &input_state_.context.overlapped); - debug_flags_ |= CALLED_CONNECT; - DWORD err = GetLastError(); if (ok) { // Uhm, the API documentation says that this function should never @@ -387,10 +350,8 @@ bool ChannelWin::ProcessConnection() { switch (err) { case ERROR_IO_PENDING: input_state_.is_pending = true; - debug_flags_ |= PENDING_CONNECT; break; case ERROR_PIPE_CONNECTED: - debug_flags_ |= PIPE_CONNECTED; waiting_connect_ = false; break; case ERROR_NO_DATA: @@ -435,19 +396,14 @@ bool ChannelWin::ProcessOutgoingMessages( // Write to pipe... Message* m = output_queue_.front(); DCHECK(m->size() <= INT_MAX); - debug_flags_ |= WRITE_MSG; - CHECK(!writing_); - writing_ = true; - write_size_ = static_cast<uint32>(m->size()); - write_error_ = 0; BOOL ok = WriteFile(pipe_.Get(), m->data(), - write_size_, + static_cast<uint32>(m->size()), NULL, &output_state_.context.overlapped); if (!ok) { - write_error_ = GetLastError(); - if (write_error_ == ERROR_IO_PENDING) { + DWORD write_error = GetLastError(); + if (write_error == ERROR_IO_PENDING) { output_state_.is_pending = true; DVLOG(2) << "sent pending message @" << m << " on channel @" << this @@ -455,9 +411,7 @@ bool ChannelWin::ProcessOutgoingMessages( return true; } - writing_ = false; - last_write_error_ = write_error_; - LOG(ERROR) << "pipe error: " << write_error_; + LOG(ERROR) << "pipe error: " << write_error; return false; } @@ -476,7 +430,6 @@ void ChannelWin::OnIOCompleted( DCHECK(thread_check_->CalledOnValidThread()); if (context == &input_state_.context) { if (waiting_connect_) { - debug_flags_ |= CONNECT_COMPLETED; if (!ProcessConnection()) return; // We may have some messages queued up to send... @@ -495,11 +448,6 @@ void ChannelWin::OnIOCompleted( // Process the new data. if (input_state_.is_pending) { // This is the normal case for everything except the initialization step. - debug_flags_ |= READ_COMPLETED; - if (debug_flags_ & WAIT_FOR_READ) { - CHECK(!(debug_flags_ & WAIT_FOR_READ_COMPLETE)); - debug_flags_ |= WAIT_FOR_READ_COMPLETE; - } input_state_.is_pending = false; if (!bytes_transfered) ok = false; @@ -514,14 +462,7 @@ void ChannelWin::OnIOCompleted( ok = ProcessIncomingMessages(); } else { DCHECK(context == &output_state_.context); - CHECK(writing_); CHECK(output_state_.is_pending); - writing_ = false; - debug_flags_ |= WRITE_COMPLETED; - if (debug_flags_ & WAIT_FOR_WRITE) { - CHECK(!(debug_flags_ & WAIT_FOR_WRITE_COMPLETE)); - debug_flags_ |= WAIT_FOR_WRITE_COMPLETE; - } ok = ProcessOutgoingMessages(context, bytes_transfered); } if (!ok && pipe_.IsValid()) { diff --git a/chromium/ipc/ipc_channel_win.h b/chromium/ipc/ipc_channel_win.h index 7a2d484159b..04990d4e84e 100644 --- a/chromium/ipc/ipc_channel_win.h +++ b/chromium/ipc/ipc_channel_win.h @@ -29,26 +29,24 @@ class ChannelWin : public Channel, // Mirror methods of Channel, see ipc_channel.h for description. ChannelWin(const IPC::ChannelHandle &channel_handle, Mode mode, Listener* listener); - ~ChannelWin(); + ~ChannelWin() override; // Channel implementation - virtual bool Connect() override; - virtual void Close() override; - virtual bool Send(Message* message) override; - virtual base::ProcessId GetPeerPID() const override; - virtual base::ProcessId GetSelfPID() const override; + bool Connect() override; + void Close() override; + bool Send(Message* message) override; + base::ProcessId GetPeerPID() const override; + base::ProcessId GetSelfPID() const override; static bool IsNamedServerInitialized(const std::string& channel_id); private: // ChannelReader implementation. - virtual ReadState ReadData(char* buffer, - int buffer_len, - int* bytes_read) override; - virtual bool WillDispatchInputMessage(Message* msg) override; + ReadState ReadData(char* buffer, int buffer_len, int* bytes_read) override; + bool WillDispatchInputMessage(Message* msg) override; bool DidEmptyInputBuffers() override; - virtual void HandleInternalMessage(const Message& msg) override; + void HandleInternalMessage(const Message& msg) override; static const base::string16 PipeName(const std::string& channel_id, int32* secret); @@ -59,9 +57,9 @@ class ChannelWin : public Channel, DWORD bytes_written); // MessageLoop::IOHandler implementation. - virtual void OnIOCompleted(base::MessageLoopForIO::IOContext* context, - DWORD bytes_transfered, - DWORD error) override; + void OnIOCompleted(base::MessageLoopForIO::IOContext* context, + DWORD bytes_transfered, + DWORD error) override; private: struct State { @@ -94,21 +92,9 @@ class ChannelWin : public Channel, // Determines if we should validate a client's secret on connection. bool validate_client_; - // True if there is a write in progress. TODO(rvargas): remove this. - bool writing_; - // Tracks the lifetime of this object, for debugging purposes. uint32 debug_flags_; - // OS result for the current write. TODO(rvargas): remove this. - uint32 write_error_; - - // OS result for a previous failed write. TODO(rvargas): remove this. - uint32 last_write_error_; - - // Size of the current write. TODO(rvargas): remove this. - uint32 write_size_; - // This is a unique per-channel value used to authenticate the client end of // a connection. If the value is non-zero, the client passes it in the hello // and the host validates. (We don't send the zero value fto preserve IPC diff --git a/chromium/ipc/ipc_fuzzing_tests.cc b/chromium/ipc/ipc_fuzzing_tests.cc index f9475a1adb0..42fa8597e08 100644 --- a/chromium/ipc/ipc_fuzzing_tests.cc +++ b/chromium/ipc/ipc_fuzzing_tests.cc @@ -7,6 +7,8 @@ #include <sstream> #include "base/message_loop/message_loop.h" +#include "base/strings/string16.h" +#include "base/strings/utf_string_conversions.h" #include "base/threading/platform_thread.h" #include "ipc/ipc_test_base.h" #include "testing/gtest/include/gtest/gtest.h" @@ -18,14 +20,14 @@ #define IPC_MESSAGE_START TestMsgStart -// Generic message class that is an int followed by a wstring. -IPC_MESSAGE_CONTROL2(MsgClassIS, int, std::wstring) +// Generic message class that is an int followed by a string16. +IPC_MESSAGE_CONTROL2(MsgClassIS, int, base::string16) -// Generic message class that is a wstring followed by an int. -IPC_MESSAGE_CONTROL2(MsgClassSI, std::wstring, int) +// Generic message class that is a string16 followed by an int. +IPC_MESSAGE_CONTROL2(MsgClassSI, base::string16, int) // Message to create a mutex in the IPC server, using the received name. -IPC_MESSAGE_CONTROL2(MsgDoMutex, std::wstring, int) +IPC_MESSAGE_CONTROL2(MsgDoMutex, base::string16, int) // Used to generate an ID for a message that should not exist. IPC_MESSAGE_CONTROL0(MsgUnhandled) @@ -35,7 +37,7 @@ IPC_MESSAGE_CONTROL0(MsgUnhandled) namespace { TEST(IPCMessageIntegrity, ReadBeyondBufferStr) { - //This was BUG 984408. + // This was BUG 984408. uint32 v1 = kuint32max - 1; int v2 = 666; IPC::Message m(0, 1, IPC::Message::PRIORITY_NORMAL); @@ -44,11 +46,11 @@ TEST(IPCMessageIntegrity, ReadBeyondBufferStr) { PickleIterator iter(m); std::string vs; - EXPECT_FALSE(m.ReadString(&iter, &vs)); + EXPECT_FALSE(iter.ReadString(&vs)); } -TEST(IPCMessageIntegrity, ReadBeyondBufferWStr) { - //This was BUG 984408. +TEST(IPCMessageIntegrity, ReadBeyondBufferStr16) { + // This was BUG 984408. uint32 v1 = kuint32max - 1; int v2 = 777; IPC::Message m(0, 1, IPC::Message::PRIORITY_NORMAL); @@ -56,8 +58,8 @@ TEST(IPCMessageIntegrity, ReadBeyondBufferWStr) { EXPECT_TRUE(m.WriteInt(v2)); PickleIterator iter(m); - std::wstring vs; - EXPECT_FALSE(m.ReadWString(&iter, &vs)); + base::string16 vs; + EXPECT_FALSE(iter.ReadString16(&vs)); } TEST(IPCMessageIntegrity, ReadBytesBadIterator) { @@ -68,7 +70,7 @@ TEST(IPCMessageIntegrity, ReadBytesBadIterator) { PickleIterator iter(m); const char* data = NULL; - EXPECT_TRUE(m.ReadBytes(&iter, &data, sizeof(int))); + EXPECT_TRUE(iter.ReadBytes(&data, sizeof(int))); } TEST(IPCMessageIntegrity, ReadVectorNegativeSize) { @@ -151,13 +153,13 @@ class FuzzerServerListener : public SimpleListener { } private: - void OnMsgClassISMessage(int value, const std::wstring& text) { + void OnMsgClassISMessage(int value, const base::string16& text) { UseData(MsgClassIS::ID, value, text); RoundtripAckReply(FUZZER_ROUTING_ID, MsgClassIS::ID, value); Cleanup(); } - void OnMsgClassSIMessage(const std::wstring& text, int value) { + void OnMsgClassSIMessage(const base::string16& text, int value) { UseData(MsgClassSI::ID, value, text); RoundtripAckReply(FUZZER_ROUTING_ID, MsgClassSI::ID, value); Cleanup(); @@ -183,12 +185,13 @@ class FuzzerServerListener : public SimpleListener { Cleanup(); } - void UseData(int caller, int value, const std::wstring& text) { - std::wostringstream wos; - wos << L"IPC fuzzer:" << caller << " [" << value << L" " << text << L"]\n"; - std::wstring output = wos.str(); - LOG(WARNING) << output.c_str(); - }; + void UseData(int caller, int value, const base::string16& text) { + std::ostringstream os; + os << "IPC fuzzer:" << caller << " [" << value << " " + << base::UTF16ToUTF8(text) << "]\n"; + std::string output = os.str(); + LOG(WARNING) << output; + } int message_count_; int pending_messages_; @@ -211,9 +214,9 @@ class FuzzerClientListener : public SimpleListener { int msg_value1 = 0; int msg_value2 = 0; PickleIterator iter(*last_msg_); - if (!last_msg_->ReadInt(&iter, &msg_value1)) + if (!iter.ReadInt(&msg_value1)) return false; - if (!last_msg_->ReadInt(&iter, &msg_value2)) + if (!iter.ReadInt(&msg_value2)) return false; if ((msg_value2 + 1) != msg_value1) return false; @@ -237,7 +240,7 @@ class FuzzerClientListener : public SimpleListener { if (FUZZER_ROUTING_ID != last_msg_->routing_id()) return false; return (type_id == last_msg_->type()); - }; + } IPC::Message* last_msg_; }; @@ -272,11 +275,11 @@ TEST_F(IPCFuzzingTest, SanityTest) { IPC::Message* msg = NULL; int value = 43; - msg = new MsgClassIS(value, L"expect 43"); + msg = new MsgClassIS(value, base::ASCIIToUTF16("expect 43")); sender()->Send(msg); EXPECT_TRUE(listener.ExpectMessage(value, MsgClassIS::ID)); - msg = new MsgClassSI(L"expect 44", ++value); + msg = new MsgClassSI(base::ASCIIToUTF16("expect 44"), ++value); sender()->Send(msg); EXPECT_TRUE(listener.ExpectMessage(value, MsgClassSI::ID)); @@ -304,7 +307,7 @@ TEST_F(IPCFuzzingTest, MsgBadPayloadShort) { sender()->Send(msg); EXPECT_TRUE(listener.ExpectMsgNotHandled(MsgClassIS::ID)); - msg = new MsgClassSI(L"expect one", 1); + msg = new MsgClassSI(base::ASCIIToUTF16("expect one"), 1); sender()->Send(msg); EXPECT_TRUE(listener.ExpectMessage(1, MsgClassSI::ID)); @@ -328,7 +331,7 @@ TEST_F(IPCFuzzingTest, MsgBadPayloadArgs) { IPC::Message* msg = new IPC::Message(MSG_ROUTING_CONTROL, MsgClassSI::ID, IPC::Message::PRIORITY_NORMAL); - msg->WriteWString(L"d"); + msg->WriteString16(base::ASCIIToUTF16("d")); msg->WriteInt(0); msg->WriteInt(0x65); // Extra argument. @@ -337,7 +340,7 @@ TEST_F(IPCFuzzingTest, MsgBadPayloadArgs) { // Now send a well formed message to make sure the receiver wasn't // thrown out of sync by the extra argument. - msg = new MsgClassIS(3, L"expect three"); + msg = new MsgClassIS(3, base::ASCIIToUTF16("expect three")); sender()->Send(msg); EXPECT_TRUE(listener.ExpectMessage(3, MsgClassIS::ID)); diff --git a/chromium/ipc/ipc_logging.cc b/chromium/ipc/ipc_logging.cc index 65d88901d93..3db4e676d15 100644 --- a/chromium/ipc/ipc_logging.cc +++ b/chromium/ipc/ipc_logging.cc @@ -13,9 +13,10 @@ #include "base/command_line.h" #include "base/location.h" #include "base/logging.h" -#include "base/message_loop/message_loop.h" +#include "base/single_thread_task_runner.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" +#include "base/thread_task_runner_handle.h" #include "base/threading/thread.h" #include "base/time/time.h" #include "ipc/ipc_message_utils.h" @@ -161,7 +162,7 @@ void Logging::OnPostDispatchMessage(const Message& message, if (base::MessageLoop::current() == main_thread_) { Log(data); } else { - main_thread_->PostTask( + main_thread_->task_runner()->PostTask( FROM_HERE, base::Bind(&Logging::Log, base::Unretained(this), data)); } } @@ -231,9 +232,8 @@ void Logging::Log(const LogData& data) { queued_logs_.push_back(data); if (!queue_invoke_later_pending_) { queue_invoke_later_pending_ = true; - base::MessageLoop::current()->PostDelayedTask( - FROM_HERE, - base::Bind(&Logging::OnSendLogs, base::Unretained(this)), + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, base::Bind(&Logging::OnSendLogs, base::Unretained(this)), base::TimeDelta::FromMilliseconds(kLogSendDelayMs)); } } diff --git a/chromium/ipc/ipc_message.cc b/chromium/ipc/ipc_message.cc index 7bd7a697c4f..2587b0b112b 100644 --- a/chromium/ipc/ipc_message.cc +++ b/chromium/ipc/ipc_message.cc @@ -7,10 +7,12 @@ #include "base/atomic_sequence_num.h" #include "base/logging.h" #include "build/build_config.h" +#include "ipc/ipc_message_attachment.h" +#include "ipc/ipc_message_attachment_set.h" #if defined(OS_POSIX) #include "base/file_descriptor_posix.h" -#include "ipc/file_descriptor_set_posix.h" +#include "ipc/ipc_platform_file_attachment_posix.h" #endif namespace { @@ -21,9 +23,10 @@ base::StaticAtomicSequenceNumber g_ref_num; // values has the reference number stored in the upper 24 bits, leaving the low // 8 bits set to 0 for use as flags. inline uint32 GetRefNumUpper24() { - base::debug::TraceLog* trace_log = base::debug::TraceLog::GetInstance(); - int32 pid = trace_log ? trace_log->process_id() : 0; - int32 count = g_ref_num.GetNext(); + base::trace_event::TraceLog* trace_log = + base::trace_event::TraceLog::GetInstance(); + uint32 pid = trace_log ? trace_log->process_id() : 0; + uint32 count = g_ref_num.GetNext(); // The 24 bit hash is composed of 14 bits of the count and 10 bits of the // Process ID. With the current trace event buffer cap, the 14-bit count did // not appear to wrap during a trace. Note that it is not a big deal if @@ -71,7 +74,7 @@ Message::Message(const char* data, int data_len) : Pickle(data, data_len) { Message::Message(const Message& other) : Pickle(other) { Init(); #if defined(OS_POSIX) - file_descriptor_set_ = other.file_descriptor_set_; + attachment_set_ = other.attachment_set_; #endif } @@ -87,7 +90,7 @@ void Message::Init() { Message& Message::operator=(const Message& other) { *static_cast<Pickle*>(this) = other; #if defined(OS_POSIX) - file_descriptor_set_ = other.file_descriptor_set_; + attachment_set_ = other.attachment_set_; #endif return *this; } @@ -101,6 +104,11 @@ void Message::SetHeaderValues(int32 routing, uint32 type, uint32 flags) { header()->flags = flags; } +void Message::EnsureMessageAttachmentSet() { + if (attachment_set_.get() == NULL) + attachment_set_ = new MessageAttachmentSet; +} + #ifdef IPC_MESSAGE_LOG_ENABLED void Message::set_sent_time(int64 time) { DCHECK((header()->flags & HAS_SENT_TIME_BIT) == 0); @@ -122,48 +130,34 @@ void Message::set_received_time(int64 time) const { } #endif -#if defined(OS_POSIX) -bool Message::WriteFile(base::ScopedFD descriptor) { +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(file_descriptor_set()->size()); - return file_descriptor_set()->AddToOwn(descriptor.Pass()); + WriteInt(attachment_set()->size()); + return attachment_set()->AddAttachment(attachment); } -bool Message::WriteBorrowingFile(const base::PlatformFile& descriptor) { - // 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(file_descriptor_set()->size()); - return file_descriptor_set()->AddToBorrow(descriptor); -} - -bool Message::ReadFile(PickleIterator* iter, base::ScopedFD* descriptor) const { +bool Message::ReadAttachment( + PickleIterator* iter, + scoped_refptr<MessageAttachment>* attachment) const { int descriptor_index; - if (!ReadInt(iter, &descriptor_index)) - return false; - - FileDescriptorSet* file_descriptor_set = file_descriptor_set_.get(); - if (!file_descriptor_set) + if (!iter->ReadInt(&descriptor_index)) return false; - base::PlatformFile file = - file_descriptor_set->TakeDescriptorAt(descriptor_index); - if (file < 0) + MessageAttachmentSet* attachment_set = attachment_set_.get(); + if (!attachment_set) return false; - descriptor->reset(file); - return true; + *attachment = attachment_set->GetAttachmentAt(descriptor_index); + return nullptr != attachment->get(); } -bool Message::HasFileDescriptors() const { - return file_descriptor_set_.get() && !file_descriptor_set_->empty(); +bool Message::HasAttachments() const { + return attachment_set_.get() && !attachment_set_->empty(); } -void Message::EnsureFileDescriptorSet() { - if (file_descriptor_set_.get() == NULL) - file_descriptor_set_ = new FileDescriptorSet; +bool Message::HasMojoHandles() const { + return attachment_set_.get() && 0 < attachment_set_->num_mojo_handles(); } -#endif - } // namespace IPC diff --git a/chromium/ipc/ipc_message.h b/chromium/ipc/ipc_message.h index a1b26e1d289..64d85d4d44c 100644 --- a/chromium/ipc/ipc_message.h +++ b/chromium/ipc/ipc_message.h @@ -8,26 +8,22 @@ #include <string> #include "base/basictypes.h" -#include "base/debug/trace_event.h" -#include "base/files/file.h" +#include "base/memory/ref_counted.h" #include "base/pickle.h" +#include "base/trace_event/trace_event.h" #include "ipc/ipc_export.h" #if !defined(NDEBUG) #define IPC_MESSAGE_LOG_ENABLED #endif -#if defined(OS_POSIX) -#include "base/memory/ref_counted.h" -#endif - -class FileDescriptorSet; - namespace IPC { //------------------------------------------------------------------------------ struct LogData; +class MessageAttachment; +class MessageAttachmentSet; class IPC_EXPORT Message : public Pickle { public: @@ -170,21 +166,17 @@ class IPC_EXPORT Message : public Pickle { return Pickle::FindNext(sizeof(Header), range_start, range_end); } -#if defined(OS_POSIX) - // On POSIX, a message supports reading / writing FileDescriptor objects. - // This is used to pass a file descriptor to the peer of an IPC channel. - - // Add a descriptor to the end of the set. Returns false if the set is full. - bool WriteFile(base::ScopedFD descriptor); - bool WriteBorrowingFile(const base::PlatformFile& descriptor); - - // Get a file descriptor from the message. Returns false on error. - // iter: a Pickle iterator to the current location in the message. - bool ReadFile(PickleIterator* iter, base::ScopedFD* file) const; - - // Returns true if there are any file descriptors in this message. - bool HasFileDescriptors() const; -#endif + // WriteAttachment appends |attachment| to the end of the set. It returns + // false iff the set is full. + bool WriteAttachment(scoped_refptr<MessageAttachment> attachment); + // ReadAttachment parses an attachment given the parsing state |iter| and + // writes it to |*attachment|. It returns true on success. + bool ReadAttachment(PickleIterator* iter, + scoped_refptr<MessageAttachment>* attachment) const; + // Returns true if there are any attachment in this message. + bool HasAttachments() const; + // Returns true if there are any MojoHandleAttachments in this message. + bool HasMojoHandles() const; #ifdef IPC_MESSAGE_LOG_ENABLED // Adds the outgoing time from Time::Now() at the end of the message and sets @@ -250,21 +242,19 @@ class IPC_EXPORT Message : public Pickle { // Used internally to support IPC::Listener::OnBadMessageReceived. mutable bool dispatch_error_; -#if defined(OS_POSIX) // The set of file descriptors associated with this message. - scoped_refptr<FileDescriptorSet> file_descriptor_set_; + scoped_refptr<MessageAttachmentSet> attachment_set_; - // Ensure that a FileDescriptorSet is allocated - void EnsureFileDescriptorSet(); + // Ensure that a MessageAttachmentSet is allocated + void EnsureMessageAttachmentSet(); - FileDescriptorSet* file_descriptor_set() { - EnsureFileDescriptorSet(); - return file_descriptor_set_.get(); + MessageAttachmentSet* attachment_set() { + EnsureMessageAttachmentSet(); + return attachment_set_.get(); } - const FileDescriptorSet* file_descriptor_set() const { - return file_descriptor_set_.get(); + const MessageAttachmentSet* attachment_set() const { + return attachment_set_.get(); } -#endif #ifdef IPC_MESSAGE_LOG_ENABLED // Used for logging. diff --git a/chromium/ipc/ipc_message_attachment.cc b/chromium/ipc/ipc_message_attachment.cc new file mode 100644 index 00000000000..83440ae8e00 --- /dev/null +++ b/chromium/ipc/ipc_message_attachment.cc @@ -0,0 +1,15 @@ +// Copyright (c) 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/ipc_message_attachment.h" + +namespace IPC { + +MessageAttachment::MessageAttachment() { +} + +MessageAttachment::~MessageAttachment() { +} + +} // namespace IPC diff --git a/chromium/ipc/ipc_message_attachment.h b/chromium/ipc/ipc_message_attachment.h new file mode 100644 index 00000000000..ba7f0e83f90 --- /dev/null +++ b/chromium/ipc/ipc_message_attachment.h @@ -0,0 +1,41 @@ +// Copyright (c) 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_IPC_MESSAGE_ATTACHMENT_H_ +#define IPC_IPC_MESSAGE_ATTACHMENT_H_ + +#include "base/files/file.h" +#include "base/macros.h" +#include "base/memory/ref_counted.h" +#include "ipc/ipc_export.h" + +namespace IPC { + +// Auxiliary data sent with |Message|. This can be a platform file descriptor +// or a mojo |MessagePipe|. |GetType()| returns the type of the subclass. +class IPC_EXPORT MessageAttachment + : public base::RefCounted<MessageAttachment> { + public: + enum Type { + TYPE_PLATFORM_FILE, // The instance is |PlatformFileAttachment|. + TYPE_MOJO_HANDLE, // The instance is |MojoHandleAttachment|. + }; + + virtual Type GetType() const = 0; + +#if defined(OS_POSIX) + virtual base::PlatformFile TakePlatformFile() = 0; +#endif // OS_POSIX + + protected: + friend class base::RefCounted<MessageAttachment>; + MessageAttachment(); + virtual ~MessageAttachment(); + + DISALLOW_COPY_AND_ASSIGN(MessageAttachment); +}; + +} // namespace IPC + +#endif // IPC_IPC_MESSAGE_ATTACHMENT_H_ diff --git a/chromium/ipc/ipc_message_attachment_set.cc b/chromium/ipc/ipc_message_attachment_set.cc new file mode 100644 index 00000000000..cb74a5aaf07 --- /dev/null +++ b/chromium/ipc/ipc_message_attachment_set.cc @@ -0,0 +1,163 @@ +// Copyright (c) 2011 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/ipc_message_attachment_set.h" + +#include <algorithm> +#include "base/logging.h" +#include "base/posix/eintr_wrapper.h" +#include "ipc/ipc_message_attachment.h" + +#if defined(OS_POSIX) +#include <sys/types.h> +#include <sys/stat.h> +#include <unistd.h> +#include "ipc/ipc_platform_file_attachment_posix.h" +#endif // OS_POSIX + +namespace IPC { + +MessageAttachmentSet::MessageAttachmentSet() + : consumed_descriptor_highwater_(0) { +} + +MessageAttachmentSet::~MessageAttachmentSet() { + if (consumed_descriptor_highwater_ == size()) + return; + + // We close all the owning descriptors. If this message should have + // been transmitted, then closing those with close flags set mirrors + // the expected behaviour. + // + // If this message was received with more descriptors than expected + // (which could a DOS against the browser by a rogue renderer) then all + // the descriptors have their close flag set and we free all the extra + // kernel resources. + LOG(WARNING) << "MessageAttachmentSet destroyed with unconsumed descriptors: " + << consumed_descriptor_highwater_ << "/" << size(); +} + +unsigned MessageAttachmentSet::num_descriptors() const { + return std::count_if(attachments_.begin(), attachments_.end(), + [](scoped_refptr<MessageAttachment> i) { + return i->GetType() == MessageAttachment::TYPE_PLATFORM_FILE; + }); +} + +unsigned MessageAttachmentSet::num_mojo_handles() const { + return std::count_if(attachments_.begin(), attachments_.end(), + [](scoped_refptr<MessageAttachment> i) { + return i->GetType() == MessageAttachment::TYPE_MOJO_HANDLE; + }); +} + +unsigned MessageAttachmentSet::size() const { + return static_cast<unsigned>(attachments_.size()); +} + +bool MessageAttachmentSet::AddAttachment( + scoped_refptr<MessageAttachment> attachment) { +#if defined(OS_POSIX) + if (attachment->GetType() == MessageAttachment::TYPE_PLATFORM_FILE && + num_descriptors() == kMaxDescriptorsPerMessage) { + DLOG(WARNING) << "Cannot add file descriptor. MessageAttachmentSet full."; + return false; + } +#endif + + attachments_.push_back(attachment); + return true; +} + +scoped_refptr<MessageAttachment> MessageAttachmentSet::GetAttachmentAt( + unsigned index) { + if (index >= size()) { + DLOG(WARNING) << "Accessing out of bound index:" << index << "/" << size(); + return scoped_refptr<MessageAttachment>(); + } + + // We should always walk the descriptors in order, so it's reasonable to + // enforce this. Consider the case where a compromised renderer sends us + // the following message: + // + // ExampleMsg: + // num_fds:2 msg:FD(index = 1) control:SCM_RIGHTS {n, m} + // + // Here the renderer sent us a message which should have a descriptor, but + // actually sent two in an attempt to fill our fd table and kill us. By + // setting the index of the descriptor in the message to 1 (it should be + // 0), we would record a highwater of 1 and then consider all the + // descriptors to have been used. + // + // So we can either track of the use of each descriptor in a bitset, or we + // can enforce that we walk the indexes strictly in order. + // + // There's one more wrinkle: When logging messages, we may reparse them. So + // we have an exception: When the consumed_descriptor_highwater_ is at the + // end of the array and index 0 is requested, we reset the highwater value. + // TODO(morrita): This is absurd. This "wringle" disallow to introduce clearer + // ownership model. Only client is NaclIPCAdapter. See crbug.com/415294 + if (index == 0 && consumed_descriptor_highwater_ == size()) + consumed_descriptor_highwater_ = 0; + + if (index != consumed_descriptor_highwater_) + return scoped_refptr<MessageAttachment>(); + + consumed_descriptor_highwater_ = index + 1; + + return attachments_[index]; +} + +void MessageAttachmentSet::CommitAll() { + attachments_.clear(); + consumed_descriptor_highwater_ = 0; +} + +#if defined(OS_POSIX) + +void MessageAttachmentSet::PeekDescriptors(base::PlatformFile* buffer) const { + for (size_t i = 0; i != attachments_.size(); ++i) + buffer[i] = internal::GetPlatformFile(attachments_[i]); +} + +bool MessageAttachmentSet::ContainsDirectoryDescriptor() const { + struct stat st; + + for (auto i = attachments_.begin(); i != attachments_.end(); ++i) { + if (fstat(internal::GetPlatformFile(*i), &st) == 0 && S_ISDIR(st.st_mode)) + return true; + } + + return false; +} + +void MessageAttachmentSet::ReleaseFDsToClose( + std::vector<base::PlatformFile>* fds) { + for (size_t i = 0; i < attachments_.size(); ++i) { + internal::PlatformFileAttachment* file = + static_cast<internal::PlatformFileAttachment*>(attachments_[i].get()); + if (file->Owns()) + fds->push_back(file->TakePlatformFile()); + } + + CommitAll(); +} + +void MessageAttachmentSet::AddDescriptorsToOwn(const base::PlatformFile* buffer, + unsigned count) { + DCHECK(count <= kMaxDescriptorsPerMessage); + DCHECK_EQ(num_descriptors(), 0u); + DCHECK_EQ(consumed_descriptor_highwater_, 0u); + + attachments_.reserve(count); + for (unsigned i = 0; i < count; ++i) + AddAttachment( + new internal::PlatformFileAttachment(base::ScopedFD(buffer[i]))); +} + +#endif // OS_POSIX + +} // namespace IPC + + diff --git a/chromium/ipc/file_descriptor_set_posix.h b/chromium/ipc/ipc_message_attachment_set.h index d454962e84f..7e848bd3f54 100644 --- a/chromium/ipc/file_descriptor_set_posix.h +++ b/chromium/ipc/ipc_message_attachment_set.h @@ -2,67 +2,70 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef IPC_FILE_DESCRIPTOR_SET_POSIX_H_ -#define IPC_FILE_DESCRIPTOR_SET_POSIX_H_ +#ifndef IPC_IPC_MESSAGE_ATTACHMENT_SET_H_ +#define IPC_IPC_MESSAGE_ATTACHMENT_SET_H_ #include <vector> #include "base/basictypes.h" -#include "base/files/file.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_vector.h" #include "ipc/ipc_export.h" -// ----------------------------------------------------------------------------- -// A FileDescriptorSet is an ordered set of POSIX file descriptors. These are -// associated with IPC messages so that descriptors can be transmitted over a -// UNIX domain socket. -// ----------------------------------------------------------------------------- -class IPC_EXPORT FileDescriptorSet - : public base::RefCountedThreadSafe<FileDescriptorSet> { - public: - FileDescriptorSet(); - - // This is the maximum number of descriptors per message. We need to know this - // because the control message kernel interface has to be given a buffer which - // is large enough to store all the descriptor numbers. Otherwise the kernel - // tells us that it truncated the control data and the extra descriptors are - // lost. - // - // In debugging mode, it's a fatal error to try and add more than this number - // of descriptors to a FileDescriptorSet. - static const size_t kMaxDescriptorsPerMessage = 7; - - // --------------------------------------------------------------------------- - // Interfaces for building during message serialisation... +#if defined(OS_POSIX) +#include "base/files/file.h" +#endif - // Add a descriptor to the end of the set. Returns false iff the set is full. - bool AddToBorrow(base::PlatformFile fd); - // Add a descriptor to the end of the set and automatically close it after - // transmission. Returns false iff the set is full. - bool AddToOwn(base::ScopedFD fd); +namespace IPC { - // --------------------------------------------------------------------------- +class MessageAttachment; +// ----------------------------------------------------------------------------- +// A MessageAttachmentSet is an ordered set of MessageAttachment objects. These +// are associated with IPC messages so that attachments, each of which is either +// a platform file or a mojo handle, can be transmitted over the underlying UNIX +// domain socket (for ChannelPosix) or Mojo MessagePipe (for ChannelMojo). +// ----------------------------------------------------------------------------- +class IPC_EXPORT MessageAttachmentSet + : public base::RefCountedThreadSafe<MessageAttachmentSet> { + public: + MessageAttachmentSet(); - // --------------------------------------------------------------------------- - // Interfaces for accessing during message deserialisation... + // Return the number of attachments + unsigned size() const; + // Return the number of file descriptors + unsigned num_descriptors() const; + // Return the number of mojo handles in the attachment set + unsigned num_mojo_handles() const; - // Return the number of descriptors - unsigned size() const { return descriptors_.size(); } // Return true if no unconsumed descriptors remain bool empty() const { return 0 == size(); } - // Take the nth descriptor from the beginning of the set, - // transferring the ownership of the descriptor taken. Code using this - // /must/ access the descriptors in order, and must do it at most once. + + bool AddAttachment(scoped_refptr<MessageAttachment> attachment); + + // Take the nth attachment from the beginning of the set, Code using this + // /must/ access the attachments in order, and must do it at most once. // // This interface is designed for the deserialising code as it doesn't // support close flags. - // returns: file descriptor, or -1 on error - base::PlatformFile TakeDescriptorAt(unsigned n); + // returns: an attachment, or nullptr on error + scoped_refptr<MessageAttachment> GetAttachmentAt(unsigned index); - // --------------------------------------------------------------------------- + // This must be called after transmitting the descriptors returned by + // PeekDescriptors. It marks all the descriptors as consumed and closes those + // which are auto-close. + void CommitAll(); +#if defined(OS_POSIX) + // This is the maximum number of descriptors per message. We need to know this + // because the control message kernel interface has to be given a buffer which + // is large enough to store all the descriptor numbers. Otherwise the kernel + // tells us that it truncated the control data and the extra descriptors are + // lost. + // + // In debugging mode, it's a fatal error to try and add more than this number + // of descriptors to a MessageAttachmentSet. + static const size_t kMaxDescriptorsPerMessage = 7; // --------------------------------------------------------------------------- // Interfaces for transmission... @@ -71,10 +74,6 @@ class IPC_EXPORT FileDescriptorSet // must be called after these descriptors have been transmitted. // buffer: (output) a buffer of, at least, size() integers. void PeekDescriptors(base::PlatformFile* buffer) const; - // This must be called after transmitting the descriptors returned by - // PeekDescriptors. It marks all the descriptors as consumed and closes those - // which are auto-close. - void CommitAll(); // Returns true if any contained file descriptors appear to be handles to a // directory. bool ContainsDirectoryDescriptor() const; @@ -84,7 +83,6 @@ class IPC_EXPORT FileDescriptorSet // --------------------------------------------------------------------------- - // --------------------------------------------------------------------------- // Interfaces for receiving... @@ -93,19 +91,18 @@ class IPC_EXPORT FileDescriptorSet // unconsumed descriptors are closed on destruction. void AddDescriptorsToOwn(const base::PlatformFile* buffer, unsigned count); +#endif // OS_POSIX + // --------------------------------------------------------------------------- private: - friend class base::RefCountedThreadSafe<FileDescriptorSet>; + friend class base::RefCountedThreadSafe<MessageAttachmentSet>; - ~FileDescriptorSet(); + ~MessageAttachmentSet(); - // A vector of descriptors and close flags. If this message is sent, then - // these descriptors are sent as control data. After sending, any descriptors - // with a true flag are closed. If this message has been received, then these - // are the descriptors which were received and all close flags are true. - std::vector<base::PlatformFile> descriptors_; - ScopedVector<base::ScopedFD> owned_descriptors_; + // A vector of attachments of the message, which might be |PlatformFile| or + // |MessagePipe|. + std::vector<scoped_refptr<MessageAttachment>> attachments_; // This contains the index of the next descriptor which should be consumed. // It's used in a couple of ways. Firstly, at destruction we can check that @@ -113,7 +110,9 @@ class IPC_EXPORT FileDescriptorSet // can check that they are read in order. mutable unsigned consumed_descriptor_highwater_; - DISALLOW_COPY_AND_ASSIGN(FileDescriptorSet); + DISALLOW_COPY_AND_ASSIGN(MessageAttachmentSet); }; -#endif // IPC_FILE_DESCRIPTOR_SET_POSIX_H_ +} // namespace IPC + +#endif // IPC_IPC_MESSAGE_ATTACHMENT_SET_H_ diff --git a/chromium/ipc/ipc_message_attachment_set_posix_unittest.cc b/chromium/ipc/ipc_message_attachment_set_posix_unittest.cc new file mode 100644 index 00000000000..416a7d2589c --- /dev/null +++ b/chromium/ipc/ipc_message_attachment_set_posix_unittest.cc @@ -0,0 +1,201 @@ +// Copyright (c) 2011 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. + +// This test is POSIX only. + +#include "ipc/ipc_message_attachment_set.h" + +#include <fcntl.h> +#include <unistd.h> + +#include "base/basictypes.h" +#include "base/posix/eintr_wrapper.h" +#include "ipc/ipc_platform_file_attachment_posix.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace IPC { +namespace { + +// Get a safe file descriptor for test purposes. +int GetSafeFd() { + return open("/dev/null", O_RDONLY); +} + +// Returns true if fd was already closed. Closes fd if not closed. +bool VerifyClosed(int fd) { + const int duped = dup(fd); + if (duped != -1) { + EXPECT_NE(IGNORE_EINTR(close(duped)), -1); + EXPECT_NE(IGNORE_EINTR(close(fd)), -1); + return false; + } + return true; +} + +// The MessageAttachmentSet will try and close some of the descriptor numbers +// which we given it. This is the base descriptor value. It's great enough such +// that no real descriptor will accidently be closed. +static const int kFDBase = 50000; + +TEST(MessageAttachmentSet, BasicAdd) { + scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet); + + ASSERT_EQ(set->size(), 0u); + ASSERT_TRUE(set->empty()); + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase))); + ASSERT_EQ(set->size(), 1u); + ASSERT_TRUE(!set->empty()); + + // Empties the set and stops a warning about deleting a set with unconsumed + // descriptors + set->CommitAll(); +} + +TEST(MessageAttachmentSet, BasicAddAndClose) { + scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet); + + ASSERT_EQ(set->size(), 0u); + ASSERT_TRUE(set->empty()); + const int fd = GetSafeFd(); + ASSERT_TRUE(set->AddAttachment( + new internal::PlatformFileAttachment(base::ScopedFD(fd)))); + ASSERT_EQ(set->size(), 1u); + ASSERT_TRUE(!set->empty()); + + set->CommitAll(); + + ASSERT_TRUE(VerifyClosed(fd)); +} +TEST(MessageAttachmentSet, MaxSize) { + scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet); + + for (size_t i = 0; i < MessageAttachmentSet::kMaxDescriptorsPerMessage; ++i) + ASSERT_TRUE(set->AddAttachment( + new internal::PlatformFileAttachment(kFDBase + 1 + i))); + + ASSERT_TRUE( + !set->AddAttachment(new internal::PlatformFileAttachment(kFDBase))); + + set->CommitAll(); +} + +TEST(MessageAttachmentSet, SetDescriptors) { + scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet); + + ASSERT_TRUE(set->empty()); + set->AddDescriptorsToOwn(NULL, 0); + ASSERT_TRUE(set->empty()); + + const int fd = GetSafeFd(); + static const int fds[] = {fd}; + set->AddDescriptorsToOwn(fds, 1); + ASSERT_TRUE(!set->empty()); + ASSERT_EQ(set->size(), 1u); + + set->CommitAll(); + + ASSERT_TRUE(VerifyClosed(fd)); +} + +TEST(MessageAttachmentSet, PeekDescriptors) { + scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet); + + set->PeekDescriptors(NULL); + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase))); + + int fds[1]; + fds[0] = 0; + set->PeekDescriptors(fds); + ASSERT_EQ(fds[0], kFDBase); + set->CommitAll(); + ASSERT_TRUE(set->empty()); +} + +TEST(MessageAttachmentSet, WalkInOrder) { + scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet); + + // TODO(morrita): This test is wrong. TakeDescriptorAt() shouldn't be + // used to retrieve borrowed descriptors. That never happens in production. + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase))); + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase + 1))); + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase + 2))); + + ASSERT_EQ(set->GetAttachmentAt(0)->TakePlatformFile(), kFDBase); + ASSERT_EQ(set->GetAttachmentAt(1)->TakePlatformFile(), kFDBase + 1); + ASSERT_EQ(set->GetAttachmentAt(2)->TakePlatformFile(), kFDBase + 2); + + set->CommitAll(); +} + +TEST(MessageAttachmentSet, WalkWrongOrder) { + scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet); + + // TODO(morrita): This test is wrong. TakeDescriptorAt() shouldn't be + // used to retrieve borrowed descriptors. That never happens in production. + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase))); + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase + 1))); + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase + 2))); + + ASSERT_EQ(set->GetAttachmentAt(0)->TakePlatformFile(), kFDBase); + ASSERT_EQ(set->GetAttachmentAt(2), nullptr); + + set->CommitAll(); +} + +TEST(MessageAttachmentSet, WalkCycle) { + scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet); + + // TODO(morrita): This test is wrong. TakeDescriptorAt() shouldn't be + // used to retrieve borrowed descriptors. That never happens in production. + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase))); + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase + 1))); + ASSERT_TRUE( + set->AddAttachment(new internal::PlatformFileAttachment(kFDBase + 2))); + + ASSERT_EQ(set->GetAttachmentAt(0)->TakePlatformFile(), kFDBase); + ASSERT_EQ(set->GetAttachmentAt(1)->TakePlatformFile(), kFDBase + 1); + ASSERT_EQ(set->GetAttachmentAt(2)->TakePlatformFile(), kFDBase + 2); + ASSERT_EQ(set->GetAttachmentAt(0)->TakePlatformFile(), kFDBase); + ASSERT_EQ(set->GetAttachmentAt(1)->TakePlatformFile(), kFDBase + 1); + ASSERT_EQ(set->GetAttachmentAt(2)->TakePlatformFile(), kFDBase + 2); + ASSERT_EQ(set->GetAttachmentAt(0)->TakePlatformFile(), kFDBase); + ASSERT_EQ(set->GetAttachmentAt(1)->TakePlatformFile(), kFDBase + 1); + ASSERT_EQ(set->GetAttachmentAt(2)->TakePlatformFile(), kFDBase + 2); + + set->CommitAll(); +} + +TEST(MessageAttachmentSet, DontClose) { + scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet); + + const int fd = GetSafeFd(); + ASSERT_TRUE(set->AddAttachment(new internal::PlatformFileAttachment(fd))); + set->CommitAll(); + + ASSERT_FALSE(VerifyClosed(fd)); +} + +TEST(MessageAttachmentSet, DoClose) { + scoped_refptr<MessageAttachmentSet> set(new MessageAttachmentSet); + + const int fd = GetSafeFd(); + ASSERT_TRUE(set->AddAttachment( + new internal::PlatformFileAttachment(base::ScopedFD(fd)))); + set->CommitAll(); + + ASSERT_TRUE(VerifyClosed(fd)); +} + +} // namespace +} // namespace IPC diff --git a/chromium/ipc/ipc_message_macros.h b/chromium/ipc/ipc_message_macros.h index 20c83350733..e0e16d55925 100644 --- a/chromium/ipc/ipc_message_macros.h +++ b/chromium/ipc/ipc_message_macros.h @@ -457,7 +457,7 @@ void (T::*func)(P*, TA)) { \ Schema::Param p; \ if (Read(msg, &p)) { \ - (obj->*func)(parameter, p.a); \ + (obj->*func)(parameter, get<0>(p)); \ return true; \ } \ return false; \ @@ -469,7 +469,7 @@ void (T::*func)(P*, TA, TB)) { \ Schema::Param p; \ if (Read(msg, &p)) { \ - (obj->*func)(parameter, p.a, p.b); \ + (obj->*func)(parameter, get<0>(p), get<1>(p)); \ return true; \ } \ return false; \ @@ -481,7 +481,7 @@ void (T::*func)(P*, TA, TB, TC)) { \ Schema::Param p; \ if (Read(msg, &p)) { \ - (obj->*func)(parameter, p.a, p.b, p.c); \ + (obj->*func)(parameter, get<0>(p), get<1>(p), get<2>(p)); \ return true; \ } \ return false; \ @@ -494,7 +494,7 @@ void (T::*func)(P*, TA, TB, TC, TD)) { \ Schema::Param p; \ if (Read(msg, &p)) { \ - (obj->*func)(parameter, p.a, p.b, p.c, p.d); \ + (obj->*func)(parameter, get<0>(p), get<1>(p), get<2>(p), get<3>(p)); \ return true; \ } \ return false; \ @@ -507,7 +507,8 @@ void (T::*func)(P*, TA, TB, TC, TD, TE)) { \ Schema::Param p; \ if (Read(msg, &p)) { \ - (obj->*func)(parameter, p.a, p.b, p.c, p.d, p.e); \ + (obj->*func)(parameter, get<0>(p), get<1>(p), get<2>(p), get<3>(p), \ + get<4>(p)); \ return true; \ } \ return false; \ @@ -817,18 +818,18 @@ #define IPC_TYPE_OUT_3(t1, t2, t3) t1* arg6, t2* arg7, t3* arg8 #define IPC_TYPE_OUT_4(t1, t2, t3, t4) t1* arg6, t2* arg7, t3* arg8, t4* arg9 -#define IPC_TUPLE_IN_0() Tuple0 -#define IPC_TUPLE_IN_1(t1) Tuple1<t1> -#define IPC_TUPLE_IN_2(t1, t2) Tuple2<t1, t2> -#define IPC_TUPLE_IN_3(t1, t2, t3) Tuple3<t1, t2, t3> -#define IPC_TUPLE_IN_4(t1, t2, t3, t4) Tuple4<t1, t2, t3, t4> -#define IPC_TUPLE_IN_5(t1, t2, t3, t4, t5) Tuple5<t1, t2, t3, t4, t5> +#define IPC_TUPLE_IN_0() Tuple<> +#define IPC_TUPLE_IN_1(t1) Tuple<t1> +#define IPC_TUPLE_IN_2(t1, t2) Tuple<t1, t2> +#define IPC_TUPLE_IN_3(t1, t2, t3) Tuple<t1, t2, t3> +#define IPC_TUPLE_IN_4(t1, t2, t3, t4) Tuple<t1, t2, t3, t4> +#define IPC_TUPLE_IN_5(t1, t2, t3, t4, t5) Tuple<t1, t2, t3, t4, t5> -#define IPC_TUPLE_OUT_0() Tuple0 -#define IPC_TUPLE_OUT_1(t1) Tuple1<t1&> -#define IPC_TUPLE_OUT_2(t1, t2) Tuple2<t1&, t2&> -#define IPC_TUPLE_OUT_3(t1, t2, t3) Tuple3<t1&, t2&, t3&> -#define IPC_TUPLE_OUT_4(t1, t2, t3, t4) Tuple4<t1&, t2&, t3&, t4&> +#define IPC_TUPLE_OUT_0() Tuple<> +#define IPC_TUPLE_OUT_1(t1) Tuple<t1&> +#define IPC_TUPLE_OUT_2(t1, t2) Tuple<t1&, t2&> +#define IPC_TUPLE_OUT_3(t1, t2, t3) Tuple<t1&, t2&, t3&> +#define IPC_TUPLE_OUT_4(t1, t2, t3, t4) Tuple<t1&, t2&, t3&, t4&> #define IPC_NAME_IN_0() MakeTuple() #define IPC_NAME_IN_1(t1) MakeRefTuple(arg1) @@ -966,14 +967,7 @@ #endif // IPC_IPC_MESSAGE_MACROS_H_ -// The following #ifdef cannot be removed. Although the code is semantically -// equivalent without the #ifdef, VS2013 contains a bug where it is -// over-aggressive in optimizing out #includes. Putting the #ifdef is a -// workaround for this bug. See http://goo.gl/eGt2Fb for more details. -// This can be removed once VS2013 is fixed. -#ifdef IPC_MESSAGE_START // Clean up IPC_MESSAGE_START in this unguarded section so that the // XXX_messages.h files need not do so themselves. This makes the // XXX_messages.h files easier to write. #undef IPC_MESSAGE_START -#endif diff --git a/chromium/ipc/ipc_message_start.h b/chromium/ipc/ipc_message_start.h index f7b4be8f526..d4966bc4196 100644 --- a/chromium/ipc/ipc_message_start.h +++ b/chromium/ipc/ipc_message_start.h @@ -35,7 +35,6 @@ enum IPCMessageStart { AutofillMsgStart, SafeBrowsingMsgStart, P2PMsgStart, - SocketStreamMsgStart, ResourceMsgStart, FileSystemMsgStart, ChildProcessMsgStart, @@ -45,8 +44,6 @@ enum IPCMessageStart { DeviceLightMsgStart, DeviceMotionMsgStart, DeviceOrientationMsgStart, - DesktopNotificationMsgStart, - GeolocationMsgStart, AudioMsgStart, MidiMsgStart, ChromeMsgStart, @@ -86,6 +83,7 @@ enum IPCMessageStart { LocalDiscoveryMsgStart, PowerMonitorMsgStart, EncryptedMediaMsgStart, + CacheStorageMsgStart, ServiceWorkerMsgStart, MessagePortMsgStart, EmbeddedWorkerMsgStart, @@ -100,7 +98,6 @@ enum IPCMessageStart { CldDataProviderMsgStart, PushMessagingMsgStart, GinJavaBridgeMsgStart, - BatteryStatusMsgStart, ChromeUtilityPrintingMsgStart, AecDumpMsgStart, OzoneGpuMsgStart, @@ -113,6 +110,20 @@ enum IPCMessageStart { ExtensionUtilityMsgStart, GeofencingMsgStart, LayoutTestMsgStart, + NetworkHintsMsgStart, + BluetoothMsgStart, + NavigatorConnectMsgStart, + CastMediaMsgStart, + AwMessagePortMsgStart, + ExtensionsGuestViewMsgStart, + GuestViewMsgStart, + // Note: CastCryptoMsgStart and CastChannelMsgStart reserved for Chromecast + // internal code. Contact gunsch@ before changing/removing. + CastCryptoMsgStart, + CastChannelMsgStart, + DataReductionProxyStart, + ContentSettingsMsgStart, + ChromeAppBannerMsgStart, LastIPCMsgStart // Must come last. }; diff --git a/chromium/ipc/ipc_message_utils.cc b/chromium/ipc/ipc_message_utils.cc index be8795b00ef..adb14d1d56c 100644 --- a/chromium/ipc/ipc_message_utils.cc +++ b/chromium/ipc/ipc_message_utils.cc @@ -13,10 +13,14 @@ #include "base/time/time.h" #include "base/values.h" #include "ipc/ipc_channel_handle.h" +#include "ipc/ipc_message_attachment.h" +#include "ipc/ipc_message_attachment_set.h" #if defined(OS_POSIX) -#include "ipc/file_descriptor_set_posix.h" -#elif defined(OS_WIN) +#include "ipc/ipc_platform_file_attachment_posix.h" +#endif + +#if defined(OS_WIN) #include <tchar.h> #endif @@ -175,7 +179,7 @@ bool ReadValue(const Message* m, PickleIterator* iter, base::Value** value, switch (type) { case base::Value::TYPE_NULL: - *value = base::Value::CreateNullValue(); + *value = base::Value::CreateNullValue().release(); break; case base::Value::TYPE_BOOLEAN: { bool val; @@ -208,7 +212,7 @@ bool ReadValue(const Message* m, PickleIterator* iter, base::Value** value, case base::Value::TYPE_BINARY: { const char* data; int length; - if (!m->ReadData(iter, &data, &length)) + if (!iter->ReadData(&data, &length)) return false; *value = base::BinaryValue::CreateWithCopiedBuffer(data, length); break; @@ -260,7 +264,7 @@ void ParamTraits<unsigned char>::Write(Message* m, const param_type& p) { bool ParamTraits<unsigned char>::Read(const Message* m, PickleIterator* iter, param_type* r) { const char* data; - if (!m->ReadBytes(iter, &data, sizeof(param_type))) + if (!iter->ReadBytes(&data, sizeof(param_type))) return false; memcpy(r, data, sizeof(param_type)); return true; @@ -277,7 +281,7 @@ void ParamTraits<unsigned short>::Write(Message* m, const param_type& p) { bool ParamTraits<unsigned short>::Read(const Message* m, PickleIterator* iter, param_type* r) { const char* data; - if (!m->ReadBytes(iter, &data, sizeof(param_type))) + if (!iter->ReadBytes(&data, sizeof(param_type))) return false; memcpy(r, data, sizeof(param_type)); return true; @@ -322,7 +326,7 @@ void ParamTraits<double>::Write(Message* m, const param_type& p) { bool ParamTraits<double>::Read(const Message* m, PickleIterator* iter, param_type* r) { const char *data; - if (!m->ReadBytes(iter, &data, sizeof(*r))) { + if (!iter->ReadBytes(&data, sizeof(*r))) { NOTREACHED(); return false; } @@ -339,15 +343,9 @@ void ParamTraits<std::string>::Log(const param_type& p, std::string* l) { l->append(p); } -void ParamTraits<std::wstring>::Log(const param_type& p, std::string* l) { - l->append(base::WideToUTF8(p)); -} - -#if !defined(WCHAR_T_IS_UTF16) void ParamTraits<base::string16>::Log(const param_type& p, std::string* l) { l->append(base::UTF16ToUTF8(p)); } -#endif void ParamTraits<std::vector<char> >::Write(Message* m, const param_type& p) { if (p.empty()) { @@ -362,7 +360,7 @@ bool ParamTraits<std::vector<char> >::Read(const Message* m, param_type* r) { const char *data; int data_size = 0; - if (!m->ReadData(iter, &data, &data_size) || data_size < 0) + if (!iter->ReadData(&data, &data_size) || data_size < 0) return false; r->resize(data_size); if (data_size) @@ -389,7 +387,7 @@ bool ParamTraits<std::vector<unsigned char> >::Read(const Message* m, param_type* r) { const char *data; int data_size = 0; - if (!m->ReadData(iter, &data, &data_size) || data_size < 0) + if (!iter->ReadData(&data, &data_size) || data_size < 0) return false; r->resize(data_size); if (data_size) @@ -416,7 +414,7 @@ bool ParamTraits<std::vector<bool> >::Read(const Message* m, param_type* r) { int size; // ReadLength() checks for < 0 itself. - if (!m->ReadLength(iter, &size)) + if (!iter->ReadLength(&size)) return false; r->resize(size); for (int i = 0; i < size; i++) { @@ -466,10 +464,11 @@ void ParamTraits<base::FileDescriptor>::Write(Message* m, const param_type& p) { return; if (p.auto_close) { - if (!m->WriteFile(base::ScopedFD(p.fd))) + if (!m->WriteAttachment( + new internal::PlatformFileAttachment(base::ScopedFD(p.fd)))) NOTREACHED(); } else { - if (!m->WriteBorrowingFile(p.fd)) + if (!m->WriteAttachment(new internal::PlatformFileAttachment(p.fd))) NOTREACHED(); } } @@ -487,11 +486,11 @@ bool ParamTraits<base::FileDescriptor>::Read(const Message* m, if (!valid) return true; - base::ScopedFD fd; - if (!m->ReadFile(iter, &fd)) + scoped_refptr<MessageAttachment> attachment; + if (!m->ReadAttachment(iter, &attachment)) return false; - *r = base::FileDescriptor(fd.release(), true); + *r = base::FileDescriptor(attachment->TakePlatformFile(), true); return true; } @@ -727,7 +726,7 @@ void ParamTraits<Message>::Write(Message* m, const Message& p) { #if defined(OS_POSIX) // We don't serialize the file descriptors in the nested message, so there // better not be any. - DCHECK(!p.HasFileDescriptors()); + DCHECK(!p.HasAttachments()); #endif // Don't just write out the message. This is used to send messages between @@ -749,14 +748,14 @@ void ParamTraits<Message>::Write(Message* m, const Message& p) { bool ParamTraits<Message>::Read(const Message* m, PickleIterator* iter, Message* r) { uint32 routing_id, type, flags; - if (!m->ReadUInt32(iter, &routing_id) || - !m->ReadUInt32(iter, &type) || - !m->ReadUInt32(iter, &flags)) + if (!iter->ReadUInt32(&routing_id) || + !iter->ReadUInt32(&type) || + !iter->ReadUInt32(&flags)) return false; int payload_size; const char* payload; - if (!m->ReadData(iter, &payload, &payload_size)) + if (!iter->ReadData(&payload, &payload_size)) return false; r->SetHeaderValues(static_cast<int32>(routing_id), type, flags); @@ -777,7 +776,7 @@ void ParamTraits<HANDLE>::Write(Message* m, const param_type& p) { bool ParamTraits<HANDLE>::Read(const Message* m, PickleIterator* iter, param_type* r) { int32 temp; - if (!m->ReadInt(iter, &temp)) + if (!iter->ReadInt(&temp)) return false; *r = LongToHandle(temp); return true; @@ -795,7 +794,7 @@ bool ParamTraits<LOGFONT>::Read(const Message* m, PickleIterator* iter, param_type* r) { const char *data; int data_size = 0; - if (m->ReadData(iter, &data, &data_size) && data_size == sizeof(LOGFONT)) { + if (iter->ReadData(&data, &data_size) && data_size == sizeof(LOGFONT)) { const LOGFONT *font = reinterpret_cast<LOGFONT*>(const_cast<char*>(data)); if (_tcsnlen(font->lfFaceName, LF_FACESIZE) < LF_FACESIZE) { memcpy(r, data, sizeof(LOGFONT)); @@ -819,7 +818,7 @@ bool ParamTraits<MSG>::Read(const Message* m, PickleIterator* iter, param_type* r) { const char *data; int data_size = 0; - bool result = m->ReadData(iter, &data, &data_size); + bool result = iter->ReadData(&data, &data_size); if (result && data_size == sizeof(MSG)) { memcpy(r, data, sizeof(MSG)); } else { diff --git a/chromium/ipc/ipc_message_utils.h b/chromium/ipc/ipc_message_utils.h index 8351b178b2b..6787c8e2eb8 100644 --- a/chromium/ipc/ipc_message_utils.h +++ b/chromium/ipc/ipc_message_utils.h @@ -117,7 +117,7 @@ struct ParamTraits<bool> { m->WriteBool(p); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return m->ReadBool(iter, r); + return iter->ReadBool(r); } IPC_EXPORT static void Log(const param_type& p, std::string* l); }; @@ -145,7 +145,7 @@ struct ParamTraits<int> { m->WriteInt(p); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return m->ReadInt(iter, r); + return iter->ReadInt(r); } IPC_EXPORT static void Log(const param_type& p, std::string* l); }; @@ -157,7 +157,7 @@ struct ParamTraits<unsigned int> { m->WriteInt(p); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return m->ReadInt(iter, reinterpret_cast<int*>(r)); + return iter->ReadInt(reinterpret_cast<int*>(r)); } IPC_EXPORT static void Log(const param_type& p, std::string* l); }; @@ -169,7 +169,7 @@ struct ParamTraits<long> { m->WriteLongUsingDangerousNonPortableLessPersistableForm(p); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return m->ReadLong(iter, r); + return iter->ReadLong(r); } IPC_EXPORT static void Log(const param_type& p, std::string* l); }; @@ -181,7 +181,7 @@ struct ParamTraits<unsigned long> { m->WriteLongUsingDangerousNonPortableLessPersistableForm(p); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return m->ReadLong(iter, reinterpret_cast<long*>(r)); + return iter->ReadLong(reinterpret_cast<long*>(r)); } IPC_EXPORT static void Log(const param_type& p, std::string* l); }; @@ -194,7 +194,7 @@ struct ParamTraits<long long> { } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return m->ReadInt64(iter, reinterpret_cast<int64*>(r)); + return iter->ReadInt64(reinterpret_cast<int64*>(r)); } IPC_EXPORT static void Log(const param_type& p, std::string* l); }; @@ -207,7 +207,7 @@ struct ParamTraits<unsigned long long> { } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return m->ReadInt64(iter, reinterpret_cast<int64*>(r)); + return iter->ReadInt64(reinterpret_cast<int64*>(r)); } IPC_EXPORT static void Log(const param_type& p, std::string* l); }; @@ -222,7 +222,7 @@ struct IPC_EXPORT ParamTraits<float> { m->WriteFloat(p); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return m->ReadFloat(iter, r); + return iter->ReadFloat(r); } static void Log(const param_type& p, std::string* l); }; @@ -245,28 +245,12 @@ struct ParamTraits<std::string> { } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return m->ReadString(iter, r); + return iter->ReadString(r); } IPC_EXPORT static void Log(const param_type& p, std::string* l); }; template <> -struct ParamTraits<std::wstring> { - typedef std::wstring param_type; - static void Write(Message* m, const param_type& p) { - m->WriteWString(p); - } - static bool Read(const Message* m, PickleIterator* iter, - param_type* r) { - return m->ReadWString(iter, r); - } - IPC_EXPORT static void Log(const param_type& p, std::string* l); -}; - -// If WCHAR_T_IS_UTF16 is defined, then string16 is a std::wstring so we don't -// need this trait. -#if !defined(WCHAR_T_IS_UTF16) -template <> struct ParamTraits<base::string16> { typedef base::string16 param_type; static void Write(Message* m, const param_type& p) { @@ -274,11 +258,10 @@ struct ParamTraits<base::string16> { } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return m->ReadString16(iter, r); + return iter->ReadString16(r); } IPC_EXPORT static void Log(const param_type& p, std::string* l); }; -#endif template <> struct IPC_EXPORT ParamTraits<std::vector<char> > { @@ -316,7 +299,7 @@ struct ParamTraits<std::vector<P> > { param_type* r) { int size; // ReadLength() checks for < 0 itself. - if (!m->ReadLength(iter, &size)) + if (!iter->ReadLength(&size)) return false; // Resizing beforehand is not safe, see BUG 1006367 for details. if (INT_MAX / sizeof(P) <= static_cast<size_t>(size)) @@ -349,7 +332,7 @@ struct ParamTraits<std::set<P> > { static bool Read(const Message* m, PickleIterator* iter, param_type* r) { int size; - if (!m->ReadLength(iter, &size)) + if (!iter->ReadLength(&size)) return false; for (int i = 0; i < size; ++i) { P item; @@ -520,8 +503,8 @@ struct IPC_EXPORT ParamTraits<base::TimeTicks> { }; template <> -struct ParamTraits<Tuple0> { - typedef Tuple0 param_type; +struct ParamTraits<Tuple<>> { + typedef Tuple<> param_type; static void Write(Message* m, const param_type& p) { } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { @@ -532,112 +515,112 @@ struct ParamTraits<Tuple0> { }; template <class A> -struct ParamTraits< Tuple1<A> > { - typedef Tuple1<A> param_type; +struct ParamTraits<Tuple<A>> { + typedef Tuple<A> param_type; static void Write(Message* m, const param_type& p) { - WriteParam(m, p.a); + WriteParam(m, get<0>(p)); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return ReadParam(m, iter, &r->a); + return ReadParam(m, iter, &get<0>(*r)); } static void Log(const param_type& p, std::string* l) { - LogParam(p.a, l); + LogParam(get<0>(p), l); } }; template <class A, class B> -struct ParamTraits< Tuple2<A, B> > { - typedef Tuple2<A, B> param_type; +struct ParamTraits< Tuple<A, B> > { + typedef Tuple<A, B> param_type; static void Write(Message* m, const param_type& p) { - WriteParam(m, p.a); - WriteParam(m, p.b); + WriteParam(m, get<0>(p)); + WriteParam(m, get<1>(p)); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return (ReadParam(m, iter, &r->a) && - ReadParam(m, iter, &r->b)); + return (ReadParam(m, iter, &get<0>(*r)) && + ReadParam(m, iter, &get<1>(*r))); } static void Log(const param_type& p, std::string* l) { - LogParam(p.a, l); + LogParam(get<0>(p), l); l->append(", "); - LogParam(p.b, l); + LogParam(get<1>(p), l); } }; template <class A, class B, class C> -struct ParamTraits< Tuple3<A, B, C> > { - typedef Tuple3<A, B, C> param_type; +struct ParamTraits< Tuple<A, B, C> > { + typedef Tuple<A, B, C> param_type; static void Write(Message* m, const param_type& p) { - WriteParam(m, p.a); - WriteParam(m, p.b); - WriteParam(m, p.c); + WriteParam(m, get<0>(p)); + WriteParam(m, get<1>(p)); + WriteParam(m, get<2>(p)); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return (ReadParam(m, iter, &r->a) && - ReadParam(m, iter, &r->b) && - ReadParam(m, iter, &r->c)); + return (ReadParam(m, iter, &get<0>(*r)) && + ReadParam(m, iter, &get<1>(*r)) && + ReadParam(m, iter, &get<2>(*r))); } static void Log(const param_type& p, std::string* l) { - LogParam(p.a, l); + LogParam(get<0>(p), l); l->append(", "); - LogParam(p.b, l); + LogParam(get<1>(p), l); l->append(", "); - LogParam(p.c, l); + LogParam(get<2>(p), l); } }; template <class A, class B, class C, class D> -struct ParamTraits< Tuple4<A, B, C, D> > { - typedef Tuple4<A, B, C, D> param_type; +struct ParamTraits< Tuple<A, B, C, D> > { + typedef Tuple<A, B, C, D> param_type; static void Write(Message* m, const param_type& p) { - WriteParam(m, p.a); - WriteParam(m, p.b); - WriteParam(m, p.c); - WriteParam(m, p.d); + WriteParam(m, get<0>(p)); + WriteParam(m, get<1>(p)); + WriteParam(m, get<2>(p)); + WriteParam(m, get<3>(p)); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return (ReadParam(m, iter, &r->a) && - ReadParam(m, iter, &r->b) && - ReadParam(m, iter, &r->c) && - ReadParam(m, iter, &r->d)); + return (ReadParam(m, iter, &get<0>(*r)) && + ReadParam(m, iter, &get<1>(*r)) && + ReadParam(m, iter, &get<2>(*r)) && + ReadParam(m, iter, &get<3>(*r))); } static void Log(const param_type& p, std::string* l) { - LogParam(p.a, l); + LogParam(get<0>(p), l); l->append(", "); - LogParam(p.b, l); + LogParam(get<1>(p), l); l->append(", "); - LogParam(p.c, l); + LogParam(get<2>(p), l); l->append(", "); - LogParam(p.d, l); + LogParam(get<3>(p), l); } }; template <class A, class B, class C, class D, class E> -struct ParamTraits< Tuple5<A, B, C, D, E> > { - typedef Tuple5<A, B, C, D, E> param_type; +struct ParamTraits< Tuple<A, B, C, D, E> > { + typedef Tuple<A, B, C, D, E> param_type; static void Write(Message* m, const param_type& p) { - WriteParam(m, p.a); - WriteParam(m, p.b); - WriteParam(m, p.c); - WriteParam(m, p.d); - WriteParam(m, p.e); + WriteParam(m, get<0>(p)); + WriteParam(m, get<1>(p)); + WriteParam(m, get<2>(p)); + WriteParam(m, get<3>(p)); + WriteParam(m, get<4>(p)); } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { - return (ReadParam(m, iter, &r->a) && - ReadParam(m, iter, &r->b) && - ReadParam(m, iter, &r->c) && - ReadParam(m, iter, &r->d) && - ReadParam(m, iter, &r->e)); + return (ReadParam(m, iter, &get<0>(*r)) && + ReadParam(m, iter, &get<1>(*r)) && + ReadParam(m, iter, &get<2>(*r)) && + ReadParam(m, iter, &get<3>(*r)) && + ReadParam(m, iter, &get<4>(*r))); } static void Log(const param_type& p, std::string* l) { - LogParam(p.a, l); + LogParam(get<0>(p), l); l->append(", "); - LogParam(p.b, l); + LogParam(get<1>(p), l); l->append(", "); - LogParam(p.c, l); + LogParam(get<2>(p), l); l->append(", "); - LogParam(p.d, l); + LogParam(get<3>(p), l); l->append(", "); - LogParam(p.e, l); + LogParam(get<4>(p), l); } }; @@ -651,7 +634,7 @@ struct ParamTraits<ScopedVector<P> > { } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { int size = 0; - if (!m->ReadLength(iter, &size)) + if (!iter->ReadLength(&size)) return false; if (INT_MAX/sizeof(P) <= static_cast<size_t>(size)) return false; @@ -690,7 +673,7 @@ struct ParamTraits<base::SmallMap<NormalMap, kArraySize, EqualKey, MapInit> > { } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { int size; - if (!m->ReadLength(iter, &size)) + if (!iter->ReadLength(&size)) return false; for (int i = 0; i < size; ++i) { K key; @@ -865,7 +848,8 @@ class ParamDeserializer : public MessageReplyDeserializer { public: explicit ParamDeserializer(const RefTuple& out) : out_(out) { } - bool SerializeOutputParameters(const IPC::Message& msg, PickleIterator iter) { + bool SerializeOutputParameters(const IPC::Message& msg, + PickleIterator iter) override { return ReadParam(&msg, &iter, &out_); } @@ -911,7 +895,7 @@ class SyncMessageSchema { Method func) { Message* reply = SyncMessage::GenerateReply(msg); if (ok) { - Tuple1<Message&> t = MakeRefTuple(*reply); + Tuple<Message&> t = MakeRefTuple(*reply); ConnectMessageAndReply(msg, reply); DispatchToMethod(obj, func, send_params, &t); } else { @@ -922,33 +906,9 @@ class SyncMessageSchema { return ok; } - template<typename TA> - static void WriteReplyParams(Message* reply, TA a) { - ReplyParam p(a); - WriteParam(reply, p); - } - - template<typename TA, typename TB> - static void WriteReplyParams(Message* reply, TA a, TB b) { - ReplyParam p(a, b); - WriteParam(reply, p); - } - - template<typename TA, typename TB, typename TC> - static void WriteReplyParams(Message* reply, TA a, TB b, TC c) { - ReplyParam p(a, b, c); - WriteParam(reply, p); - } - - template<typename TA, typename TB, typename TC, typename TD> - static void WriteReplyParams(Message* reply, TA a, TB b, TC c, TD d) { - ReplyParam p(a, b, c, d); - WriteParam(reply, p); - } - - template<typename TA, typename TB, typename TC, typename TD, typename TE> - static void WriteReplyParams(Message* reply, TA a, TB b, TC c, TD d, TE e) { - ReplyParam p(a, b, c, d, e); + template <typename... Ts> + static void WriteReplyParams(Message* reply, Ts... args) { + ReplyParam p(args...); WriteParam(reply, p); } }; diff --git a/chromium/ipc/ipc_nacl.gyp b/chromium/ipc/ipc_nacl.gyp index 6f0d522d562..5faf9af1f4c 100644 --- a/chromium/ipc/ipc_nacl.gyp +++ b/chromium/ipc/ipc_nacl.gyp @@ -26,7 +26,6 @@ }, 'dependencies': [ '../base/base_nacl.gyp:base_nacl', - '../native_client/tools.gyp:prep_toolchain', ], }, { @@ -56,7 +55,6 @@ ], 'dependencies': [ '../base/base_nacl.gyp:base_nacl_nonsfi', - '../native_client/tools.gyp:prep_toolchain', ], }, ], diff --git a/chromium/ipc/ipc_perftest_support.cc b/chromium/ipc/ipc_perftest_support.cc index d0cbe5ef803..ae8be7fb2d1 100644 --- a/chromium/ipc/ipc_perftest_support.cc +++ b/chromium/ipc/ipc_perftest_support.cc @@ -26,6 +26,10 @@ namespace IPC { namespace test { +// Avoid core 0 due to conflicts with Intel's Power Gadget. +// Setting thread affinity will fail harmlessly on single/dual core machines. +const int kSharedCore = 2; + // This class simply collects stats about abstract "events" (each of which has a // start time and an end time). class EventTimeTracker { @@ -99,8 +103,8 @@ class ChannelReflectorListener : public Listener { EXPECT_TRUE(iter.ReadInt64(&time_internal)); int msgid; EXPECT_TRUE(iter.ReadInt(&msgid)); - std::string payload; - EXPECT_TRUE(iter.ReadString(&payload)); + base::StringPiece payload; + EXPECT_TRUE(iter.ReadStringPiece(&payload)); // Include message deserialization in latency. base::TimeTicks now = base::TimeTicks::Now(); @@ -229,7 +233,7 @@ IPCChannelPerfTestBase::GetDefaultTestParams() { list.push_back(PingPongTestParams(144, 50000)); list.push_back(PingPongTestParams(1728, 50000)); list.push_back(PingPongTestParams(20736, 12000)); - list.push_back(PingPongTestParams(248832, 100)); + list.push_back(PingPongTestParams(248832, 1000)); return list; } @@ -244,6 +248,7 @@ void IPCChannelPerfTestBase::RunTestChannelPingPong( ASSERT_TRUE(ConnectChannel()); ASSERT_TRUE(StartClient()); + LockThreadAffinity thread_locker(kSharedCore); for (size_t i = 0; i < params.size(); i++) { listener.SetTestParams(params[i].message_count(), params[i].message_size()); @@ -284,6 +289,7 @@ void IPCChannelPerfTestBase::RunTestChannelProxyPingPong( listener.Init(channel_proxy()); ASSERT_TRUE(StartClient()); + LockThreadAffinity thread_locker(kSharedCore); for (size_t i = 0; i < params.size(); i++) { listener.SetTestParams(params[i].message_count(), params[i].message_size()); @@ -326,6 +332,7 @@ scoped_ptr<Channel> PingPongTestClient::CreateChannel( } int PingPongTestClient::RunMain() { + LockThreadAffinity thread_locker(kSharedCore); scoped_ptr<Channel> channel = CreateChannel(listener_.get()); listener_->Init(channel.get()); CHECK(channel->Connect()); @@ -335,7 +342,39 @@ int PingPongTestClient::RunMain() { } scoped_refptr<base::TaskRunner> PingPongTestClient::task_runner() { - return main_message_loop_.message_loop_proxy(); + return main_message_loop_.task_runner(); +} + +LockThreadAffinity::LockThreadAffinity(int cpu_number) + : affinity_set_ok_(false) { +#if defined(OS_WIN) + const DWORD_PTR thread_mask = 1 << cpu_number; + old_affinity_ = SetThreadAffinityMask(GetCurrentThread(), thread_mask); + affinity_set_ok_ = old_affinity_ != 0; +#elif defined(OS_LINUX) + cpu_set_t cpuset; + CPU_ZERO(&cpuset); + CPU_SET(cpu_number, &cpuset); + auto get_result = sched_getaffinity(0, sizeof(old_cpuset_), &old_cpuset_); + DCHECK_EQ(0, get_result); + auto set_result = sched_setaffinity(0, sizeof(cpuset), &cpuset); + // Check for get_result failure, even though it should always succeed. + affinity_set_ok_ = (set_result == 0) && (get_result == 0); +#endif + if (!affinity_set_ok_) + LOG(WARNING) << "Failed to set thread affinity to CPU " << cpu_number; +} + +LockThreadAffinity::~LockThreadAffinity() { + if (!affinity_set_ok_) + return; +#if defined(OS_WIN) + auto set_result = SetThreadAffinityMask(GetCurrentThread(), old_affinity_); + DCHECK_NE(0u, set_result); +#elif defined(OS_LINUX) + auto set_result = sched_setaffinity(0, sizeof(old_cpuset_), &old_cpuset_); + DCHECK_EQ(0, set_result); +#endif } } // namespace test diff --git a/chromium/ipc/ipc_perftest_support.h b/chromium/ipc/ipc_perftest_support.h index 611a2096330..578256f8b11 100644 --- a/chromium/ipc/ipc_perftest_support.h +++ b/chromium/ipc/ipc_perftest_support.h @@ -53,6 +53,27 @@ class PingPongTestClient { scoped_ptr<Channel> channel_; }; +// This class locks the current thread to a particular CPU core. This is +// important because otherwise the different threads and processes of these +// tests end up on different CPU cores which means that all of the cores are +// lightly loaded so the OS (Windows and Linux) fails to ramp up the CPU +// frequency, leading to unpredictable and often poor performance. +class LockThreadAffinity { + public: + explicit LockThreadAffinity(int cpu_number); + ~LockThreadAffinity(); + + private: + bool affinity_set_ok_; +#if defined(OS_WIN) + DWORD_PTR old_affinity_; +#elif defined(OS_LINUX) + cpu_set_t old_cpuset_; +#endif + + DISALLOW_COPY_AND_ASSIGN(LockThreadAffinity); +}; + } } diff --git a/chromium/ipc/ipc_platform_file_attachment_posix.cc b/chromium/ipc/ipc_platform_file_attachment_posix.cc new file mode 100644 index 00000000000..b704750c156 --- /dev/null +++ b/chromium/ipc/ipc_platform_file_attachment_posix.cc @@ -0,0 +1,37 @@ +// Copyright (c) 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/ipc_platform_file_attachment_posix.h" + +namespace IPC { +namespace internal { + +PlatformFileAttachment::PlatformFileAttachment(base::PlatformFile file) + : file_(file) { +} + +PlatformFileAttachment::PlatformFileAttachment(base::ScopedFD file) + : file_(file.get()), owning_(file.Pass()) { +} + +PlatformFileAttachment::~PlatformFileAttachment() { +} + +MessageAttachment::Type PlatformFileAttachment::GetType() const { + return TYPE_PLATFORM_FILE; +} + +base::PlatformFile PlatformFileAttachment::TakePlatformFile() { + ignore_result(owning_.release()); + return file_; +} + +base::PlatformFile GetPlatformFile( + scoped_refptr<MessageAttachment> attachment) { + DCHECK_EQ(attachment->GetType(), MessageAttachment::TYPE_PLATFORM_FILE); + return static_cast<PlatformFileAttachment*>(attachment.get())->file(); +} + +} // namespace internal +} // namespace IPC diff --git a/chromium/ipc/ipc_platform_file_attachment_posix.h b/chromium/ipc/ipc_platform_file_attachment_posix.h new file mode 100644 index 00000000000..d1eff609c53 --- /dev/null +++ b/chromium/ipc/ipc_platform_file_attachment_posix.h @@ -0,0 +1,43 @@ +// Copyright (c) 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_IPC_PLATFORM_FILE_ATTACHMENT_H_ +#define IPC_IPC_PLATFORM_FILE_ATTACHMENT_H_ + +#include "ipc/ipc_export.h" +#include "ipc/ipc_message_attachment.h" + +namespace IPC { +namespace internal { + +// A platform file that is sent over |Channel| as a part of |Message|. +// PlatformFileAttachment optionally owns the file and |owning_| is set in that +// case. Also, |file_| is not cleared even after the ownership is taken. +// Some old clients require this strange behavior. +class IPC_EXPORT PlatformFileAttachment : public MessageAttachment { + public: + // Non-owning constructor + explicit PlatformFileAttachment(base::PlatformFile file); + // Owning constructor + explicit PlatformFileAttachment(base::ScopedFD file); + + Type GetType() const override; + base::PlatformFile TakePlatformFile() override; + + base::PlatformFile file() const { return file_; } + bool Owns() const { return owning_.is_valid(); } + + private: + ~PlatformFileAttachment() override; + + base::PlatformFile file_; + base::ScopedFD owning_; +}; + +base::PlatformFile GetPlatformFile(scoped_refptr<MessageAttachment> attachment); + +} // namespace internal +} // namespace IPC + +#endif // IPC_IPC_PLATFORM_FILE_ATTACHMENT_H_ diff --git a/chromium/ipc/ipc_security_test_util.cc b/chromium/ipc/ipc_security_test_util.cc new file mode 100644 index 00000000000..4ae5a06096d --- /dev/null +++ b/chromium/ipc/ipc_security_test_util.cc @@ -0,0 +1,25 @@ +// Copyright 2014 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/ipc_security_test_util.h" + +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/run_loop.h" +#include "ipc/ipc_channel_proxy.h" + +namespace IPC { + +void IpcSecurityTestUtil::PwnMessageReceived(ChannelProxy* channel, + const IPC::Message& message) { + base::RunLoop run_loop; + base::Closure inject_message = base::Bind( + base::IgnoreResult(&IPC::ChannelProxy::Context::OnMessageReceived), + channel->context(), message); + channel->context()->ipc_task_runner()->PostTaskAndReply( + FROM_HERE, inject_message, run_loop.QuitClosure()); + run_loop.Run(); +} + +} // namespace IPC diff --git a/chromium/ipc/ipc_security_test_util.h b/chromium/ipc/ipc_security_test_util.h new file mode 100644 index 00000000000..1ec2555fce7 --- /dev/null +++ b/chromium/ipc/ipc_security_test_util.h @@ -0,0 +1,40 @@ +// Copyright 2014 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_IPC_SECURITY_TEST_UTIL_H_ +#define IPC_IPC_SECURITY_TEST_UTIL_H_ + +#include "base/basictypes.h" + +namespace IPC { + +class ChannelProxy; +class Message; + +class IpcSecurityTestUtil { + public: + // Enables testing of security exploit scenarios where a compromised child + // process can send a malicious message of an arbitrary type. + // + // This function will post the message to the IPC channel's thread, where it + // is offered to the channel's listeners. Afterwards, a reply task is posted + // back to the current thread. This function blocks until the reply task is + // received. For messages forwarded back to the current thread, we won't + // return until after the message has been handled here. + // + // Use this only for testing security bugs in a browsertest; other uses are + // likely perilous. Unit tests should be using IPC::TestSink which has an + // OnMessageReceived method you can call directly. Non-security browsertests + // should just exercise the child process's normal codepaths to send messages. + static void PwnMessageReceived(ChannelProxy* channel, const Message& message); + + private: + IpcSecurityTestUtil(); // Not instantiable. + + DISALLOW_COPY_AND_ASSIGN(IpcSecurityTestUtil); +}; + +} // namespace IPC + +#endif // IPC_IPC_SECURITY_TEST_UTIL_H_ diff --git a/chromium/ipc/ipc_send_fds_test.cc b/chromium/ipc/ipc_send_fds_test.cc index cf2c68003de..148eecab125 100644 --- a/chromium/ipc/ipc_send_fds_test.cc +++ b/chromium/ipc/ipc_send_fds_test.cc @@ -19,28 +19,40 @@ extern "C" { #include "base/callback.h" #include "base/file_descriptor_posix.h" -#include "base/message_loop/message_loop.h" +#include "base/location.h" #include "base/pickle.h" #include "base/posix/eintr_wrapper.h" +#include "base/single_thread_task_runner.h" #include "base/synchronization/waitable_event.h" +#include "ipc/ipc_message_attachment_set.h" #include "ipc/ipc_message_utils.h" #include "ipc/ipc_test_base.h" +#if defined(OS_POSIX) +#include "base/macros.h" +#endif + namespace { -const unsigned kNumFDsToSend = 20; +const unsigned kNumFDsToSend = 7; // per message +const unsigned kNumMessages = 20; const char* kDevZeroPath = "/dev/zero"; +#if defined(OS_POSIX) +static_assert(kNumFDsToSend == + IPC::MessageAttachmentSet::kMaxDescriptorsPerMessage, + "The number of FDs to send must be kMaxDescriptorsPerMessage."); +#endif + class MyChannelDescriptorListenerBase : public IPC::Listener { public: bool OnMessageReceived(const IPC::Message& message) override { PickleIterator iter(message); - base::FileDescriptor descriptor; - - IPC::ParamTraits<base::FileDescriptor>::Read(&message, &iter, &descriptor); - - HandleFD(descriptor.fd); + while (IPC::ParamTraits<base::FileDescriptor>::Read( + &message, &iter, &descriptor)) { + HandleFD(descriptor.fd); + } return true; } @@ -57,7 +69,7 @@ class MyChannelDescriptorListener : public MyChannelDescriptorListenerBase { } bool GotExpectedNumberOfDescriptors() const { - return num_fds_received_ == kNumFDsToSend; + return num_fds_received_ == kNumFDsToSend * kNumMessages; } void OnChannelError() override { @@ -66,6 +78,7 @@ class MyChannelDescriptorListener : public MyChannelDescriptorListenerBase { protected: void HandleFD(int fd) override { + ASSERT_GE(fd, 0); // Check that we can read from the FD. char buf; ssize_t amt_read = read(fd, &buf, 1); @@ -82,7 +95,7 @@ class MyChannelDescriptorListener : public MyChannelDescriptorListenerBase { ASSERT_EQ(expected_inode_num_, st.st_ino); ++num_fds_received_; - if (num_fds_received_ == kNumFDsToSend) + if (num_fds_received_ == kNumFDsToSend * kNumMessages) base::MessageLoop::current()->Quit(); } @@ -101,14 +114,15 @@ class IPCSendFdsTest : public IPCTestBase { ASSERT_TRUE(ConnectChannel()); ASSERT_TRUE(StartClient()); - for (unsigned i = 0; i < kNumFDsToSend; ++i) { - const int fd = open(kDevZeroPath, O_RDONLY); - ASSERT_GE(fd, 0); - base::FileDescriptor descriptor(fd, true); - + for (unsigned i = 0; i < kNumMessages; ++i) { IPC::Message* message = new IPC::Message(0, 3, IPC::Message::PRIORITY_NORMAL); - IPC::ParamTraits<base::FileDescriptor>::Write(message, descriptor); + for (unsigned j = 0; j < kNumFDsToSend; ++j) { + const int fd = open(kDevZeroPath, O_RDONLY); + ASSERT_GE(fd, 0); + base::FileDescriptor descriptor(fd, true); + IPC::ParamTraits<base::FileDescriptor>::Write(message, descriptor); + } ASSERT_TRUE(sender()->Send(message)); } @@ -237,12 +251,10 @@ class PipeChannelHelper { out = IPC::Channel::CreateClient(out_handle, &cb_listener_); // PostTask the connect calls to make sure the callbacks happens // on the right threads. - in_thread_->message_loop()->PostTask( - FROM_HERE, - base::Bind(&PipeChannelHelper::Connect, in.get())); - out_thread_->message_loop()->PostTask( - FROM_HERE, - base::Bind(&PipeChannelHelper::Connect, out.get())); + in_thread_->task_runner()->PostTask( + FROM_HERE, base::Bind(&PipeChannelHelper::Connect, in.get())); + out_thread_->task_runner()->PostTask( + FROM_HERE, base::Bind(&PipeChannelHelper::Connect, out.get())); } static void DestroyChannel(scoped_ptr<IPC::Channel> *c, @@ -254,12 +266,10 @@ class PipeChannelHelper { ~PipeChannelHelper() { base::WaitableEvent a(true, false); base::WaitableEvent b(true, false); - in_thread_->message_loop()->PostTask( - FROM_HERE, - base::Bind(&PipeChannelHelper::DestroyChannel, &in, &a)); - out_thread_->message_loop()->PostTask( - FROM_HERE, - base::Bind(&PipeChannelHelper::DestroyChannel, &out, &b)); + in_thread_->task_runner()->PostTask( + FROM_HERE, base::Bind(&PipeChannelHelper::DestroyChannel, &in, &a)); + out_thread_->task_runner()->PostTask( + FROM_HERE, base::Bind(&PipeChannelHelper::DestroyChannel, &out, &b)); a.Wait(); b.Wait(); } @@ -308,11 +318,9 @@ class IPCMultiSendingFdsTest : public testing::Test { for (int i = 0; i < pipes_to_send; i++) { received_.Reset(); std::pair<int, int> pipe_fds = make_socket_pair(); - t->message_loop()->PostTask( - FROM_HERE, - base::Bind(&PipeChannelHelper::Send, - base::Unretained(dest), - pipe_fds.second)); + t->task_runner()->PostTask( + FROM_HERE, base::Bind(&PipeChannelHelper::Send, + base::Unretained(dest), pipe_fds.second)); char tmp = 'x'; CHECK_EQ(1, HANDLE_EINTR(write(pipe_fds.first, &tmp, 1))); CHECK_EQ(0, IGNORE_EINTR(close(pipe_fds.first))); diff --git a/chromium/ipc/ipc_sync_channel.cc b/chromium/ipc/ipc_sync_channel.cc index a6ef27ada67..06cd46f1ea6 100644 --- a/chromium/ipc/ipc_sync_channel.cc +++ b/chromium/ipc/ipc_sync_channel.cc @@ -5,7 +5,6 @@ #include "ipc/ipc_sync_channel.h" #include "base/bind.h" -#include "base/debug/trace_event.h" #include "base/lazy_instance.h" #include "base/location.h" #include "base/logging.h" @@ -13,6 +12,7 @@ #include "base/synchronization/waitable_event_watcher.h" #include "base/thread_task_runner_handle.h" #include "base/threading/thread_local.h" +#include "base/trace_event/trace_event.h" #include "ipc/ipc_channel_factory.h" #include "ipc/ipc_logging.h" #include "ipc/ipc_message_macros.h" diff --git a/chromium/ipc/ipc_sync_channel_unittest.cc b/chromium/ipc/ipc_sync_channel_unittest.cc index f834ec3b5bb..7e81d5deabc 100644 --- a/chromium/ipc/ipc_sync_channel_unittest.cc +++ b/chromium/ipc/ipc_sync_channel_unittest.cc @@ -9,13 +9,15 @@ #include "base/basictypes.h" #include "base/bind.h" +#include "base/location.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" -#include "base/message_loop/message_loop.h" #include "base/process/process_handle.h" #include "base/run_loop.h" +#include "base/single_thread_task_runner.h" #include "base/strings/string_util.h" #include "base/synchronization/waitable_event.h" +#include "base/thread_task_runner_handle.h" #include "base/threading/platform_thread.h" #include "base/threading/thread.h" #include "ipc/ipc_listener.h" @@ -72,7 +74,7 @@ class Worker : public Listener, public Sender { } void Start() { StartThread(&listener_thread_, base::MessageLoop::TYPE_DEFAULT); - ListenerThread()->message_loop()->PostTask( + ListenerThread()->task_runner()->PostTask( FROM_HERE, base::Bind(&Worker::OnStart, this)); } void Shutdown() { @@ -80,7 +82,7 @@ class Worker : public Listener, public Sender { // ~Worker(), since that'll reset the vtable pointer (to Worker's), which // may result in a race conditions. See http://crbug.com/25841. WaitableEvent listener_done(false, false), ipc_done(false, false); - ListenerThread()->message_loop()->PostTask( + ListenerThread()->task_runner()->PostTask( FROM_HERE, base::Bind(&Worker::OnListenerThreadShutdown1, this, &listener_done, &ipc_done)); listener_done.Wait(); @@ -124,7 +126,7 @@ class Worker : public Listener, public Sender { protected: SyncChannel* channel() { return channel_.get(); } - // Functions for dervied classes to implement if they wish. + // Functions for derived classes to implement if they wish. virtual void Run() { } virtual void OnAnswer(int* answer) { NOTREACHED(); } virtual void OnAnswerDelay(Message* reply_msg) { @@ -152,8 +154,8 @@ class Worker : public Listener, public Sender { virtual SyncChannel* CreateChannel() { scoped_ptr<SyncChannel> channel = SyncChannel::Create( - channel_name_, mode_, this, ipc_thread_.message_loop_proxy().get(), - true, &shutdown_event_); + channel_name_, mode_, this, ipc_thread_.task_runner().get(), true, + &shutdown_event_); return channel.release(); } @@ -190,9 +192,9 @@ class Worker : public Listener, public Sender { base::RunLoop().RunUntilIdle(); ipc_event->Signal(); - listener_thread_.message_loop()->PostTask( - FROM_HERE, base::Bind(&Worker::OnListenerThreadShutdown2, this, - listener_event)); + listener_thread_.task_runner()->PostTask( + FROM_HERE, + base::Bind(&Worker::OnListenerThreadShutdown2, this, listener_event)); } void OnListenerThreadShutdown2(WaitableEvent* listener_event) { @@ -324,8 +326,7 @@ class TwoStepServer : public Worker { SyncChannel* CreateChannel() override { SyncChannel* channel = SyncChannel::Create(channel_name(), mode(), this, - ipc_thread().message_loop_proxy().get(), - create_pipe_now_, + ipc_thread().task_runner().get(), create_pipe_now_, shutdown_event()).release(); return channel; } @@ -347,8 +348,7 @@ class TwoStepClient : public Worker { SyncChannel* CreateChannel() override { SyncChannel* channel = SyncChannel::Create(channel_name(), mode(), this, - ipc_thread().message_loop_proxy().get(), - create_pipe_now_, + ipc_thread().task_runner().get(), create_pipe_now_, shutdown_event()).release(); return channel; } @@ -896,11 +896,10 @@ class DoneEventRaceServer : public Worker { : Worker(Channel::MODE_SERVER, "done_event_race_server") { } void Run() override { - base::MessageLoop::current()->PostTask(FROM_HERE, - base::Bind(&NestedCallback, this)); - base::MessageLoop::current()->PostDelayedTask( - FROM_HERE, - base::Bind(&TimeoutCallback), + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::Bind(&NestedCallback, this)); + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, base::Bind(&TimeoutCallback), base::TimeDelta::FromSeconds(9)); // Even though we have a timeout on the Send, it will succeed since for this // bug, the reply message comes back and is deserialized, however the done @@ -926,17 +925,17 @@ TEST_F(IPCSyncChannelTest, DoneEventRace) { class TestSyncMessageFilter : public SyncMessageFilter { public: - TestSyncMessageFilter(base::WaitableEvent* shutdown_event, - Worker* worker, - scoped_refptr<base::MessageLoopProxy> message_loop) + TestSyncMessageFilter( + base::WaitableEvent* shutdown_event, + Worker* worker, + scoped_refptr<base::SingleThreadTaskRunner> task_runner) : SyncMessageFilter(shutdown_event), worker_(worker), - message_loop_(message_loop) { - } + task_runner_(task_runner) {} void OnFilterAdded(Sender* sender) override { SyncMessageFilter::OnFilterAdded(sender); - message_loop_->PostTask( + task_runner_->PostTask( FROM_HERE, base::Bind(&TestSyncMessageFilter::SendMessageOnHelperThread, this)); } @@ -954,7 +953,7 @@ class TestSyncMessageFilter : public SyncMessageFilter { ~TestSyncMessageFilter() override {} Worker* worker_; - scoped_refptr<base::MessageLoopProxy> message_loop_; + scoped_refptr<base::SingleThreadTaskRunner> task_runner_; }; class SyncMessageFilterServer : public Worker { @@ -966,7 +965,7 @@ class SyncMessageFilterServer : public Worker { options.message_loop_type = base::MessageLoop::TYPE_DEFAULT; thread_.StartWithOptions(options); filter_ = new TestSyncMessageFilter(shutdown_event(), this, - thread_.message_loop_proxy()); + thread_.task_runner()); } void Run() override { @@ -987,7 +986,7 @@ class ServerSendAfterClose : public Worker { } bool SendDummy() { - ListenerThread()->message_loop()->PostTask( + ListenerThread()->task_runner()->PostTask( FROM_HERE, base::Bind(base::IgnoreResult(&ServerSendAfterClose::Send), this, new SyncChannelTestMsg_NoArgs)); return true; @@ -1053,7 +1052,7 @@ class RestrictedDispatchServer : public Worker { Send(msg); // Signal the event after the message has been sent on the channel, on the // IPC thread. - ipc_thread().message_loop()->PostTask( + ipc_thread().task_runner()->PostTask( FROM_HERE, base::Bind(&RestrictedDispatchServer::OnPingSent, this)); } @@ -1128,7 +1127,7 @@ class RestrictedDispatchClient : public Worker { // send a message on that same channel. channel()->SetRestrictDispatchChannelGroup(1); - server_->ListenerThread()->message_loop()->PostTask( + server_->ListenerThread()->task_runner()->PostTask( FROM_HERE, base::Bind(&RestrictedDispatchServer::OnDoPing, server_, 1)); sent_ping_event_->Wait(); Send(new SyncChannelTestMsg_NoArgs); @@ -1137,15 +1136,11 @@ class RestrictedDispatchClient : public Worker { else LOG(ERROR) << "Send failed to dispatch incoming message on same channel"; - non_restricted_channel_ = - SyncChannel::Create("non_restricted_channel", - IPC::Channel::MODE_CLIENT, - this, - ipc_thread().message_loop_proxy().get(), - true, - shutdown_event()); + non_restricted_channel_ = SyncChannel::Create( + "non_restricted_channel", IPC::Channel::MODE_CLIENT, this, + ipc_thread().task_runner().get(), true, shutdown_event()); - server_->ListenerThread()->message_loop()->PostTask( + server_->ListenerThread()->task_runner()->PostTask( FROM_HERE, base::Bind(&RestrictedDispatchServer::OnDoPing, server_, 2)); sent_ping_event_->Wait(); // Check that the incoming message is *not* dispatched when sending on the @@ -1170,7 +1165,7 @@ class RestrictedDispatchClient : public Worker { // Check that the incoming message on the non-restricted channel is // dispatched when sending on the restricted channel. - server2_->ListenerThread()->message_loop()->PostTask( + server2_->ListenerThread()->task_runner()->PostTask( FROM_HERE, base::Bind(&NonRestrictedDispatchServer::OnDoPingTTL, server2_, 3)); int value = 0; @@ -1392,10 +1387,10 @@ class RestrictedDispatchDeadlockClient1 : public Worker { void Run() override { server_ready_event_->Wait(); - server_->ListenerThread()->message_loop()->PostTask( + server_->ListenerThread()->task_runner()->PostTask( FROM_HERE, base::Bind(&RestrictedDispatchDeadlockServer::OnDoServerTask, server_)); - peer_->ListenerThread()->message_loop()->PostTask( + peer_->ListenerThread()->task_runner()->PostTask( FROM_HERE, base::Bind(&RestrictedDispatchDeadlockClient2::OnDoClient2Task, peer_)); events_[0]->Wait(); @@ -1528,13 +1523,9 @@ class RestrictedDispatchPipeWorker : public Worker { if (is_first()) event1_->Signal(); event2_->Wait(); - other_channel_ = - SyncChannel::Create(other_channel_name_, - IPC::Channel::MODE_CLIENT, - this, - ipc_thread().message_loop_proxy().get(), - true, - shutdown_event()); + other_channel_ = SyncChannel::Create( + other_channel_name_, IPC::Channel::MODE_CLIENT, this, + ipc_thread().task_runner().get(), true, shutdown_event()); other_channel_->SetRestrictDispatchChannelGroup(group_); if (!is_first()) { event1_->Signal(); @@ -1608,13 +1599,9 @@ class ReentrantReplyServer1 : public Worker { server_ready_(server_ready) { } void Run() override { - server2_channel_ = - SyncChannel::Create("reentrant_reply2", - IPC::Channel::MODE_CLIENT, - this, - ipc_thread().message_loop_proxy().get(), - true, - shutdown_event()); + server2_channel_ = SyncChannel::Create( + "reentrant_reply2", IPC::Channel::MODE_CLIENT, this, + ipc_thread().task_runner().get(), true, shutdown_event()); server_ready_->Signal(); Message* msg = new SyncChannelTestMsg_Reentrant1(); server2_channel_->Send(msg); diff --git a/chromium/ipc/ipc_sync_message.cc b/chromium/ipc/ipc_sync_message.cc index 9e3acf8e1fa..fd6dc471d59 100644 --- a/chromium/ipc/ipc_sync_message.cc +++ b/chromium/ipc/ipc_sync_message.cc @@ -113,7 +113,7 @@ bool SyncMessage::ReadSyncHeader(const Message& msg, SyncHeader* header) { DCHECK(msg.is_sync() || msg.is_reply()); PickleIterator iter(msg); - bool result = msg.ReadInt(&iter, &header->message_id); + bool result = iter.ReadInt(&header->message_id); if (!result) { NOTREACHED(); return false; diff --git a/chromium/ipc/ipc_sync_message_filter.cc b/chromium/ipc/ipc_sync_message_filter.cc index e2ea1bfb1ee..6a408ec12cf 100644 --- a/chromium/ipc/ipc_sync_message_filter.cc +++ b/chromium/ipc/ipc_sync_message_filter.cc @@ -7,33 +7,33 @@ #include "base/bind.h" #include "base/location.h" #include "base/logging.h" -#include "base/message_loop/message_loop_proxy.h" +#include "base/single_thread_task_runner.h" #include "base/synchronization/waitable_event.h" +#include "base/thread_task_runner_handle.h" #include "ipc/ipc_channel.h" #include "ipc/ipc_sync_message.h" -using base::MessageLoopProxy; - namespace IPC { SyncMessageFilter::SyncMessageFilter(base::WaitableEvent* shutdown_event) : sender_(NULL), - listener_loop_(MessageLoopProxy::current()), + listener_task_runner_(base::ThreadTaskRunnerHandle::Get()), shutdown_event_(shutdown_event) { } bool SyncMessageFilter::Send(Message* message) { { base::AutoLock auto_lock(lock_); - if (!io_loop_.get()) { + if (!io_task_runner_.get()) { delete message; return false; } } if (!message->is_sync()) { - io_loop_->PostTask( - FROM_HERE, base::Bind(&SyncMessageFilter::SendOnIOThread, this, message)); + io_task_runner_->PostTask( + FROM_HERE, + base::Bind(&SyncMessageFilter::SendOnIOThread, this, message)); return true; } @@ -47,12 +47,14 @@ bool SyncMessageFilter::Send(Message* message) { base::AutoLock auto_lock(lock_); // Can't use this class on the main thread or else it can lead to deadlocks. // Also by definition, can't use this on IO thread since we're blocking it. - DCHECK(MessageLoopProxy::current().get() != listener_loop_.get()); - DCHECK(MessageLoopProxy::current().get() != io_loop_.get()); + if (base::ThreadTaskRunnerHandle::IsSet()) { + DCHECK(base::ThreadTaskRunnerHandle::Get() != listener_task_runner_); + DCHECK(base::ThreadTaskRunnerHandle::Get() != io_task_runner_); + } pending_sync_messages_.insert(&pending_message); } - io_loop_->PostTask( + io_task_runner_->PostTask( FROM_HERE, base::Bind(&SyncMessageFilter::SendOnIOThread, this, message)); base::WaitableEvent* events[2] = { shutdown_event_, &done_event }; @@ -70,7 +72,7 @@ bool SyncMessageFilter::Send(Message* message) { void SyncMessageFilter::OnFilterAdded(Sender* sender) { sender_ = sender; base::AutoLock auto_lock(lock_); - io_loop_ = MessageLoopProxy::current(); + io_task_runner_ = base::ThreadTaskRunnerHandle::Get(); } void SyncMessageFilter::OnChannelError() { diff --git a/chromium/ipc/ipc_sync_message_filter.h b/chromium/ipc/ipc_sync_message_filter.h index 49c3ca03171..ee6677ef310 100644 --- a/chromium/ipc/ipc_sync_message_filter.h +++ b/chromium/ipc/ipc_sync_message_filter.h @@ -15,7 +15,7 @@ #include "ipc/message_filter.h" namespace base { -class MessageLoopProxy; +class SingleThreadTaskRunner; class WaitableEvent; } @@ -51,10 +51,10 @@ class IPC_EXPORT SyncMessageFilter : public MessageFilter, public Sender { Sender* sender_; // The process's main thread. - scoped_refptr<base::MessageLoopProxy> listener_loop_; + scoped_refptr<base::SingleThreadTaskRunner> listener_task_runner_; // The message loop where the Channel lives. - scoped_refptr<base::MessageLoopProxy> io_loop_; + scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_; typedef std::set<PendingSyncMsg*> PendingSyncMessages; PendingSyncMessages pending_sync_messages_; diff --git a/chromium/ipc/ipc_test_base.cc b/chromium/ipc/ipc_test_base.cc index ba4e71156af..0d662822691 100644 --- a/chromium/ipc/ipc_test_base.cc +++ b/chromium/ipc/ipc_test_base.cc @@ -4,6 +4,7 @@ #include "build/build_config.h" +#include "base/single_thread_task_runner.h" #include "ipc/ipc_test_base.h" #include "base/command_line.h" @@ -22,8 +23,7 @@ std::string IPCTestBase::GetChannelName(const std::string& test_client_name) { return test_client_name + "__Channel"; } -IPCTestBase::IPCTestBase() - : client_process_(base::kNullProcessHandle) { +IPCTestBase::IPCTestBase() { } IPCTestBase::~IPCTestBase() { @@ -104,8 +104,8 @@ std::string IPCTestBase::GetTestMainName() const { } bool IPCTestBase::DidStartClient() { - DCHECK_NE(base::kNullProcessHandle, client_process_); - return client_process_ != base::kNullProcessHandle; + DCHECK(client_process_.IsValid()); + return client_process_.IsValid(); } #if defined(OS_POSIX) @@ -117,7 +117,7 @@ bool IPCTestBase::StartClient() { } bool IPCTestBase::StartClientWithFD(int ipcfd) { - DCHECK_EQ(client_process_, base::kNullProcessHandle); + DCHECK(!client_process_.IsValid()); base::FileHandleMappingVector fds_to_map; if (ipcfd > -1) @@ -133,7 +133,7 @@ bool IPCTestBase::StartClientWithFD(int ipcfd) { #elif defined(OS_WIN) bool IPCTestBase::StartClient() { - DCHECK_EQ(client_process_, base::kNullProcessHandle); + DCHECK(!client_process_.IsValid()); client_process_ = SpawnChild(GetTestMainName()); return DidStartClient(); } @@ -141,12 +141,12 @@ bool IPCTestBase::StartClient() { #endif bool IPCTestBase::WaitForClientShutdown() { - DCHECK(client_process_ != base::kNullProcessHandle); + DCHECK(client_process_.IsValid()); - bool rv = base::WaitForSingleProcess(client_process_, - base::TimeDelta::FromSeconds(5)); - base::CloseProcessHandle(client_process_); - client_process_ = base::kNullProcessHandle; + int exit_code; + bool rv = client_process_.WaitForExitWithTimeout( + base::TimeDelta::FromSeconds(5), &exit_code); + client_process_.Close(); return rv; } @@ -154,12 +154,12 @@ IPC::ChannelHandle IPCTestBase::GetTestChannelHandle() { return GetChannelName(test_client_name_); } -scoped_refptr<base::TaskRunner> IPCTestBase::task_runner() { - return message_loop_->message_loop_proxy(); +scoped_refptr<base::SequencedTaskRunner> IPCTestBase::task_runner() { + return message_loop_->task_runner(); } scoped_ptr<IPC::ChannelFactory> IPCTestBase::CreateChannelFactory( const IPC::ChannelHandle& handle, - base::TaskRunner* runner) { + base::SequencedTaskRunner* runner) { return IPC::ChannelFactory::Create(handle, IPC::Channel::MODE_SERVER); } diff --git a/chromium/ipc/ipc_test_base.h b/chromium/ipc/ipc_test_base.h index 89d53db58cd..2a45f7230ef 100644 --- a/chromium/ipc/ipc_test_base.h +++ b/chromium/ipc/ipc_test_base.h @@ -32,9 +32,9 @@ class IPCTestBase : public base::MultiProcessTest { protected: IPCTestBase(); - virtual ~IPCTestBase(); + ~IPCTestBase() override; - virtual void TearDown() override; + void TearDown() override; // Initializes the test to use the given client and creates an IO message loop // on the current thread. @@ -100,11 +100,11 @@ class IPCTestBase : public base::MultiProcessTest { IPC::Channel* channel() { return channel_.get(); } IPC::ChannelProxy* channel_proxy() { return channel_proxy_.get(); } - const base::ProcessHandle& client_process() const { return client_process_; } - scoped_refptr<base::TaskRunner> task_runner(); + const base::Process& client_process() const { return client_process_; } + scoped_refptr<base::SequencedTaskRunner> task_runner(); virtual scoped_ptr<IPC::ChannelFactory> CreateChannelFactory( - const IPC::ChannelHandle& handle, base::TaskRunner* runner); + const IPC::ChannelHandle& handle, base::SequencedTaskRunner* runner); virtual bool DidStartClient(); @@ -117,7 +117,7 @@ class IPCTestBase : public base::MultiProcessTest { scoped_ptr<IPC::Channel> channel_; scoped_ptr<IPC::ChannelProxy> channel_proxy_; - base::ProcessHandle client_process_; + base::Process client_process_; DISALLOW_COPY_AND_ASSIGN(IPCTestBase); }; diff --git a/chromium/ipc/ipc_test_sink.cc b/chromium/ipc/ipc_test_sink.cc index 1f76c0484c3..53d29fc7845 100644 --- a/chromium/ipc/ipc_test_sink.cc +++ b/chromium/ipc/ipc_test_sink.cc @@ -41,7 +41,7 @@ base::ProcessId TestSink::GetSelfPID() const { } bool TestSink::OnMessageReceived(const Message& msg) { - ObserverListBase<Listener>::Iterator it(filter_list_); + ObserverListBase<Listener>::Iterator it(&filter_list_); Listener* observer; while ((observer = it.GetNext()) != NULL) { if (observer->OnMessageReceived(msg)) diff --git a/chromium/ipc/mojo/BUILD.gn b/chromium/ipc/mojo/BUILD.gn index 324cc9ac9aa..50241fe4641 100644 --- a/chromium/ipc/mojo/BUILD.gn +++ b/chromium/ipc/mojo/BUILD.gn @@ -2,7 +2,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -import("//mojo/public/tools/bindings/mojom.gni") +import("//testing/test.gni") +import("//third_party/mojo/src/mojo/public/tools/bindings/mojom.gni") mojom("client_channel") { sources = [ @@ -12,17 +13,25 @@ mojom("client_channel") { component("mojo") { sources = [ + "async_handle_waiter.cc", + "async_handle_waiter.h", "client_channel.mojom", "ipc_channel_mojo.cc", "ipc_channel_mojo.h", "ipc_channel_mojo_host.cc", "ipc_channel_mojo_host.h", - "ipc_channel_mojo_readers.cc", - "ipc_channel_mojo_readers.h", - "ipc_mojo_bootstrap.cc", - "ipc_mojo_bootstrap.h", "ipc_message_pipe_reader.cc", "ipc_message_pipe_reader.h", + "ipc_mojo_bootstrap.cc", + "ipc_mojo_bootstrap.h", + "ipc_mojo_handle_attachment.cc", + "ipc_mojo_handle_attachment.h", + "ipc_mojo_message_helper.cc", + "ipc_mojo_message_helper.h", + "ipc_mojo_param_traits.cc", + "ipc_mojo_param_traits.h", + "scoped_ipc_support.cc", + "scoped_ipc_support.h", ] defines = [ "IPC_MOJO_IMPLEMENTATION" ] @@ -32,14 +41,16 @@ component("mojo") { "//base/third_party/dynamic_annotations", "//ipc", "//mojo/environment:chromium", - "//mojo/public/cpp/bindings", - "//mojo/edk/system", + "//third_party/mojo/src/mojo/edk/system", + "//third_party/mojo/src/mojo/public/c/environment:environment", + "//third_party/mojo/src/mojo/public/cpp/bindings", ":client_channel", ] } test("ipc_mojo_unittests") { sources = [ + "async_handle_waiter_unittest.cc", "ipc_channel_mojo_unittest.cc", "ipc_mojo_bootstrap_unittest.cc", "run_all_unittests.cc", @@ -52,8 +63,9 @@ test("ipc_mojo_unittests") { "//ipc", "//ipc:test_support", "//ipc/mojo", - "//mojo/edk/system", "//mojo/environment:chromium", + "//testing/gtest", + "//third_party/mojo/src/mojo/edk/system", "//url", ] } @@ -71,8 +83,8 @@ test("ipc_mojo_perftests") { "//ipc", "//ipc:test_support", "//ipc/mojo", - "//mojo/edk/system", "//mojo/environment:chromium", + "//third_party/mojo/src/mojo/edk/system", "//url", ] } diff --git a/chromium/ipc/mojo/DEPS b/chromium/ipc/mojo/DEPS index 039e2432bf7..fb36693916b 100644 --- a/chromium/ipc/mojo/DEPS +++ b/chromium/ipc/mojo/DEPS @@ -1,4 +1,4 @@ include_rules = [ - "+mojo/public", - "+mojo/edk/embedder", + "+third_party/mojo/src/mojo/public", + "+third_party/mojo/src/mojo/edk/embedder", ] diff --git a/chromium/ipc/mojo/async_handle_waiter.cc b/chromium/ipc/mojo/async_handle_waiter.cc new file mode 100644 index 00000000000..b9e68d7c453 --- /dev/null +++ b/chromium/ipc/mojo/async_handle_waiter.cc @@ -0,0 +1,165 @@ +// 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/mojo/async_handle_waiter.h" + +#include "base/atomic_ref_count.h" +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/location.h" +#include "base/logging.h" +#include "third_party/mojo/src/mojo/edk/embedder/embedder.h" + +namespace IPC { +namespace internal { + +class AsyncHandleWaiterContextTraits { + public: + static void Destruct(const AsyncHandleWaiter::Context* context); +}; + +// The thread-safe part of |AsyncHandleWaiter|. +// As |AsyncWait()| invokes the given callback from an arbitrary thread, +// |HandleIsReady()| and the bound |this| have to be thread-safe. +class AsyncHandleWaiter::Context + : public base::RefCountedThreadSafe<AsyncHandleWaiter::Context, + AsyncHandleWaiterContextTraits>, + public base::MessageLoopForIO::IOObserver { + public: + Context(base::WeakPtr<AsyncHandleWaiter> waiter) + : io_runner_(base::MessageLoopForIO::current()->task_runner()), + waiter_(waiter), + last_result_(MOJO_RESULT_INTERNAL), + io_loop_level_(0), + should_invoke_callback_(false) { + base::MessageLoopForIO::current()->AddIOObserver(this); + } + + void HandleIsReady(MojoResult result) { + last_result_ = result; + + // If the signaling happens in the IO handler, use |IOObserver| callback + // to invoke the callback. + if (IsCalledFromIOHandler()) { + should_invoke_callback_ = true; + return; + } + + io_runner_->PostTask(FROM_HERE, + base::Bind(&Context::InvokeWaiterCallback, this)); + } + + private: + friend void base::DeletePointer<const Context>(const Context* self); + friend class AsyncHandleWaiterContextTraits; + friend class base::RefCountedThreadSafe<Context>; + + ~Context() override { + DCHECK(base::MessageLoopForIO::current()->task_runner() == io_runner_); + base::MessageLoopForIO::current()->RemoveIOObserver(this); + } + + bool IsCalledFromIOHandler() const { + base::MessageLoop* loop = base::MessageLoop::current(); + if (!loop) + return false; + if (loop->task_runner() != io_runner_) + return false; + return io_loop_level_ > 0; + } + + // Called from |io_runner_| thus safe to touch |waiter_|. + void InvokeWaiterCallback() { + MojoResult result = last_result_; + last_result_ = MOJO_RESULT_INTERNAL; + if (waiter_) + waiter_->InvokeCallback(result); + } + + // IOObserver implementation: + + void WillProcessIOEvent() override { + DCHECK(io_loop_level_ != 0 || !should_invoke_callback_); + DCHECK_GE(io_loop_level_, 0); + io_loop_level_++; + } + + void DidProcessIOEvent() override { + DCHECK_GE(io_loop_level_, 1); + + // Leaving a nested loop. + if (io_loop_level_ > 1) { + io_loop_level_--; + return; + } + + // The zero |waiter_| indicates that |this| have lost the owner and can be + // under destruction. So we cannot wrap it with a |scoped_refptr| anymore. + if (!waiter_) { + should_invoke_callback_ = false; + io_loop_level_--; + return; + } + + // We have to protect |this| because |AsyncHandleWaiter| can be + // deleted during the callback. + scoped_refptr<Context> protect(this); + while (should_invoke_callback_) { + should_invoke_callback_ = false; + InvokeWaiterCallback(); + } + + io_loop_level_--; + } + + // Only |io_runner_| is accessed from arbitrary threads. Others are touched + // only from the IO thread. + const scoped_refptr<base::TaskRunner> io_runner_; + + const base::WeakPtr<AsyncHandleWaiter> waiter_; + MojoResult last_result_; + int io_loop_level_; + bool should_invoke_callback_; + + DISALLOW_COPY_AND_ASSIGN(Context); +}; + +AsyncHandleWaiter::AsyncHandleWaiter(base::Callback<void(MojoResult)> callback) + : callback_(callback), + weak_factory_(this) { + context_ = new Context(weak_factory_.GetWeakPtr()); +} + +AsyncHandleWaiter::~AsyncHandleWaiter() { +} + +MojoResult AsyncHandleWaiter::Wait(MojoHandle handle, + MojoHandleSignals signals) { + return mojo::embedder::AsyncWait( + handle, signals, base::Bind(&Context::HandleIsReady, context_)); +} + +void AsyncHandleWaiter::InvokeCallback(MojoResult result) { + callback_.Run(result); +} + +base::MessageLoopForIO::IOObserver* AsyncHandleWaiter::GetIOObserverForTest() { + return context_.get(); +} + +base::Callback<void(MojoResult)> AsyncHandleWaiter::GetWaitCallbackForTest() { + return base::Bind(&Context::HandleIsReady, context_); +} + +// static +void AsyncHandleWaiterContextTraits::Destruct( + const AsyncHandleWaiter::Context* context) { + context->io_runner_->PostTask( + FROM_HERE, + base::Bind(&base::DeletePointer<const AsyncHandleWaiter::Context>, + base::Unretained(context))); +} + +} // namespace internal +} // namespace IPC diff --git a/chromium/ipc/mojo/async_handle_waiter.h b/chromium/ipc/mojo/async_handle_waiter.h new file mode 100644 index 00000000000..d6cc74510b2 --- /dev/null +++ b/chromium/ipc/mojo/async_handle_waiter.h @@ -0,0 +1,52 @@ +// 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_MOJO_ASYNC_HANDLE_WAITER_H_ +#define IPC_MOJO_ASYNC_HANDLE_WAITER_H_ + +#include "base/callback.h" +#include "base/memory/ref_counted.h" +#include "base/memory/weak_ptr.h" +#include "base/message_loop/message_loop.h" +#include "ipc/ipc_export.h" +#include "third_party/mojo/src/mojo/public/c/system/types.h" + +namespace IPC { +namespace internal { + +// |AsyncHandleWaiter| waits on a mojo handle asynchronously and +// invokes the given |callback| through |runner| when it is signaled. +// * To start waiting, the client must call |AsyncHandleWaiter::Wait()|. +// The client can call |Wait()| again once it is signaled and +// the |callback| is invoked. +// * To cancel waiting, delete the instance. +// +// |AsyncHandleWaiter| must be created, used and deleted only from the IO +// |thread. +class IPC_MOJO_EXPORT AsyncHandleWaiter { + public: + class Context; + + explicit AsyncHandleWaiter(base::Callback<void(MojoResult)> callback); + ~AsyncHandleWaiter(); + + MojoResult Wait(MojoHandle handle, MojoHandleSignals signals); + + base::MessageLoopForIO::IOObserver* GetIOObserverForTest(); + base::Callback<void(MojoResult)> GetWaitCallbackForTest(); + + private: + void InvokeCallback(MojoResult result); + + scoped_refptr<Context> context_; + base::Callback<void(MojoResult)> callback_; + base::WeakPtrFactory<AsyncHandleWaiter> weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(AsyncHandleWaiter); +}; + +} // namespace internal +} // namespace IPC + +#endif // IPC_MOJO_ASYNC_HANDLE_WAITER_H_ diff --git a/chromium/ipc/mojo/async_handle_waiter_unittest.cc b/chromium/ipc/mojo/async_handle_waiter_unittest.cc new file mode 100644 index 00000000000..441b4ecd316 --- /dev/null +++ b/chromium/ipc/mojo/async_handle_waiter_unittest.cc @@ -0,0 +1,257 @@ +// 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/mojo/async_handle_waiter.h" + +#include "base/bind.h" +#include "base/location.h" +#include "base/run_loop.h" +#include "base/single_thread_task_runner.h" +#include "base/threading/thread.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/mojo/src/mojo/public/cpp/system/message_pipe.h" + +namespace IPC { +namespace internal { +namespace { + +void ReadOneByteOfX(MojoHandle pipe) { + uint32_t size = 1; + char buffer = ' '; + MojoResult rv = MojoReadMessage(pipe, &buffer, &size, nullptr, nullptr, + MOJO_READ_MESSAGE_FLAG_NONE); + CHECK_EQ(rv, MOJO_RESULT_OK); + CHECK_EQ(size, 1U); + CHECK_EQ(buffer, 'X'); +} + +class AsyncHandleWaiterTest : public testing::Test { + public: + AsyncHandleWaiterTest() : worker_("test_worker") { + worker_.StartWithOptions( + base::Thread::Options(base::MessageLoop::TYPE_IO, 0)); + } + + void SetUp() override { + message_loop_.reset(new base::MessageLoopForIO()); + ResetSignaledStates(); + mojo::CreateMessagePipe(nullptr, &pipe_to_write_, &pipe_to_read_); + target_.reset(new AsyncHandleWaiter(base::Bind( + &AsyncHandleWaiterTest::HandleIsReady, base::Unretained(this)))); + } + + protected: + MojoResult Start() { + return target_->Wait(pipe_to_read_.get().value(), + MOJO_HANDLE_SIGNAL_READABLE); + } + + void ResetSignaledStates() { + signaled_result_ = MOJO_RESULT_UNKNOWN; + run_loop_.reset(new base::RunLoop()); + } + + void WriteToPipe() { + MojoResult rv = MojoWriteMessage(pipe_to_write_.get().value(), "X", 1, + nullptr, 0, MOJO_WRITE_MESSAGE_FLAG_NONE); + CHECK_EQ(rv, MOJO_RESULT_OK); + } + + void WriteToPipeFromWorker() { + worker_.task_runner()->PostTask( + FROM_HERE, base::Bind(&AsyncHandleWaiterTest::WriteToPipe, + base::Unretained(this))); + } + + void WaitAndAssertSignaledAndMessageIsArrived() { + run_loop_->Run(); + EXPECT_EQ(MOJO_RESULT_OK, signaled_result_); + + ReadOneByteOfX(pipe_to_read_.get().value()); + } + + void WaitAndAssertNotSignaled() { + run_loop_->RunUntilIdle(); + EXPECT_EQ(MOJO_RESULT_OK, MojoWait(pipe_to_read_.get().value(), + MOJO_HANDLE_SIGNAL_READABLE, 0, + nullptr)); + EXPECT_EQ(MOJO_RESULT_UNKNOWN, signaled_result_); + } + + void HandleIsReady(MojoResult result) { + CHECK_EQ(base::MessageLoop::current(), message_loop_.get()); + CHECK_EQ(signaled_result_, MOJO_RESULT_UNKNOWN); + signaled_result_ = result; + run_loop_->Quit(); + } + + base::Thread worker_; + scoped_ptr<base::MessageLoop> message_loop_; + scoped_ptr<base::RunLoop> run_loop_; + mojo::ScopedMessagePipeHandle pipe_to_write_; + mojo::ScopedMessagePipeHandle pipe_to_read_; + + scoped_ptr<AsyncHandleWaiter> target_; + MojoResult signaled_result_; +}; + +TEST_F(AsyncHandleWaiterTest, SignalFromSameThread) { + EXPECT_EQ(MOJO_RESULT_OK, Start()); + WriteToPipe(); + WaitAndAssertSignaledAndMessageIsArrived(); + + // Ensures that the waiter is reusable. + ResetSignaledStates(); + + EXPECT_EQ(MOJO_RESULT_OK, Start()); + WriteToPipe(); + WaitAndAssertSignaledAndMessageIsArrived(); +} + +TEST_F(AsyncHandleWaiterTest, SignalFromDifferentThread) { + EXPECT_EQ(MOJO_RESULT_OK, Start()); + WriteToPipeFromWorker(); + WaitAndAssertSignaledAndMessageIsArrived(); + + // Ensures that the waiter is reusable. + ResetSignaledStates(); + + EXPECT_EQ(MOJO_RESULT_OK, Start()); + WriteToPipeFromWorker(); + WaitAndAssertSignaledAndMessageIsArrived(); +} + +TEST_F(AsyncHandleWaiterTest, DeleteWaiterBeforeWrite) { + EXPECT_EQ(MOJO_RESULT_OK, Start()); + + target_.reset(); + + WriteToPipe(); + WaitAndAssertNotSignaled(); +} + +TEST_F(AsyncHandleWaiterTest, DeleteWaiterBeforeSignal) { + EXPECT_EQ(MOJO_RESULT_OK, Start()); + WriteToPipe(); + + target_.reset(); + + WaitAndAssertNotSignaled(); +} + +class HandlerThatReenters { + public: + HandlerThatReenters(base::RunLoop* loop, MojoHandle handle) + : target_(nullptr), handle_(handle), loop_(loop), step_(0) {} + + void set_target(AsyncHandleWaiter* target) { target_ = target; } + + void HandleIsReady(MojoResult result) { + switch (step_) { + case 0: + RestartAndClose(result); + break; + case 1: + HandleClosingSignal(result); + break; + default: + NOTREACHED(); + break; + } + } + + void RestartAndClose(MojoResult result) { + CHECK_EQ(step_, 0); + CHECK_EQ(result, MOJO_RESULT_OK); + step_ = 1; + + ReadOneByteOfX(handle_); + target_->Wait(handle_, MOJO_HANDLE_SIGNAL_READABLE); + + // This signals the |AsyncHandleWaiter|. + MojoResult rv = MojoClose(handle_); + CHECK_EQ(rv, MOJO_RESULT_OK); + } + + void HandleClosingSignal(MojoResult result) { + CHECK_EQ(step_, 1); + CHECK_EQ(result, MOJO_RESULT_CANCELLED); + step_ = 2; + loop_->Quit(); + } + + bool IsClosingHandled() const { return step_ == 2; } + + AsyncHandleWaiter* target_; + MojoHandle handle_; + base::RunLoop* loop_; + int step_; +}; + +TEST_F(AsyncHandleWaiterTest, RestartWaitingWhileSignaled) { + HandlerThatReenters handler(run_loop_.get(), pipe_to_read_.get().value()); + target_.reset(new AsyncHandleWaiter(base::Bind( + &HandlerThatReenters::HandleIsReady, base::Unretained(&handler)))); + handler.set_target(target_.get()); + + EXPECT_EQ(MOJO_RESULT_OK, Start()); + WriteToPipe(); + run_loop_->Run(); + + EXPECT_TRUE(handler.IsClosingHandled()); + + // |HandlerThatReenters::RestartAndClose| already closed it. + ignore_result(pipe_to_read_.release()); +} + +class AsyncHandleWaiterIOObserverTest : public testing::Test { + public: + void SetUp() override { + message_loop_.reset(new base::MessageLoopForIO()); + target_.reset(new AsyncHandleWaiter( + base::Bind(&AsyncHandleWaiterIOObserverTest::HandleIsReady, + base::Unretained(this)))); + invocation_count_ = 0; + } + + void HandleIsReady(MojoResult result) { invocation_count_++; } + + scoped_ptr<base::MessageLoop> message_loop_; + scoped_ptr<AsyncHandleWaiter> target_; + size_t invocation_count_; +}; + +TEST_F(AsyncHandleWaiterIOObserverTest, OutsideIOEvnet) { + target_->GetWaitCallbackForTest().Run(MOJO_RESULT_OK); + EXPECT_EQ(0U, invocation_count_); + message_loop_->RunUntilIdle(); + EXPECT_EQ(1U, invocation_count_); +} + +TEST_F(AsyncHandleWaiterIOObserverTest, InsideIOEvnet) { + target_->GetIOObserverForTest()->WillProcessIOEvent(); + target_->GetWaitCallbackForTest().Run(MOJO_RESULT_OK); + EXPECT_EQ(0U, invocation_count_); + target_->GetIOObserverForTest()->DidProcessIOEvent(); + EXPECT_EQ(1U, invocation_count_); +} + +TEST_F(AsyncHandleWaiterIOObserverTest, Reenter) { + target_->GetIOObserverForTest()->WillProcessIOEvent(); + target_->GetWaitCallbackForTest().Run(MOJO_RESULT_OK); + EXPECT_EQ(0U, invocation_count_); + + // As if some other io handler start nested loop. + target_->GetIOObserverForTest()->WillProcessIOEvent(); + target_->GetWaitCallbackForTest().Run(MOJO_RESULT_OK); + target_->GetIOObserverForTest()->DidProcessIOEvent(); + EXPECT_EQ(0U, invocation_count_); + + target_->GetIOObserverForTest()->DidProcessIOEvent(); + EXPECT_EQ(1U, invocation_count_); +} + +} // namespace +} // namespace internal +} // namespace IPC diff --git a/chromium/ipc/mojo/ipc_channel_mojo.cc b/chromium/ipc/mojo/ipc_channel_mojo.cc index 4051bb5c21a..e38a7e94ae8 100644 --- a/chromium/ipc/mojo/ipc_channel_mojo.cc +++ b/chromium/ipc/mojo/ipc_channel_mojo.cc @@ -7,15 +7,19 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/lazy_instance.h" +#include "base/thread_task_runner_handle.h" #include "ipc/ipc_listener.h" +#include "ipc/ipc_logging.h" +#include "ipc/ipc_message_attachment_set.h" +#include "ipc/ipc_message_macros.h" #include "ipc/mojo/client_channel.mojom.h" -#include "ipc/mojo/ipc_channel_mojo_readers.h" #include "ipc/mojo/ipc_mojo_bootstrap.h" -#include "mojo/edk/embedder/embedder.h" -#include "mojo/public/cpp/bindings/error_handler.h" +#include "ipc/mojo/ipc_mojo_handle_attachment.h" +#include "third_party/mojo/src/mojo/edk/embedder/embedder.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/error_handler.h" #if defined(OS_POSIX) && !defined(OS_NACL) -#include "ipc/file_descriptor_set_posix.h" +#include "ipc/ipc_platform_file_attachment_posix.h" #endif namespace IPC { @@ -25,37 +29,44 @@ namespace { class MojoChannelFactory : public ChannelFactory { public: MojoChannelFactory(ChannelMojo::Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, ChannelHandle channel_handle, Channel::Mode mode) - : delegate_(delegate), channel_handle_(channel_handle), mode_(mode) {} + : delegate_(delegate), + io_runner_(io_runner), + channel_handle_(channel_handle), + mode_(mode) {} std::string GetName() const override { return channel_handle_.name; } scoped_ptr<Channel> BuildChannel(Listener* listener) override { - return ChannelMojo::Create(delegate_, channel_handle_, mode_, listener); + return ChannelMojo::Create(delegate_, io_runner_, channel_handle_, mode_, + listener); } private: ChannelMojo::Delegate* delegate_; + scoped_refptr<base::TaskRunner> io_runner_; ChannelHandle channel_handle_; Channel::Mode mode_; }; //------------------------------------------------------------------------------ -class ClientChannelMojo - : public ChannelMojo, - public NON_EXPORTED_BASE(mojo::InterfaceImpl<ClientChannel>) { +class ClientChannelMojo : public ChannelMojo, + public ClientChannel, + public mojo::ErrorHandler { public: ClientChannelMojo(ChannelMojo::Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, const ChannelHandle& handle, Listener* listener); ~ClientChannelMojo() override; // MojoBootstrap::Delegate implementation void OnPipeAvailable(mojo::embedder::ScopedPlatformHandle handle) override; - // InterfaceImpl implementation + // mojo::ErrorHandler implementation void OnConnectionError() override; // ClientChannel implementation void Init( @@ -63,13 +74,22 @@ class ClientChannelMojo int32_t peer_pid, const mojo::Callback<void(int32_t)>& callback) override; + private: + void BindPipe(mojo::ScopedMessagePipeHandle handle); + + mojo::Binding<ClientChannel> binding_; + base::WeakPtrFactory<ClientChannelMojo> weak_factory_; + DISALLOW_COPY_AND_ASSIGN(ClientChannelMojo); }; ClientChannelMojo::ClientChannelMojo(ChannelMojo::Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, const ChannelHandle& handle, Listener* listener) - : ChannelMojo(delegate, handle, Channel::MODE_CLIENT, listener) { + : ChannelMojo(delegate, io_runner, handle, Channel::MODE_CLIENT, listener), + binding_(this), + weak_factory_(this) { } ClientChannelMojo::~ClientChannelMojo() { @@ -77,7 +97,8 @@ ClientChannelMojo::~ClientChannelMojo() { void ClientChannelMojo::OnPipeAvailable( mojo::embedder::ScopedPlatformHandle handle) { - mojo::WeakBindToPipe(this, CreateMessagingPipe(handle.Pass())); + CreateMessagingPipe(handle.Pass(), base::Bind(&ClientChannelMojo::BindPipe, + weak_factory_.GetWeakPtr())); } void ClientChannelMojo::OnConnectionError() { @@ -92,36 +113,47 @@ void ClientChannelMojo::Init( callback.Run(GetSelfPID()); } +void ClientChannelMojo::BindPipe(mojo::ScopedMessagePipeHandle handle) { + binding_.Bind(handle.Pass()); +} + //------------------------------------------------------------------------------ class ServerChannelMojo : public ChannelMojo, public mojo::ErrorHandler { public: ServerChannelMojo(ChannelMojo::Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, const ChannelHandle& handle, Listener* listener); ~ServerChannelMojo() override; // MojoBootstrap::Delegate implementation void OnPipeAvailable(mojo::embedder::ScopedPlatformHandle handle) override; - // ErrorHandler implementation + // mojo::ErrorHandler implementation void OnConnectionError() override; // Channel override void Close() override; private: + void InitClientChannel(mojo::ScopedMessagePipeHandle peer_handle, + mojo::ScopedMessagePipeHandle handle); + // ClientChannelClient implementation void ClientChannelWasInitialized(int32_t peer_pid); mojo::InterfacePtr<ClientChannel> client_channel_; mojo::ScopedMessagePipeHandle message_pipe_; + base::WeakPtrFactory<ServerChannelMojo> weak_factory_; DISALLOW_COPY_AND_ASSIGN(ServerChannelMojo); }; ServerChannelMojo::ServerChannelMojo(ChannelMojo::Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, const ChannelHandle& handle, Listener* listener) - : ChannelMojo(delegate, handle, Channel::MODE_SERVER, listener) { + : ChannelMojo(delegate, io_runner, handle, Channel::MODE_SERVER, listener), + weak_factory_(this) { } ServerChannelMojo::~ServerChannelMojo() { @@ -134,16 +166,24 @@ void ServerChannelMojo::OnPipeAvailable( MojoResult create_result = mojo::CreateMessagePipe(nullptr, &message_pipe_, &peer); if (create_result != MOJO_RESULT_OK) { - DLOG(WARNING) << "mojo::CreateMessagePipe failed: " << create_result; + LOG(WARNING) << "mojo::CreateMessagePipe failed: " << create_result; listener()->OnChannelError(); return; } + CreateMessagingPipe( + handle.Pass(), + base::Bind(&ServerChannelMojo::InitClientChannel, + weak_factory_.GetWeakPtr(), base::Passed(&peer))); +} - client_channel_.Bind(CreateMessagingPipe(handle.Pass())); +void ServerChannelMojo::InitClientChannel( + mojo::ScopedMessagePipeHandle peer_handle, + mojo::ScopedMessagePipeHandle handle) { + client_channel_.Bind( + mojo::InterfacePtrInfo<ClientChannel>(handle.Pass(), 0u)); client_channel_.set_error_handler(this); client_channel_->Init( - peer.Pass(), - static_cast<int32_t>(GetSelfPID()), + peer_handle.Pass(), static_cast<int32_t>(GetSelfPID()), base::Bind(&ServerChannelMojo::ClientChannelWasInitialized, base::Unretained(this))); } @@ -162,35 +202,59 @@ void ServerChannelMojo::Close() { ChannelMojo::Close(); } -} // namespace +#if defined(OS_POSIX) && !defined(OS_NACL) + +base::ScopedFD TakeOrDupFile(internal::PlatformFileAttachment* attachment) { + return attachment->Owns() ? base::ScopedFD(attachment->TakePlatformFile()) + : base::ScopedFD(dup(attachment->file())); +} + +#endif + +} // namespace //------------------------------------------------------------------------------ +ChannelMojo::ChannelInfoDeleter::ChannelInfoDeleter( + scoped_refptr<base::TaskRunner> io_runner) + : io_runner(io_runner) { +} + +ChannelMojo::ChannelInfoDeleter::~ChannelInfoDeleter() { +} + void ChannelMojo::ChannelInfoDeleter::operator()( mojo::embedder::ChannelInfo* ptr) const { - mojo::embedder::DestroyChannelOnIOThread(ptr); + if (base::ThreadTaskRunnerHandle::Get() == io_runner) { + mojo::embedder::DestroyChannelOnIOThread(ptr); + } else { + io_runner->PostTask( + FROM_HERE, base::Bind(&mojo::embedder::DestroyChannelOnIOThread, ptr)); + } } //------------------------------------------------------------------------------ // static bool ChannelMojo::ShouldBeUsed() { - // TODO(morrita): Turn this on for a set of platforms. - return false; + // TODO(morrita): Remove this if it sticks. + return true; } // static -scoped_ptr<ChannelMojo> ChannelMojo::Create(ChannelMojo::Delegate* delegate, - const ChannelHandle& channel_handle, - Mode mode, - Listener* listener) { +scoped_ptr<ChannelMojo> ChannelMojo::Create( + ChannelMojo::Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, + const ChannelHandle& channel_handle, + Mode mode, + Listener* listener) { switch (mode) { case Channel::MODE_CLIENT: return make_scoped_ptr( - new ClientChannelMojo(delegate, channel_handle, listener)); + new ClientChannelMojo(delegate, io_runner, channel_handle, listener)); case Channel::MODE_SERVER: return make_scoped_ptr( - new ServerChannelMojo(delegate, channel_handle, listener)); + new ServerChannelMojo(delegate, io_runner, channel_handle, listener)); default: NOTREACHED(); return nullptr; @@ -200,39 +264,41 @@ scoped_ptr<ChannelMojo> ChannelMojo::Create(ChannelMojo::Delegate* delegate, // static scoped_ptr<ChannelFactory> ChannelMojo::CreateServerFactory( ChannelMojo::Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, const ChannelHandle& channel_handle) { - return make_scoped_ptr( - new MojoChannelFactory(delegate, channel_handle, Channel::MODE_SERVER)); + return make_scoped_ptr(new MojoChannelFactory( + delegate, io_runner, channel_handle, Channel::MODE_SERVER)); } // static scoped_ptr<ChannelFactory> ChannelMojo::CreateClientFactory( + ChannelMojo::Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, const ChannelHandle& channel_handle) { - return make_scoped_ptr( - new MojoChannelFactory(NULL, channel_handle, Channel::MODE_CLIENT)); + return make_scoped_ptr(new MojoChannelFactory( + delegate, io_runner, channel_handle, Channel::MODE_CLIENT)); } ChannelMojo::ChannelMojo(ChannelMojo::Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, const ChannelHandle& handle, Mode mode, Listener* listener) : mode_(mode), listener_(listener), peer_pid_(base::kNullProcessId), + io_runner_(io_runner), + channel_info_(nullptr, ChannelInfoDeleter(nullptr)), weak_factory_(this) { // Create MojoBootstrap after all members are set as it touches // ChannelMojo from a different thread. bootstrap_ = MojoBootstrap::Create(handle, mode, this); - if (delegate) { - if (delegate->GetIOTaskRunner() == - base::MessageLoop::current()->message_loop_proxy()) { - InitDelegate(delegate); - } else { - delegate->GetIOTaskRunner()->PostTask( - FROM_HERE, - base::Bind( - &ChannelMojo::InitDelegate, base::Unretained(this), delegate)); - } + if (io_runner == base::MessageLoop::current()->task_runner()) { + InitOnIOThread(delegate); + } else { + io_runner->PostTask(FROM_HERE, + base::Bind(&ChannelMojo::InitOnIOThread, + base::Unretained(this), delegate)); } } @@ -240,19 +306,56 @@ ChannelMojo::~ChannelMojo() { Close(); } -void ChannelMojo::InitDelegate(ChannelMojo::Delegate* delegate) { +void ChannelMojo::InitOnIOThread(ChannelMojo::Delegate* delegate) { + ipc_support_.reset( + new ScopedIPCSupport(base::MessageLoop::current()->task_runner())); + if (!delegate) + return; delegate_ = delegate->ToWeakPtr(); delegate_->OnChannelCreated(weak_factory_.GetWeakPtr()); } -mojo::ScopedMessagePipeHandle ChannelMojo::CreateMessagingPipe( - mojo::embedder::ScopedPlatformHandle handle) { - DCHECK(!channel_info_.get()); +void ChannelMojo::CreateMessagingPipe( + mojo::embedder::ScopedPlatformHandle handle, + const CreateMessagingPipeCallback& callback) { + auto return_callback = base::Bind(&ChannelMojo::OnMessagingPipeCreated, + weak_factory_.GetWeakPtr(), callback); + if (base::ThreadTaskRunnerHandle::Get() == io_runner_) { + CreateMessagingPipeOnIOThread( + handle.Pass(), base::ThreadTaskRunnerHandle::Get(), return_callback); + } else { + io_runner_->PostTask( + FROM_HERE, + base::Bind(&ChannelMojo::CreateMessagingPipeOnIOThread, + base::Passed(&handle), base::ThreadTaskRunnerHandle::Get(), + return_callback)); + } +} + +// static +void ChannelMojo::CreateMessagingPipeOnIOThread( + mojo::embedder::ScopedPlatformHandle handle, + scoped_refptr<base::TaskRunner> callback_runner, + const CreateMessagingPipeOnIOThreadCallback& callback) { mojo::embedder::ChannelInfo* channel_info; mojo::ScopedMessagePipeHandle pipe = mojo::embedder::CreateChannelOnIOThread(handle.Pass(), &channel_info); - channel_info_.reset(channel_info); - return pipe.Pass(); + if (base::ThreadTaskRunnerHandle::Get() == callback_runner) { + callback.Run(pipe.Pass(), channel_info); + } else { + callback_runner->PostTask( + FROM_HERE, base::Bind(callback, base::Passed(&pipe), channel_info)); + } +} + +void ChannelMojo::OnMessagingPipeCreated( + const CreateMessagingPipeCallback& callback, + mojo::ScopedMessagePipeHandle handle, + mojo::embedder::ChannelInfo* channel_info) { + DCHECK(!channel_info_.get()); + channel_info_ = scoped_ptr<mojo::embedder::ChannelInfo, ChannelInfoDeleter>( + channel_info, ChannelInfoDeleter(io_runner_)); + callback.Run(handle.Pass()); } bool ChannelMojo::Connect() { @@ -261,33 +364,68 @@ bool ChannelMojo::Connect() { } void ChannelMojo::Close() { - message_reader_.reset(); + scoped_ptr<internal::MessagePipeReader, ReaderDeleter> to_be_deleted; + + { + // |message_reader_| has to be cleared inside the lock, + // but the instance has to be deleted outside. + base::AutoLock l(lock_); + to_be_deleted = message_reader_.Pass(); + } + channel_info_.reset(); + ipc_support_.reset(); + to_be_deleted.reset(); } void ChannelMojo::OnBootstrapError() { listener_->OnChannelError(); } +namespace { + +// ClosingDeleter calls |CloseWithErrorIfPending| before deleting the +// |MessagePipeReader|. +struct ClosingDeleter { + typedef base::DefaultDeleter<internal::MessagePipeReader> DefaultType; + + void operator()(internal::MessagePipeReader* ptr) const { + ptr->CloseWithErrorIfPending(); + delete ptr; + } +}; + +} // namespace + void ChannelMojo::InitMessageReader(mojo::ScopedMessagePipeHandle pipe, int32_t peer_pid) { - message_reader_ = - make_scoped_ptr(new internal::MessageReader(pipe.Pass(), this)); - - for (size_t i = 0; i < pending_messages_.size(); ++i) { - bool sent = message_reader_->Send(make_scoped_ptr(pending_messages_[i])); - pending_messages_[i] = NULL; - if (!sent) { - pending_messages_.clear(); - listener_->OnChannelError(); - return; + scoped_ptr<internal::MessagePipeReader, ClosingDeleter> reader( + new internal::MessagePipeReader(pipe.Pass(), this)); + + { + base::AutoLock l(lock_); + for (size_t i = 0; i < pending_messages_.size(); ++i) { + bool sent = reader->Send(make_scoped_ptr(pending_messages_[i])); + pending_messages_[i] = nullptr; + if (!sent) { + // OnChannelError() is notified through ClosingDeleter. + pending_messages_.clear(); + LOG(ERROR) << "Failed to flush pending messages"; + return; + } } - } - pending_messages_.clear(); + // We set |message_reader_| here and won't get any |pending_messages_| + // hereafter. Although we might have some if there is an error, we don't + // care. They cannot be sent anyway. + message_reader_.reset(reader.release()); + pending_messages_.clear(); + } set_peer_pid(peer_pid); listener_->OnChannelConnected(static_cast<int32_t>(GetPeerPID())); + if (message_reader_) + message_reader_->ReadMessagesThenWait(); } void ChannelMojo::OnPipeClosed(internal::MessagePipeReader* reader) { @@ -299,7 +437,9 @@ void ChannelMojo::OnPipeError(internal::MessagePipeReader* reader) { } +// Warning: Keep the implementation thread-safe. bool ChannelMojo::Send(Message* message) { + base::AutoLock l(lock_); if (!message_reader_) { pending_messages_.push_back(message); return true; @@ -308,12 +448,16 @@ bool ChannelMojo::Send(Message* message) { return message_reader_->Send(make_scoped_ptr(message)); } +bool ChannelMojo::IsSendThreadSafe() const { + return false; +} + base::ProcessId ChannelMojo::GetPeerPID() const { return peer_pid_; } base::ProcessId ChannelMojo::GetSelfPID() const { - return base::GetCurrentProcId(); + return bootstrap_->GetSelfPID(); } void ChannelMojo::OnClientLaunched(base::ProcessHandle handle) { @@ -321,6 +465,9 @@ void ChannelMojo::OnClientLaunched(base::ProcessHandle handle) { } void ChannelMojo::OnMessageReceived(Message& message) { + TRACE_EVENT2("ipc,toplevel", "ChannelMojo::OnMessageReceived", + "class", IPC_MESSAGE_ID_CLASS(message.type()), + "line", IPC_MESSAGE_ID_LINE(message.type())); listener_->OnMessageReceived(message); if (message.dispatch_error()) listener_->OnBadMessageReceived(message); @@ -334,69 +481,81 @@ int ChannelMojo::GetClientFileDescriptor() const { base::ScopedFD ChannelMojo::TakeClientFileDescriptor() { return bootstrap_->TakeClientFileDescriptor(); } +#endif // defined(OS_POSIX) && !defined(OS_NACL) // static -MojoResult ChannelMojo::WriteToFileDescriptorSet( - const std::vector<MojoHandle>& handle_buffer, - Message* message) { - for (size_t i = 0; i < handle_buffer.size(); ++i) { - mojo::embedder::ScopedPlatformHandle platform_handle; - MojoResult unwrap_result = mojo::embedder::PassWrappedPlatformHandle( - handle_buffer[i], &platform_handle); - if (unwrap_result != MOJO_RESULT_OK) { - DLOG(WARNING) << "Pipe failed to covert handles. Closing: " - << unwrap_result; - return unwrap_result; +MojoResult ChannelMojo::ReadFromMessageAttachmentSet( + Message* message, + std::vector<MojoHandle>* handles) { + // We dup() the handles in IPC::Message to transmit. + // IPC::MessageAttachmentSet has intricate lifecycle semantics + // of FDs, so just to dup()-and-own them is the safest option. + if (message->HasAttachments()) { + MessageAttachmentSet* set = message->attachment_set(); + for (unsigned i = 0; i < set->size(); ++i) { + scoped_refptr<MessageAttachment> attachment = set->GetAttachmentAt(i); + switch (attachment->GetType()) { + case MessageAttachment::TYPE_PLATFORM_FILE: +#if defined(OS_POSIX) && !defined(OS_NACL) + { + base::ScopedFD file = + TakeOrDupFile(static_cast<IPC::internal::PlatformFileAttachment*>( + attachment.get())); + if (!file.is_valid()) { + DPLOG(WARNING) << "Failed to dup FD to transmit."; + set->CommitAll(); + return MOJO_RESULT_UNKNOWN; + } + + MojoHandle wrapped_handle; + MojoResult wrap_result = CreatePlatformHandleWrapper( + mojo::embedder::ScopedPlatformHandle( + mojo::embedder::PlatformHandle(file.release())), + &wrapped_handle); + if (MOJO_RESULT_OK != wrap_result) { + LOG(WARNING) << "Pipe failed to wrap handles. Closing: " + << wrap_result; + set->CommitAll(); + return wrap_result; + } + + handles->push_back(wrapped_handle); + } +#else + NOTREACHED(); +#endif // defined(OS_POSIX) && !defined(OS_NACL) + break; + case MessageAttachment::TYPE_MOJO_HANDLE: { + mojo::ScopedHandle handle = + static_cast<IPC::internal::MojoHandleAttachment*>( + attachment.get())->TakeHandle(); + handles->push_back(handle.release().value()); + } break; + } } - bool ok = message->file_descriptor_set()->AddToOwn( - base::ScopedFD(platform_handle.release().fd)); - DCHECK(ok); + set->CommitAll(); } return MOJO_RESULT_OK; } // static -MojoResult ChannelMojo::ReadFromFileDescriptorSet( - Message* message, - std::vector<MojoHandle>* handles) { - // We dup() the handles in IPC::Message to transmit. - // IPC::FileDescriptorSet has intricate lifecycle semantics - // of FDs, so just to dup()-and-own them is the safest option. - if (message->HasFileDescriptors()) { - FileDescriptorSet* fdset = message->file_descriptor_set(); - std::vector<base::PlatformFile> fds_to_send(fdset->size()); - fdset->PeekDescriptors(&fds_to_send[0]); - for (size_t i = 0; i < fds_to_send.size(); ++i) { - int fd_to_send = dup(fds_to_send[i]); - if (-1 == fd_to_send) { - DPLOG(WARNING) << "Failed to dup FD to transmit."; - fdset->CommitAll(); - return MOJO_RESULT_UNKNOWN; - } - - MojoHandle wrapped_handle; - MojoResult wrap_result = CreatePlatformHandleWrapper( - mojo::embedder::ScopedPlatformHandle( - mojo::embedder::PlatformHandle(fd_to_send)), - &wrapped_handle); - if (MOJO_RESULT_OK != wrap_result) { - DLOG(WARNING) << "Pipe failed to wrap handles. Closing: " - << wrap_result; - fdset->CommitAll(); - return wrap_result; - } - - handles->push_back(wrapped_handle); +MojoResult ChannelMojo::WriteToMessageAttachmentSet( + const std::vector<MojoHandle>& handle_buffer, + Message* message) { + for (size_t i = 0; i < handle_buffer.size(); ++i) { + bool ok = message->attachment_set()->AddAttachment( + new IPC::internal::MojoHandleAttachment( + mojo::MakeScopedHandle(mojo::Handle(handle_buffer[i])))); + DCHECK(ok); + if (!ok) { + LOG(ERROR) << "Failed to add new Mojo handle."; + return MOJO_RESULT_UNKNOWN; } - - fdset->CommitAll(); } return MOJO_RESULT_OK; } -#endif // defined(OS_POSIX) && !defined(OS_NACL) - } // namespace IPC diff --git a/chromium/ipc/mojo/ipc_channel_mojo.h b/chromium/ipc/mojo/ipc_channel_mojo.h index 29d5f29b3e7..3a1d98a8a97 100644 --- a/chromium/ipc/mojo/ipc_channel_mojo.h +++ b/chromium/ipc/mojo/ipc_channel_mojo.h @@ -10,23 +10,18 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_vector.h" #include "base/memory/weak_ptr.h" +#include "base/synchronization/lock.h" #include "ipc/ipc_channel.h" #include "ipc/ipc_channel_factory.h" #include "ipc/ipc_export.h" #include "ipc/mojo/ipc_message_pipe_reader.h" #include "ipc/mojo/ipc_mojo_bootstrap.h" -#include "mojo/edk/embedder/channel_info_forward.h" -#include "mojo/public/cpp/system/core.h" +#include "ipc/mojo/scoped_ipc_support.h" +#include "third_party/mojo/src/mojo/edk/embedder/channel_info_forward.h" +#include "third_party/mojo/src/mojo/public/cpp/system/core.h" namespace IPC { -namespace internal { -class ControlReader; -class ServerControlReader; -class ClientControlReader; -class MessageReader; -} - // Mojo-based IPC::Channel implementation over a platform handle. // // ChannelMojo builds Mojo MessagePipe using underlying pipe given by @@ -51,14 +46,21 @@ class MessageReader; // TODO(morrita): Add APIs to create extra MessagePipes to let // Mojo-based objects talk over this Channel. // -class IPC_MOJO_EXPORT ChannelMojo : public Channel, - public MojoBootstrap::Delegate { +class IPC_MOJO_EXPORT ChannelMojo + : public Channel, + public MojoBootstrap::Delegate, + public NON_EXPORTED_BASE(internal::MessagePipeReader::Delegate) { public: + using CreateMessagingPipeCallback = + base::Callback<void(mojo::ScopedMessagePipeHandle)>; + using CreateMessagingPipeOnIOThreadCallback = + base::Callback<void(mojo::ScopedMessagePipeHandle, + mojo::embedder::ChannelInfo*)>; + class Delegate { public: virtual ~Delegate() {} virtual base::WeakPtr<Delegate> ToWeakPtr() = 0; - virtual scoped_refptr<base::TaskRunner> GetIOTaskRunner() = 0; virtual void OnChannelCreated(base::WeakPtr<ChannelMojo> channel) = 0; }; @@ -67,19 +69,24 @@ class IPC_MOJO_EXPORT ChannelMojo : public Channel, // Create ChannelMojo. A bootstrap channel is created as well. // |host| must not be null for server channels. - static scoped_ptr<ChannelMojo> Create(Delegate* delegate, - const ChannelHandle& channel_handle, - Mode mode, - Listener* listener); + static scoped_ptr<ChannelMojo> Create( + Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, + const ChannelHandle& channel_handle, + Mode mode, + Listener* listener); // Create a factory object for ChannelMojo. // The factory is used to create Mojo-based ChannelProxy family. // |host| must not be null. static scoped_ptr<ChannelFactory> CreateServerFactory( Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, const ChannelHandle& channel_handle); static scoped_ptr<ChannelFactory> CreateClientFactory( + Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, const ChannelHandle& channel_handle); ~ChannelMojo() override; @@ -91,39 +98,41 @@ class IPC_MOJO_EXPORT ChannelMojo : public Channel, bool Connect() override; void Close() override; bool Send(Message* message) override; + bool IsSendThreadSafe() const override; base::ProcessId GetPeerPID() const override; base::ProcessId GetSelfPID() const override; #if defined(OS_POSIX) && !defined(OS_NACL) int GetClientFileDescriptor() const override; base::ScopedFD TakeClientFileDescriptor() override; +#endif // defined(OS_POSIX) && !defined(OS_NACL) // These access protected API of IPC::Message, which has ChannelMojo // as a friend class. - static MojoResult WriteToFileDescriptorSet( + static MojoResult WriteToMessageAttachmentSet( const std::vector<MojoHandle>& handle_buffer, Message* message); - static MojoResult ReadFromFileDescriptorSet(Message* message, - std::vector<MojoHandle>* handles); - -#endif // defined(OS_POSIX) && !defined(OS_NACL) + static MojoResult ReadFromMessageAttachmentSet( + Message* message, + std::vector<MojoHandle>* handles); // MojoBootstrapDelegate implementation void OnBootstrapError() override; - // Called from MessagePipeReader implementations - void OnMessageReceived(Message& message); - void OnPipeClosed(internal::MessagePipeReader* reader); - void OnPipeError(internal::MessagePipeReader* reader); + // MessagePipeReader::Delegate + void OnMessageReceived(Message& message) override; + void OnPipeClosed(internal::MessagePipeReader* reader) override; + void OnPipeError(internal::MessagePipeReader* reader) override; protected: ChannelMojo(Delegate* delegate, + scoped_refptr<base::TaskRunner> io_runner, const ChannelHandle& channel_handle, Mode mode, Listener* listener); - mojo::ScopedMessagePipeHandle CreateMessagingPipe( - mojo::embedder::ScopedPlatformHandle handle); + void CreateMessagingPipe(mojo::embedder::ScopedPlatformHandle handle, + const CreateMessagingPipeCallback& callback); void InitMessageReader(mojo::ScopedMessagePipeHandle pipe, int32_t peer_pid); Listener* listener() const { return listener_; } @@ -131,7 +140,12 @@ class IPC_MOJO_EXPORT ChannelMojo : public Channel, private: struct ChannelInfoDeleter { + explicit ChannelInfoDeleter(scoped_refptr<base::TaskRunner> io_runner); + ~ChannelInfoDeleter(); + void operator()(mojo::embedder::ChannelInfo* ptr) const; + + scoped_refptr<base::TaskRunner> io_runner; }; // ChannelMojo needs to kill its MessagePipeReader in delayed manner @@ -139,19 +153,36 @@ class IPC_MOJO_EXPORT ChannelMojo : public Channel, // notifications invoked by them. typedef internal::MessagePipeReader::DelayedDeleter ReaderDeleter; - void InitDelegate(ChannelMojo::Delegate* delegate); + void InitOnIOThread(ChannelMojo::Delegate* delegate); + + static void CreateMessagingPipeOnIOThread( + mojo::embedder::ScopedPlatformHandle handle, + scoped_refptr<base::TaskRunner> callback_runner, + const CreateMessagingPipeOnIOThreadCallback& callback); + void OnMessagingPipeCreated(const CreateMessagingPipeCallback& callback, + mojo::ScopedMessagePipeHandle handle, + mojo::embedder::ChannelInfo* channel_info); scoped_ptr<MojoBootstrap> bootstrap_; base::WeakPtr<Delegate> delegate_; Mode mode_; Listener* listener_; base::ProcessId peer_pid_; + scoped_refptr<base::TaskRunner> io_runner_; scoped_ptr<mojo::embedder::ChannelInfo, ChannelInfoDeleter> channel_info_; - scoped_ptr<internal::MessageReader, ReaderDeleter> message_reader_; + // Guards |message_reader_| and |pending_messages_| + // + // * The contents of |pending_messages_| can be modified from any thread. + // * |message_reader_| is modified only from the IO thread, + // but they can be referenced from other threads. + base::Lock lock_; + scoped_ptr<internal::MessagePipeReader, ReaderDeleter> message_reader_; ScopedVector<Message> pending_messages_; + scoped_ptr<ScopedIPCSupport> ipc_support_; + base::WeakPtrFactory<ChannelMojo> weak_factory_; DISALLOW_COPY_AND_ASSIGN(ChannelMojo); diff --git a/chromium/ipc/mojo/ipc_channel_mojo_host.cc b/chromium/ipc/mojo/ipc_channel_mojo_host.cc index beb18ee5e99..ac914ad2aff 100644 --- a/chromium/ipc/mojo/ipc_channel_mojo_host.cc +++ b/chromium/ipc/mojo/ipc_channel_mojo_host.cc @@ -5,32 +5,44 @@ #include "ipc/mojo/ipc_channel_mojo_host.h" #include "base/bind.h" -#include "base/message_loop/message_loop.h" +#include "base/location.h" +#include "base/single_thread_task_runner.h" #include "ipc/mojo/ipc_channel_mojo.h" namespace IPC { +class ChannelMojoHost::ChannelDelegateTraits { + public: + static void Destruct(const ChannelMojoHost::ChannelDelegate* ptr); +}; + // The delete class lives on the IO thread to talk to ChannelMojo on // behalf of ChannelMojoHost. // // The object must be touched only on the IO thread. -class ChannelMojoHost::ChannelDelegate : public ChannelMojo::Delegate { +class ChannelMojoHost::ChannelDelegate + : public base::RefCountedThreadSafe<ChannelMojoHost::ChannelDelegate, + ChannelMojoHost::ChannelDelegateTraits>, + public ChannelMojo::Delegate { public: - explicit ChannelDelegate(scoped_refptr<base::TaskRunner> io_task_runner); - ~ChannelDelegate() override; + explicit ChannelDelegate( + scoped_refptr<base::SequencedTaskRunner> io_task_runner); // ChannelMojo::Delegate base::WeakPtr<Delegate> ToWeakPtr() override; void OnChannelCreated(base::WeakPtr<ChannelMojo> channel) override; - scoped_refptr<base::TaskRunner> GetIOTaskRunner() override; // Returns an weak ptr of ChannelDelegate instead of Delegate base::WeakPtr<ChannelDelegate> GetWeakPtr(); void OnClientLaunched(base::ProcessHandle process); - void DeleteThisSoon(); + void DeleteThisSoon() const; private: - scoped_refptr<base::TaskRunner> io_task_runner_; + friend class base::DeleteHelper<ChannelDelegate>; + + ~ChannelDelegate() override; + + scoped_refptr<base::SequencedTaskRunner> io_task_runner_; base::WeakPtr<ChannelMojo> channel_; base::WeakPtrFactory<ChannelDelegate> weak_factory_; @@ -38,7 +50,7 @@ class ChannelMojoHost::ChannelDelegate : public ChannelMojo::Delegate { }; ChannelMojoHost::ChannelDelegate::ChannelDelegate( - scoped_refptr<base::TaskRunner> io_task_runner) + scoped_refptr<base::SequencedTaskRunner> io_task_runner) : io_task_runner_(io_task_runner), weak_factory_(this) { } @@ -61,29 +73,22 @@ void ChannelMojoHost::ChannelDelegate::OnChannelCreated( channel_ = channel; } -scoped_refptr<base::TaskRunner> -ChannelMojoHost::ChannelDelegate::GetIOTaskRunner() { - return io_task_runner_; -} - void ChannelMojoHost::ChannelDelegate::OnClientLaunched( base::ProcessHandle process) { if (channel_) channel_->OnClientLaunched(process); } -void ChannelMojoHost::ChannelDelegate::DeleteThisSoon() { - io_task_runner_->PostTask( - FROM_HERE, - base::Bind(&base::DeletePointer<ChannelMojoHost::ChannelDelegate>, - base::Unretained(this))); +void ChannelMojoHost::ChannelDelegate::DeleteThisSoon() const { + io_task_runner_->DeleteSoon(FROM_HERE, this); } // // ChannelMojoHost // -ChannelMojoHost::ChannelMojoHost(scoped_refptr<base::TaskRunner> io_task_runner) +ChannelMojoHost::ChannelMojoHost( + scoped_refptr<base::SequencedTaskRunner> io_task_runner) : io_task_runner_(io_task_runner), channel_delegate_(new ChannelDelegate(io_task_runner)), weak_factory_(this) { @@ -93,13 +98,12 @@ ChannelMojoHost::~ChannelMojoHost() { } void ChannelMojoHost::OnClientLaunched(base::ProcessHandle process) { - if (io_task_runner_ == base::MessageLoop::current()->message_loop_proxy()) { + if (io_task_runner_ == base::MessageLoop::current()->task_runner()) { channel_delegate_->OnClientLaunched(process); } else { io_task_runner_->PostTask(FROM_HERE, base::Bind(&ChannelDelegate::OnClientLaunched, - channel_delegate_->GetWeakPtr(), - process)); + channel_delegate_, process)); } } @@ -107,8 +111,9 @@ ChannelMojo::Delegate* ChannelMojoHost::channel_delegate() const { return channel_delegate_.get(); } -void ChannelMojoHost::DelegateDeleter::operator()( - ChannelMojoHost::ChannelDelegate* ptr) const { +// static +void ChannelMojoHost::ChannelDelegateTraits::Destruct( + const ChannelMojoHost::ChannelDelegate* ptr) { ptr->DeleteThisSoon(); } diff --git a/chromium/ipc/mojo/ipc_channel_mojo_host.h b/chromium/ipc/mojo/ipc_channel_mojo_host.h index 8289515fc31..db60b12a4cc 100644 --- a/chromium/ipc/mojo/ipc_channel_mojo_host.h +++ b/chromium/ipc/mojo/ipc_channel_mojo_host.h @@ -12,7 +12,7 @@ #include "ipc/mojo/ipc_channel_mojo.h" namespace base { -class TaskRunner; +class SequencedTaskRunner; } namespace IPC { @@ -23,7 +23,8 @@ namespace IPC { // instance and call OnClientLaunched(). class IPC_MOJO_EXPORT ChannelMojoHost { public: - explicit ChannelMojoHost(scoped_refptr<base::TaskRunner> io_task_runner); + explicit ChannelMojoHost( + scoped_refptr<base::SequencedTaskRunner> io_task_runner); ~ChannelMojoHost(); void OnClientLaunched(base::ProcessHandle process); @@ -31,16 +32,10 @@ class IPC_MOJO_EXPORT ChannelMojoHost { private: class ChannelDelegate; + class ChannelDelegateTraits; - // Delegate talks to ChannelMojo, whch lives in IO thread, thus - // the Delegate should also live and dies in the IO thread as well. - class DelegateDeleter { - public: - void operator()(ChannelDelegate* ptr) const; - }; - - const scoped_refptr<base::TaskRunner> io_task_runner_; - scoped_ptr<ChannelDelegate, DelegateDeleter> channel_delegate_; + const scoped_refptr<base::SequencedTaskRunner> io_task_runner_; + scoped_refptr<ChannelDelegate> channel_delegate_; base::WeakPtrFactory<ChannelMojoHost> weak_factory_; DISALLOW_COPY_AND_ASSIGN(ChannelMojoHost); diff --git a/chromium/ipc/mojo/ipc_channel_mojo_readers.cc b/chromium/ipc/mojo/ipc_channel_mojo_readers.cc deleted file mode 100644 index e4a35c584e5..00000000000 --- a/chromium/ipc/mojo/ipc_channel_mojo_readers.cc +++ /dev/null @@ -1,84 +0,0 @@ -// Copyright 2014 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/mojo/ipc_channel_mojo_readers.h" - -#include "ipc/mojo/ipc_channel_mojo.h" - -namespace IPC { -namespace internal { - -//------------------------------------------------------------------------------ - -MessageReader::MessageReader(mojo::ScopedMessagePipeHandle pipe, - ChannelMojo* owner) - : internal::MessagePipeReader(pipe.Pass()), owner_(owner) { -} - -void MessageReader::OnMessageReceived() { - Message message(data_buffer().empty() ? "" : &data_buffer()[0], - static_cast<uint32>(data_buffer().size())); - - std::vector<MojoHandle> handle_buffer; - TakeHandleBuffer(&handle_buffer); -#if defined(OS_POSIX) && !defined(OS_NACL) - MojoResult write_result = - ChannelMojo::WriteToFileDescriptorSet(handle_buffer, &message); - if (write_result != MOJO_RESULT_OK) { - CloseWithError(write_result); - return; - } -#else - DCHECK(handle_buffer.empty()); -#endif - - message.TraceMessageEnd(); - owner_->OnMessageReceived(message); -} - -void MessageReader::OnPipeClosed() { - if (!owner_) - return; - owner_->OnPipeClosed(this); - owner_ = NULL; -} - -void MessageReader::OnPipeError(MojoResult error) { - if (!owner_) - return; - owner_->OnPipeError(this); -} - -bool MessageReader::Send(scoped_ptr<Message> message) { - DCHECK(IsValid()); - - message->TraceMessageBegin(); - std::vector<MojoHandle> handles; -#if defined(OS_POSIX) && !defined(OS_NACL) - MojoResult read_result = - ChannelMojo::ReadFromFileDescriptorSet(message.get(), &handles); - if (read_result != MOJO_RESULT_OK) { - std::for_each(handles.begin(), handles.end(), &MojoClose); - CloseWithError(read_result); - return false; - } -#endif - MojoResult write_result = - MojoWriteMessage(handle(), - message->data(), - static_cast<uint32>(message->size()), - handles.empty() ? NULL : &handles[0], - static_cast<uint32>(handles.size()), - MOJO_WRITE_MESSAGE_FLAG_NONE); - if (MOJO_RESULT_OK != write_result) { - std::for_each(handles.begin(), handles.end(), &MojoClose); - CloseWithError(write_result); - return false; - } - - return true; -} - -} // namespace internal -} // namespace IPC diff --git a/chromium/ipc/mojo/ipc_channel_mojo_readers.h b/chromium/ipc/mojo/ipc_channel_mojo_readers.h deleted file mode 100644 index ffcc08bac5f..00000000000 --- a/chromium/ipc/mojo/ipc_channel_mojo_readers.h +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright 2014 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_MOJO_IPC_CHANNEL_MOJO_READERS_H_ -#define IPC_MOJO_IPC_CHANNEL_MOJO_READERS_H_ - -#include <vector> - -#include "base/macros.h" -#include "base/memory/scoped_ptr.h" -#include "ipc/mojo/ipc_message_pipe_reader.h" -#include "mojo/public/cpp/system/core.h" - -namespace IPC { - -class ChannelMojo; -class Message; - -namespace internal { - -// A MessagePipeReader implementation for IPC::Message communication. -class MessageReader : public MessagePipeReader { - public: - MessageReader(mojo::ScopedMessagePipeHandle pipe, ChannelMojo* owner); - - bool Send(scoped_ptr<Message> message); - - // MessagePipeReader implementation - void OnMessageReceived() override; - void OnPipeClosed() override; - void OnPipeError(MojoResult error) override; - - private: - ChannelMojo* owner_; - - DISALLOW_COPY_AND_ASSIGN(MessageReader); -}; - -// MessagePipeReader implementation for control messages. -// Actual message handling is implemented by sublcasses. -class ControlReader : public MessagePipeReader { - public: - ControlReader(mojo::ScopedMessagePipeHandle pipe, ChannelMojo* owner); - - virtual bool Connect(); - - // MessagePipeReader implementation - void OnPipeClosed() override; - void OnPipeError(MojoResult error) override; - - protected: - ChannelMojo* owner_; - - DISALLOW_COPY_AND_ASSIGN(ControlReader); -}; - -// ControlReader for server-side ChannelMojo. -class ServerControlReader : public ControlReader { - public: - ServerControlReader(mojo::ScopedMessagePipeHandle pipe, ChannelMojo* owner); - ~ServerControlReader() override; - - // ControlReader override - bool Connect() override; - - // MessagePipeReader implementation - void OnMessageReceived() override; - - private: - MojoResult SendHelloRequest(); - MojoResult RespondHelloResponse(); - - mojo::ScopedMessagePipeHandle message_pipe_; - - DISALLOW_COPY_AND_ASSIGN(ServerControlReader); -}; - -// ControlReader for client-side ChannelMojo. -class ClientControlReader : public ControlReader { - public: - ClientControlReader(mojo::ScopedMessagePipeHandle pipe, ChannelMojo* owner); - - // MessagePipeReader implementation - void OnMessageReceived() override; - - private: - MojoResult RespondHelloRequest(MojoHandle message_channel); - - DISALLOW_COPY_AND_ASSIGN(ClientControlReader); -}; - -} // namespace internal -} // namespace IPC - -#endif // IPC_MOJO_IPC_CHANNEL_MOJO_READERS_H_ diff --git a/chromium/ipc/mojo/ipc_channel_mojo_unittest.cc b/chromium/ipc/mojo/ipc_channel_mojo_unittest.cc index 07218c69297..53c6ecfd04d 100644 --- a/chromium/ipc/mojo/ipc_channel_mojo_unittest.cc +++ b/chromium/ipc/mojo/ipc_channel_mojo_unittest.cc @@ -6,18 +6,26 @@ #include "base/base_paths.h" #include "base/files/file.h" -#include "base/message_loop/message_loop.h" +#include "base/location.h" #include "base/path_service.h" #include "base/pickle.h" +#include "base/run_loop.h" +#include "base/single_thread_task_runner.h" +#include "base/test/test_timeouts.h" +#include "base/thread_task_runner_handle.h" #include "base/threading/thread.h" #include "ipc/ipc_message.h" #include "ipc/ipc_test_base.h" #include "ipc/ipc_test_channel_listener.h" #include "ipc/mojo/ipc_channel_mojo_host.h" -#include "ipc/mojo/ipc_channel_mojo_readers.h" +#include "ipc/mojo/ipc_mojo_handle_attachment.h" +#include "ipc/mojo/ipc_mojo_message_helper.h" +#include "ipc/mojo/ipc_mojo_param_traits.h" +#include "ipc/mojo/scoped_ipc_support.h" #if defined(OS_POSIX) #include "base/file_descriptor_posix.h" +#include "ipc/ipc_platform_file_attachment_posix.h" #endif namespace { @@ -60,16 +68,24 @@ class ListenerThatExpectsOK : public IPC::Listener { class ChannelClient { public: explicit ChannelClient(IPC::Listener* listener, const char* name) { - channel_ = IPC::ChannelMojo::Create(NULL, + channel_ = IPC::ChannelMojo::Create(NULL, main_message_loop_.task_runner(), IPCTestBase::GetChannelName(name), - IPC::Channel::MODE_CLIENT, - listener); + IPC::Channel::MODE_CLIENT, listener); } void Connect() { CHECK(channel_->Connect()); } + void Close() { + channel_->Close(); + + base::RunLoop run_loop; + base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, + run_loop.QuitClosure()); + run_loop.Run(); + } + IPC::ChannelMojo* channel() const { return channel_.get(); } private: @@ -77,20 +93,37 @@ class ChannelClient { scoped_ptr<IPC::ChannelMojo> channel_; }; -class IPCChannelMojoTest : public IPCTestBase { +class IPCChannelMojoTestBase : public IPCTestBase { + public: + void InitWithMojo(const std::string& test_client_name) { + Init(test_client_name); + } + + void TearDown() override { + // Make sure Mojo IPC support is properly shutdown on the I/O loop before + // TearDown continues. + base::RunLoop run_loop; + task_runner()->PostTask(FROM_HERE, run_loop.QuitClosure()); + run_loop.Run(); + + IPCTestBase::TearDown(); + } +}; + +class IPCChannelMojoTest : public IPCChannelMojoTestBase { protected: scoped_ptr<IPC::ChannelFactory> CreateChannelFactory( const IPC::ChannelHandle& handle, - base::TaskRunner* runner) override { + base::SequencedTaskRunner* runner) override { host_.reset(new IPC::ChannelMojoHost(task_runner())); return IPC::ChannelMojo::CreateServerFactory(host_->channel_delegate(), - handle); + task_runner(), handle); } bool DidStartClient() override { bool ok = IPCTestBase::DidStartClient(); DCHECK(ok); - host_->OnClientLaunched(client_process()); + host_->OnClientLaunched(client_process().Handle()); return ok; } @@ -119,7 +152,7 @@ class TestChannelListenerWithExtraExpectations }; TEST_F(IPCChannelMojoTest, ConnectedFromClient) { - Init("IPCChannelMojoTestClient"); + InitWithMojo("IPCChannelMojoTestClient"); // Set up IPC channel and start client. TestChannelListenerWithExtraExpectations listener; @@ -156,6 +189,8 @@ MULTIPROCESS_IPC_TEST_CLIENT_MAIN(IPCChannelMojoTestClient) { EXPECT_TRUE(listener.is_connected_called()); EXPECT_TRUE(listener.HasSentAll()); + client.Close(); + return 0; } @@ -183,20 +218,20 @@ class ListenerExpectingErrors : public IPC::Listener { }; -class IPCChannelMojoErrorTest : public IPCTestBase { +class IPCChannelMojoErrorTest : public IPCChannelMojoTestBase { protected: scoped_ptr<IPC::ChannelFactory> CreateChannelFactory( const IPC::ChannelHandle& handle, - base::TaskRunner* runner) override { + base::SequencedTaskRunner* runner) override { host_.reset(new IPC::ChannelMojoHost(task_runner())); return IPC::ChannelMojo::CreateServerFactory(host_->channel_delegate(), - handle); + task_runner(), handle); } bool DidStartClient() override { bool ok = IPCTestBase::DidStartClient(); DCHECK(ok); - host_->OnClientLaunched(client_process()); + host_->OnClientLaunched(client_process().Handle()); return ok; } @@ -226,11 +261,13 @@ MULTIPROCESS_IPC_TEST_CLIENT_MAIN(IPCChannelMojoErraticTestClient) { base::MessageLoop::current()->Run(); + client.Close(); + return 0; } TEST_F(IPCChannelMojoErrorTest, SendFailWithPendingMessages) { - Init("IPCChannelMojoErraticTestClient"); + InitWithMojo("IPCChannelMojoErraticTestClient"); // Set up IPC channel and start client. ListenerExpectingErrors listener; @@ -257,23 +294,281 @@ TEST_F(IPCChannelMojoErrorTest, SendFailWithPendingMessages) { DestroyChannel(); } +struct TestingMessagePipe { + TestingMessagePipe() { + EXPECT_EQ(MOJO_RESULT_OK, mojo::CreateMessagePipe(nullptr, &self, &peer)); + } + + mojo::ScopedMessagePipeHandle self; + mojo::ScopedMessagePipeHandle peer; +}; + +class HandleSendingHelper { + public: + static std::string GetSendingFileContent() { return "Hello"; } + + static void WritePipe(IPC::Message* message, TestingMessagePipe* pipe) { + std::string content = HandleSendingHelper::GetSendingFileContent(); + EXPECT_EQ(MOJO_RESULT_OK, + mojo::WriteMessageRaw(pipe->self.get(), &content[0], + static_cast<uint32_t>(content.size()), + nullptr, 0, 0)); + EXPECT_TRUE( + IPC::MojoMessageHelper::WriteMessagePipeTo(message, pipe->peer.Pass())); + } + + static void WritePipeThenSend(IPC::Sender* sender, TestingMessagePipe* pipe) { + IPC::Message* message = + new IPC::Message(0, 2, IPC::Message::PRIORITY_NORMAL); + WritePipe(message, pipe); + ASSERT_TRUE(sender->Send(message)); + } + + static void ReadReceivedPipe(const IPC::Message& message, + PickleIterator* iter) { + mojo::ScopedMessagePipeHandle pipe; + EXPECT_TRUE( + IPC::MojoMessageHelper::ReadMessagePipeFrom(&message, iter, &pipe)); + std::string content(GetSendingFileContent().size(), ' '); + + uint32_t num_bytes = static_cast<uint32_t>(content.size()); + EXPECT_EQ(MOJO_RESULT_OK, + mojo::ReadMessageRaw(pipe.get(), &content[0], &num_bytes, nullptr, + nullptr, 0)); + EXPECT_EQ(content, GetSendingFileContent()); + } + +#if defined(OS_POSIX) + static base::FilePath GetSendingFilePath() { + base::FilePath path; + bool ok = PathService::Get(base::DIR_CACHE, &path); + EXPECT_TRUE(ok); + return path.Append("ListenerThatExpectsFile.txt"); + } + + static void WriteFile(IPC::Message* message, base::File& file) { + std::string content = GetSendingFileContent(); + file.WriteAtCurrentPos(content.data(), content.size()); + file.Flush(); + message->WriteAttachment(new IPC::internal::PlatformFileAttachment( + base::ScopedFD(file.TakePlatformFile()))); + } + + static void WriteFileThenSend(IPC::Sender* sender, base::File& file) { + IPC::Message* message = + new IPC::Message(0, 2, IPC::Message::PRIORITY_NORMAL); + WriteFile(message, file); + ASSERT_TRUE(sender->Send(message)); + } + + static void WriteFileAndPipeThenSend(IPC::Sender* sender, + base::File& file, + TestingMessagePipe* pipe) { + IPC::Message* message = + new IPC::Message(0, 2, IPC::Message::PRIORITY_NORMAL); + WriteFile(message, file); + WritePipe(message, pipe); + ASSERT_TRUE(sender->Send(message)); + } + + static void ReadReceivedFile(const IPC::Message& message, + PickleIterator* iter) { + base::ScopedFD fd; + scoped_refptr<IPC::MessageAttachment> attachment; + EXPECT_TRUE(message.ReadAttachment(iter, &attachment)); + base::File file(attachment->TakePlatformFile()); + std::string content(GetSendingFileContent().size(), ' '); + file.Read(0, &content[0], content.size()); + EXPECT_EQ(content, GetSendingFileContent()); + } +#endif +}; + +class ListenerThatExpectsMessagePipe : public IPC::Listener { + public: + ListenerThatExpectsMessagePipe() : sender_(NULL) {} + + ~ListenerThatExpectsMessagePipe() override {} + + bool OnMessageReceived(const IPC::Message& message) override { + PickleIterator iter(message); + HandleSendingHelper::ReadReceivedPipe(message, &iter); + base::MessageLoop::current()->Quit(); + ListenerThatExpectsOK::SendOK(sender_); + return true; + } + + void OnChannelError() override { NOTREACHED(); } + + void set_sender(IPC::Sender* sender) { sender_ = sender; } + + private: + IPC::Sender* sender_; +}; + +TEST_F(IPCChannelMojoTest, SendMessagePipe) { + InitWithMojo("IPCChannelMojoTestSendMessagePipeClient"); + + ListenerThatExpectsOK listener; + CreateChannel(&listener); + ASSERT_TRUE(ConnectChannel()); + ASSERT_TRUE(StartClient()); + + TestingMessagePipe pipe; + HandleSendingHelper::WritePipeThenSend(channel(), &pipe); + + base::MessageLoop::current()->Run(); + this->channel()->Close(); + + EXPECT_TRUE(WaitForClientShutdown()); + DestroyChannel(); +} + +MULTIPROCESS_IPC_TEST_CLIENT_MAIN(IPCChannelMojoTestSendMessagePipeClient) { + ListenerThatExpectsMessagePipe listener; + ChannelClient client(&listener, "IPCChannelMojoTestSendMessagePipeClient"); + client.Connect(); + listener.set_sender(client.channel()); + + base::MessageLoop::current()->Run(); + + client.Close(); + + return 0; +} + +void ReadOK(mojo::MessagePipeHandle pipe) { + std::string should_be_ok("xx"); + uint32_t num_bytes = static_cast<uint32_t>(should_be_ok.size()); + CHECK_EQ(MOJO_RESULT_OK, + mojo::ReadMessageRaw(pipe, &should_be_ok[0], &num_bytes, nullptr, + nullptr, 0)); + EXPECT_EQ(should_be_ok, std::string("OK")); +} + +void WriteOK(mojo::MessagePipeHandle pipe) { + std::string ok("OK"); + CHECK_EQ(MOJO_RESULT_OK, + mojo::WriteMessageRaw(pipe, &ok[0], static_cast<uint32_t>(ok.size()), + nullptr, 0, 0)); +} + +class ListenerThatExpectsMessagePipeUsingParamTrait : public IPC::Listener { + public: + explicit ListenerThatExpectsMessagePipeUsingParamTrait(bool receiving_valid) + : sender_(NULL), receiving_valid_(receiving_valid) {} + + ~ListenerThatExpectsMessagePipeUsingParamTrait() override {} + + bool OnMessageReceived(const IPC::Message& message) override { + PickleIterator iter(message); + mojo::MessagePipeHandle handle; + EXPECT_TRUE(IPC::ParamTraits<mojo::MessagePipeHandle>::Read(&message, &iter, + &handle)); + EXPECT_EQ(handle.is_valid(), receiving_valid_); + if (receiving_valid_) { + ReadOK(handle); + MojoClose(handle.value()); + } + + base::MessageLoop::current()->Quit(); + ListenerThatExpectsOK::SendOK(sender_); + return true; + } + + void OnChannelError() override { NOTREACHED(); } + void set_sender(IPC::Sender* sender) { sender_ = sender; } + + private: + IPC::Sender* sender_; + bool receiving_valid_; +}; + +void ParamTraitMessagePipeClient(bool receiving_valid_handle, + const char* channel_name) { + ListenerThatExpectsMessagePipeUsingParamTrait listener( + receiving_valid_handle); + ChannelClient client(&listener, channel_name); + client.Connect(); + listener.set_sender(client.channel()); + + base::MessageLoop::current()->Run(); + + client.Close(); +} + +TEST_F(IPCChannelMojoTest, ParamTraitValidMessagePipe) { + InitWithMojo("ParamTraitValidMessagePipeClient"); + + ListenerThatExpectsOK listener; + CreateChannel(&listener); + ASSERT_TRUE(ConnectChannel()); + ASSERT_TRUE(StartClient()); + + TestingMessagePipe pipe; + + scoped_ptr<IPC::Message> message(new IPC::Message()); + IPC::ParamTraits<mojo::MessagePipeHandle>::Write(message.get(), + pipe.peer.release()); + WriteOK(pipe.self.get()); + + this->channel()->Send(message.release()); + base::MessageLoop::current()->Run(); + this->channel()->Close(); + + EXPECT_TRUE(WaitForClientShutdown()); + DestroyChannel(); +} + +MULTIPROCESS_IPC_TEST_CLIENT_MAIN(ParamTraitValidMessagePipeClient) { + ParamTraitMessagePipeClient(true, "ParamTraitValidMessagePipeClient"); + return 0; +} + +TEST_F(IPCChannelMojoTest, ParamTraitInvalidMessagePipe) { + InitWithMojo("ParamTraitInvalidMessagePipeClient"); + + ListenerThatExpectsOK listener; + CreateChannel(&listener); + ASSERT_TRUE(ConnectChannel()); + ASSERT_TRUE(StartClient()); + + mojo::MessagePipeHandle invalid_handle; + scoped_ptr<IPC::Message> message(new IPC::Message()); + IPC::ParamTraits<mojo::MessagePipeHandle>::Write(message.get(), + invalid_handle); + + this->channel()->Send(message.release()); + base::MessageLoop::current()->Run(); + this->channel()->Close(); + + EXPECT_TRUE(WaitForClientShutdown()); + DestroyChannel(); +} + +MULTIPROCESS_IPC_TEST_CLIENT_MAIN(ParamTraitInvalidMessagePipeClient) { + ParamTraitMessagePipeClient(false, "ParamTraitInvalidMessagePipeClient"); + return 0; +} + #if defined(OS_WIN) -class IPCChannelMojoDeadHandleTest : public IPCTestBase { +class IPCChannelMojoDeadHandleTest : public IPCChannelMojoTestBase { protected: - virtual scoped_ptr<IPC::ChannelFactory> CreateChannelFactory( + scoped_ptr<IPC::ChannelFactory> CreateChannelFactory( const IPC::ChannelHandle& handle, - base::TaskRunner* runner) override { + base::SequencedTaskRunner* runner) override { host_.reset(new IPC::ChannelMojoHost(task_runner())); return IPC::ChannelMojo::CreateServerFactory(host_->channel_delegate(), - handle); + task_runner(), handle); } - virtual bool DidStartClient() override { + bool DidStartClient() override { IPCTestBase::DidStartClient(); - base::ProcessHandle client = client_process(); + const base::ProcessHandle client = client_process().Handle(); // Forces GetFileHandleForProcess() fail. It happens occasionally // in production, so we should exercise it somehow. - ::CloseHandle(client); + // TODO(morrita): figure out how to safely test this. See crbug.com/464109. + // ::CloseHandle(client); host_->OnClientLaunched(client); return true; } @@ -284,7 +579,7 @@ class IPCChannelMojoDeadHandleTest : public IPCTestBase { TEST_F(IPCChannelMojoDeadHandleTest, InvalidClientHandle) { // Any client type is fine as it is going to be killed anyway. - Init("IPCChannelMojoTestDoNothingClient"); + InitWithMojo("IPCChannelMojoTestDoNothingClient"); // Set up IPC channel and start client. ListenerExpectingErrors listener; @@ -296,9 +591,11 @@ TEST_F(IPCChannelMojoDeadHandleTest, InvalidClientHandle) { this->channel()->Close(); - // WaitForClientShutdown() fails as client_hanadle() is already - // closed. - EXPECT_FALSE(WaitForClientShutdown()); + // TODO(morrita): We need CloseHandle() call in DidStartClient(), + // which has been disabled since crrev.com/843113003, to + // make this fail. See crbug.com/464109. + // EXPECT_FALSE(WaitForClientShutdown()); + WaitForClientShutdown(); EXPECT_TRUE(listener.has_error()); DestroyChannel(); @@ -326,13 +623,7 @@ class ListenerThatExpectsFile : public IPC::Listener { bool OnMessageReceived(const IPC::Message& message) override { PickleIterator iter(message); - - base::ScopedFD fd; - EXPECT_TRUE(message.ReadFile(&iter, &fd)); - base::File file(fd.release()); - std::string content(GetSendingFileContent().size(), ' '); - file.Read(0, &content[0], content.size()); - EXPECT_EQ(content, GetSendingFileContent()); + HandleSendingHelper::ReadReceivedFile(message, &iter); base::MessageLoop::current()->Quit(); ListenerThatExpectsOK::SendOK(sender_); return true; @@ -342,27 +633,6 @@ class ListenerThatExpectsFile : public IPC::Listener { NOTREACHED(); } - static std::string GetSendingFileContent() { - return "Hello"; - } - - static base::FilePath GetSendingFilePath() { - base::FilePath path; - bool ok = PathService::Get(base::DIR_CACHE, &path); - EXPECT_TRUE(ok); - return path.Append("ListenerThatExpectsFile.txt"); - } - - static void WriteAndSendFile(IPC::Sender* sender, base::File& file) { - std::string content = GetSendingFileContent(); - file.WriteAtCurrentPos(content.data(), content.size()); - file.Flush(); - IPC::Message* message = new IPC::Message( - 0, 2, IPC::Message::PRIORITY_NORMAL); - message->WriteFile(base::ScopedFD(file.TakePlatformFile())); - ASSERT_TRUE(sender->Send(message)); - } - void set_sender(IPC::Sender* sender) { sender_ = sender; } private: @@ -371,17 +641,17 @@ class ListenerThatExpectsFile : public IPC::Listener { TEST_F(IPCChannelMojoTest, SendPlatformHandle) { - Init("IPCChannelMojoTestSendPlatformHandleClient"); + InitWithMojo("IPCChannelMojoTestSendPlatformHandleClient"); ListenerThatExpectsOK listener; CreateChannel(&listener); ASSERT_TRUE(ConnectChannel()); ASSERT_TRUE(StartClient()); - base::File file(ListenerThatExpectsFile::GetSendingFilePath(), + base::File file(HandleSendingHelper::GetSendingFilePath(), base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE | - base::File::FLAG_READ); - ListenerThatExpectsFile::WriteAndSendFile(channel(), file); + base::File::FLAG_READ); + HandleSendingHelper::WriteFileThenSend(channel(), file); base::MessageLoop::current()->Run(); this->channel()->Close(); @@ -399,8 +669,118 @@ MULTIPROCESS_IPC_TEST_CLIENT_MAIN(IPCChannelMojoTestSendPlatformHandleClient) { base::MessageLoop::current()->Run(); + client.Close(); + return 0; } + +class ListenerThatExpectsFileAndPipe : public IPC::Listener { + public: + ListenerThatExpectsFileAndPipe() : sender_(NULL) {} + + ~ListenerThatExpectsFileAndPipe() override {} + + bool OnMessageReceived(const IPC::Message& message) override { + PickleIterator iter(message); + HandleSendingHelper::ReadReceivedFile(message, &iter); + HandleSendingHelper::ReadReceivedPipe(message, &iter); + base::MessageLoop::current()->Quit(); + ListenerThatExpectsOK::SendOK(sender_); + return true; + } + + void OnChannelError() override { NOTREACHED(); } + + void set_sender(IPC::Sender* sender) { sender_ = sender; } + + private: + IPC::Sender* sender_; +}; + +TEST_F(IPCChannelMojoTest, SendPlatformHandleAndPipe) { + InitWithMojo("IPCChannelMojoTestSendPlatformHandleAndPipeClient"); + + ListenerThatExpectsOK listener; + CreateChannel(&listener); + ASSERT_TRUE(ConnectChannel()); + ASSERT_TRUE(StartClient()); + + base::File file(HandleSendingHelper::GetSendingFilePath(), + base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE | + base::File::FLAG_READ); + TestingMessagePipe pipe; + HandleSendingHelper::WriteFileAndPipeThenSend(channel(), file, &pipe); + + base::MessageLoop::current()->Run(); + this->channel()->Close(); + + EXPECT_TRUE(WaitForClientShutdown()); + DestroyChannel(); +} + +MULTIPROCESS_IPC_TEST_CLIENT_MAIN( + IPCChannelMojoTestSendPlatformHandleAndPipeClient) { + ListenerThatExpectsFileAndPipe listener; + ChannelClient client(&listener, + "IPCChannelMojoTestSendPlatformHandleAndPipeClient"); + client.Connect(); + listener.set_sender(client.channel()); + + base::MessageLoop::current()->Run(); + + client.Close(); + + return 0; +} + #endif +#if defined(OS_LINUX) + +const base::ProcessId kMagicChildId = 54321; + +class ListenerThatVerifiesPeerPid : public IPC::Listener { + public: + void OnChannelConnected(int32 peer_pid) override { + EXPECT_EQ(peer_pid, kMagicChildId); + base::MessageLoop::current()->Quit(); + } + + bool OnMessageReceived(const IPC::Message& message) override { + NOTREACHED(); + return true; + } +}; + +TEST_F(IPCChannelMojoTest, VerifyGlobalPid) { + InitWithMojo("IPCChannelMojoTestVerifyGlobalPidClient"); + + ListenerThatVerifiesPeerPid listener; + CreateChannel(&listener); + ASSERT_TRUE(ConnectChannel()); + ASSERT_TRUE(StartClient()); + + base::MessageLoop::current()->Run(); + channel()->Close(); + + EXPECT_TRUE(WaitForClientShutdown()); + DestroyChannel(); +} + +MULTIPROCESS_IPC_TEST_CLIENT_MAIN(IPCChannelMojoTestVerifyGlobalPidClient) { + IPC::Channel::SetGlobalPid(kMagicChildId); + ListenerThatQuits listener; + ChannelClient client(&listener, + "IPCChannelMojoTestVerifyGlobalPidClient"); + client.Connect(); + + base::MessageLoop::current()->Run(); + + client.Close(); + + return 0; +} + +#endif // OS_LINUX + } // namespace diff --git a/chromium/ipc/mojo/ipc_message_pipe_reader.cc b/chromium/ipc/mojo/ipc_message_pipe_reader.cc index b0df9976b3c..35ba7fd6397 100644 --- a/chromium/ipc/mojo/ipc_message_pipe_reader.cc +++ b/chromium/ipc/mojo/ipc_message_pipe_reader.cc @@ -8,24 +8,34 @@ #include "base/bind_helpers.h" #include "base/location.h" #include "base/logging.h" -#include "base/message_loop/message_loop_proxy.h" -#include "mojo/public/cpp/environment/environment.h" +#include "base/single_thread_task_runner.h" +#include "base/thread_task_runner_handle.h" +#include "ipc/mojo/async_handle_waiter.h" +#include "ipc/mojo/ipc_channel_mojo.h" namespace IPC { namespace internal { -MessagePipeReader::MessagePipeReader(mojo::ScopedMessagePipeHandle handle) - : pipe_wait_id_(0), - pipe_(handle.Pass()) { - StartWaiting(); +MessagePipeReader::MessagePipeReader(mojo::ScopedMessagePipeHandle handle, + MessagePipeReader::Delegate* delegate) + : pipe_(handle.Pass()), + delegate_(delegate), + async_waiter_( + new AsyncHandleWaiter(base::Bind(&MessagePipeReader::PipeIsReady, + base::Unretained(this)))), + pending_send_error_(MOJO_RESULT_OK) { } MessagePipeReader::~MessagePipeReader() { + // The pipe should be closed before deletion. CHECK(!IsValid()); + DCHECK_EQ(pending_send_error_, MOJO_RESULT_OK); } void MessagePipeReader::Close() { - StopWaiting(); + // All pending errors should be signaled before Close(). + DCHECK_EQ(pending_send_error_, MOJO_RESULT_OK); + async_waiter_.reset(); pipe_.reset(); OnPipeClosed(); } @@ -35,73 +45,76 @@ void MessagePipeReader::CloseWithError(MojoResult error) { Close(); } -// static -void MessagePipeReader::InvokePipeIsReady(void* closure, MojoResult result) { - reinterpret_cast<MessagePipeReader*>(closure)->PipeIsReady(result); +void MessagePipeReader::CloseWithErrorIfPending() { + if (pending_send_error_ == MOJO_RESULT_OK) + return; + MojoResult error = pending_send_error_; + pending_send_error_ = MOJO_RESULT_OK; + CloseWithError(error); + return; } -void MessagePipeReader::StartWaiting() { - DCHECK(pipe_.is_valid()); - DCHECK(!pipe_wait_id_); - // Not using MOJO_HANDLE_SIGNAL_WRITABLE here, expecting buffer in - // MessagePipe. - // - // TODO(morrita): Should we re-set the signal when we get new - // message to send? - pipe_wait_id_ = mojo::Environment::GetDefaultAsyncWaiter()->AsyncWait( - pipe_.get().value(), - MOJO_HANDLE_SIGNAL_READABLE, - MOJO_DEADLINE_INDEFINITE, - &InvokePipeIsReady, - this); +void MessagePipeReader::CloseWithErrorLater(MojoResult error) { + pending_send_error_ = error; } -void MessagePipeReader::StopWaiting() { - if (!pipe_wait_id_) - return; - mojo::Environment::GetDefaultAsyncWaiter()->CancelWait(pipe_wait_id_); - pipe_wait_id_ = 0; -} +bool MessagePipeReader::Send(scoped_ptr<Message> message) { + DCHECK(IsValid()); + + message->TraceMessageBegin(); + std::vector<MojoHandle> handles; + MojoResult result = MOJO_RESULT_OK; + result = ChannelMojo::ReadFromMessageAttachmentSet(message.get(), &handles); + if (result == MOJO_RESULT_OK) { + result = MojoWriteMessage(handle(), + message->data(), + static_cast<uint32>(message->size()), + handles.empty() ? nullptr : &handles[0], + static_cast<uint32>(handles.size()), + MOJO_WRITE_MESSAGE_FLAG_NONE); + } -void MessagePipeReader::PipeIsReady(MojoResult wait_result) { - pipe_wait_id_ = 0; + if (result != MOJO_RESULT_OK) { + std::for_each(handles.begin(), handles.end(), &MojoClose); + // We cannot call CloseWithError() here as Send() is protected by + // ChannelMojo's lock and CloseWithError() could re-enter ChannelMojo. We + // cannot call CloseWithError() also because Send() can be called from + // non-UI thread while OnPipeError() expects to be called on IO thread. + CloseWithErrorLater(result); + return false; + } - if (wait_result != MOJO_RESULT_OK) { - if (wait_result != MOJO_RESULT_ABORTED) { - // FAILED_PRECONDITION happens every time the peer is dead so - // it isn't worth polluting the log message. - DLOG_IF(WARNING, wait_result != MOJO_RESULT_FAILED_PRECONDITION) - << "Pipe got error from the waiter. Closing: " - << wait_result; - OnPipeError(wait_result); - } + return true; +} - Close(); +void MessagePipeReader::OnMessageReceived() { + Message message(data_buffer().empty() ? "" : &data_buffer()[0], + static_cast<uint32>(data_buffer().size())); + + std::vector<MojoHandle> handle_buffer; + TakeHandleBuffer(&handle_buffer); + MojoResult write_result = + ChannelMojo::WriteToMessageAttachmentSet(handle_buffer, &message); + if (write_result != MOJO_RESULT_OK) { + CloseWithError(write_result); return; } - while (pipe_.is_valid()) { - MojoResult read_result = ReadMessageBytes(); - if (read_result == MOJO_RESULT_SHOULD_WAIT) - break; - if (read_result != MOJO_RESULT_OK) { - // FAILED_PRECONDITION means that all the received messages - // got consumed and the peer is already closed. - if (read_result != MOJO_RESULT_FAILED_PRECONDITION) { - DLOG(WARNING) - << "Pipe got error from ReadMessage(). Closing: " << read_result; - OnPipeError(read_result); - } - - Close(); - break; - } + message.TraceMessageEnd(); + delegate_->OnMessageReceived(message); +} - OnMessageReceived(); - } +void MessagePipeReader::OnPipeClosed() { + if (!delegate_) + return; + delegate_->OnPipeClosed(this); + delegate_ = nullptr; +} - if (pipe_.is_valid()) - StartWaiting(); +void MessagePipeReader::OnPipeError(MojoResult error) { + if (!delegate_) + return; + delegate_->OnPipeError(this); } MojoResult MessagePipeReader::ReadMessageBytes() { @@ -110,9 +123,9 @@ MojoResult MessagePipeReader::ReadMessageBytes() { uint32_t num_bytes = static_cast<uint32_t>(data_buffer_.size()); uint32_t num_handles = 0; MojoResult result = MojoReadMessage(pipe_.get().value(), - num_bytes ? &data_buffer_[0] : NULL, + num_bytes ? &data_buffer_[0] : nullptr, &num_bytes, - NULL, + nullptr, &num_handles, MOJO_READ_MESSAGE_FLAG_NONE); data_buffer_.resize(num_bytes); @@ -121,9 +134,9 @@ MojoResult MessagePipeReader::ReadMessageBytes() { // MOJO_RESULT_RESOURCE_EXHAUSTED was asking the caller that // it needs more bufer. So we re-read it with resized buffers. result = MojoReadMessage(pipe_.get().value(), - num_bytes ? &data_buffer_[0] : NULL, + num_bytes ? &data_buffer_[0] : nullptr, &num_bytes, - num_handles ? &handle_buffer_[0] : NULL, + num_handles ? &handle_buffer_[0] : nullptr, &num_handles, MOJO_READ_MESSAGE_FLAG_NONE); } @@ -133,11 +146,84 @@ MojoResult MessagePipeReader::ReadMessageBytes() { return result; } +void MessagePipeReader::ReadAvailableMessages() { + while (pipe_.is_valid()) { + MojoResult read_result = ReadMessageBytes(); + if (read_result == MOJO_RESULT_SHOULD_WAIT) + break; + if (read_result != MOJO_RESULT_OK) { + // FAILED_PRECONDITION means that all the received messages + // got consumed and the peer is already closed. + if (read_result != MOJO_RESULT_FAILED_PRECONDITION) { + DLOG(WARNING) + << "Pipe got error from ReadMessage(). Closing: " << read_result; + OnPipeError(read_result); + } + + Close(); + break; + } + + OnMessageReceived(); + } + +} + +void MessagePipeReader::ReadMessagesThenWait() { + while (true) { + ReadAvailableMessages(); + if (!pipe_.is_valid()) + break; + // |Wait()| is safe to call only after all messages are read. + // If can fail with |MOJO_RESULT_ALREADY_EXISTS| otherwise. + // Also, we don't use MOJO_HANDLE_SIGNAL_WRITABLE here, expecting buffer in + // MessagePipe. + MojoResult result = + async_waiter_->Wait(pipe_.get().value(), MOJO_HANDLE_SIGNAL_READABLE); + // If the result is |MOJO_RESULT_ALREADY_EXISTS|, there could be messages + // that have been arrived after the last |ReadAvailableMessages()|. + // We have to consume then and retry in that case. + if (result != MOJO_RESULT_ALREADY_EXISTS) { + if (result != MOJO_RESULT_OK) { + LOG(ERROR) << "Failed to wait on the pipe. Result is " << result; + OnPipeError(result); + Close(); + } + + break; + } + } +} + +void MessagePipeReader::PipeIsReady(MojoResult wait_result) { + CloseWithErrorIfPending(); + if (!IsValid()) { + // There was a pending error and it closed the pipe. + // We cannot do the work anymore. + return; + } + + if (wait_result != MOJO_RESULT_OK) { + if (wait_result != MOJO_RESULT_ABORTED) { + // FAILED_PRECONDITION happens every time the peer is dead so + // it isn't worth polluting the log message. + LOG_IF(WARNING, wait_result != MOJO_RESULT_FAILED_PRECONDITION) + << "Pipe got error from the waiter. Closing: " << wait_result; + OnPipeError(wait_result); + } + + Close(); + return; + } + + ReadMessagesThenWait(); +} + void MessagePipeReader::DelayedDeleter::operator()( MessagePipeReader* ptr) const { ptr->Close(); - base::MessageLoopProxy::current()->PostTask( - FROM_HERE, base::Bind(&DeleteNow, ptr)); + base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, + base::Bind(&DeleteNow, ptr)); } } // namespace internal diff --git a/chromium/ipc/mojo/ipc_message_pipe_reader.h b/chromium/ipc/mojo/ipc_message_pipe_reader.h index ecfa018b5e0..937930eef33 100644 --- a/chromium/ipc/mojo/ipc_message_pipe_reader.h +++ b/chromium/ipc/mojo/ipc_message_pipe_reader.h @@ -7,13 +7,17 @@ #include <vector> +#include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" -#include "mojo/public/c/environment/async_waiter.h" -#include "mojo/public/cpp/system/core.h" +#include "ipc/ipc_message.h" +#include "third_party/mojo/src/mojo/public/c/environment/async_waiter.h" +#include "third_party/mojo/src/mojo/public/cpp/system/core.h" namespace IPC { namespace internal { +class AsyncHandleWaiter; + // A helper class to handle bytestream directly over mojo::MessagePipe // in template-method pattern. MessagePipeReader manages the lifetime // of given MessagePipe and participates the event loop, and @@ -30,6 +34,13 @@ namespace internal { // class MessagePipeReader { public: + class Delegate { + public: + virtual void OnMessageReceived(Message& message) = 0; + virtual void OnPipeClosed(MessagePipeReader* reader) = 0; + virtual void OnPipeError(MessagePipeReader* reader) = 0; + }; + // Delay the object deletion using the current message loop. // This is intended to used by MessagePipeReader owners. class DelayedDeleter { @@ -39,13 +50,16 @@ class MessagePipeReader { static void DeleteNow(MessagePipeReader* ptr) { delete ptr; } DelayedDeleter() {} - DelayedDeleter(const DefaultType&) {} + explicit DelayedDeleter(const DefaultType&) {} DelayedDeleter& operator=(const DefaultType&) { return *this; } void operator()(MessagePipeReader* ptr) const; }; - explicit MessagePipeReader(mojo::ScopedMessagePipeHandle handle); + // Both parameters must be non-null. + // Build a reader that reads messages from |handle| and lets |delegate| know. + // Note that MessagePipeReader doesn't delete |delete|. + MessagePipeReader(mojo::ScopedMessagePipeHandle handle, Delegate* delegate); virtual ~MessagePipeReader(); MojoHandle handle() const { return pipe_.get().value(); } @@ -65,29 +79,31 @@ class MessagePipeReader { void Close(); // Close the mesage pipe with notifying the client with the error. void CloseWithError(MojoResult error); + void CloseWithErrorLater(MojoResult error); + void CloseWithErrorIfPending(); + // Return true if the MessagePipe is alive. bool IsValid() { return pipe_.is_valid(); } - // - // The client have to implment these callback to get the readiness - // event from the reader - // - virtual void OnMessageReceived() = 0; - virtual void OnPipeClosed() = 0; - virtual void OnPipeError(MojoResult error) = 0; + bool Send(scoped_ptr<Message> message); + void ReadMessagesThenWait(); private: - static void InvokePipeIsReady(void* closure, MojoResult result); + void OnMessageReceived(); + void OnPipeClosed(); + void OnPipeError(MojoResult error); MojoResult ReadMessageBytes(); void PipeIsReady(MojoResult wait_result); - void StartWaiting(); - void StopWaiting(); + void ReadAvailableMessages(); std::vector<char> data_buffer_; std::vector<MojoHandle> handle_buffer_; - MojoAsyncWaitID pipe_wait_id_; mojo::ScopedMessagePipeHandle pipe_; + // |delegate_| and |async_waiter_| are null once the message pipe is closed. + Delegate* delegate_; + scoped_ptr<AsyncHandleWaiter> async_waiter_; + MojoResult pending_send_error_; DISALLOW_COPY_AND_ASSIGN(MessagePipeReader); }; diff --git a/chromium/ipc/mojo/ipc_mojo.gyp b/chromium/ipc/mojo/ipc_mojo.gyp index a9a26737828..a392283263d 100644 --- a/chromium/ipc/mojo/ipc_mojo.gyp +++ b/chromium/ipc/mojo/ipc_mojo.gyp @@ -17,27 +17,35 @@ 'defines': [ 'IPC_MOJO_IMPLEMENTATION', ], - 'includes': [ '../../mojo/public/tools/bindings/mojom_bindings_generator.gypi' ], + 'includes': [ '../../third_party/mojo/mojom_bindings_generator.gypi' ], 'dependencies': [ '../ipc.gyp:ipc', '../../base/base.gyp:base', '../../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations', - '../../mojo/edk/mojo_edk.gyp:mojo_system_impl', '../../mojo/mojo_base.gyp:mojo_environment_chromium', - '../../mojo/public/mojo_public.gyp:mojo_cpp_bindings', + '../../third_party/mojo/mojo_edk.gyp:mojo_system_impl', + '../../third_party/mojo/mojo_public.gyp:mojo_cpp_bindings', ], 'sources': [ 'client_channel.mojom', + 'async_handle_waiter.cc', + 'async_handle_waiter.h', 'ipc_channel_mojo.cc', 'ipc_channel_mojo.h', 'ipc_channel_mojo_host.cc', 'ipc_channel_mojo_host.h', - 'ipc_channel_mojo_readers.cc', - 'ipc_channel_mojo_readers.h', 'ipc_mojo_bootstrap.cc', 'ipc_mojo_bootstrap.h', + 'ipc_mojo_handle_attachment.cc', + 'ipc_mojo_handle_attachment.h', + 'ipc_mojo_message_helper.cc', + 'ipc_mojo_message_helper.h', + 'ipc_mojo_param_traits.cc', + 'ipc_mojo_param_traits.h', 'ipc_message_pipe_reader.cc', 'ipc_message_pipe_reader.h', + 'scoped_ipc_support.cc', + 'scoped_ipc_support.h', ], # TODO(gregoryd): direct_dependent_settings should be shared with the # 64-bit target, but it doesn't work due to a bug in gyp @@ -56,16 +64,17 @@ '../../base/base.gyp:base', '../../base/base.gyp:base_i18n', '../../base/base.gyp:test_support_base', - '../../mojo/edk/mojo_edk.gyp:mojo_system_impl', '../../mojo/mojo_base.gyp:mojo_environment_chromium', - '../../mojo/public/mojo_public.gyp:mojo_cpp_bindings', '../../testing/gtest.gyp:gtest', + '../../third_party/mojo/mojo_edk.gyp:mojo_system_impl', + '../../third_party/mojo/mojo_public.gyp:mojo_cpp_bindings', 'ipc_mojo', ], 'include_dirs': [ '..' ], 'sources': [ + 'async_handle_waiter_unittest.cc', 'run_all_unittests.cc', 'ipc_channel_mojo_unittest.cc', 'ipc_mojo_bootstrap_unittest.cc', @@ -83,10 +92,10 @@ '../../base/base.gyp:base_i18n', '../../base/base.gyp:test_support_base', '../../base/base.gyp:test_support_perf', - '../../mojo/edk/mojo_edk.gyp:mojo_system_impl', '../../mojo/mojo_base.gyp:mojo_environment_chromium', - '../../mojo/public/mojo_public.gyp:mojo_cpp_bindings', '../../testing/gtest.gyp:gtest', + '../../third_party/mojo/mojo_edk.gyp:mojo_system_impl', + '../../third_party/mojo/mojo_public.gyp:mojo_cpp_bindings', 'ipc_mojo', ], 'include_dirs': [ diff --git a/chromium/ipc/mojo/ipc_mojo_bootstrap.cc b/chromium/ipc/mojo/ipc_mojo_bootstrap.cc index 4af4e503a9d..d307246033f 100644 --- a/chromium/ipc/mojo/ipc_mojo_bootstrap.cc +++ b/chromium/ipc/mojo/ipc_mojo_bootstrap.cc @@ -8,7 +8,7 @@ #include "base/process/process_handle.h" #include "ipc/ipc_message_utils.h" #include "ipc/ipc_platform_file.h" -#include "mojo/edk/embedder/platform_channel_pair.h" +#include "third_party/mojo/src/mojo/edk/embedder/platform_channel_pair.h" namespace IPC { @@ -61,7 +61,7 @@ void MojoServerBootstrap::SendClientPipe() { // GetFileHandleForProcess() only fails on Windows. NOTREACHED(); #endif - DLOG(WARNING) << "Failed to translate file handle for client process."; + LOG(WARNING) << "Failed to translate file handle for client process."; Fail(); return; } @@ -100,9 +100,14 @@ void MojoServerBootstrap::OnChannelConnected(int32 peer_pid) { } bool MojoServerBootstrap::OnMessageReceived(const Message&) { - DCHECK_EQ(state(), STATE_WAITING_ACK); - set_state(STATE_READY); + if (state() != STATE_WAITING_ACK) { + set_state(STATE_ERROR); + LOG(ERROR) << "Got inconsistent message from client."; + return false; + } + set_state(STATE_READY); + CHECK(server_pipe_.is_valid()); delegate()->OnPipeAvailable( mojo::embedder::ScopedPlatformHandle(server_pipe_.release())); @@ -129,10 +134,16 @@ MojoClientBootstrap::MojoClientBootstrap() { } bool MojoClientBootstrap::OnMessageReceived(const Message& message) { + if (state() != STATE_INITIALIZED) { + set_state(STATE_ERROR); + LOG(ERROR) << "Got inconsistent message from server."; + return false; + } + PlatformFileForTransit pipe; PickleIterator iter(message); if (!ParamTraits<PlatformFileForTransit>::Read(&message, &iter, &pipe)) { - DLOG(WARNING) << "Failed to read a file handle from bootstrap channel."; + LOG(WARNING) << "Failed to read a file handle from bootstrap channel."; message.set_dispatch_error(); return false; } @@ -189,6 +200,10 @@ bool MojoBootstrap::Connect() { return channel_->Connect(); } +base::ProcessId MojoBootstrap::GetSelfPID() const { + return channel_->GetSelfPID(); +} + void MojoBootstrap::OnBadMessageReceived(const Message& message) { Fail(); } diff --git a/chromium/ipc/mojo/ipc_mojo_bootstrap.h b/chromium/ipc/mojo/ipc_mojo_bootstrap.h index ad5283d96ca..1f71d2e6d4f 100644 --- a/chromium/ipc/mojo/ipc_mojo_bootstrap.h +++ b/chromium/ipc/mojo/ipc_mojo_bootstrap.h @@ -9,7 +9,7 @@ #include "base/process/process_handle.h" #include "ipc/ipc_channel.h" #include "ipc/ipc_listener.h" -#include "mojo/edk/embedder/scoped_platform_handle.h" +#include "third_party/mojo/src/mojo/edk/embedder/scoped_platform_handle.h" namespace IPC { @@ -46,6 +46,9 @@ class IPC_MOJO_EXPORT MojoBootstrap : public Listener { // Start the handshake over the underlying platform channel. bool Connect(); + // GetSelfPID returns the PID associated with |channel_|. + base::ProcessId GetSelfPID() const; + // Each client should call this once the process handle becomes known. virtual void OnClientLaunched(base::ProcessHandle process) = 0; @@ -55,6 +58,9 @@ class IPC_MOJO_EXPORT MojoBootstrap : public Listener { #endif // defined(OS_POSIX) && !defined(OS_NACL) protected: + // On MojoServerBootstrap: INITIALIZED -> WAITING_ACK -> READY + // On MojoClientBootstrap: INITIALIZED -> READY + // STATE_ERROR is a catch-all state that captures any observed error. enum State { STATE_INITIALIZED, STATE_WAITING_ACK, STATE_READY, STATE_ERROR }; Delegate* delegate() const { return delegate_; } diff --git a/chromium/ipc/mojo/ipc_mojo_bootstrap_unittest.cc b/chromium/ipc/mojo/ipc_mojo_bootstrap_unittest.cc index a071d66a7e8..fbe0fa89589 100644 --- a/chromium/ipc/mojo/ipc_mojo_bootstrap_unittest.cc +++ b/chromium/ipc/mojo/ipc_mojo_bootstrap_unittest.cc @@ -55,7 +55,7 @@ TEST_F(IPCMojoBootstrapTest, Connect) { #else ASSERT_TRUE(StartClient()); #endif - bootstrap->OnClientLaunched(client_process()); + bootstrap->OnClientLaunched(client_process().Handle()); base::MessageLoop::current()->Run(); diff --git a/chromium/ipc/mojo/ipc_mojo_handle_attachment.cc b/chromium/ipc/mojo/ipc_mojo_handle_attachment.cc new file mode 100644 index 00000000000..98ac5c3c5e0 --- /dev/null +++ b/chromium/ipc/mojo/ipc_mojo_handle_attachment.cc @@ -0,0 +1,43 @@ +// Copyright (c) 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/mojo/ipc_mojo_handle_attachment.h" + +#include "ipc/ipc_message_attachment_set.h" +#include "third_party/mojo/src/mojo/edk/embedder/embedder.h" + +namespace IPC { +namespace internal { + +MojoHandleAttachment::MojoHandleAttachment(mojo::ScopedHandle handle) + : handle_(handle.Pass()) { +} + +MojoHandleAttachment::~MojoHandleAttachment() { +} + +MessageAttachment::Type MojoHandleAttachment::GetType() const { + return TYPE_MOJO_HANDLE; +} + +#if defined(OS_POSIX) +base::PlatformFile MojoHandleAttachment::TakePlatformFile() { + mojo::embedder::ScopedPlatformHandle platform_handle; + MojoResult unwrap_result = mojo::embedder::PassWrappedPlatformHandle( + handle_.release().value(), &platform_handle); + if (unwrap_result != MOJO_RESULT_OK) { + LOG(ERROR) << "Pipe failed to covert handles. Closing: " << unwrap_result; + return -1; + } + + return platform_handle.release().fd; +} +#endif // OS_POSIX + +mojo::ScopedHandle MojoHandleAttachment::TakeHandle() { + return handle_.Pass(); +} + +} // namespace internal +} // namespace IPC diff --git a/chromium/ipc/mojo/ipc_mojo_handle_attachment.h b/chromium/ipc/mojo/ipc_mojo_handle_attachment.h new file mode 100644 index 00000000000..ef5a318ec72 --- /dev/null +++ b/chromium/ipc/mojo/ipc_mojo_handle_attachment.h @@ -0,0 +1,50 @@ +// Copyright (c) 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_MOJO_IPC_MOJO_HANDLE_ATTACHMENT_H_ +#define IPC_MOJO_IPC_MOJO_HANDLE_ATTACHMENT_H_ + +#include "base/files/file.h" +#include "ipc/ipc_export.h" +#include "ipc/ipc_message_attachment.h" +#include "third_party/mojo/src/mojo/public/cpp/system/handle.h" + +namespace IPC { + +namespace internal { + +// A MessageAttachment that holds a MojoHandle. +// * On the sending side, every Mojo handle is a MessagePipe. This is because +// any platform files are wrapped by PlatformFileAttachment. +// * On the receiving side, the handle can be either MessagePipe or wrapped +// platform file: All files, not only MessagePipes are wrapped as a +// MojoHandle. The message deserializer should know which type of the object +// the handle wraps. +class IPC_MOJO_EXPORT MojoHandleAttachment : public MessageAttachment { + public: + explicit MojoHandleAttachment(mojo::ScopedHandle handle); + + Type GetType() const override; + +#if defined(OS_POSIX) + // Returns wrapped file if it wraps a file, or + // an invalid fd otherwise. The ownership of handle + // is passed to the caller. + base::PlatformFile TakePlatformFile() override; +#endif // OS_POSIX + + // Returns the owning handle transferring the ownership. + mojo::ScopedHandle TakeHandle(); + + private: + ~MojoHandleAttachment() override; + mojo::ScopedHandle handle_; + + DISALLOW_COPY_AND_ASSIGN(MojoHandleAttachment); +}; + +} // namespace internal +} // namespace IPC + +#endif // IPC_MOJO_IPC_MOJO_HANDLE_ATTACHMENT_H_ diff --git a/chromium/ipc/mojo/ipc_mojo_message_helper.cc b/chromium/ipc/mojo/ipc_mojo_message_helper.cc new file mode 100644 index 00000000000..38b870dde50 --- /dev/null +++ b/chromium/ipc/mojo/ipc_mojo_message_helper.cc @@ -0,0 +1,47 @@ +// Copyright (c) 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/mojo/ipc_mojo_message_helper.h" + +#include "ipc/mojo/ipc_mojo_handle_attachment.h" + +namespace IPC { + +// static +bool MojoMessageHelper::WriteMessagePipeTo( + Message* message, + mojo::ScopedMessagePipeHandle handle) { + message->WriteAttachment(new internal::MojoHandleAttachment( + mojo::ScopedHandle::From(handle.Pass()))); + return true; +} + +// static +bool MojoMessageHelper::ReadMessagePipeFrom( + const Message* message, + PickleIterator* iter, + mojo::ScopedMessagePipeHandle* handle) { + scoped_refptr<MessageAttachment> attachment; + if (!message->ReadAttachment(iter, &attachment)) { + LOG(ERROR) << "Failed to read attachment for message pipe."; + return false; + } + + if (attachment->GetType() != MessageAttachment::TYPE_MOJO_HANDLE) { + LOG(ERROR) << "Unxpected attachment type:" << attachment->GetType(); + return false; + } + + handle->reset(mojo::MessagePipeHandle( + static_cast<internal::MojoHandleAttachment*>(attachment.get()) + ->TakeHandle() + .release() + .value())); + return true; +} + +MojoMessageHelper::MojoMessageHelper() { +} + +} // namespace IPC diff --git a/chromium/ipc/mojo/ipc_mojo_message_helper.h b/chromium/ipc/mojo/ipc_mojo_message_helper.h new file mode 100644 index 00000000000..efb48c33a94 --- /dev/null +++ b/chromium/ipc/mojo/ipc_mojo_message_helper.h @@ -0,0 +1,29 @@ +// Copyright (c) 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_MOJO_IPC_MOJO_MESSAGE_HELPER_H_ +#define IPC_MOJO_IPC_MOJO_MESSAGE_HELPER_H_ + +#include "ipc/ipc_export.h" +#include "ipc/ipc_message.h" +#include "third_party/mojo/src/mojo/public/cpp/system/message_pipe.h" + +namespace IPC { + +// Reads and writes |mojo::MessagePipe| from/to |Message|. +class IPC_MOJO_EXPORT MojoMessageHelper { + public: + static bool WriteMessagePipeTo(Message* message, + mojo::ScopedMessagePipeHandle handle); + static bool ReadMessagePipeFrom(const Message* message, + PickleIterator* iter, + mojo::ScopedMessagePipeHandle* handle); + + private: + MojoMessageHelper(); +}; + +} // namespace IPC + +#endif // IPC_MOJO_IPC_MOJO_MESSAGE_HELPER_H_ diff --git a/chromium/ipc/mojo/ipc_mojo_param_traits.cc b/chromium/ipc/mojo/ipc_mojo_param_traits.cc new file mode 100644 index 00000000000..80c3ca70c92 --- /dev/null +++ b/chromium/ipc/mojo/ipc_mojo_param_traits.cc @@ -0,0 +1,43 @@ +// 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/mojo/ipc_mojo_param_traits.h" + +#include "ipc/ipc_message_utils.h" +#include "ipc/mojo/ipc_mojo_message_helper.h" + +namespace IPC { + +void ParamTraits<mojo::MessagePipeHandle>::Write(Message* m, + const param_type& p) { + WriteParam(m, p.is_valid()); + if (p.is_valid()) + MojoMessageHelper::WriteMessagePipeTo(m, mojo::ScopedMessagePipeHandle(p)); +} + +bool ParamTraits<mojo::MessagePipeHandle>::Read(const Message* m, + PickleIterator* iter, + param_type* r) { + bool is_valid; + if (!ReadParam(m, iter, &is_valid)) + return false; + if (!is_valid) + return true; + + mojo::ScopedMessagePipeHandle handle; + if (!MojoMessageHelper::ReadMessagePipeFrom(m, iter, &handle)) + return false; + DCHECK(handle.is_valid()); + *r = handle.release(); + return true; +} + +void ParamTraits<mojo::MessagePipeHandle>::Log(const param_type& p, + std::string* l) { + l->append("mojo::MessagePipeHandle("); + LogParam(p.value(), l); + l->append(")"); +} + +} // namespace IPC diff --git a/chromium/ipc/mojo/ipc_mojo_param_traits.h b/chromium/ipc/mojo/ipc_mojo_param_traits.h new file mode 100644 index 00000000000..d12e44fccd8 --- /dev/null +++ b/chromium/ipc/mojo/ipc_mojo_param_traits.h @@ -0,0 +1,30 @@ +// 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_MOJO_IPC_MOJO_PARAM_TRAITS_H_ +#define IPC_MOJO_IPC_MOJO_PARAM_TRAITS_H_ + +#include <string> + +#include "ipc/ipc_export.h" +#include "ipc/ipc_param_traits.h" +#include "third_party/mojo/src/mojo/public/cpp/system/message_pipe.h" + +class PickleIterator; + +namespace IPC { + +class Message; + +template <> +struct IPC_MOJO_EXPORT ParamTraits<mojo::MessagePipeHandle> { + typedef mojo::MessagePipeHandle param_type; + static void Write(Message* m, const param_type& p); + static bool Read(const Message* m, PickleIterator* iter, param_type* r); + static void Log(const param_type& p, std::string* l); +}; + +} // namespace IPC + +#endif // IPC_MOJO_IPC_MOJO_PARAM_TRAITS_H_ diff --git a/chromium/ipc/mojo/ipc_mojo_perftest.cc b/chromium/ipc/mojo/ipc_mojo_perftest.cc index 29e78397ce2..dc96dd19a14 100644 --- a/chromium/ipc/mojo/ipc_mojo_perftest.cc +++ b/chromium/ipc/mojo/ipc_mojo_perftest.cc @@ -3,10 +3,11 @@ // found in the LICENSE file. #include "base/lazy_instance.h" +#include "base/run_loop.h" #include "ipc/ipc_perftest_support.h" #include "ipc/mojo/ipc_channel_mojo.h" #include "ipc/mojo/ipc_channel_mojo_host.h" -#include "mojo/edk/embedder/test_embedder.h" +#include "third_party/mojo/src/mojo/edk/embedder/test_embedder.h" namespace { @@ -29,42 +30,46 @@ public: MojoChannelPerfTest(); + void TearDown() override { + IPC::test::IPCChannelPerfTestBase::TearDown(); + } + scoped_ptr<IPC::ChannelFactory> CreateChannelFactory( const IPC::ChannelHandle& handle, - base::TaskRunner* runner) override { - host_.reset(new IPC::ChannelMojoHost(task_runner())); + base::SequencedTaskRunner* runner) override { + host_.reset(new IPC::ChannelMojoHost(runner)); return IPC::ChannelMojo::CreateServerFactory(host_->channel_delegate(), - handle); + runner, handle); } bool DidStartClient() override { bool ok = IPCTestBase::DidStartClient(); DCHECK(ok); - host_->OnClientLaunched(client_process()); + host_->OnClientLaunched(client_process().Handle()); return ok; } - void set_io_thread_task_runner(base::TaskRunner* runner) { - io_thread_task_runner_ = runner; - } - private: - base::TaskRunner* io_thread_task_runner_; scoped_ptr<IPC::ChannelMojoHost> host_; }; -MojoChannelPerfTest::MojoChannelPerfTest() - : io_thread_task_runner_() { +MojoChannelPerfTest::MojoChannelPerfTest() { g_mojo_initializer.Get(); } TEST_F(MojoChannelPerfTest, ChannelPingPong) { RunTestChannelPingPong(GetDefaultTestParams()); + + base::RunLoop run_loop; + run_loop.RunUntilIdle(); } TEST_F(MojoChannelPerfTest, ChannelProxyPingPong) { RunTestChannelProxyPingPong(GetDefaultTestParams()); + + base::RunLoop run_loop; + run_loop.RunUntilIdle(); } class MojoTestClient : public IPC::test::PingPongTestClient { @@ -82,16 +87,19 @@ MojoTestClient::MojoTestClient() { scoped_ptr<IPC::Channel> MojoTestClient::CreateChannel( IPC::Listener* listener) { - return scoped_ptr<IPC::Channel>( - IPC::ChannelMojo::Create(NULL, - IPCTestBase::GetChannelName("PerformanceClient"), - IPC::Channel::MODE_CLIENT, - listener)); + return scoped_ptr<IPC::Channel>(IPC::ChannelMojo::Create( + NULL, task_runner(), IPCTestBase::GetChannelName("PerformanceClient"), + IPC::Channel::MODE_CLIENT, listener)); } MULTIPROCESS_IPC_TEST_CLIENT_MAIN(PerformanceClient) { MojoTestClient client; - return client.RunMain(); + int rv = client.RunMain(); + + base::RunLoop run_loop; + run_loop.RunUntilIdle(); + + return rv; } } // namespace diff --git a/chromium/ipc/mojo/run_all_unittests.cc b/chromium/ipc/mojo/run_all_unittests.cc index cc5b1028c89..fe2bf85c7a1 100644 --- a/chromium/ipc/mojo/run_all_unittests.cc +++ b/chromium/ipc/mojo/run_all_unittests.cc @@ -6,7 +6,7 @@ #include "base/bind.h" #include "base/test/launcher/unit_test_launcher.h" #include "base/test/test_suite.h" -#include "mojo/edk/embedder/test_embedder.h" +#include "third_party/mojo/src/mojo/edk/embedder/test_embedder.h" #if defined(OS_ANDROID) #include "base/android/jni_android.h" diff --git a/chromium/ipc/mojo/scoped_ipc_support.cc b/chromium/ipc/mojo/scoped_ipc_support.cc new file mode 100644 index 00000000000..b1d530e0463 --- /dev/null +++ b/chromium/ipc/mojo/scoped_ipc_support.cc @@ -0,0 +1,178 @@ +// 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/mojo/scoped_ipc_support.h" + +#include "base/bind.h" +#include "base/lazy_instance.h" +#include "base/logging.h" +#include "base/message_loop/message_loop.h" +#include "base/synchronization/condition_variable.h" +#include "base/synchronization/lock.h" +#include "base/synchronization/waitable_event.h" +#include "base/thread_task_runner_handle.h" +#include "third_party/mojo/src/mojo/edk/embedder/embedder.h" +#include "third_party/mojo/src/mojo/edk/embedder/process_delegate.h" + +namespace IPC { + +namespace { + +class IPCSupportInitializer : public mojo::embedder::ProcessDelegate { + public: + IPCSupportInitializer() + : init_count_(0), + shutting_down_(false), + was_shut_down_(false), + observer_(nullptr) {} + + ~IPCSupportInitializer() override { DCHECK(!observer_); } + + void Init(scoped_refptr<base::TaskRunner> io_thread_task_runner); + void ShutDown(); + + // Forces the initializer to shut down even if scopers are still holding it. + void ForceShutdown(); + + private: + // This watches for destruction of the MessageLoop that IPCSupportInitializer + // uses for IO, and guarantees that the initializer is shut down if it still + // exists when the loop is being destroyed. + class MessageLoopObserver : public base::MessageLoop::DestructionObserver { + public: + MessageLoopObserver(IPCSupportInitializer* initializer) + : initializer_(initializer) {} + + ~MessageLoopObserver() override { + base::MessageLoop::current()->RemoveDestructionObserver(this); + } + + private: + // base::MessageLoop::DestructionObserver: + void WillDestroyCurrentMessageLoop() override { + initializer_->ForceShutdown(); + } + + IPCSupportInitializer* initializer_; + + DISALLOW_COPY_AND_ASSIGN(MessageLoopObserver); + }; + + void ShutDownOnIOThread(); + + // mojo::embedder::ProcessDelegate: + void OnShutdownComplete() override {} + + static void WatchMessageLoopOnIOThread(MessageLoopObserver* observer); + + base::Lock lock_; + size_t init_count_; + bool shutting_down_; + + // This is used to track whether shutdown has occurred yet, since we can be + // shut down by either the scoper or IO MessageLoop destruction. + bool was_shut_down_; + + // The message loop destruction observer we have watching our IO loop. This + // is created on the initializer's own thread but is used and destroyed on the + // IO thread. + MessageLoopObserver* observer_; + + scoped_refptr<base::TaskRunner> io_thread_task_runner_; + + DISALLOW_COPY_AND_ASSIGN(IPCSupportInitializer); +}; + +void IPCSupportInitializer::Init( + scoped_refptr<base::TaskRunner> io_thread_task_runner) { + base::AutoLock locker(lock_); + DCHECK((init_count_ == 0 && !io_thread_task_runner_) || + io_thread_task_runner_ == io_thread_task_runner); + + if (shutting_down_) { + // If reinitialized before a pending shutdown task is executed, we + // effectively cancel the shutdown task. + DCHECK(init_count_ == 1); + shutting_down_ = false; + return; + } + + init_count_++; + if (init_count_ == 1) { + was_shut_down_ = false; + observer_ = new MessageLoopObserver(this); + io_thread_task_runner_ = io_thread_task_runner; + io_thread_task_runner_->PostTask( + FROM_HERE, base::Bind(&WatchMessageLoopOnIOThread, observer_)); + mojo::embedder::InitIPCSupport( + mojo::embedder::ProcessType::NONE, io_thread_task_runner_, this, + io_thread_task_runner_, mojo::embedder::ScopedPlatformHandle()); + } +} + +void IPCSupportInitializer::ShutDown() { + { + base::AutoLock locker(lock_); + if (shutting_down_ || was_shut_down_) + return; + DCHECK(init_count_ > 0); + if (init_count_ > 1) { + init_count_--; + return; + } + } + ForceShutdown(); +} + +void IPCSupportInitializer::ForceShutdown() { + base::AutoLock locker(lock_); + if (shutting_down_ || was_shut_down_) + return; + shutting_down_ = true; + if (base::MessageLoop::current() && + base::MessageLoop::current()->task_runner() == io_thread_task_runner_) { + base::AutoUnlock unlocker_(lock_); + ShutDownOnIOThread(); + } else { + io_thread_task_runner_->PostTask( + FROM_HERE, base::Bind(&IPCSupportInitializer::ShutDownOnIOThread, + base::Unretained(this))); + } +} + +void IPCSupportInitializer::ShutDownOnIOThread() { + base::AutoLock locker(lock_); + if (shutting_down_ && !was_shut_down_) { + mojo::embedder::ShutdownIPCSupportOnIOThread(); + init_count_ = 0; + shutting_down_ = false; + io_thread_task_runner_ = nullptr; + was_shut_down_ = true; + if (observer_) { + delete observer_; + observer_ = nullptr; + } + } +} + +// static +void IPCSupportInitializer::WatchMessageLoopOnIOThread( + MessageLoopObserver* observer) { + base::MessageLoop::current()->AddDestructionObserver(observer); +} + +base::LazyInstance<IPCSupportInitializer>::Leaky ipc_support_initializer; + +} // namespace + +ScopedIPCSupport::ScopedIPCSupport( + scoped_refptr<base::TaskRunner> io_thread_task_runner) { + ipc_support_initializer.Get().Init(io_thread_task_runner); +} + +ScopedIPCSupport::~ScopedIPCSupport() { + ipc_support_initializer.Get().ShutDown(); +} + +} // namespace IPC diff --git a/chromium/ipc/mojo/scoped_ipc_support.h b/chromium/ipc/mojo/scoped_ipc_support.h new file mode 100644 index 00000000000..21013fa75c0 --- /dev/null +++ b/chromium/ipc/mojo/scoped_ipc_support.h @@ -0,0 +1,34 @@ +// 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_MOJO_SCOPED_IPC_SUPPORT_H_ +#define IPC_MOJO_SCOPED_IPC_SUPPORT_H_ + +#include "base/macros.h" +#include "base/memory/ref_counted.h" +#include "base/task_runner.h" +#include "ipc/ipc_export.h" + +namespace IPC { + +// Perform any necessary Mojo IPC initialization. A ScopedIPCSupport object +// should be instantiated and retained by any component which makes direct calls +// to the Mojo EDK. This is used to ensure that the EDK is initialized within +// the current process and that it is shutdown cleanly when no longer in use. +// +// NOTE: Unless you are making explicit calls to functions in the +// mojo::embedder namespace, you almost definitely DO NOT need this and should +// not be using it. +class IPC_MOJO_EXPORT ScopedIPCSupport { + public: + ScopedIPCSupport(scoped_refptr<base::TaskRunner> io_thread_task_runner); + ~ScopedIPCSupport(); + + private: + DISALLOW_COPY_AND_ASSIGN(ScopedIPCSupport); +}; + +} // namespace IPC + +#endif // IPC_MOJO_SCOPED_IPC_SUPPORT_H_ diff --git a/chromium/ipc/param_traits_read_macros.h b/chromium/ipc/param_traits_read_macros.h index 1656d14228b..683a1de261c 100644 --- a/chromium/ipc/param_traits_read_macros.h +++ b/chromium/ipc/param_traits_read_macros.h @@ -35,7 +35,7 @@ bool ParamTraits<enum_name>:: \ Read(const Message* m, PickleIterator* iter, param_type* p) { \ int value; \ - if (!m->ReadInt(iter, &value)) \ + if (!iter->ReadInt(&value)) \ return false; \ if (!(validation_expression)) \ return false; \ diff --git a/chromium/ipc/sync_socket_unittest.cc b/chromium/ipc/sync_socket_unittest.cc index 77ae72d39e3..82ab0cd0992 100644 --- a/chromium/ipc/sync_socket_unittest.cc +++ b/chromium/ipc/sync_socket_unittest.cc @@ -5,11 +5,12 @@ #include "base/sync_socket.h" #include <stdio.h> -#include <string> #include <sstream> +#include <string> #include "base/bind.h" -#include "base/message_loop/message_loop.h" +#include "base/location.h" +#include "base/single_thread_task_runner.h" #include "base/threading/thread.h" #include "ipc/ipc_test_base.h" #include "testing/gtest/include/gtest/gtest.h" @@ -184,7 +185,7 @@ TEST_F(SyncSocketTest, SanityTest) { #if defined(OS_WIN) // On windows we need to duplicate the handle into the server process. BOOL retval = DuplicateHandle(GetCurrentProcess(), pair[1].handle(), - client_process(), &target_handle, + client_process().Handle(), &target_handle, 0, FALSE, DUPLICATE_SAME_ACCESS); EXPECT_TRUE(retval); // Set up a message to pass the handle to the server. @@ -227,7 +228,8 @@ TEST_F(SyncSocketTest, DisconnectTest) { // Try to do a blocking read from one of the sockets on the worker thread. char buf[0xff]; size_t received = 1U; // Initialize to an unexpected value. - worker.message_loop()->PostTask(FROM_HERE, + worker.task_runner()->PostTask( + FROM_HERE, base::Bind(&BlockingRead, &pair[0], &buf[0], arraysize(buf), &received)); // Wait for the worker thread to say hello. @@ -257,9 +259,9 @@ TEST_F(SyncSocketTest, BlockingReceiveTest) { // Try to do a blocking read from one of the sockets on the worker thread. char buf[kHelloStringLength] = {0}; size_t received = 1U; // Initialize to an unexpected value. - worker.message_loop()->PostTask(FROM_HERE, - base::Bind(&BlockingRead, &pair[0], &buf[0], - kHelloStringLength, &received)); + worker.task_runner()->PostTask(FROM_HERE, + base::Bind(&BlockingRead, &pair[0], &buf[0], + kHelloStringLength, &received)); // Wait for the worker thread to say hello. char hello[kHelloStringLength] = {0}; diff --git a/chromium/ipc/unix_domain_socket_util.cc b/chromium/ipc/unix_domain_socket_util.cc index d1ddd83e5ce..74053445b7a 100644 --- a/chromium/ipc/unix_domain_socket_util.cc +++ b/chromium/ipc/unix_domain_socket_util.cc @@ -20,8 +20,8 @@ namespace IPC { // Verify that kMaxSocketNameLength is a decent size. -COMPILE_ASSERT(sizeof(((sockaddr_un*)0)->sun_path) >= kMaxSocketNameLength, - BAD_SUN_PATH_LENGTH); +static_assert(sizeof(((sockaddr_un*)0)->sun_path) >= kMaxSocketNameLength, + "sun_path is too long."); namespace { diff --git a/chromium/ipc/unix_domain_socket_util_unittest.cc b/chromium/ipc/unix_domain_socket_util_unittest.cc index 813d4979260..57365a5840d 100644 --- a/chromium/ipc/unix_domain_socket_util_unittest.cc +++ b/chromium/ipc/unix_domain_socket_util_unittest.cc @@ -6,8 +6,10 @@ #include "base/bind.h" #include "base/files/file_path.h" +#include "base/location.h" #include "base/path_service.h" #include "base/posix/eintr_wrapper.h" +#include "base/single_thread_task_runner.h" #include "base/synchronization/waitable_event.h" #include "base/threading/thread.h" #include "base/threading/thread_restrictions.h" @@ -18,7 +20,7 @@ namespace { class SocketAcceptor : public base::MessageLoopForIO::Watcher { public: - SocketAcceptor(int fd, base::MessageLoopProxy* target_thread) + SocketAcceptor(int fd, base::SingleThreadTaskRunner* target_thread) : server_fd_(-1), target_thread_(target_thread), started_watching_event_(false, false), @@ -69,7 +71,7 @@ class SocketAcceptor : public base::MessageLoopForIO::Watcher { void OnFileCanWriteWithoutBlocking(int fd) override {} int server_fd_; - base::MessageLoopProxy* target_thread_; + base::SingleThreadTaskRunner* target_thread_; scoped_ptr<base::MessageLoopForIO::FileDescriptorWatcher> watcher_; base::WaitableEvent started_watching_event_; base::WaitableEvent accepted_event_; @@ -78,13 +80,9 @@ class SocketAcceptor : public base::MessageLoopForIO::Watcher { }; const base::FilePath GetChannelDir() { -#if defined(OS_ANDROID) base::FilePath tmp_dir; - PathService::Get(base::DIR_CACHE, &tmp_dir); + PathService::Get(base::DIR_TEMP, &tmp_dir); return tmp_dir; -#else - return base::FilePath("/var/tmp"); -#endif } class TestUnixSocketConnection { @@ -107,8 +105,8 @@ class TestUnixSocketConnection { struct stat socket_stat; stat(socket_name_.value().c_str(), &socket_stat); EXPECT_TRUE(S_ISSOCK(socket_stat.st_mode)); - acceptor_.reset(new SocketAcceptor(server_listen_fd_, - worker_.message_loop_proxy().get())); + acceptor_.reset( + new SocketAcceptor(server_listen_fd_, worker_.task_runner().get())); acceptor_->WaitUntilReady(); return true; } |