EDK: Remove Core::AddDispatcher(). Use AddHandle() instead. Also, add kDefaultHandleRights static member variables to the various dispatcher implementations. (Arguably, their Create() static factories should produce a Handle instead, and thus automatically have the default handle rights.) R=azani@chromium.org Review URL: https://codereview.chromium.org/1946423002 .
diff --git a/mojo/edk/embedder/embedder.cc b/mojo/edk/embedder/embedder.cc index 6622de9..ac25362 100644 --- a/mojo/edk/embedder/embedder.cc +++ b/mojo/edk/embedder/embedder.cc
@@ -8,6 +8,7 @@ #include "mojo/edk/embedder/embedder_internal.h" #include "mojo/edk/system/configuration.h" #include "mojo/edk/system/core.h" +#include "mojo/edk/system/handle.h" #include "mojo/edk/system/platform_handle_dispatcher.h" #include "mojo/edk/util/ref_ptr.h" @@ -54,7 +55,9 @@ system::PlatformHandleDispatcher::Create(platform_handle.Pass()); DCHECK(internal::g_core); - MojoHandle h = internal::g_core->AddDispatcher(dispatcher.get()); + MojoHandle h = internal::g_core->AddHandle( + system::Handle(dispatcher.Clone(), + system::PlatformHandleDispatcher::kDefaultHandleRights)); if (h == MOJO_HANDLE_INVALID) { LOG(ERROR) << "Handle table full"; dispatcher->Close();
diff --git a/mojo/edk/embedder/multiprocess_embedder.cc b/mojo/edk/embedder/multiprocess_embedder.cc index 25e6355..4968191 100644 --- a/mojo/edk/embedder/multiprocess_embedder.cc +++ b/mojo/edk/embedder/multiprocess_embedder.cc
@@ -15,6 +15,7 @@ #include "mojo/edk/system/channel.h" #include "mojo/edk/system/channel_manager.h" #include "mojo/edk/system/core.h" +#include "mojo/edk/system/handle.h" #include "mojo/edk/system/ipc_support.h" #include "mojo/edk/system/message_pipe_dispatcher.h" #include "mojo/edk/system/raw_channel.h" @@ -128,8 +129,9 @@ std::move(did_connect_to_slave_runner), &channel_id); *channel_info = new ChannelInfo(channel_id); - ScopedMessagePipeHandle rv( - MessagePipeHandle(internal::g_core->AddDispatcher(dispatcher.get()))); + ScopedMessagePipeHandle rv(MessagePipeHandle(internal::g_core->AddHandle( + system::Handle(std::move(dispatcher), + system::MessagePipeDispatcher::kDefaultHandleRights)))); CHECK(rv.is_valid()); return rv; } @@ -154,8 +156,9 @@ std::move(did_connect_to_master_runner), &channel_id); *channel_info = new ChannelInfo(channel_id); - ScopedMessagePipeHandle rv( - MessagePipeHandle(internal::g_core->AddDispatcher(dispatcher.get()))); + ScopedMessagePipeHandle rv(MessagePipeHandle(internal::g_core->AddHandle( + system::Handle(std::move(dispatcher), + system::MessagePipeDispatcher::kDefaultHandleRights)))); CHECK(rv.is_valid()); return rv; } @@ -176,8 +179,9 @@ channel_manager->CreateChannelOnIOThread((*channel_info)->channel_id, platform_handle.Pass()); - ScopedMessagePipeHandle rv( - MessagePipeHandle(internal::g_core->AddDispatcher(dispatcher.get()))); + ScopedMessagePipeHandle rv(MessagePipeHandle(internal::g_core->AddHandle( + system::Handle(std::move(dispatcher), + system::MessagePipeDispatcher::kDefaultHandleRights)))); CHECK(rv.is_valid()); return rv; } @@ -204,8 +208,9 @@ }, std::move(did_create_channel_runner)); - ScopedMessagePipeHandle rv( - MessagePipeHandle(internal::g_core->AddDispatcher(dispatcher.get()))); + ScopedMessagePipeHandle rv(MessagePipeHandle(internal::g_core->AddHandle( + system::Handle(std::move(dispatcher), + system::MessagePipeDispatcher::kDefaultHandleRights)))); CHECK(rv.is_valid()); return rv; }
diff --git a/mojo/edk/embedder/system_impl_private_entrypoints.cc b/mojo/edk/embedder/system_impl_private_entrypoints.cc index 1ec1da9..79cd309 100644 --- a/mojo/edk/embedder/system_impl_private_entrypoints.cc +++ b/mojo/edk/embedder/system_impl_private_entrypoints.cc
@@ -6,6 +6,7 @@ #include "mojo/edk/embedder/embedder_internal.h" #include "mojo/edk/system/core.h" #include "mojo/edk/system/dispatcher.h" +#include "mojo/edk/system/handle.h" #include "mojo/edk/util/ref_ptr.h" #include "mojo/public/c/system/buffer.h" #include "mojo/public/c/system/data_pipe.h" @@ -59,7 +60,11 @@ if (result != MOJO_RESULT_OK) return result; - MojoHandle created_handle = to_core->AddDispatcher(d.get()); + // TODO(vtl): The rights should come from the original handle (to be dealt + // with when I fix/replace |Core::GetAndRemoveDispatcher()|. + MojoHandle created_handle = to_core->AddHandle(mojo::system::Handle( + d.Clone(), MOJO_HANDLE_RIGHT_TRANSFER | MOJO_HANDLE_RIGHT_READ | + MOJO_HANDLE_RIGHT_WRITE)); if (created_handle == MOJO_HANDLE_INVALID) { // The handle has been lost, unfortunately. There's no guarentee we can put // it back where it came from, or get the original ID back. Holding locks
diff --git a/mojo/edk/system/core.cc b/mojo/edk/system/core.cc index a534a2f..95cb703 100644 --- a/mojo/edk/system/core.cc +++ b/mojo/edk/system/core.cc
@@ -97,12 +97,6 @@ return handle_table_.AddHandle(std::move(handle)); } -// FIXME -MojoHandle Core::AddDispatcher(Dispatcher* dispatcher) { - return AddHandle( - Handle(RefPtr<Dispatcher>(dispatcher), MOJO_HANDLE_RIGHT_TRANSFER)); -} - MojoResult Core::GetDispatcher(MojoHandle handle, RefPtr<Dispatcher>* dispatcher) { if (handle == MOJO_HANDLE_INVALID) @@ -227,12 +221,10 @@ { MutexLocker locker(&handle_table_mutex_); handle_pair = handle_table_.AddHandlePair( - Handle(dispatcher0.Clone(), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE), - Handle(dispatcher1.Clone(), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE)); + Handle(dispatcher0.Clone(), + MessagePipeDispatcher::kDefaultHandleRights), + Handle(dispatcher1.Clone(), + MessagePipeDispatcher::kDefaultHandleRights)); } if (handle_pair.first == MOJO_HANDLE_INVALID) { DCHECK_EQ(handle_pair.second, MOJO_HANDLE_INVALID); @@ -398,9 +390,9 @@ MutexLocker locker(&handle_table_mutex_); handle_pair = handle_table_.AddHandlePair( Handle(producer_dispatcher.Clone(), - MOJO_HANDLE_RIGHT_TRANSFER | MOJO_HANDLE_RIGHT_WRITE), + DataPipeProducerDispatcher::kDefaultHandleRights), Handle(consumer_dispatcher.Clone(), - MOJO_HANDLE_RIGHT_TRANSFER | MOJO_HANDLE_RIGHT_READ)); + DataPipeConsumerDispatcher::kDefaultHandleRights)); } if (handle_pair.first == MOJO_HANDLE_INVALID) { DCHECK_EQ(handle_pair.second, MOJO_HANDLE_INVALID); @@ -551,12 +543,8 @@ return result; } - // Note that shared buffer handles are duplicatable (by default). - MojoHandle h = AddHandle(Handle( - dispatcher.Clone(), MOJO_HANDLE_RIGHT_DUPLICATE | - MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | MOJO_HANDLE_RIGHT_WRITE | - MOJO_HANDLE_RIGHT_EXECUTE)); + MojoHandle h = AddHandle( + Handle(dispatcher.Clone(), SharedBufferDispatcher::kDefaultHandleRights)); if (h == MOJO_HANDLE_INVALID) { LOG(ERROR) << "Handle table full"; dispatcher->Close(); @@ -582,7 +570,9 @@ if (result != MOJO_RESULT_OK) return result; - MojoHandle new_handle = AddDispatcher(new_dispatcher.get()); + // TODO(vtl): This should be done with the original handle's rights. + MojoHandle new_handle = AddHandle(Handle( + new_dispatcher.Clone(), SharedBufferDispatcher::kDefaultHandleRights)); if (new_handle == MOJO_HANDLE_INVALID) { LOG(ERROR) << "Handle table full"; new_dispatcher->Close();
diff --git a/mojo/edk/system/core.h b/mojo/edk/system/core.h index cc480dd..5611541 100644 --- a/mojo/edk/system/core.h +++ b/mojo/edk/system/core.h
@@ -52,12 +52,6 @@ // if the handle table is full. MojoHandle AddHandle(Handle&& handle); - // DEPRECATED. TODO(vtl): Remove this. - // Adds |dispatcher| to the handle table, returning the handle value for it. - // Returns |MOJO_HANDLE_INVALID| on failure, namely if the handle table is - // full. - MojoHandle AddDispatcher(Dispatcher* dispatcher); - // Looks up the dispatcher for the given handle. On success, gets the // dispatcher for a given handle. On failure, returns an appropriate result // and leaves |dispatcher| alone), namely |MOJO_RESULT_INVALID_ARGUMENT| if
diff --git a/mojo/edk/system/core_test_base.cc b/mojo/edk/system/core_test_base.cc index 2ea2da3..6297456 100644 --- a/mojo/edk/system/core_test_base.cc +++ b/mojo/edk/system/core_test_base.cc
@@ -4,6 +4,7 @@ #include "mojo/edk/system/core_test_base.h" +#include <utility> #include <vector> #include "base/logging.h" @@ -11,6 +12,7 @@ #include "mojo/edk/system/configuration.h" #include "mojo/edk/system/core.h" #include "mojo/edk/system/dispatcher.h" +#include "mojo/edk/system/handle.h" #include "mojo/edk/system/memory.h" #include "mojo/edk/util/ref_ptr.h" #include "mojo/public/cpp/system/macros.h" @@ -224,7 +226,11 @@ MojoHandle CoreTestBase::CreateMockHandle(CoreTestBase::MockHandleInfo* info) { CHECK(core_); auto dispatcher = MockDispatcher::Create(info); - MojoHandle rv = core_->AddDispatcher(dispatcher.get()); + MojoHandle rv = core_->AddHandle( + Handle(std::move(dispatcher), + MOJO_HANDLE_RIGHT_DUPLICATE | MOJO_HANDLE_RIGHT_TRANSFER | + MOJO_HANDLE_RIGHT_READ | MOJO_HANDLE_RIGHT_WRITE | + MOJO_HANDLE_RIGHT_EXECUTE)); CHECK_NE(rv, MOJO_HANDLE_INVALID); return rv; }
diff --git a/mojo/edk/system/data_pipe_consumer_dispatcher.cc b/mojo/edk/system/data_pipe_consumer_dispatcher.cc index cdf553f..1d515b2 100644 --- a/mojo/edk/system/data_pipe_consumer_dispatcher.cc +++ b/mojo/edk/system/data_pipe_consumer_dispatcher.cc
@@ -18,6 +18,9 @@ namespace mojo { namespace system { +// static +constexpr MojoHandleRights DataPipeConsumerDispatcher::kDefaultHandleRights; + void DataPipeConsumerDispatcher::Init(RefPtr<DataPipe>&& data_pipe) { DCHECK(data_pipe); data_pipe_ = std::move(data_pipe);
diff --git a/mojo/edk/system/data_pipe_consumer_dispatcher.h b/mojo/edk/system/data_pipe_consumer_dispatcher.h index aa4455a..a9c99c3 100644 --- a/mojo/edk/system/data_pipe_consumer_dispatcher.h +++ b/mojo/edk/system/data_pipe_consumer_dispatcher.h
@@ -20,6 +20,11 @@ // thread-safe. class DataPipeConsumerDispatcher final : public Dispatcher { public: + // The default/standard rights for a data pipe consumer handle. + static constexpr MojoHandleRights kDefaultHandleRights = + MOJO_HANDLE_RIGHT_TRANSFER | MOJO_HANDLE_RIGHT_READ | + MOJO_HANDLE_RIGHT_WRITE; + static util::RefPtr<DataPipeConsumerDispatcher> Create() { return AdoptRef(new DataPipeConsumerDispatcher()); }
diff --git a/mojo/edk/system/data_pipe_impl_unittest.cc b/mojo/edk/system/data_pipe_impl_unittest.cc index 985b401..8841fd4 100644 --- a/mojo/edk/system/data_pipe_impl_unittest.cc +++ b/mojo/edk/system/data_pipe_impl_unittest.cc
@@ -385,9 +385,8 @@ // This is the producer dispatcher we'll send. auto to_send = DataPipeProducerDispatcher::Create(); to_send->Init(dp()); - Handle handle_to_send(std::move(to_send), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle handle_to_send(std::move(to_send), + DataPipeProducerDispatcher::kDefaultHandleRights); RefPtr<Dispatcher> to_receive; SendHandle(0, handle_to_send, &to_receive); // |handle_to_send.dispatcher| should have been closed. This is |DCHECK()|ed @@ -437,9 +436,8 @@ // This is the consumer dispatcher we'll send. auto to_send = DataPipeConsumerDispatcher::Create(); to_send->Init(dp()); - Handle handle_to_send(std::move(to_send), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle handle_to_send(std::move(to_send), + DataPipeConsumerDispatcher::kDefaultHandleRights); RefPtr<Dispatcher> to_receive; SendHandle(0, handle_to_send, &to_receive); // |handle_to_send.dispatcher| should have been closed. This is |DCHECK()|ed @@ -494,9 +492,8 @@ // This is the producer dispatcher we'll send. auto to_send = DataPipeProducerDispatcher::Create(); to_send->Init(dp()); - Handle handle_to_send(std::move(to_send), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle handle_to_send(std::move(to_send), + DataPipeProducerDispatcher::kDefaultHandleRights); RefPtr<Dispatcher> to_receive; SendHandle(0, handle_to_send, &to_receive); // |handle_to_send.dispatcher| should have been closed. This is |DCHECK()|ed @@ -504,9 +501,8 @@ EXPECT_TRUE(handle_to_send.dispatcher->HasOneRef()); handle_to_send.reset(); ASSERT_EQ(Dispatcher::Type::DATA_PIPE_PRODUCER, to_receive->GetType()); - handle_to_send = Handle(std::move(to_receive), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + handle_to_send = Handle(std::move(to_receive), + DataPipeProducerDispatcher::kDefaultHandleRights); // Now send it back the other way. SendHandle(1, handle_to_send, &to_receive); @@ -544,9 +540,8 @@ // This is the consumer dispatcher we'll send. auto to_send = DataPipeConsumerDispatcher::Create(); to_send->Init(dp()); - Handle handle_to_send(std::move(to_send), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle handle_to_send(std::move(to_send), + DataPipeConsumerDispatcher::kDefaultHandleRights); RefPtr<Dispatcher> to_receive; SendHandle(0, handle_to_send, &to_receive); // |handle_to_send.dispatcher| should have been closed. This is |DCHECK()|ed @@ -554,9 +549,8 @@ EXPECT_TRUE(handle_to_send.dispatcher->HasOneRef()); handle_to_send.reset(); ASSERT_EQ(Dispatcher::Type::DATA_PIPE_CONSUMER, to_receive->GetType()); - handle_to_send = Handle(std::move(to_receive), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + handle_to_send = Handle(std::move(to_receive), + DataPipeConsumerDispatcher::kDefaultHandleRights); // Now send it back the other way. SendHandle(1, handle_to_send, &to_receive);
diff --git a/mojo/edk/system/data_pipe_producer_dispatcher.cc b/mojo/edk/system/data_pipe_producer_dispatcher.cc index e89f44f..86d022c 100644 --- a/mojo/edk/system/data_pipe_producer_dispatcher.cc +++ b/mojo/edk/system/data_pipe_producer_dispatcher.cc
@@ -18,6 +18,9 @@ namespace mojo { namespace system { +// static +constexpr MojoHandleRights DataPipeProducerDispatcher::kDefaultHandleRights; + void DataPipeProducerDispatcher::Init(RefPtr<DataPipe>&& data_pipe) { DCHECK(data_pipe); data_pipe_ = std::move(data_pipe);
diff --git a/mojo/edk/system/data_pipe_producer_dispatcher.h b/mojo/edk/system/data_pipe_producer_dispatcher.h index c024c18..415b756 100644 --- a/mojo/edk/system/data_pipe_producer_dispatcher.h +++ b/mojo/edk/system/data_pipe_producer_dispatcher.h
@@ -20,6 +20,11 @@ // thread-safe. class DataPipeProducerDispatcher final : public Dispatcher { public: + // The default/standard rights for a data pipe consumer handle. + static constexpr MojoHandleRights kDefaultHandleRights = + MOJO_HANDLE_RIGHT_TRANSFER | MOJO_HANDLE_RIGHT_READ | + MOJO_HANDLE_RIGHT_WRITE; + static util::RefPtr<DataPipeProducerDispatcher> Create() { return AdoptRef(new DataPipeProducerDispatcher()); }
diff --git a/mojo/edk/system/ipc_support_unittest.cc b/mojo/edk/system/ipc_support_unittest.cc index 95d57d5..a3f6d56 100644 --- a/mojo/edk/system/ipc_support_unittest.cc +++ b/mojo/edk/system/ipc_support_unittest.cc
@@ -87,9 +87,8 @@ RefPtr<MessagePipeDispatcher>&& mp_to_send) { CHECK_NE(mp_to_send.get(), write_mp); CHECK_NE(mp_to_send.get(), read_mp); - Handle mp_handle_to_send(std::move(mp_to_send), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle mp_handle_to_send(std::move(mp_to_send), + MessagePipeDispatcher::kDefaultHandleRights); // Set up waiting on the read end first (to avoid racing). Waiter waiter;
diff --git a/mojo/edk/system/message_pipe_dispatcher.cc b/mojo/edk/system/message_pipe_dispatcher.cc index fcdb5d2..8a8edaf 100644 --- a/mojo/edk/system/message_pipe_dispatcher.cc +++ b/mojo/edk/system/message_pipe_dispatcher.cc
@@ -26,6 +26,9 @@ // MessagePipeDispatcher ------------------------------------------------------- // static +constexpr MojoHandleRights MessagePipeDispatcher::kDefaultHandleRights; + +// static const MojoCreateMessagePipeOptions MessagePipeDispatcher::kDefaultCreateOptions = { static_cast<uint32_t>(sizeof(MojoCreateMessagePipeOptions)),
diff --git a/mojo/edk/system/message_pipe_dispatcher.h b/mojo/edk/system/message_pipe_dispatcher.h index 62ee563..f712ce0 100644 --- a/mojo/edk/system/message_pipe_dispatcher.h +++ b/mojo/edk/system/message_pipe_dispatcher.h
@@ -21,6 +21,11 @@ // Mojo primitive |MojoCreateMessagePipe()|). This class is thread-safe. class MessagePipeDispatcher final : public Dispatcher { public: + // The default/standard rights for a message pipe handle. + static constexpr MojoHandleRights kDefaultHandleRights = + MOJO_HANDLE_RIGHT_TRANSFER | MOJO_HANDLE_RIGHT_READ | + MOJO_HANDLE_RIGHT_WRITE; + // The default options to use for |MojoCreateMessagePipe()|. (Real uses // should obtain this via |ValidateCreateOptions()| with a null |in_options|; // this is exposed directly for testing convenience.)
diff --git a/mojo/edk/system/multiprocess_message_pipe_unittest.cc b/mojo/edk/system/multiprocess_message_pipe_unittest.cc index 519379d..c02f078 100644 --- a/mojo/edk/system/multiprocess_message_pipe_unittest.cc +++ b/mojo/edk/system/multiprocess_message_pipe_unittest.cc
@@ -325,8 +325,7 @@ EXPECT_EQ(MOJO_RESULT_OK, result); ASSERT_TRUE(dispatcher); Handle handle(std::move(dispatcher), - MOJO_HANDLE_RIGHT_DUPLICATE | MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | MOJO_HANDLE_RIGHT_WRITE); + SharedBufferDispatcher::kDefaultHandleRights); // Make a mapping. std::unique_ptr<PlatformSharedBufferMapping> mapping; @@ -480,8 +479,7 @@ Handle handle(PlatformHandleDispatcher::Create(ScopedPlatformHandle( PlatformHandleFromFILE(std::move(fp)))), - MOJO_HANDLE_RIGHT_TRANSFER | MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + PlatformHandleDispatcher::kDefaultHandleRights); handles.push_back(std::move(handle)); HandleTransport transport(test::HandleTryStartTransport(handles.back())); ASSERT_TRUE(transport.is_valid());
diff --git a/mojo/edk/system/platform_handle_dispatcher.cc b/mojo/edk/system/platform_handle_dispatcher.cc index 93d2b5c..0d52529 100644 --- a/mojo/edk/system/platform_handle_dispatcher.cc +++ b/mojo/edk/system/platform_handle_dispatcher.cc
@@ -26,6 +26,9 @@ } // namespace +// static +constexpr MojoHandleRights PlatformHandleDispatcher::kDefaultHandleRights; + ScopedPlatformHandle PlatformHandleDispatcher::PassPlatformHandle() { MutexLocker locker(&mutex()); return platform_handle_.Pass();
diff --git a/mojo/edk/system/platform_handle_dispatcher.h b/mojo/edk/system/platform_handle_dispatcher.h index 577323b..ed3edf8 100644 --- a/mojo/edk/system/platform_handle_dispatcher.h +++ b/mojo/edk/system/platform_handle_dispatcher.h
@@ -20,6 +20,11 @@ // the embedder). class PlatformHandleDispatcher final : public SimpleDispatcher { public: + // The default/standard rights for a platform handle (wrapper) handle. + static constexpr MojoHandleRights kDefaultHandleRights = + MOJO_HANDLE_RIGHT_TRANSFER | MOJO_HANDLE_RIGHT_READ | + MOJO_HANDLE_RIGHT_WRITE; + static util::RefPtr<PlatformHandleDispatcher> Create( platform::ScopedPlatformHandle platform_handle) { return AdoptRef(new PlatformHandleDispatcher(platform_handle.Pass()));
diff --git a/mojo/edk/system/platform_handle_dispatcher_unittest.cc b/mojo/edk/system/platform_handle_dispatcher_unittest.cc index 8b47bd3..626991a 100644 --- a/mojo/edk/system/platform_handle_dispatcher_unittest.cc +++ b/mojo/edk/system/platform_handle_dispatcher_unittest.cc
@@ -97,11 +97,8 @@ auto dispatcher = PlatformHandleDispatcher::Create(PlatformHandleFromFILE(std::move(fp))); - // TODO(vtl): Are these the correct rights for a |PlatformHandleDispatcher|? - const MojoHandleRights kRights = MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE; - Handle handle(std::move(dispatcher), kRights); + Handle handle(std::move(dispatcher), + PlatformHandleDispatcher::kDefaultHandleRights); HandleTransport transport(test::HandleTryStartTransport(handle)); EXPECT_TRUE(transport.is_valid()); @@ -111,7 +108,8 @@ Handle equivalent_handle = transport.CreateEquivalentHandleAndClose(nullptr, 0u); ASSERT_TRUE(equivalent_handle.dispatcher); - EXPECT_EQ(kRights, equivalent_handle.rights); + EXPECT_EQ(PlatformHandleDispatcher::kDefaultHandleRights, + equivalent_handle.rights); transport.End(); EXPECT_TRUE(handle.dispatcher->HasOneRef());
diff --git a/mojo/edk/system/remote_data_pipe_impl_unittest.cc b/mojo/edk/system/remote_data_pipe_impl_unittest.cc index ee1dd45..91f7160 100644 --- a/mojo/edk/system/remote_data_pipe_impl_unittest.cc +++ b/mojo/edk/system/remote_data_pipe_impl_unittest.cc
@@ -177,9 +177,8 @@ // This is the consumer dispatcher we'll send. auto consumer = DataPipeConsumerDispatcher::Create(); consumer->Init(dp.Clone()); - Handle consumer_handle(std::move(consumer), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle consumer_handle(std::move(consumer), + DataPipeConsumerDispatcher::kDefaultHandleRights); // Write to the producer and close it, before sending the consumer. int32_t elements[10] = {123}; @@ -300,9 +299,8 @@ // This is the consumer dispatcher we'll send. auto consumer = DataPipeConsumerDispatcher::Create(); consumer->Init(dp.Clone()); - Handle consumer_handle(std::move(consumer), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle consumer_handle(std::move(consumer), + DataPipeConsumerDispatcher::kDefaultHandleRights); void* write_ptr = nullptr; uint32_t num_bytes = 0u; @@ -415,9 +413,8 @@ // This is the consumer dispatcher we'll send. auto consumer = DataPipeConsumerDispatcher::Create(); consumer->Init(dp.Clone()); - Handle consumer_handle(std::move(consumer), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle consumer_handle(std::move(consumer), + DataPipeConsumerDispatcher::kDefaultHandleRights); void* write_ptr = nullptr; uint32_t num_bytes = 0u;
diff --git a/mojo/edk/system/remote_message_pipe_unittest.cc b/mojo/edk/system/remote_message_pipe_unittest.cc index 63faa89..7d91d57 100644 --- a/mojo/edk/system/remote_message_pipe_unittest.cc +++ b/mojo/edk/system/remote_message_pipe_unittest.cc
@@ -50,10 +50,6 @@ namespace system { namespace { -const MojoHandleSignals kAllSignals = MOJO_HANDLE_SIGNAL_READABLE | - MOJO_HANDLE_SIGNAL_WRITABLE | - MOJO_HANDLE_SIGNAL_PEER_CLOSED; - class RemoteMessagePipeTest : public testing::Test { public: RemoteMessagePipeTest() @@ -216,7 +212,9 @@ mp1->RemoveAwakable(1, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from MP 1, port 1. EXPECT_EQ(MOJO_RESULT_OK, @@ -244,7 +242,9 @@ mp0->RemoveAwakable(0, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); buffer_size = static_cast<uint32_t>(sizeof(buffer)); EXPECT_EQ(MOJO_RESULT_OK, @@ -368,7 +368,9 @@ mp1->RemoveAwakable(1, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); EXPECT_EQ(endpoint_info_size, channels(1)->GetSerializedEndpointSize()); std::unique_ptr<char[]> received_endpoint_info(new char[endpoint_info_size]); @@ -406,7 +408,9 @@ mp3->RemoveAwakable(0, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Make sure there's nothing on MP 0, port 0 or MP 1, port 1 or MP 2, port 0. buffer_size = static_cast<uint32_t>(sizeof(buffer)); @@ -452,7 +456,8 @@ mp1->RemoveAwakable(1, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals | MOJO_HANDLE_SIGNAL_PEER_CLOSED, + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED | MOJO_HANDLE_SIGNAL_PEER_CLOSED, hss.satisfiable_signals); // Make sure there's nothing on the other ports. @@ -627,9 +632,8 @@ MessagePipeDispatcher::kDefaultCreateOptions); auto local_mp = MessagePipe::CreateLocalLocal(); dispatcher->Init(local_mp.Clone(), 0); - Handle handle(std::move(dispatcher), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle handle(std::move(dispatcher), + MessagePipeDispatcher::kDefaultHandleRights); // Prepare to wait on MP 1, port 1. (Add the waiter now. Otherwise, if we do // it later, it might already be readable.) @@ -664,7 +668,9 @@ mp1->RemoveAwakable(1, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from MP 1, port 1. char read_buffer[100] = {0}; @@ -709,7 +715,9 @@ dispatcher->RemoveAwakable(&waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from the dispatcher. memset(read_buffer, 0, sizeof(read_buffer)); @@ -739,7 +747,9 @@ local_mp->RemoveAwakable(1, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from "local_mp", port 1. memset(read_buffer, 0, sizeof(read_buffer)); @@ -774,13 +784,14 @@ MessagePipeDispatcher::kDefaultCreateOptions); auto local_mp = MessagePipe::CreateLocalLocal(); dispatcher->Init(local_mp.Clone(), 0); - Handle handle(std::move(dispatcher), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle handle(std::move(dispatcher), + MessagePipeDispatcher::kDefaultHandleRights); hss = local_mp->GetHandleSignalsState(0); EXPECT_EQ(MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Write to the other end (|local_mp|, port 1), and then close it. EXPECT_EQ( MOJO_RESULT_OK, @@ -789,7 +800,9 @@ hss = local_mp->GetHandleSignalsState(0); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Then the second message.... EXPECT_EQ( MOJO_RESULT_OK, @@ -798,7 +811,9 @@ hss = local_mp->GetHandleSignalsState(0); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Then close it. local_mp->Close(1); @@ -841,7 +856,9 @@ mp1->RemoveAwakable(1, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from MP 1, port 1. char read_buffer[100] = {0}; @@ -925,8 +942,7 @@ EXPECT_EQ(MOJO_RESULT_OK, result); ASSERT_TRUE(dispatcher); Handle handle(std::move(dispatcher), - MOJO_HANDLE_RIGHT_DUPLICATE | MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | MOJO_HANDLE_RIGHT_WRITE); + SharedBufferDispatcher::kDefaultHandleRights); // Make a mapping. std::unique_ptr<PlatformSharedBufferMapping> mapping0; @@ -972,7 +988,9 @@ mp1->RemoveAwakable(1, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from MP 1, port 1. char read_buffer[100] = {0}; @@ -1047,10 +1065,8 @@ // be passed. auto dispatcher = PlatformHandleDispatcher::Create(PlatformHandleFromFILE(std::move(fp))); - // TODO(vtl): Are these the correct rights for a |PlatformHandleDispatcher|? - Handle handle(std::move(dispatcher), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle handle(std::move(dispatcher), + PlatformHandleDispatcher::kDefaultHandleRights); // Prepare to wait on MP 1, port 1. (Add the waiter now. Otherwise, if we do // it later, it might already be readable.) @@ -1085,7 +1101,9 @@ mp1->RemoveAwakable(1, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from MP 1, port 1. char read_buffer[100] = {0}; @@ -1185,9 +1203,8 @@ MessagePipeDispatcher::kDefaultCreateOptions); auto local_mp = MessagePipe::CreateLocalLocal(); dispatcher->Init(local_mp.Clone(), 0); - Handle handle(std::move(dispatcher), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + Handle handle(std::move(dispatcher), + MessagePipeDispatcher::kDefaultHandleRights); // Prepare to wait on MP 1, port 1. (Add the waiter now. Otherwise, if we do // it later, it might already be readable.) @@ -1222,7 +1239,9 @@ mp1->RemoveAwakable(1, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from MP 1, port 1. char read_buffer[100] = {0}; @@ -1246,9 +1265,8 @@ static_cast<MessagePipeDispatcher*>(read_dispatchers[0].get())); read_dispatchers.clear(); // TODO(vtl): We should really get |handle| from |ReadMessage()|. - handle = Handle(std::move(dispatcher), MOJO_HANDLE_RIGHT_TRANSFER | - MOJO_HANDLE_RIGHT_READ | - MOJO_HANDLE_RIGHT_WRITE); + handle = Handle(std::move(dispatcher), + MessagePipeDispatcher::kDefaultHandleRights); // Now pass it back. @@ -1285,7 +1303,9 @@ mp0->RemoveAwakable(0, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from MP 0, port 0. read_buffer_size = static_cast<uint32_t>(sizeof(read_buffer)); @@ -1326,7 +1346,9 @@ dispatcher->RemoveAwakable(&waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from the dispatcher. memset(read_buffer, 0, sizeof(read_buffer)); @@ -1356,7 +1378,9 @@ local_mp->RemoveAwakable(1, &waiter, &hss); EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE, hss.satisfied_signals); - EXPECT_EQ(kAllSignals, hss.satisfiable_signals); + EXPECT_EQ(MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE | + MOJO_HANDLE_SIGNAL_PEER_CLOSED, + hss.satisfiable_signals); // Read from "local_mp", port 1. memset(read_buffer, 0, sizeof(read_buffer));
diff --git a/mojo/edk/system/shared_buffer_dispatcher.cc b/mojo/edk/system/shared_buffer_dispatcher.cc index ee428cd..43af5f1 100644 --- a/mojo/edk/system/shared_buffer_dispatcher.cc +++ b/mojo/edk/system/shared_buffer_dispatcher.cc
@@ -34,6 +34,9 @@ } // namespace // static +constexpr MojoHandleRights SharedBufferDispatcher::kDefaultHandleRights; + +// static const MojoCreateSharedBufferOptions SharedBufferDispatcher::kDefaultCreateOptions = { static_cast<uint32_t>(sizeof(MojoCreateSharedBufferOptions)),
diff --git a/mojo/edk/system/shared_buffer_dispatcher.h b/mojo/edk/system/shared_buffer_dispatcher.h index 0286520..0dd2fc0 100644 --- a/mojo/edk/system/shared_buffer_dispatcher.h +++ b/mojo/edk/system/shared_buffer_dispatcher.h
@@ -11,6 +11,7 @@ #include "mojo/edk/system/simple_dispatcher.h" #include "mojo/edk/util/ref_ptr.h" #include "mojo/edk/util/thread_annotations.h" +#include "mojo/public/c/system/handle.h" #include "mojo/public/cpp/system/macros.h" namespace mojo { @@ -31,6 +32,13 @@ // (which would entail overriding |GetHandleSignalsStateImplNoLock()|, etc.). class SharedBufferDispatcher final : public SimpleDispatcher { public: + // The default/standard rights for a shared buffer handle. Note that they're + // duplicable by default. + static constexpr MojoHandleRights kDefaultHandleRights = + MOJO_HANDLE_RIGHT_DUPLICATE | MOJO_HANDLE_RIGHT_TRANSFER | + MOJO_HANDLE_RIGHT_READ | MOJO_HANDLE_RIGHT_WRITE | + MOJO_HANDLE_RIGHT_EXECUTE; + // The default options to use for |MojoCreateSharedBuffer()|. (Real uses // should obtain this via |ValidateCreateOptions()| with a null |in_options|; // this is exposed directly for testing convenience.)