diff options
Diffstat (limited to 'chromium/mojo')
110 files changed, 1221 insertions, 457 deletions
diff --git a/chromium/mojo/PRESUBMIT.py b/chromium/mojo/PRESUBMIT.py index 47196cf24bd..67910aca87a 100644 --- a/chromium/mojo/PRESUBMIT.py +++ b/chromium/mojo/PRESUBMIT.py @@ -10,6 +10,9 @@ for more details about the presubmit API built into depot_tools. import os.path +USE_PYTHON3 = True + + def CheckChangeOnUpload(input_api, output_api): # Additional python module paths (we're in src/mojo/); not everyone needs # them, but it's easiest to add them to everyone's path. diff --git a/chromium/mojo/core/broker_win.cc b/chromium/mojo/core/broker_win.cc index 32d10dfc550..5014be61b6b 100644 --- a/chromium/mojo/core/broker_win.cc +++ b/chromium/mojo/core/broker_win.cc @@ -58,7 +58,8 @@ Channel::MessagePtr WaitForBrokerMessage(HANDLE pipe_handle, } Channel::MessagePtr message = - Channel::Message::Deserialize(buffer, static_cast<size_t>(bytes_read)); + Channel::Message::Deserialize(buffer, static_cast<size_t>(bytes_read), + Channel::HandlePolicy::kAcceptHandles); if (!message || message->payload_size() < sizeof(BrokerMessageHeader)) { LOG(ERROR) << "Invalid broker message"; diff --git a/chromium/mojo/core/channel.cc b/chromium/mojo/core/channel.cc index 24dc3742ef8..5935ef3cecf 100644 --- a/chromium/mojo/core/channel.cc +++ b/chromium/mojo/core/channel.cc @@ -229,6 +229,7 @@ Channel::MessagePtr Channel::Message::CreateRawForFuzzing( Channel::MessagePtr Channel::Message::Deserialize( const void* data, size_t data_num_bytes, + HandlePolicy handle_policy, base::ProcessHandle from_process) { if (data_num_bytes < sizeof(LegacyHeader)) return nullptr; @@ -305,6 +306,11 @@ Channel::MessagePtr Channel::Message::Deserialize( return nullptr; } + if (num_handles > 0 && handle_policy == HandlePolicy::kRejectHandles) { + DLOG(ERROR) << "Rejecting message with unexpected handle attachments."; + return nullptr; + } + MessagePtr message = CreateMessage(payload_size, max_handles, legacy_header->message_type); DCHECK_EQ(message->data_num_bytes(), data_num_bytes); @@ -843,8 +849,6 @@ Channel::DispatchResult Channel::TryDispatchMessage( TRACE_EVENT(TRACE_DISABLED_BY_DEFAULT("toplevel.ipc"), "Mojo dispatch message"); - bool did_consume_message = false; - // We have at least enough data available for a LegacyHeader. const Message::LegacyHeader* legacy_header = reinterpret_cast<const Message::LegacyHeader*>(buffer.data()); @@ -919,12 +923,8 @@ Channel::DispatchResult Channel::TryDispatchMessage( std::move(handles))) { return DispatchResult::kError; } - did_consume_message = true; - } else if (deferred) { - did_consume_message = true; - } else if (delegate_) { + } else if (!deferred && delegate_) { delegate_->OnChannelMessage(payload, payload_size, std::move(handles)); - did_consume_message = true; } *size_hint = legacy_header->num_bytes; diff --git a/chromium/mojo/core/channel.h b/chromium/mojo/core/channel.h index 88761ddb2f0..7288dd87c25 100644 --- a/chromium/mojo/core/channel.h +++ b/chromium/mojo/core/channel.h @@ -188,6 +188,7 @@ class MOJO_SYSTEM_IMPL_EXPORT Channel static MessagePtr Deserialize( const void* data, size_t data_num_bytes, + HandlePolicy handle_policy, base::ProcessHandle from_process = base::kNullProcessHandle); virtual const void* data() const = 0; diff --git a/chromium/mojo/core/channel_fuchsia.cc b/chromium/mojo/core/channel_fuchsia.cc index eb4356c0c56..7b112ded3fe 100644 --- a/chromium/mojo/core/channel_fuchsia.cc +++ b/chromium/mojo/core/channel_fuchsia.cc @@ -16,13 +16,13 @@ #include "base/bind.h" #include "base/containers/circular_deque.h" +#include "base/cxx17_backports.h" #include "base/files/scoped_file.h" #include "base/fuchsia/fuchsia_logging.h" #include "base/location.h" #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/message_loop/message_pump_for_io.h" -#include "base/stl_util.h" #include "base/synchronization/lock.h" #include "base/task/current_thread.h" #include "base/task_runner.h" diff --git a/chromium/mojo/core/channel_linux.cc b/chromium/mojo/core/channel_linux.cc index 38e738732fa..20333a7d19b 100644 --- a/chromium/mojo/core/channel_linux.cc +++ b/chromium/mojo/core/channel_linux.cc @@ -20,6 +20,7 @@ #include <memory> #include "base/bind.h" +#include "base/bits.h" #include "base/callback.h" #include "base/files/scoped_file.h" #include "base/location.h" @@ -133,7 +134,7 @@ struct UpgradeOfferMessage { }; constexpr size_t RoundUpToWordBoundary(size_t size) { - return (size + (sizeof(void*) - 1)) & ~(sizeof(void*) - 1); + return base::bits::AlignUp(size, sizeof(void*)); } base::ScopedFD CreateSealedMemFD(size_t size) { diff --git a/chromium/mojo/core/channel_mac.cc b/chromium/mojo/core/channel_mac.cc index 71877bf3995..51ccc3200f2 100644 --- a/chromium/mojo/core/channel_mac.cc +++ b/chromium/mojo/core/channel_mac.cc @@ -284,20 +284,21 @@ class ChannelMac : public Channel, return false; } - // Record the audit token of the sender. All messages received by the - // channel must be from this same sender. - auto* trailer = buffer.Object<mach_msg_audit_trailer_t>(); - peer_audit_token_ = std::make_unique<audit_token_t>(); - memcpy(peer_audit_token_.get(), &trailer->msgh_audit, - sizeof(audit_token_t)); - send_port_ = base::mac::ScopedMachSendRight(message->msgh_remote_port); if (!RequestSendDeadNameNotification()) { + send_port_.reset(); OnError(Error::kConnectionFailed); return false; } + // Record the audit token of the sender. All messages received by the + // channel must be from this same sender. + auto* trailer = buffer.Object<mach_msg_audit_trailer_t>(); + peer_audit_token_ = std::make_unique<audit_token_t>(); + memcpy(peer_audit_token_.get(), &trailer->msgh_audit, + sizeof(audit_token_t)); + base::AutoLock lock(write_lock_); handshake_done_ = true; SendPendingMessagesLocked(); diff --git a/chromium/mojo/core/channel_posix.cc b/chromium/mojo/core/channel_posix.cc index 001a3b62053..782d4f03307 100644 --- a/chromium/mojo/core/channel_posix.cc +++ b/chromium/mojo/core/channel_posix.cc @@ -709,15 +709,14 @@ scoped_refptr<Channel> Channel::Create( ConnectionParams connection_params, HandlePolicy handle_policy, scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) { -#if !defined(OS_NACL) -#if (defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_ANDROID)) +#if !defined(OS_NACL) && \ + (defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_ANDROID)) return new ChannelLinux(delegate, std::move(connection_params), handle_policy, io_task_runner); -#endif -#endif - +#else return new ChannelPosix(delegate, std::move(connection_params), handle_policy, io_task_runner); +#endif } #if !defined(OS_NACL) diff --git a/chromium/mojo/core/channel_unittest.cc b/chromium/mojo/core/channel_unittest.cc index e5eee1e4051..0a377457310 100644 --- a/chromium/mojo/core/channel_unittest.cc +++ b/chromium/mojo/core/channel_unittest.cc @@ -136,7 +136,8 @@ void TestMessagesAreEqual(Channel::Message* message1, TEST(ChannelTest, LegacyMessageDeserialization) { Channel::MessagePtr message = CreateDefaultMessage(true /* legacy_message */); Channel::MessagePtr deserialized_message = - Channel::Message::Deserialize(message->data(), message->data_num_bytes()); + Channel::Message::Deserialize(message->data(), message->data_num_bytes(), + Channel::HandlePolicy::kAcceptHandles); TestMessagesAreEqual(message.get(), deserialized_message.get(), true /* legacy_message */); } @@ -145,7 +146,8 @@ TEST(ChannelTest, NonLegacyMessageDeserialization) { Channel::MessagePtr message = CreateDefaultMessage(false /* legacy_message */); Channel::MessagePtr deserialized_message = - Channel::Message::Deserialize(message->data(), message->data_num_bytes()); + Channel::Message::Deserialize(message->data(), message->data_num_bytes(), + Channel::HandlePolicy::kAcceptHandles); TestMessagesAreEqual(message.get(), deserialized_message.get(), false /* legacy_message */); } @@ -370,8 +372,10 @@ TEST(ChannelTest, DeserializeMessage_BadExtraHeaderSize) { header->num_header_bytes = kTotalHeaderSize; header->message_type = Channel::Message::MessageType::NORMAL; header->num_handles = 0; - EXPECT_EQ(nullptr, Channel::Message::Deserialize(&message[0], kMessageSize, - base::kNullProcessHandle)); + EXPECT_EQ(nullptr, + Channel::Message::Deserialize(&message[0], kMessageSize, + Channel::HandlePolicy::kAcceptHandles, + base::kNullProcessHandle)); } #if !defined(OS_WIN) && !defined(OS_APPLE) && !defined(OS_FUCHSIA) @@ -391,8 +395,10 @@ TEST(ChannelTest, DeserializeMessage_NonZeroExtraHeaderSize) { header->num_header_bytes = kTotalHeaderSize; header->message_type = Channel::Message::MessageType::NORMAL; header->num_handles = 0; - EXPECT_EQ(nullptr, Channel::Message::Deserialize(&message[0], kMessageSize, - base::kNullProcessHandle)); + EXPECT_EQ(nullptr, + Channel::Message::Deserialize(&message[0], kMessageSize, + Channel::HandlePolicy::kAcceptHandles, + base::kNullProcessHandle)); } #endif diff --git a/chromium/mojo/core/core_unittest.cc b/chromium/mojo/core/core_unittest.cc index 0d2024ccd9a..546aba020c8 100644 --- a/chromium/mojo/core/core_unittest.cc +++ b/chromium/mojo/core/core_unittest.cc @@ -9,6 +9,7 @@ #include <limits> #include "base/bind.h" +#include "base/threading/platform_thread.h" #include "build/build_config.h" #include "mojo/core/core_test_base.h" #include "mojo/public/cpp/system/wait.h" diff --git a/chromium/mojo/core/data_pipe_consumer_dispatcher.cc b/chromium/mojo/core/data_pipe_consumer_dispatcher.cc index 4cf36093574..e64ac7eccdf 100644 --- a/chromium/mojo/core/data_pipe_consumer_dispatcher.cc +++ b/chromium/mojo/core/data_pipe_consumer_dispatcher.cc @@ -14,6 +14,7 @@ #include "base/bind.h" #include "base/logging.h" #include "base/memory/ref_counted.h" +#include "base/trace_event/trace_event.h" #include "mojo/core/core.h" #include "mojo/core/data_pipe_control_message.h" #include "mojo/core/node_controller.h" @@ -570,6 +571,9 @@ void DataPipeConsumerDispatcher::UpdateSignalsStateNoLock() { break; } + TRACE_EVENT0("ipc", + "DataPipeConsumerDispatcher received DATA_WAS_WRITTEN"); + if (static_cast<size_t>(bytes_available_) + m->num_bytes > options_.capacity_num_bytes) { DLOG(ERROR) << "Producer claims to have written too many bytes."; diff --git a/chromium/mojo/core/data_pipe_producer_dispatcher.cc b/chromium/mojo/core/data_pipe_producer_dispatcher.cc index cebe8dda57d..80ff8ee2ac4 100644 --- a/chromium/mojo/core/data_pipe_producer_dispatcher.cc +++ b/chromium/mojo/core/data_pipe_producer_dispatcher.cc @@ -12,6 +12,7 @@ #include "base/bind.h" #include "base/logging.h" #include "base/memory/ref_counted.h" +#include "base/trace_event/trace_event.h" #include "mojo/core/configuration.h" #include "mojo/core/core.h" #include "mojo/core/data_pipe_control_message.h" @@ -517,6 +518,9 @@ void DataPipeProducerDispatcher::UpdateSignalsStateNoLock() { break; } + TRACE_EVENT0("ipc", + "DataPipeProducerDispatcher received DATA_WAS_READ"); + if (static_cast<size_t>(available_capacity_) + m->num_bytes > options_.capacity_num_bytes) { DLOG(ERROR) << "Consumer claims to have read too many bytes."; diff --git a/chromium/mojo/core/data_pipe_unittest.cc b/chromium/mojo/core/data_pipe_unittest.cc index 395e8d8fdac..330c957cf1c 100644 --- a/chromium/mojo/core/data_pipe_unittest.cc +++ b/chromium/mojo/core/data_pipe_unittest.cc @@ -9,9 +9,9 @@ #include "base/bind.h" #include "base/check_op.h" +#include "base/cxx17_backports.h" #include "base/location.h" #include "base/run_loop.h" -#include "base/stl_util.h" #include "base/test/task_environment.h" #include "build/build_config.h" #include "mojo/core/embedder/embedder.h" diff --git a/chromium/mojo/core/embedder_unittest.cc b/chromium/mojo/core/embedder_unittest.cc index 7f24ea6b159..79a5ef254ca 100644 --- a/chromium/mojo/core/embedder_unittest.cc +++ b/chromium/mojo/core/embedder_unittest.cc @@ -13,6 +13,7 @@ #include "base/base_paths.h" #include "base/bind.h" #include "base/command_line.h" +#include "base/cxx17_backports.h" #include "base/files/file.h" #include "base/macros.h" #include "base/memory/ptr_util.h" @@ -22,7 +23,6 @@ #include "base/path_service.h" #include "base/rand_util.h" #include "base/run_loop.h" -#include "base/stl_util.h" #include "base/strings/string_number_conversions.h" #include "base/synchronization/waitable_event.h" #include "base/test/test_timeouts.h" diff --git a/chromium/mojo/core/entrypoints.cc b/chromium/mojo/core/entrypoints.cc index 7b342a516f5..ba5781ea0bc 100644 --- a/chromium/mojo/core/entrypoints.cc +++ b/chromium/mojo/core/entrypoints.cc @@ -6,6 +6,7 @@ #include <stdint.h> +#include "base/no_destructor.h" #include "mojo/core/core.h" #include "mojo/public/c/system/buffer.h" #include "mojo/public/c/system/data_pipe.h" @@ -416,7 +417,8 @@ Core* Core::Get() { } void InitializeCore() { - g_core = new Core; + static base::NoDestructor<Core> core_instance; + g_core = core_instance.get(); } const MojoSystemThunks& GetSystemThunks() { diff --git a/chromium/mojo/core/mojo_core.cc b/chromium/mojo/core/mojo_core.cc index 5f2253302c0..bcbe12105fb 100644 --- a/chromium/mojo/core/mojo_core.cc +++ b/chromium/mojo/core/mojo_core.cc @@ -13,7 +13,6 @@ #include "base/logging.h" #include "base/macros.h" #include "base/message_loop/message_pump_type.h" -#include "base/no_destructor.h" #include "base/rand_util.h" #include "base/synchronization/waitable_event.h" #include "base/threading/thread.h" @@ -146,9 +145,9 @@ MojoResult InitializeImpl(const struct MojoInitializeOptions* options) { argv = options->argv; } - static base::NoDestructor<GlobalStateInitializer> global_state_initializer; + static GlobalStateInitializer global_state_initializer; const bool was_global_state_already_initialized = - !global_state_initializer->Initialize(argc, argv); + !global_state_initializer.Initialize(argc, argv); if (!should_initialize_ipc_support) { if (was_global_state_already_initialized) diff --git a/chromium/mojo/core/multiprocess_message_pipe_unittest.cc b/chromium/mojo/core/multiprocess_message_pipe_unittest.cc index 14d7bc90718..cd1fd73f223 100644 --- a/chromium/mojo/core/multiprocess_message_pipe_unittest.cc +++ b/chromium/mojo/core/multiprocess_message_pipe_unittest.cc @@ -13,13 +13,13 @@ #include <vector> #include "base/bind.h" +#include "base/cxx17_backports.h" #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/files/scoped_file.h" #include "base/files/scoped_temp_dir.h" #include "base/logging.h" #include "base/run_loop.h" -#include "base/stl_util.h" #include "base/strings/string_split.h" #include "base/test/task_environment.h" #include "build/build_config.h" diff --git a/chromium/mojo/core/node_channel.cc b/chromium/mojo/core/node_channel.cc index 52fed81f5c2..88d74742d3b 100644 --- a/chromium/mojo/core/node_channel.cc +++ b/chromium/mojo/core/node_channel.cc @@ -734,6 +734,11 @@ void NodeChannel::OnChannelMessage(const void* payload, // through the extent of this call because |this| is kept alive and // |remote_process_handle_| is never reset once set. from_process = remote_process_handle_.Handle(); + + // If we don't have a handle to the remote process, we should not be + // receiving relay requests from them because we're not the broker. + if (from_process == base::kNullProcessHandle) + break; } RelayEventMessageData data; if (GetMessagePayload(payload, payload_size, &data)) { @@ -745,7 +750,7 @@ void NodeChannel::OnChannelMessage(const void* payload, sizeof(Header) + sizeof(data); Channel::MessagePtr message = Channel::Message::Deserialize( message_start, payload_size - sizeof(Header) - sizeof(data), - from_process); + Channel::HandlePolicy::kAcceptHandles, from_process); if (!message) { DLOG(ERROR) << "Dropping invalid relay message."; break; @@ -764,8 +769,9 @@ void NodeChannel::OnChannelMessage(const void* payload, const void* data = static_cast<const void*>( reinterpret_cast<const Header*>(payload) + 1); Channel::MessagePtr message = - Channel::Message::Deserialize(data, payload_size - sizeof(Header)); - if (!message || message->has_handles()) { + Channel::Message::Deserialize(data, payload_size - sizeof(Header), + Channel::HandlePolicy::kRejectHandles); + if (!message) { DLOG(ERROR) << "Dropping invalid broadcast message."; break; } diff --git a/chromium/mojo/core/node_controller.cc b/chromium/mojo/core/node_controller.cc index cd857fba973..9f2e79b9e04 100644 --- a/chromium/mojo/core/node_controller.cc +++ b/chromium/mojo/core/node_controller.cc @@ -17,8 +17,6 @@ #include "base/rand_util.h" #include "base/strings/string_piece.h" #include "base/task/current_thread.h" -#include "base/time/time.h" -#include "base/timer/elapsed_timer.h" #include "mojo/core/broker.h" #include "mojo/core/broker_host.h" #include "mojo/core/configuration.h" @@ -197,7 +195,6 @@ void NodeController::AcceptBrokerClientInvitation( // Use the bootstrap channel for the broker and receive the node's channel // synchronously as the first message from the broker. DCHECK(connection_params.endpoint().is_valid()); - base::ElapsedTimer timer; broker_ = std::make_unique<Broker>( connection_params.TakeEndpoint().TakePlatformHandle(), /*wait_for_channel_handle=*/true); diff --git a/chromium/mojo/core/port_event_fuzzer.cc b/chromium/mojo/core/port_event_fuzzer.cc index 8c03adaf816..6d91eeaf6f0 100644 --- a/chromium/mojo/core/port_event_fuzzer.cc +++ b/chromium/mojo/core/port_event_fuzzer.cc @@ -5,7 +5,6 @@ #include <stdint.h> #include "base/containers/span.h" -#include "base/no_destructor.h" #include "mojo/core/entrypoints.h" #include "mojo/core/node_controller.h" @@ -16,7 +15,7 @@ struct Environment { }; extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) { - static base::NoDestructor<Environment> environment; + static Environment environment; // Try using the fuzz as the full contents of a port event. mojo::core::NodeController::DeserializeRawBytesAsEventForFuzzer( diff --git a/chromium/mojo/core/shared_buffer_dispatcher_unittest.cc b/chromium/mojo/core/shared_buffer_dispatcher_unittest.cc index fe206b63146..108359aeb71 100644 --- a/chromium/mojo/core/shared_buffer_dispatcher_unittest.cc +++ b/chromium/mojo/core/shared_buffer_dispatcher_unittest.cc @@ -9,10 +9,10 @@ #include <limits> +#include "base/cxx17_backports.h" #include "base/memory/platform_shared_memory_region.h" #include "base/memory/ref_counted.h" #include "base/memory/writable_shared_memory_region.h" -#include "base/stl_util.h" #include "mojo/core/dispatcher.h" #include "mojo/core/platform_shared_memory_mapping.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/chromium/mojo/core/user_message_fuzzer.cc b/chromium/mojo/core/user_message_fuzzer.cc index 6b6091d782c..7365d2d6321 100644 --- a/chromium/mojo/core/user_message_fuzzer.cc +++ b/chromium/mojo/core/user_message_fuzzer.cc @@ -5,7 +5,6 @@ #include <stdint.h> #include "base/containers/span.h" -#include "base/no_destructor.h" #include "mojo/core/entrypoints.h" #include "mojo/core/node_controller.h" #include "mojo/core/user_message_impl.h" @@ -17,7 +16,7 @@ struct Environment { }; extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) { - static base::NoDestructor<Environment> environment; + static Environment environment; // Try using our fuzz input as the payload of an otherwise well-formed user // message event. diff --git a/chromium/mojo/core/user_message_impl.h b/chromium/mojo/core/user_message_impl.h index 4f439a4a6b4..118f3f67270 100644 --- a/chromium/mojo/core/user_message_impl.h +++ b/chromium/mojo/core/user_message_impl.h @@ -19,7 +19,6 @@ #include "mojo/core/system_impl_export.h" #include "mojo/public/c/system/message_pipe.h" #include "mojo/public/c/system/types.h" -#include "third_party/abseil-cpp/absl/types/optional.h" namespace mojo { namespace core { diff --git a/chromium/mojo/public/c/system/thunks.cc b/chromium/mojo/public/c/system/thunks.cc index b348bb5d76b..d7dc6e3d8a4 100644 --- a/chromium/mojo/public/c/system/thunks.cc +++ b/chromium/mojo/public/c/system/thunks.cc @@ -142,7 +142,9 @@ class CoreLibraryInitializer { extern "C" { MojoResult MojoInitialize(const struct MojoInitializeOptions* options) { - static base::NoDestructor<mojo::CoreLibraryInitializer> initializer; + static base::NoDestructor<mojo::CoreLibraryInitializer, + base::AllowForTriviallyDestructibleType> + initializer; base::StringPiece library_path_utf8; if (options) { @@ -490,6 +492,10 @@ MojoResult MojoSetDefaultProcessErrorHandler( } // extern "C" +const MojoSystemThunks* MojoEmbedderGetSystemThunks() { + return &g_thunks; +} + void MojoEmbedderSetSystemThunks(const MojoSystemThunks* thunks) { // Assume embedders will always use matching versions of the Mojo Core and // public APIs. diff --git a/chromium/mojo/public/c/system/thunks.h b/chromium/mojo/public/c/system/thunks.h index 4b41b939e24..04e405282ed 100644 --- a/chromium/mojo/public/c/system/thunks.h +++ b/chromium/mojo/public/c/system/thunks.h @@ -236,6 +236,8 @@ struct MojoSystemThunks { }; #pragma pack(pop) +MOJO_SYSTEM_EXPORT const struct MojoSystemThunks* MojoEmbedderGetSystemThunks(); + // A function for setting up the embedder's own system thunks. This should only // be called by Mojo embedder code. MOJO_SYSTEM_EXPORT void MojoEmbedderSetSystemThunks( diff --git a/chromium/mojo/public/cpp/base/BUILD.gn b/chromium/mojo/public/cpp/base/BUILD.gn index 201a6a81c00..9760da63921 100644 --- a/chromium/mojo/public/cpp/base/BUILD.gn +++ b/chromium/mojo/public/cpp/base/BUILD.gn @@ -88,6 +88,8 @@ component("shared_typemap_traits") { "file_mojom_traits.h", "file_path_mojom_traits.cc", "file_path_mojom_traits.h", + "generic_pending_associated_receiver_mojom_traits.cc", + "generic_pending_associated_receiver_mojom_traits.h", "generic_pending_receiver_mojom_traits.cc", "generic_pending_receiver_mojom_traits.h", "read_only_buffer_mojom_traits.cc", diff --git a/chromium/mojo/public/cpp/base/big_buffer.cc b/chromium/mojo/public/cpp/base/big_buffer.cc index d0883ff124a..d5824e30897 100644 --- a/chromium/mojo/public/cpp/base/big_buffer.cc +++ b/chromium/mojo/public/cpp/base/big_buffer.cc @@ -11,7 +11,7 @@ namespace mojo_base { namespace internal { -BigBufferSharedMemoryRegion::BigBufferSharedMemoryRegion() = default; +BigBufferSharedMemoryRegion::BigBufferSharedMemoryRegion() : size_(0) {} BigBufferSharedMemoryRegion::BigBufferSharedMemoryRegion( mojo::ScopedSharedBufferHandle buffer_handle, diff --git a/chromium/mojo/public/cpp/base/byte_string_unittest.cc b/chromium/mojo/public/cpp/base/byte_string_unittest.cc index db5d8cbbd94..fa3941444dc 100644 --- a/chromium/mojo/public/cpp/base/byte_string_unittest.cc +++ b/chromium/mojo/public/cpp/base/byte_string_unittest.cc @@ -14,7 +14,7 @@ namespace mojo_base { TEST(ByteStringTest, Test) { const std::string kCases[] = { "hello", // C-string - {0xEF, 0xB7, 0xAF}, // invalid UTF-8 + {'\xEF', '\xB7', '\xAF'}, // invalid UTF-8 {'h', '\0', 'w', 'd', 'y'}, // embedded null }; for (size_t i = 0; i < base::size(kCases); ++i) { diff --git a/chromium/mojo/public/cpp/base/generic_pending_associated_receiver_mojom_traits.cc b/chromium/mojo/public/cpp/base/generic_pending_associated_receiver_mojom_traits.cc new file mode 100644 index 00000000000..7e6e91c50fc --- /dev/null +++ b/chromium/mojo/public/cpp/base/generic_pending_associated_receiver_mojom_traits.cc @@ -0,0 +1,38 @@ +// Copyright 2021 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 "mojo/public/cpp/base/generic_pending_associated_receiver_mojom_traits.h" + +#include "base/strings/string_piece.h" + +namespace mojo { + +// static +bool StructTraits<mojo_base::mojom::GenericPendingAssociatedReceiverDataView, + GenericPendingAssociatedReceiver>:: + IsNull(const GenericPendingAssociatedReceiver& receiver) { + return !receiver.is_valid(); +} + +// static +void StructTraits<mojo_base::mojom::GenericPendingAssociatedReceiverDataView, + GenericPendingAssociatedReceiver>:: + SetToNull(GenericPendingAssociatedReceiver* receiver) { + receiver->reset(); +} + +// static +bool StructTraits<mojo_base::mojom::GenericPendingAssociatedReceiverDataView, + GenericPendingAssociatedReceiver>:: + Read(mojo_base::mojom::GenericPendingAssociatedReceiverDataView data, + GenericPendingAssociatedReceiver* out) { + base::StringPiece interface_name; + if (!data.ReadInterfaceName(&interface_name)) + return false; + *out = GenericPendingAssociatedReceiver( + interface_name, data.TakeReceiver<ScopedInterfaceEndpointHandle>()); + return true; +} + +} // namespace mojo diff --git a/chromium/mojo/public/cpp/base/generic_pending_associated_receiver_mojom_traits.h b/chromium/mojo/public/cpp/base/generic_pending_associated_receiver_mojom_traits.h new file mode 100644 index 00000000000..1dda94188d2 --- /dev/null +++ b/chromium/mojo/public/cpp/base/generic_pending_associated_receiver_mojom_traits.h @@ -0,0 +1,42 @@ +// Copyright 2021 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 MOJO_PUBLIC_CPP_BASE_GENERIC_PENDING_ASSOCIATED_RECEIVER_MOJOM_TRAITS_H_ +#define MOJO_PUBLIC_CPP_BASE_GENERIC_PENDING_ASSOCIATED_RECEIVER_MOJOM_TRAITS_H_ + +#include "base/component_export.h" +#include "base/strings/string_piece.h" +#include "mojo/public/cpp/bindings/generic_pending_associated_receiver.h" +#include "mojo/public/cpp/bindings/pending_associated_receiver.h" +#include "mojo/public/cpp/bindings/struct_traits.h" +#include "mojo/public/mojom/base/generic_pending_associated_receiver.mojom-shared.h" + +namespace mojo { + +template <> +struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS) + StructTraits<mojo_base::mojom::GenericPendingAssociatedReceiverDataView, + GenericPendingAssociatedReceiver> { + static bool IsNull(const GenericPendingAssociatedReceiver& receiver); + static void SetToNull(GenericPendingAssociatedReceiver* receiver); + + static base::StringPiece interface_name( + const GenericPendingAssociatedReceiver& receiver) { + DCHECK(receiver.interface_name().has_value()); + return receiver.interface_name().value(); + } + + static mojo::ScopedInterfaceEndpointHandle receiver( + GenericPendingAssociatedReceiver& receiver) { + return receiver.PassHandle(); + } + + static bool Read( + mojo_base::mojom::GenericPendingAssociatedReceiverDataView data, + GenericPendingAssociatedReceiver* out); +}; + +} // namespace mojo + +#endif // MOJO_PUBLIC_CPP_BASE_GENERIC_PENDING_ASSOCIATED_RECEIVER_MOJOM_TRAITS_H_ diff --git a/chromium/mojo/public/cpp/base/ref_counted_memory_unittest.cc b/chromium/mojo/public/cpp/base/ref_counted_memory_unittest.cc index 7dfe2e70148..24c32dcbc46 100644 --- a/chromium/mojo/public/cpp/base/ref_counted_memory_unittest.cc +++ b/chromium/mojo/public/cpp/base/ref_counted_memory_unittest.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/stl_util.h" +#include "base/cxx17_backports.h" #include "mojo/public/cpp/base/big_buffer_mojom_traits.h" #include "mojo/public/cpp/base/ref_counted_memory_mojom_traits.h" #include "mojo/public/cpp/test_support/test_utils.h" diff --git a/chromium/mojo/public/cpp/bindings/BUILD.gn b/chromium/mojo/public/cpp/bindings/BUILD.gn index 785c7a55349..13ad53b3ed3 100644 --- a/chromium/mojo/public/cpp/bindings/BUILD.gn +++ b/chromium/mojo/public/cpp/bindings/BUILD.gn @@ -138,6 +138,8 @@ component("bindings") { "callback_helpers.h", "connection_error_callback.h", "connector.h", + "generic_pending_associated_receiver.cc", + "generic_pending_associated_receiver.h", "generic_pending_receiver.cc", "generic_pending_receiver.h", "interface_endpoint_client.h", @@ -234,7 +236,6 @@ component("bindings") { ":bindings_base", ":struct_traits", "//base", - "//base/util/type_safety", "//ipc:message_support", "//ipc:param_traits", "//mojo/public/cpp/system", diff --git a/chromium/mojo/public/cpp/bindings/associated_receiver.h b/chromium/mojo/public/cpp/bindings/associated_receiver.h index 92b4c0e8800..82bf604dc8b 100644 --- a/chromium/mojo/public/cpp/bindings/associated_receiver.h +++ b/chromium/mojo/public/cpp/bindings/associated_receiver.h @@ -45,6 +45,12 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) AssociatedReceiverBase { void FlushForTesting(); + // Please see comments on the same method of InterfaceEndpointClient. + void ResetFromAnotherSequenceUnsafe() { + if (endpoint_client_) + endpoint_client_->ResetFromAnotherSequenceUnsafe(); + } + protected: ~AssociatedReceiverBase(); diff --git a/chromium/mojo/public/cpp/bindings/connector.h b/chromium/mojo/public/cpp/bindings/connector.h index 86ff9a7e474..3975d01a434 100644 --- a/chromium/mojo/public/cpp/bindings/connector.h +++ b/chromium/mojo/public/cpp/bindings/connector.h @@ -81,11 +81,21 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) Connector : public MessageReceiver { kSerializeBeforeDispatchForTesting, }; - // The Connector takes ownership of |message_pipe|. + // The Connector takes ownership of `message_pipe`. A Connector is essentially + // inert upon construction, though it may be used to send messages + // immediately. In order to receive incoming messages or error events, + // StartReceiving() must be called. + Connector(ScopedMessagePipeHandle message_pipe, + ConnectorConfig config, + const char* interface_name = "unknown interface"); + + // Same as above but automatically calls StartReceiving() with `runner` before + // returning. Connector(ScopedMessagePipeHandle message_pipe, ConnectorConfig config, scoped_refptr<base::SequencedTaskRunner> runner, const char* interface_name = "unknown interface"); + ~Connector() override; const char* interface_name() const { return interface_name_; } @@ -131,6 +141,18 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) Connector : public MessageReceiver { return error_; } + // Starts receiving on the Connector's message pipe, allowing incoming + // messages and error events to be dispatched. Once called, the Connector is + // effectively bound to `task_runner`. Initialization methods like + // `set_incoming_receiver` may be called before this, but if called after they + // must be called from the same sequence as `task_runner`. + // + // If `allow_woken_up_by_others` is true, the receiving sequence will allow + // this connector to process incoming messages during any sync wait by any + // Mojo object on the same sequence. + void StartReceiving(scoped_refptr<base::SequencedTaskRunner> task_runner, + bool allow_woken_up_by_others = false); + // Closes the pipe. The connector is put into a quiescent state. // // Please note that this method shouldn't be called unless it results from an @@ -303,8 +325,10 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) Connector : public MessageReceiver { // The quota checker associate with this connector, if any. scoped_refptr<internal::MessageQuotaChecker> quota_checker_; - base::Lock connected_lock_; - bool connected_ = true; + // Indicates whether the Connector is configured to actively read from its + // message pipe. As long as this is true, the Connector is only safe to + // destroy in sequence with `task_runner_` tasks. + bool is_receiving_ = false; // The tag used to track heap allocations that originated from a Watcher // notification. diff --git a/chromium/mojo/public/cpp/bindings/generic_pending_associated_receiver.cc b/chromium/mojo/public/cpp/bindings/generic_pending_associated_receiver.cc new file mode 100644 index 00000000000..2e781d4a466 --- /dev/null +++ b/chromium/mojo/public/cpp/bindings/generic_pending_associated_receiver.cc @@ -0,0 +1,51 @@ +// Copyright 2021 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 "mojo/public/cpp/bindings/generic_pending_associated_receiver.h" + +#include <utility> + +#include "base/check.h" +#include "base/strings/string_piece.h" + +namespace mojo { + +GenericPendingAssociatedReceiver::GenericPendingAssociatedReceiver() = default; + +GenericPendingAssociatedReceiver::GenericPendingAssociatedReceiver( + base::StringPiece interface_name, + mojo::ScopedInterfaceEndpointHandle handle) + : interface_name_(std::string(interface_name)), + handle_(std::move(handle)) {} + +GenericPendingAssociatedReceiver::GenericPendingAssociatedReceiver( + GenericPendingAssociatedReceiver&&) = default; + +GenericPendingAssociatedReceiver& GenericPendingAssociatedReceiver::operator=( + GenericPendingAssociatedReceiver&&) = default; + +GenericPendingAssociatedReceiver::~GenericPendingAssociatedReceiver() = default; + +void GenericPendingAssociatedReceiver::reset() { + interface_name_.reset(); + handle_.reset(); +} + +mojo::ScopedInterfaceEndpointHandle +GenericPendingAssociatedReceiver::PassHandle() { + DCHECK(is_valid()); + interface_name_.reset(); + return std::move(handle_); +} + +mojo::ScopedInterfaceEndpointHandle +GenericPendingAssociatedReceiver::PassHandleIfNameIs( + const char* interface_name) { + DCHECK(is_valid()); + if (interface_name_ == interface_name) + return PassHandle(); + return mojo::ScopedInterfaceEndpointHandle(); +} + +} // namespace mojo diff --git a/chromium/mojo/public/cpp/bindings/generic_pending_associated_receiver.h b/chromium/mojo/public/cpp/bindings/generic_pending_associated_receiver.h new file mode 100644 index 00000000000..56d937ddd14 --- /dev/null +++ b/chromium/mojo/public/cpp/bindings/generic_pending_associated_receiver.h @@ -0,0 +1,82 @@ +// Copyright 2021 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 MOJO_PUBLIC_CPP_BINDINGS_GENERIC_PENDING_ASSOCIATED_RECEIVER_H_ +#define MOJO_PUBLIC_CPP_BINDINGS_GENERIC_PENDING_ASSOCIATED_RECEIVER_H_ + +#include <string> + +#include "base/component_export.h" +#include "base/strings/string_piece.h" +#include "mojo/public/cpp/bindings/pending_associated_receiver.h" +#include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h" +#include "third_party/abseil-cpp/absl/types/optional.h" + +namespace mojo { + +// GenericPendingAssociatedReceiver encapsulates a pairing of a receiving +// associated interface endponit with the name of the mojom interface assumed by +// the corresponding remote endpoint. +// +// This is used by mojom C++ bindings to represent +// |mojo_base.mojom.GenericAssociatedPendingReceiver|, and it serves as a +// semi-safe wrapper for transporting arbitrary associated interface receivers +// in a generic object. +// +// It is intended to be used in the (relatively rare) scenario where an +// interface needs to support sharing its message ordering with interfaces +// defined at higher application layers, such that knowledge of those associated +// interface(s) would constitute a layering violation. +class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) GenericPendingAssociatedReceiver { + public: + GenericPendingAssociatedReceiver(); + GenericPendingAssociatedReceiver(base::StringPiece interface_name, + mojo::ScopedInterfaceEndpointHandle handle); + + template <typename Interface> + GenericPendingAssociatedReceiver( + mojo::PendingAssociatedReceiver<Interface> receiver) + : GenericPendingAssociatedReceiver(Interface::Name_, + receiver.PassHandle()) {} + + GenericPendingAssociatedReceiver(const GenericPendingAssociatedReceiver&) = + delete; + GenericPendingAssociatedReceiver(GenericPendingAssociatedReceiver&&); + GenericPendingAssociatedReceiver& operator=( + const GenericPendingAssociatedReceiver&) = delete; + GenericPendingAssociatedReceiver& operator=( + GenericPendingAssociatedReceiver&&); + ~GenericPendingAssociatedReceiver(); + + bool is_valid() const { return handle_.is_valid(); } + explicit operator bool() const { return is_valid(); } + + void reset(); + + const absl::optional<std::string>& interface_name() const { + return interface_name_; + } + + // Takes ownership of the endpoint, invalidating this object. + mojo::ScopedInterfaceEndpointHandle PassHandle(); + + // Takes ownership of the endpoint, strongly typed as an `Interface` receiver, + // if and only if that interface's name matches the stored interface name. + template <typename Interface> + mojo::PendingAssociatedReceiver<Interface> As() { + return mojo::PendingAssociatedReceiver<Interface>( + PassHandleIfNameIs(Interface::Name_)); + } + + private: + mojo::ScopedInterfaceEndpointHandle PassHandleIfNameIs( + const char* interface_name); + + absl::optional<std::string> interface_name_; + mojo::ScopedInterfaceEndpointHandle handle_; +}; + +} // namespace mojo + +#endif // MOJO_PUBLIC_CPP_BINDINGS_GENERIC_PENDING_ASSOCIATED_RECEIVER_H_ diff --git a/chromium/mojo/public/cpp/bindings/interface_endpoint_client.h b/chromium/mojo/public/cpp/bindings/interface_endpoint_client.h index 738a2ab316f..68963ea1fd0 100644 --- a/chromium/mojo/public/cpp/bindings/interface_endpoint_client.h +++ b/chromium/mojo/public/cpp/bindings/interface_endpoint_client.h @@ -21,6 +21,7 @@ #include "base/memory/weak_ptr.h" #include "base/sequence_checker.h" #include "base/sequenced_task_runner.h" +#include "base/synchronization/lock.h" #include "base/synchronization/waitable_event.h" #include "base/time/time.h" #include "base/timer/timer.h" @@ -85,6 +86,7 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) InterfaceEndpointClient // Returns true if this endpoint has any pending callbacks. bool has_pending_responders() const { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + base::AutoLock lock(async_responders_lock_); return !async_responders_.empty() || !sync_responses_.empty(); } @@ -192,6 +194,20 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) InterfaceEndpointClient } #endif + // This allows the endpoint to be reset from a sequence other than the one on + // which it was bound. This should only be used with caution, and it is + // critical that the calling sequence cannot run tasks concurrently with the + // bound sequence. There's no practical way for this to be asserted, so we + // have to take your word for it. If this constraint is not upheld, there will + // be data races internal to the bindings object which can lead to UAFs or + // surprise message dispatches. + void ResetFromAnotherSequenceUnsafe(); + + // Tells the client to forget about a pending async request for which it still + // hasn't seen a response. Called by the router, possibly from other threads. + // The router lock must be held when calling this. + void ForgetAsyncRequest(uint64_t request_id); + private: // Maps from the id of a response to the MessageReceiver that handles the // response. @@ -275,7 +291,8 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) InterfaceEndpointClient HandleIncomingMessageThunk thunk_{this}; MessageDispatcher dispatcher_; - AsyncResponderMap async_responders_; + mutable base::Lock async_responders_lock_; + AsyncResponderMap async_responders_ GUARDED_BY(async_responders_lock_); SyncResponseMap sync_responses_; uint64_t next_request_id_ = 1; diff --git a/chromium/mojo/public/cpp/bindings/interface_request.h b/chromium/mojo/public/cpp/bindings/interface_request.h index 1189b028d39..89c4a2066e7 100644 --- a/chromium/mojo/public/cpp/bindings/interface_request.h +++ b/chromium/mojo/public/cpp/bindings/interface_request.h @@ -18,7 +18,6 @@ #include "mojo/public/cpp/bindings/lib/pending_receiver_state.h" #include "mojo/public/cpp/bindings/pipe_control_message_proxy.h" #include "mojo/public/cpp/system/message_pipe.h" -#include "third_party/abseil-cpp/absl/types/optional.h" namespace mojo { diff --git a/chromium/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h b/chromium/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h index 6a3f529d11c..29e3390e17b 100644 --- a/chromium/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h +++ b/chromium/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h @@ -105,7 +105,10 @@ class AssociatedInterfacePtrState : public AssociatedInterfacePtrStateBase { public: using Proxy = typename Interface::Proxy_; - AssociatedInterfacePtrState() {} + AssociatedInterfacePtrState() = default; + AssociatedInterfacePtrState(const AssociatedInterfacePtrState&) = delete; + AssociatedInterfacePtrState& operator=(const AssociatedInterfacePtrState&) = + delete; ~AssociatedInterfacePtrState() = default; Proxy* instance() { @@ -125,7 +128,7 @@ class AssociatedInterfacePtrState : public AssociatedInterfacePtrStateBase { info.PassHandle(), info.version(), std::make_unique<typename Interface::ResponseValidator_>(), std::move(runner), Interface::Name_); - proxy_.reset(new Proxy(endpoint_client())); + proxy_ = std::make_unique<Proxy>(endpoint_client()); } // After this method is called, the object is in an invalid state and @@ -138,8 +141,6 @@ class AssociatedInterfacePtrState : public AssociatedInterfacePtrStateBase { private: std::unique_ptr<Proxy> proxy_; - - DISALLOW_COPY_AND_ASSIGN(AssociatedInterfacePtrState); }; } // namespace internal diff --git a/chromium/mojo/public/cpp/bindings/lib/associated_receiver.cc b/chromium/mojo/public/cpp/bindings/lib/associated_receiver.cc index 98f820ccea3..221fbf1d45c 100644 --- a/chromium/mojo/public/cpp/bindings/lib/associated_receiver.cc +++ b/chromium/mojo/public/cpp/bindings/lib/associated_receiver.cc @@ -74,7 +74,7 @@ void AssociatedReceiverBase::BindImpl( void AssociateWithDisconnectedPipe(ScopedInterfaceEndpointHandle handle) { MessagePipe pipe; scoped_refptr<internal::MultiplexRouter> router = - internal::MultiplexRouter::Create( + internal::MultiplexRouter::CreateAndStartReceiving( std::move(pipe.handle0), internal::MultiplexRouter::MULTI_INTERFACE, false, base::SequencedTaskRunnerHandle::Get()); router->AssociateInterface(std::move(handle)); diff --git a/chromium/mojo/public/cpp/bindings/lib/binding_state.cc b/chromium/mojo/public/cpp/bindings/lib/binding_state.cc index fd20c1e5bf8..1efeb0791c3 100644 --- a/chromium/mojo/public/cpp/bindings/lib/binding_state.cc +++ b/chromium/mojo/public/cpp/bindings/lib/binding_state.cc @@ -122,8 +122,9 @@ void BindingStateBase::BindInternal( : (has_sync_methods ? MultiplexRouter::SINGLE_INTERFACE_WITH_SYNC_METHODS : MultiplexRouter::SINGLE_INTERFACE); - router_ = MultiplexRouter::Create(std::move(receiver_state->pipe), config, - false, sequenced_runner, interface_name); + router_ = MultiplexRouter::CreateAndStartReceiving( + std::move(receiver_state->pipe), config, false, sequenced_runner, + interface_name); router_->SetConnectionGroup(std::move(receiver_state->connection_group)); endpoint_client_ = std::make_unique<InterfaceEndpointClient>( diff --git a/chromium/mojo/public/cpp/bindings/lib/binding_state.h b/chromium/mojo/public/cpp/bindings/lib/binding_state.h index def412c37ea..e111ffa5a59 100644 --- a/chromium/mojo/public/cpp/bindings/lib/binding_state.h +++ b/chromium/mojo/public/cpp/bindings/lib/binding_state.h @@ -132,9 +132,9 @@ class BindingState : public BindingStateBase { Interface* impl() { return ImplRefTraits::GetRawPointer(&stub_.sink()); } ImplPointerType SwapImplForTesting(ImplPointerType new_impl) { - Interface* old_impl = impl(); - stub_.set_sink(std::move(new_impl)); - return old_impl; + using std::swap; + swap(new_impl, stub_.sink()); + return new_impl; } private: diff --git a/chromium/mojo/public/cpp/bindings/lib/connector.cc b/chromium/mojo/public/cpp/bindings/lib/connector.cc index daae5a3a451..92a9756cbae 100644 --- a/chromium/mojo/public/cpp/bindings/lib/connector.cc +++ b/chromium/mojo/public/cpp/bindings/lib/connector.cc @@ -16,7 +16,6 @@ #include "base/memory/ptr_util.h" #include "base/metrics/field_trial_params.h" #include "base/metrics/histogram_macros.h" -#include "base/no_destructor.h" #include "base/rand_util.h" #include "base/run_loop.h" #include "base/strings/strcat.h" @@ -105,10 +104,9 @@ class Connector::RunLoopNestingObserver // The NestingObserver for each thread. Note that this is always a // Connector::RunLoopNestingObserver; we use the base type here because that // subclass is private to Connector. - static base::NoDestructor< - base::SequenceLocalStorageSlot<RunLoopNestingObserver>> + static base::SequenceLocalStorageSlot<RunLoopNestingObserver> sls_nesting_observer; - return &sls_nesting_observer->GetOrCreateValue(); + return &sls_nesting_observer.GetOrCreateValue(); } private: @@ -148,10 +146,8 @@ void Connector::ActiveDispatchTracker::NotifyBeginNesting() { Connector::Connector(ScopedMessagePipeHandle message_pipe, ConnectorConfig config, - scoped_refptr<base::SequencedTaskRunner> runner, const char* interface_name) : message_pipe_(std::move(message_pipe)), - task_runner_(std::move(runner)), error_(false), force_immediate_dispatch_(!EnableTaskPerMessage()), outgoing_serialization_mode_(g_default_outgoing_serialization_mode), @@ -166,17 +162,14 @@ Connector::Connector(ScopedMessagePipeHandle message_pipe, #endif weak_self_ = weak_factory_.GetWeakPtr(); +} - DETACH_FROM_SEQUENCE(sequence_checker_); - - // Even though we don't have an incoming receiver, we still want to monitor - // the message pipe to know if is closed or encounters an error. - if (task_runner_->RunsTasksInCurrentSequence()) { - WaitToReadMore(); - } else { - task_runner_->PostTask( - FROM_HERE, base::BindOnce(&Connector::WaitToReadMore, weak_self_)); - } +Connector::Connector(ScopedMessagePipeHandle message_pipe, + ConnectorConfig config, + scoped_refptr<base::SequencedTaskRunner> runner, + const char* interface_name) + : Connector(std::move(message_pipe), config, interface_name) { + StartReceiving(std::move(runner)); } Connector::~Connector() { @@ -187,16 +180,10 @@ Connector::~Connector() { quota_checker_->GetMaxQuotaUsage()); } - { - // Allow for quick destruction on any sequence if the pipe is already - // closed. - base::AutoLock lock(connected_lock_); - if (!connected_) - return; + if (is_receiving_) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + CancelWait(); } - - DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - CancelWait(); } void Connector::SetOutgoingSerializationMode(OutgoingSerializationMode mode) { @@ -209,6 +196,23 @@ void Connector::SetIncomingSerializationMode(IncomingSerializationMode mode) { incoming_serialization_mode_ = mode; } +void Connector::StartReceiving( + scoped_refptr<base::SequencedTaskRunner> task_runner, + bool allow_woken_up_by_others) { + DCHECK(!task_runner_); + task_runner_ = std::move(task_runner); + allow_woken_up_by_others_ = allow_woken_up_by_others; + + DETACH_FROM_SEQUENCE(sequence_checker_); + if (task_runner_->RunsTasksInCurrentSequence()) { + WaitToReadMore(); + } else { + task_runner_->PostTask( + FROM_HERE, + base::BindOnce(&Connector::WaitToReadMore, weak_factory_.GetWeakPtr())); + } +} + void Connector::CloseMessagePipe() { // Throw away the returned message pipe. PassMessagePipe(); @@ -223,8 +227,6 @@ ScopedMessagePipeHandle Connector::PassMessagePipe() { weak_factory_.InvalidateWeakPtrs(); sync_handle_watcher_callback_count_ = 0; - base::AutoLock lock(connected_lock_); - connected_ = false; return message_pipe; } @@ -310,7 +312,7 @@ bool Connector::PrefersSerializedMessages() { } bool Connector::Accept(Message* message) { - if (!lock_) + if (!lock_ && task_runner_) DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (error_) @@ -331,9 +333,11 @@ bool Connector::Accept(Message* message) { if (!message->is_serialized()) { // The caller is sending an unserialized message. If we haven't set up a // remoteness tracker yet, do so now. See PrefersSerializedMessages() above - // for more details. + // for more details. Note that if the Connector is not yet bound to a + // TaskRunner and activaly reading the pipe, we don't bother setting this up + // yet. DCHECK_EQ(outgoing_serialization_mode_, OutgoingSerializationMode::kLazy); - if (!peer_remoteness_tracker_) { + if (!peer_remoteness_tracker_ && task_runner_) { peer_remoteness_tracker_.emplace( message_pipe_.get(), MOJO_HANDLE_SIGNAL_PEER_REMOTE, task_runner_); } @@ -437,6 +441,7 @@ void Connector::OnHandleReadyInternal(MojoResult result) { } void Connector::WaitToReadMore() { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); CHECK(!paused_); DCHECK(!handle_watcher_); @@ -466,6 +471,8 @@ void Connector::WaitToReadMore() { EnsureSyncWatcherExists(); sync_watcher_->AllowWokenUpBySyncWatchOnSameThread(); } + + is_receiving_ = true; } uint64_t Connector::QueryPendingMessageCount() const { @@ -627,6 +634,8 @@ void Connector::ReadAllAvailableMessages() { } void Connector::CancelWait() { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + is_receiving_ = false; peer_remoteness_tracker_.reset(); handle_watcher_.reset(); sync_watcher_.reset(); diff --git a/chromium/mojo/public/cpp/bindings/lib/control_message_proxy.cc b/chromium/mojo/public/cpp/bindings/lib/control_message_proxy.cc index 321bbe2e250..57b61edc862 100644 --- a/chromium/mojo/public/cpp/bindings/lib/control_message_proxy.cc +++ b/chromium/mojo/public/cpp/bindings/lib/control_message_proxy.cc @@ -13,6 +13,7 @@ #include "base/macros.h" #include "base/run_loop.h" #include "base/threading/sequenced_task_runner_handle.h" +#include "base/time/time.h" #include "mojo/public/cpp/bindings/interface_endpoint_client.h" #include "mojo/public/cpp/bindings/lib/serialization.h" #include "mojo/public/cpp/bindings/lib/validation_util.h" diff --git a/chromium/mojo/public/cpp/bindings/lib/control_message_proxy.h b/chromium/mojo/public/cpp/bindings/lib/control_message_proxy.h index 7510bb99dce..b6d7cc89622 100644 --- a/chromium/mojo/public/cpp/bindings/lib/control_message_proxy.h +++ b/chromium/mojo/public/cpp/bindings/lib/control_message_proxy.h @@ -11,6 +11,10 @@ #include "base/component_export.h" #include "base/macros.h" +namespace base { +class TimeDelta; +} + namespace mojo { class InterfaceEndpointClient; diff --git a/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc b/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc index 9888636a620..4e8d1c3c6d2 100644 --- a/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc +++ b/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc @@ -10,13 +10,14 @@ #include "base/bind_post_task.h" #include "base/check.h" #include "base/containers/contains.h" +#include "base/containers/cxx20_erase.h" +#include "base/cxx17_backports.h" #include "base/location.h" #include "base/logging.h" #include "base/macros.h" #include "base/memory/ptr_util.h" #include "base/memory/weak_ptr.h" #include "base/sequenced_task_runner.h" -#include "base/stl_util.h" #include "mojo/public/cpp/bindings/associated_group.h" #include "mojo/public/cpp/bindings/associated_group_controller.h" #include "mojo/public/cpp/bindings/interface_endpoint_controller.h" @@ -442,11 +443,6 @@ InterfaceEndpointClient::InterfaceEndpointClient( base::BindOnce(&InterfaceEndpointClient::OnAssociationEvent, weak_ptr_factory_.GetWeakPtr()))); } - } else if (!task_runner_->RunsTasksInCurrentSequence()) { - task_runner_->PostTask( - FROM_HERE, - base::BindOnce(&InterfaceEndpointClient::InitControllerIfNecessary, - weak_ptr_factory_.GetWeakPtr())); } else { InitControllerIfNecessary(); } @@ -608,13 +604,14 @@ bool InterfaceEndpointClient::SendMessageWithResponder( ++num_unacked_messages_; if (!is_sync || sync_send_mode == SyncSendMode::kForceAsync) { - async_responders_[request_id] = std::move(responder); if (is_sync) { // This was forced to send async. Leave a placeholder in the map of // expected sync responses so HandleValidatedMessage knows what to do. sync_responses_.emplace(request_id, nullptr); controller_->RegisterExternalSyncWaiter(request_id); } + base::AutoLock lock(async_responders_lock_); + async_responders_[request_id] = std::move(responder); return true; } @@ -676,7 +673,11 @@ void InterfaceEndpointClient::NotifyError( // them alive any longer. Note that it's allowed that a pending response // callback may own this endpoint, so we simply move the responders onto the // stack here and let them be destroyed when the stack unwinds. - AsyncResponderMap responders = std::move(async_responders_); + AsyncResponderMap responders; + { + base::AutoLock lock(async_responders_lock_); + std::swap(responders, async_responders_); + } control_message_proxy_.OnConnectionError(); @@ -784,13 +785,36 @@ void InterfaceEndpointClient::MaybeSendNotifyIdle() { } } +void InterfaceEndpointClient::ResetFromAnotherSequenceUnsafe() { + DETACH_FROM_SEQUENCE(sequence_checker_); + + if (controller_) { + controller_ = nullptr; + handle_.group_controller()->DetachEndpointClient(handle_); + } + + handle_.reset(); +} + +void InterfaceEndpointClient::ForgetAsyncRequest(uint64_t request_id) { + std::unique_ptr<MessageReceiver> responder; + { + base::AutoLock lock(async_responders_lock_); + auto it = async_responders_.find(request_id); + if (it == async_responders_.end()) + return; + responder = std::move(it->second); + async_responders_.erase(it); + } +} + void InterfaceEndpointClient::InitControllerIfNecessary() { if (controller_ || handle_.pending_association()) return; controller_ = handle_.group_controller()->AttachEndpointClient(handle_, this, task_runner_); - if (expect_sync_requests_) + if (expect_sync_requests_ && task_runner_->RunsTasksInCurrentSequence()) controller_->AllowWokenUpBySyncWatchOnSameThread(); } @@ -855,11 +879,15 @@ bool InterfaceEndpointClient::HandleValidatedMessage(Message* message) { sync_responses_.erase(it); } - auto it = async_responders_.find(request_id); - if (it == async_responders_.end()) - return false; - std::unique_ptr<MessageReceiver> responder = std::move(it->second); - async_responders_.erase(it); + std::unique_ptr<MessageReceiver> responder; + { + base::AutoLock lock(async_responders_lock_); + auto it = async_responders_.find(request_id); + if (it == async_responders_.end()) + return false; + responder = std::move(it->second); + async_responders_.erase(it); + } internal::MessageDispatchContext dispatch_context(message); return responder->Accept(message); diff --git a/chromium/mojo/public/cpp/bindings/lib/interface_ptr_state.cc b/chromium/mojo/public/cpp/bindings/lib/interface_ptr_state.cc index 7f095b7e2f5..ea1c937f80c 100644 --- a/chromium/mojo/public/cpp/bindings/lib/interface_ptr_state.cc +++ b/chromium/mojo/public/cpp/bindings/lib/interface_ptr_state.cc @@ -98,6 +98,14 @@ bool InterfacePtrStateBase::InitializeEndpointClient( // The version is only queried from the client so the value passed here // will not be used. 0u, interface_name); + + // Note that we defer this until after attaching the endpoint. This is in case + // `runner_` does not run tasks in the current sequence but MultiplexRouter is + // in SINGLE_INTERFACE mode. In that case, MultiplexRouter elides some + // internal synchronization, so we need to ensure that messages aren't + // processed by the router before the endpoint above is fully attached. + router_->StartReceiving(); + return true; } diff --git a/chromium/mojo/public/cpp/bindings/lib/interface_serialization.h b/chromium/mojo/public/cpp/bindings/lib/interface_serialization.h index 544457dca52..10e5df1991f 100644 --- a/chromium/mojo/public/cpp/bindings/lib/interface_serialization.h +++ b/chromium/mojo/public/cpp/bindings/lib/interface_serialization.h @@ -131,6 +131,24 @@ struct Serializer<AssociatedInterfaceRequestDataView<Base>, } }; +template <typename T> +struct Serializer<AssociatedInterfaceRequestDataView<T>, + ScopedInterfaceEndpointHandle> { + static void Serialize(ScopedInterfaceEndpointHandle& input, + AssociatedEndpointHandle_Data* output, + Message* message) { + DCHECK(!input.is_valid() || input.pending_association()); + SerializeAssociatedEndpoint(std::move(input), *message, *output); + } + + static bool Deserialize(AssociatedEndpointHandle_Data* input, + ScopedInterfaceEndpointHandle* output, + Message* message) { + *output = DeserializeAssociatedEndpointHandle(*input, *message); + return true; + } +}; + template <typename Base, typename T> struct Serializer<InterfacePtrDataView<Base>, InterfacePtr<T>> { static_assert(std::is_base_of<Base, T>::value, "Interface type mismatch."); diff --git a/chromium/mojo/public/cpp/bindings/lib/message.cc b/chromium/mojo/public/cpp/bindings/lib/message.cc index 385a668f5c6..7c8f2e293e0 100644 --- a/chromium/mojo/public/cpp/bindings/lib/message.cc +++ b/chromium/mojo/public/cpp/bindings/lib/message.cc @@ -569,6 +569,10 @@ ReportBadMessageCallback GetBadMessageCallback() { return context->GetBadMessageCallback(); } +bool IsInMessageDispatch() { + return internal::MessageDispatchContext::current(); +} + namespace internal { MessageHeaderV2::MessageHeaderV2() = default; diff --git a/chromium/mojo/public/cpp/bindings/lib/message_fragment.h b/chromium/mojo/public/cpp/bindings/lib/message_fragment.h index e466d62250e..748d3bf4099 100644 --- a/chromium/mojo/public/cpp/bindings/lib/message_fragment.h +++ b/chromium/mojo/public/cpp/bindings/lib/message_fragment.h @@ -149,8 +149,7 @@ class MessageFragment<Array_Data<T>> { static_assert( std::numeric_limits<uint32_t>::max() > Traits::kMaxNumElements, "Max num elements castable to 32bit"); - if (num_elements > Traits::kMaxNumElements) - return; + CHECK_LE(num_elements, Traits::kMaxNumElements); const uint32_t num_bytes = Traits::GetStorageSize(static_cast<uint32_t>(num_elements)); diff --git a/chromium/mojo/public/cpp/bindings/lib/message_quota_checker.cc b/chromium/mojo/public/cpp/bindings/lib/message_quota_checker.cc index 7cd5d1e32d6..dedb4085c88 100644 --- a/chromium/mojo/public/cpp/bindings/lib/message_quota_checker.cc +++ b/chromium/mojo/public/cpp/bindings/lib/message_quota_checker.cc @@ -10,10 +10,12 @@ #include "base/debug/activity_tracker.h" #include "base/debug/alias.h" #include "base/debug/dump_without_crashing.h" +#include "base/memory/scoped_refptr.h" #include "base/metrics/field_trial_params.h" #include "base/no_destructor.h" #include "base/rand_util.h" #include "base/synchronization/lock.h" +#include "base/types/pass_key.h" #include "mojo/public/c/system/quota.h" #include "mojo/public/cpp/bindings/features.h" #include "mojo/public/cpp/bindings/mojo_buildflags.h" @@ -179,7 +181,8 @@ scoped_refptr<MessageQuotaChecker> MessageQuotaChecker::MaybeCreateForTesting( return MaybeCreateImpl(config); } -MessageQuotaChecker::MessageQuotaChecker(const Configuration* config) +MessageQuotaChecker::MessageQuotaChecker(const Configuration* config, + base::PassKey<MessageQuotaChecker>) : config_(config), creation_time_(base::TimeTicks::Now()) {} MessageQuotaChecker::~MessageQuotaChecker() = default; @@ -210,7 +213,8 @@ scoped_refptr<MessageQuotaChecker> MessageQuotaChecker::MaybeCreateImpl( if (base::RandInt(0, config.sample_rate - 1) != 0) return nullptr; - return new MessageQuotaChecker(&config); + return base::MakeRefCounted<MessageQuotaChecker>( + &config, base::PassKey<MessageQuotaChecker>()); } absl::optional<size_t> MessageQuotaChecker::GetCurrentMessagePipeQuota() { diff --git a/chromium/mojo/public/cpp/bindings/lib/message_quota_checker.h b/chromium/mojo/public/cpp/bindings/lib/message_quota_checker.h index c9b021956e9..40cb6c0c65d 100644 --- a/chromium/mojo/public/cpp/bindings/lib/message_quota_checker.h +++ b/chromium/mojo/public/cpp/bindings/lib/message_quota_checker.h @@ -13,6 +13,7 @@ #include "base/synchronization/lock.h" #include "base/thread_annotations.h" #include "base/time/time.h" +#include "base/types/pass_key.h" #include "mojo/public/cpp/system/message_pipe.h" #include "third_party/abseil-cpp/absl/types/optional.h" @@ -79,10 +80,17 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) MessageQuotaChecker double decayed_average_ = 0.0; }; + // Exposed for use in tests. + struct Configuration; + // Returns a new instance if this invocation has been sampled for quota // checking. static scoped_refptr<MessageQuotaChecker> MaybeCreate(); + // Public for base::MakeRefCounted(). Use MaybeCreate(). + MessageQuotaChecker(const Configuration* config, + base::PassKey<MessageQuotaChecker>); + // Call before writing a message to |message_pipe_|. void BeforeWrite(); @@ -99,14 +107,12 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) MessageQuotaChecker // Test support. size_t GetCurrentQuotaStatusForTesting(); - struct Configuration; static Configuration GetConfigurationForTesting(); static scoped_refptr<MessageQuotaChecker> MaybeCreateForTesting( const Configuration& config); private: friend class base::RefCountedThreadSafe<MessageQuotaChecker>; - explicit MessageQuotaChecker(const Configuration* config); ~MessageQuotaChecker(); static Configuration GetConfiguration(); static scoped_refptr<MessageQuotaChecker> MaybeCreateImpl( diff --git a/chromium/mojo/public/cpp/bindings/lib/multiplex_router.cc b/chromium/mojo/public/cpp/bindings/lib/multiplex_router.cc index 6039e764a71..605e51344d6 100644 --- a/chromium/mojo/public/cpp/bindings/lib/multiplex_router.cc +++ b/chromium/mojo/public/cpp/bindings/lib/multiplex_router.cc @@ -14,9 +14,11 @@ #include "base/location.h" #include "base/macros.h" #include "base/memory/ptr_util.h" +#include "base/memory/scoped_refptr.h" #include "base/sequenced_task_runner.h" #include "base/strings/string_util.h" #include "base/synchronization/waitable_event.h" +#include "base/types/pass_key.h" #include "mojo/public/cpp/bindings/interface_endpoint_client.h" #include "mojo/public/cpp/bindings/interface_endpoint_controller.h" #include "mojo/public/cpp/bindings/lib/may_auto_lock.h" @@ -88,7 +90,6 @@ class MultiplexRouter::InterfaceEndpoint router_->AssertLockAcquired(); DCHECK(!client_); DCHECK(!closed_); - DCHECK(runner->RunsTasksInCurrentSequence()); task_runner_ = std::move(runner); client_ = client; @@ -99,7 +100,6 @@ class MultiplexRouter::InterfaceEndpoint void DetachClient() { router_->AssertLockAcquired(); DCHECK(client_); - DCHECK(task_runner_->RunsTasksInCurrentSequence()); DCHECK(!closed_); task_runner_ = nullptr; @@ -112,6 +112,13 @@ class MultiplexRouter::InterfaceEndpoint return requests_with_external_sync_waiter_.erase(request_id) != 0; } + base::flat_set<uint64_t> UnregisterAllExternalSyncWaiters() { + router_->AssertLockAcquired(); + base::flat_set<uint64_t> request_ids; + std::swap(request_ids, requests_with_external_sync_waiter_); + return request_ids; + } + void SignalSyncMessageEvent() { router_->AssertLockAcquired(); if (sync_message_event_signaled_) @@ -299,21 +306,26 @@ class MultiplexRouter::MessageWrapper { struct MultiplexRouter::Task { public: + enum Type { MESSAGE, NOTIFY_ERROR }; + // Doesn't take ownership of |message| but takes its contents. static std::unique_ptr<Task> CreateMessageTask( MessageWrapper message_wrapper) { - Task* task = new Task(MESSAGE); + auto task = std::make_unique<Task>(MESSAGE, base::PassKey<Task>()); task->message_wrapper = std::move(message_wrapper); - return base::WrapUnique(task); + return task; } static std::unique_ptr<Task> CreateNotifyErrorTask( InterfaceEndpoint* endpoint) { - Task* task = new Task(NOTIFY_ERROR); + auto task = std::make_unique<Task>(NOTIFY_ERROR, base::PassKey<Task>()); task->endpoint_to_notify = endpoint; - return base::WrapUnique(task); + return task; } - ~Task() {} + explicit Task(Type in_type, base::PassKey<Task>) : type(in_type) {} + Task(const Task&) = delete; + Task& operator=(const Task&) = delete; + ~Task() = default; bool IsMessageTask() const { return type == MESSAGE; } bool IsNotifyErrorTask() const { return type == NOTIFY_ERROR; } @@ -321,13 +333,7 @@ struct MultiplexRouter::Task { MessageWrapper message_wrapper; scoped_refptr<InterfaceEndpoint> endpoint_to_notify; - enum Type { MESSAGE, NOTIFY_ERROR }; Type type; - - private: - explicit Task(Type in_type) : type(in_type) {} - - DISALLOW_COPY_AND_ASSIGN(Task); }; // static @@ -337,16 +343,22 @@ scoped_refptr<MultiplexRouter> MultiplexRouter::Create( bool set_interface_id_namespace_bit, scoped_refptr<base::SequencedTaskRunner> runner, const char* primary_interface_name) { - auto router = base::MakeRefCounted<MultiplexRouter>( + return base::MakeRefCounted<MultiplexRouter>( base::PassKey<MultiplexRouter>(), std::move(message_pipe), config, set_interface_id_namespace_bit, runner, primary_interface_name); - if (runner->RunsTasksInCurrentSequence()) { - router->BindToCurrentSequence(); - } else { - runner->PostTask( - FROM_HERE, - base::BindOnce(&MultiplexRouter::BindToCurrentSequence, router)); - } +} + +// static +scoped_refptr<MultiplexRouter> MultiplexRouter::CreateAndStartReceiving( + ScopedMessagePipeHandle message_pipe, + Config config, + bool set_interface_id_namespace_bit, + scoped_refptr<base::SequencedTaskRunner> runner, + const char* primary_interface_name) { + auto router = + Create(std::move(message_pipe), config, set_interface_id_namespace_bit, + runner, primary_interface_name); + router->StartReceiving(); return router; } @@ -364,31 +376,13 @@ MultiplexRouter::MultiplexRouter( connector_(std::move(message_pipe), config == MULTI_INTERFACE ? Connector::MULTI_THREADED_SEND : Connector::SINGLE_THREADED_SEND, - std::move(runner), primary_interface_name), control_message_handler_(this), control_message_proxy_(&connector_) { - DETACH_FROM_SEQUENCE(sequence_checker_); if (config_ == MULTI_INTERFACE) lock_.emplace(); -} -void MultiplexRouter::BindToCurrentSequence() { - DCHECK(task_runner_->RunsTasksInCurrentSequence()); - DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - - if (config_ == SINGLE_INTERFACE_WITH_SYNC_METHODS || - config_ == MULTI_INTERFACE) { - // Always participate in sync handle watching in multi-interface mode, - // because even if it doesn't expect sync requests during sync handle - // watching, it may still need to dispatch messages to associated endpoints - // on a different sequence. - connector_.AllowWokenUpBySyncWatchOnSameThread(); - } connector_.set_incoming_receiver(&dispatcher_); - connector_.set_connection_error_handler( - base::BindOnce(&MultiplexRouter::OnPipeConnectionError, - base::Unretained(this), false /* force_async_dispatch */)); scoped_refptr<internal::MessageQuotaChecker> quota_checker = internal::MessageQuotaChecker::MaybeCreate(); @@ -400,7 +394,6 @@ void MultiplexRouter::BindToCurrentSequence() { header_validator_ = header_validator.get(); dispatcher_.SetValidator(std::move(header_validator)); - const char* primary_interface_name = connector_.interface_name(); if (primary_interface_name) { header_validator_->SetDescription(base::JoinString( {primary_interface_name, "[primary] MessageHeaderValidator"}, " ")); @@ -409,6 +402,23 @@ void MultiplexRouter::BindToCurrentSequence() { } } +void MultiplexRouter::StartReceiving() { + connector_.set_connection_error_handler( + base::BindOnce(&MultiplexRouter::OnPipeConnectionError, + base::Unretained(this), false /* force_async_dispatch */)); + + // Always participate in sync handle watching in multi-interface mode, + // because even if it doesn't expect sync requests during sync handle + // watching, it may still need to dispatch messages to associated endpoints + // on a different sequence. + const bool allow_woken_up_by_others = + config_ == SINGLE_INTERFACE_WITH_SYNC_METHODS || + config_ == MULTI_INTERFACE; + + DETACH_FROM_SEQUENCE(sequence_checker_); + connector_.StartReceiving(task_runner_, allow_woken_up_by_others); +} + MultiplexRouter::~MultiplexRouter() { MayAutoLock locker(&lock_); @@ -444,8 +454,11 @@ InterfaceId MultiplexRouter::AssociateInterface( id |= kInterfaceIdNamespaceMask; } while (base::Contains(endpoints_, id)); - InterfaceEndpoint* endpoint = new InterfaceEndpoint(this, id); - endpoints_[id] = endpoint; + auto endpoint_ref = base::MakeRefCounted<InterfaceEndpoint>(this, id); + // Raw pointer use is safe because the InterfaceEndpoint will remain alive + // as long as a reference to it exists in the `endpoints_` map. + InterfaceEndpoint* endpoint = endpoint_ref.get(); + endpoints_[id] = std::move(endpoint_ref); if (encountered_error_) UpdateEndpointStateMayRemove(endpoint, PEER_ENDPOINT_CLOSED); endpoint->set_handle_created(); @@ -805,8 +818,17 @@ void MultiplexRouter::OnPipeConnectionError(bool force_async_dispatch) { endpoint_vector.push_back(pair.second); for (const auto& endpoint : endpoint_vector) { - if (endpoint->client()) + if (endpoint->client()) { + base::flat_set<uint64_t> request_ids = + endpoint->UnregisterAllExternalSyncWaiters(); + // NOTE: Accessing the InterfaceEndpointClient from off-thread must be + // safe here, because the client can only be detached from us while + // holding `lock_`. + for (uint64_t request_id : request_ids) + endpoint->client()->ForgetAsyncRequest(request_id); + tasks_.push_back(Task::CreateNotifyErrorTask(endpoint.get())); + } UpdateEndpointStateMayRemove(endpoint.get(), PEER_ENDPOINT_CLOSED); } @@ -1136,8 +1158,11 @@ MultiplexRouter::InterfaceEndpoint* MultiplexRouter::FindOrInsertEndpoint( InterfaceEndpoint* endpoint = FindEndpoint(id); if (!endpoint) { - endpoint = new InterfaceEndpoint(this, id); - endpoints_[id] = endpoint; + auto endpoint_ref = base::MakeRefCounted<InterfaceEndpoint>(this, id); + // Raw pointer use is safe because the InterfaceEndpoint will remain alive + // as long as a reference to it exists in the `endpoints_` map. + endpoint = endpoint_ref.get(); + endpoints_[id] = std::move(endpoint_ref); if (inserted) *inserted = true; } diff --git a/chromium/mojo/public/cpp/bindings/lib/multiplex_router.h b/chromium/mojo/public/cpp/bindings/lib/multiplex_router.h index d61f3ae6d96..3d3bbb16e25 100644 --- a/chromium/mojo/public/cpp/bindings/lib/multiplex_router.h +++ b/chromium/mojo/public/cpp/bindings/lib/multiplex_router.h @@ -76,8 +76,10 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) MultiplexRouter // If `set_interface_id_namespace_bit` is true, the interface IDs generated by // this router will have the highest bit set. // - // NOTE: CloseMessagePipe() or PassMessagePipe() MUST be called on the - // `runner` sequence before this object is destroyed. + // Note that the MultiplexRouter will not initially receive any messages or + // disconnect events until StartReceiving() is explicitly called. To create a + // MultiplexRouter which calls this automatically at construction time, use + // CreateAndStartReceiving(). static scoped_refptr<MultiplexRouter> Create( ScopedMessagePipeHandle message_pipe, Config config, @@ -85,6 +87,23 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) MultiplexRouter scoped_refptr<base::SequencedTaskRunner> runner, const char* primary_interface_name = "unknown interface"); + // Same as above, but automatically calls StartReceiving() before returning. + // If `runner` does not run tasks in sequence with the caller, the returned + // MultiplexRouter may already begin receiving messages and events on `runner` + // before this call returns. + static scoped_refptr<MultiplexRouter> CreateAndStartReceiving( + ScopedMessagePipeHandle message_pipe, + Config config, + bool set_interface_id_namespace_bit, + scoped_refptr<base::SequencedTaskRunner> runner, + const char* primary_interface_name = "unknown interface"); + + // Starts receiving messages on the MultiplexRouter. Once this is called, + // CloseMessagePipe() or PassMessagePipe() MUST be called in sequence with + // the MultiplexRouter's `task_runner_` prior to destroying the + // MultiplexRouter. + void StartReceiving(); + MultiplexRouter(base::PassKey<MultiplexRouter>, ScopedMessagePipeHandle message_pipe, Config config, @@ -188,9 +207,6 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) MultiplexRouter ~MultiplexRouter() override; - // Completes initialization of the MultiplexRouter. - void BindToCurrentSequence(); - // Indicates whether `message` can unblock any active external sync waiter. bool CanUnblockExternalSyncWait(const Message& message); diff --git a/chromium/mojo/public/cpp/bindings/lib/scoped_interface_endpoint_handle.cc b/chromium/mojo/public/cpp/bindings/lib/scoped_interface_endpoint_handle.cc index 7b1712c28bc..874d6d52c6b 100644 --- a/chromium/mojo/public/cpp/bindings/lib/scoped_interface_endpoint_handle.cc +++ b/chromium/mojo/public/cpp/bindings/lib/scoped_interface_endpoint_handle.cc @@ -296,11 +296,11 @@ void ScopedInterfaceEndpointHandle::CreatePairPendingAssociation( } ScopedInterfaceEndpointHandle::ScopedInterfaceEndpointHandle() - : state_(new State) {} + : state_(base::MakeRefCounted<State>()) {} ScopedInterfaceEndpointHandle::ScopedInterfaceEndpointHandle( ScopedInterfaceEndpointHandle&& other) - : state_(new State) { + : state_(base::MakeRefCounted<State>()) { state_.swap(other.state_); } @@ -355,7 +355,7 @@ void ScopedInterfaceEndpointHandle::ResetWithReason( ScopedInterfaceEndpointHandle::ScopedInterfaceEndpointHandle( InterfaceId id, scoped_refptr<AssociatedGroupController> group_controller) - : state_(new State(id, std::move(group_controller))) { + : state_(base::MakeRefCounted<State>(id, std::move(group_controller))) { DCHECK(!IsValidInterfaceId(state_->id()) || state_->group_controller()); } @@ -367,7 +367,7 @@ bool ScopedInterfaceEndpointHandle::NotifyAssociation( void ScopedInterfaceEndpointHandle::ResetInternal( const absl::optional<DisconnectReason>& reason) { - scoped_refptr<State> new_state(new State); + auto new_state = base::MakeRefCounted<State>(); state_->Close(reason); state_.swap(new_state); } diff --git a/chromium/mojo/public/cpp/bindings/lib/sequence_local_sync_event_watcher.cc b/chromium/mojo/public/cpp/bindings/lib/sequence_local_sync_event_watcher.cc index 1d9178799df..03ff19e3a9b 100644 --- a/chromium/mojo/public/cpp/bindings/lib/sequence_local_sync_event_watcher.cc +++ b/chromium/mojo/public/cpp/bindings/lib/sequence_local_sync_event_watcher.cc @@ -14,7 +14,6 @@ #include "base/memory/ptr_util.h" #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" -#include "base/no_destructor.h" #include "base/synchronization/lock.h" #include "base/synchronization/waitable_event.h" #include "base/threading/sequence_local_storage_slot.h" @@ -173,8 +172,8 @@ class SequenceLocalSyncEventWatcher::SequenceLocalState { private: using StorageSlotType = base::SequenceLocalStorageSlot<SequenceLocalState>; static StorageSlotType& GetStorageSlot() { - static base::NoDestructor<StorageSlotType> storage; - return *storage; + static StorageSlotType storage; + return storage; } void OnEventSignaled(); diff --git a/chromium/mojo/public/cpp/bindings/lib/sync_call_restrictions.cc b/chromium/mojo/public/cpp/bindings/lib/sync_call_restrictions.cc index b6cb0a7d129..45103aff08b 100644 --- a/chromium/mojo/public/cpp/bindings/lib/sync_call_restrictions.cc +++ b/chromium/mojo/public/cpp/bindings/lib/sync_call_restrictions.cc @@ -47,8 +47,8 @@ GlobalSyncCallSettings& GetGlobalSettings() { } size_t& GetSequenceLocalScopedAllowCount() { - static base::NoDestructor<base::SequenceLocalStorageSlot<size_t>> count; - return count->GetOrCreateValue(); + static base::SequenceLocalStorageSlot<size_t> count; + return count.GetOrCreateValue(); } // Sometimes sync calls need to be made while sequence-local storage is not diff --git a/chromium/mojo/public/cpp/bindings/lib/sync_event_watcher.cc b/chromium/mojo/public/cpp/bindings/lib/sync_event_watcher.cc index ab69ade2d57..97356dcb4c8 100644 --- a/chromium/mojo/public/cpp/bindings/lib/sync_event_watcher.cc +++ b/chromium/mojo/public/cpp/bindings/lib/sync_event_watcher.cc @@ -5,9 +5,11 @@ #include "mojo/public/cpp/bindings/sync_event_watcher.h" #include <algorithm> +#include <utility> #include "base/check_op.h" #include "base/containers/stack_container.h" +#include "base/memory/scoped_refptr.h" namespace mojo { @@ -16,7 +18,7 @@ SyncEventWatcher::SyncEventWatcher(base::WaitableEvent* event, : event_(event), callback_(std::move(callback)), registry_(SyncHandleRegistry::current()), - destroyed_(new base::RefCountedData<bool>(false)) {} + destroyed_(base::MakeRefCounted<base::RefCountedData<bool>>(false)) {} SyncEventWatcher::~SyncEventWatcher() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); diff --git a/chromium/mojo/public/cpp/bindings/lib/sync_handle_registry.cc b/chromium/mojo/public/cpp/bindings/lib/sync_handle_registry.cc index 32c2d724de8..215bac399e8 100644 --- a/chromium/mojo/public/cpp/bindings/lib/sync_handle_registry.cc +++ b/chromium/mojo/public/cpp/bindings/lib/sync_handle_registry.cc @@ -10,10 +10,12 @@ #include "base/auto_reset.h" #include "base/check_op.h" #include "base/containers/contains.h" +#include "base/containers/cxx20_erase.h" +#include "base/memory/scoped_refptr.h" #include "base/no_destructor.h" -#include "base/stl_util.h" #include "base/threading/sequence_local_storage_slot.h" #include "base/threading/sequenced_task_runner_handle.h" +#include "base/types/pass_key.h" #include "mojo/public/c/system/core.h" namespace mojo { @@ -33,20 +35,26 @@ SyncHandleRegistry::Subscription::~Subscription() = default; // static scoped_refptr<SyncHandleRegistry> SyncHandleRegistry::current() { - static base::NoDestructor< - base::SequenceLocalStorageSlot<scoped_refptr<SyncHandleRegistry>>> + static base::SequenceLocalStorageSlot<scoped_refptr<SyncHandleRegistry>> g_current_sync_handle_watcher; // SyncMessageFilter can be used on threads without sequence-local storage // being available. Those receive a unique, standalone SyncHandleRegistry. - if (!base::SequencedTaskRunnerHandle::IsSet()) - return new SyncHandleRegistry(); + if (!base::SequencedTaskRunnerHandle::IsSet()) { + return base::MakeRefCounted<SyncHandleRegistry>( + base::PassKey<SyncHandleRegistry>()); + } - if (!*g_current_sync_handle_watcher) - g_current_sync_handle_watcher->emplace(new SyncHandleRegistry()); - return *g_current_sync_handle_watcher->GetValuePointer(); + if (!g_current_sync_handle_watcher) { + g_current_sync_handle_watcher.emplace( + base::MakeRefCounted<SyncHandleRegistry>( + base::PassKey<SyncHandleRegistry>())); + } + return *g_current_sync_handle_watcher.GetValuePointer(); } +SyncHandleRegistry::SyncHandleRegistry(base::PassKey<SyncHandleRegistry>) {} + bool SyncHandleRegistry::RegisterHandle(const Handle& handle, MojoHandleSignals handle_signals, HandleCallback callback) { @@ -155,13 +163,9 @@ bool SyncHandleRegistry::Wait(const bool* should_stop[], size_t count) { [](const auto& entry) { return entry.second->empty(); }); } } - }; - - return false; + } } -SyncHandleRegistry::SyncHandleRegistry() = default; - SyncHandleRegistry::~SyncHandleRegistry() = default; } // namespace mojo diff --git a/chromium/mojo/public/cpp/bindings/lib/sync_handle_watcher.cc b/chromium/mojo/public/cpp/bindings/lib/sync_handle_watcher.cc index 53d8db15c07..a475c750ce1 100644 --- a/chromium/mojo/public/cpp/bindings/lib/sync_handle_watcher.cc +++ b/chromium/mojo/public/cpp/bindings/lib/sync_handle_watcher.cc @@ -5,6 +5,7 @@ #include "mojo/public/cpp/bindings/sync_handle_watcher.h" #include "base/check_op.h" +#include "base/memory/scoped_refptr.h" namespace mojo { @@ -18,7 +19,7 @@ SyncHandleWatcher::SyncHandleWatcher( registered_(false), register_request_count_(0), registry_(SyncHandleRegistry::current()), - destroyed_(new base::RefCountedData<bool>(false)) {} + destroyed_(base::MakeRefCounted<base::RefCountedData<bool>>(false)) {} SyncHandleWatcher::~SyncHandleWatcher() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); diff --git a/chromium/mojo/public/cpp/bindings/lib/unserialized_message_context.h b/chromium/mojo/public/cpp/bindings/lib/unserialized_message_context.h index 445446fea34..baac62045a9 100644 --- a/chromium/mojo/public/cpp/bindings/lib/unserialized_message_context.h +++ b/chromium/mojo/public/cpp/bindings/lib/unserialized_message_context.h @@ -12,7 +12,6 @@ #include "mojo/public/c/system/types.h" #include "mojo/public/cpp/bindings/lib/buffer.h" #include "mojo/public/cpp/bindings/lib/message_internal.h" -#include "third_party/abseil-cpp/absl/types/optional.h" namespace mojo { namespace internal { diff --git a/chromium/mojo/public/cpp/bindings/message.h b/chromium/mojo/public/cpp/bindings/message.h index 3f1aa07f0e3..70723f6eccd 100644 --- a/chromium/mojo/public/cpp/bindings/message.h +++ b/chromium/mojo/public/cpp/bindings/message.h @@ -406,6 +406,11 @@ void ReportBadMessage(const std::string& error); COMPONENT_EXPORT(MOJO_CPP_BINDINGS_BASE) ReportBadMessageCallback GetBadMessageCallback(); +// Returns true if called directly within the stack frame of a message dispatch. +// Unlike GetBadMessageCallback(), this can be called multiple times. +COMPONENT_EXPORT(MOJO_CPP_BINDINGS_BASE) +bool IsInMessageDispatch(); + } // namespace mojo #endif // MOJO_PUBLIC_CPP_BINDINGS_MESSAGE_H_ diff --git a/chromium/mojo/public/cpp/bindings/pending_associated_receiver.h b/chromium/mojo/public/cpp/bindings/pending_associated_receiver.h index 487cf5cbabe..fb6837c9bdf 100644 --- a/chromium/mojo/public/cpp/bindings/pending_associated_receiver.h +++ b/chromium/mojo/public/cpp/bindings/pending_associated_receiver.h @@ -107,11 +107,11 @@ class PendingAssociatedReceiver { MessagePipe pipe; scoped_refptr<internal::MultiplexRouter> router0 = - internal::MultiplexRouter::Create( + internal::MultiplexRouter::CreateAndStartReceiving( std::move(pipe.handle0), internal::MultiplexRouter::MULTI_INTERFACE, false, base::SequencedTaskRunnerHandle::Get()); scoped_refptr<internal::MultiplexRouter> router1 = - internal::MultiplexRouter::Create( + internal::MultiplexRouter::CreateAndStartReceiving( std::move(pipe.handle1), internal::MultiplexRouter::MULTI_INTERFACE, true, base::SequencedTaskRunnerHandle::Get()); diff --git a/chromium/mojo/public/cpp/bindings/pending_associated_remote.h b/chromium/mojo/public/cpp/bindings/pending_associated_remote.h index cd4d28c87c9..bb68431e3eb 100644 --- a/chromium/mojo/public/cpp/bindings/pending_associated_remote.h +++ b/chromium/mojo/public/cpp/bindings/pending_associated_remote.h @@ -103,11 +103,11 @@ class PendingAssociatedRemote { MessagePipe pipe; scoped_refptr<internal::MultiplexRouter> router0 = - internal::MultiplexRouter::Create( + internal::MultiplexRouter::CreateAndStartReceiving( std::move(pipe.handle0), internal::MultiplexRouter::MULTI_INTERFACE, false, base::SequencedTaskRunnerHandle::Get()); scoped_refptr<internal::MultiplexRouter> router1 = - internal::MultiplexRouter::Create( + internal::MultiplexRouter::CreateAndStartReceiving( std::move(pipe.handle1), internal::MultiplexRouter::MULTI_INTERFACE, true, base::SequencedTaskRunnerHandle::Get()); diff --git a/chromium/mojo/public/cpp/bindings/pending_receiver.h b/chromium/mojo/public/cpp/bindings/pending_receiver.h index c3c74b69a83..9ca1d24d8b3 100644 --- a/chromium/mojo/public/cpp/bindings/pending_receiver.h +++ b/chromium/mojo/public/cpp/bindings/pending_receiver.h @@ -92,7 +92,7 @@ class PendingReceiver { return request; } - // Indicates whether the PendingReceiver is valid, meaning it can ne used to + // Indicates whether the PendingReceiver is valid, meaning it can be used to // bind a Receiver that wants to begin dispatching method calls made by the // entangled Remote. bool is_valid() const { return state_.pipe.is_valid(); } diff --git a/chromium/mojo/public/cpp/bindings/receiver.h b/chromium/mojo/public/cpp/bindings/receiver.h index cba81fb6dfa..87818aa0e61 100644 --- a/chromium/mojo/public/cpp/bindings/receiver.h +++ b/chromium/mojo/public/cpp/bindings/receiver.h @@ -52,7 +52,7 @@ class Receiver { using ImplPointerType = typename ImplRefTraits::PointerType; // Constructs an unbound Receiver linked to |impl| for the duration of the - // Receive's lifetime. The Receiver can be bound later by calling |Bind()| or + // Receiver's lifetime. The Receiver can be bound later by calling |Bind()| or // |BindNewPipeAndPassRemote()|. An unbound Receiver does not schedule any // asynchronous tasks. explicit Receiver(ImplPointerType impl) : internal_state_(std::move(impl)) {} @@ -139,7 +139,7 @@ class Receiver { return remote; } - // Like above, but the returne PendingRemote has the version annotated. + // Like above, but the returned PendingRemote has the version annotated. PendingRemote<Interface> BindNewPipeAndPassRemoteWithVersion( scoped_refptr<base::SequencedTaskRunner> task_runner = nullptr) WARN_UNUSED_RESULT { @@ -271,7 +271,7 @@ class Receiver { // Allows test code to swap the interface implementation. ImplPointerType SwapImplForTesting(ImplPointerType new_impl) { - return internal_state_.SwapImplForTesting(new_impl); + return internal_state_.SwapImplForTesting(std::move(new_impl)); } // Reports the currently dispatching message as bad and resets this receiver. diff --git a/chromium/mojo/public/cpp/bindings/receiver_set.cc b/chromium/mojo/public/cpp/bindings/receiver_set.cc index bd764bea0b7..38dc3cc2174 100644 --- a/chromium/mojo/public/cpp/bindings/receiver_set.cc +++ b/chromium/mojo/public/cpp/bindings/receiver_set.cc @@ -29,7 +29,9 @@ class ReceiverSetState::Entry::DispatchFilter : public MessageFilter { return true; } - void DidDispatchOrReject(Message* message, bool accepted) override {} + void DidDispatchOrReject(Message* message, bool accepted) override { + entry_.DidDispatchOrReject(); + } Entry& entry_; }; @@ -50,6 +52,10 @@ void ReceiverSetState::Entry::WillDispatch() { state_.SetDispatchContext(receiver_->GetContext(), id_); } +void ReceiverSetState::Entry::DidDispatchOrReject() { + state_.SetDispatchContext(nullptr, 0); +} + void ReceiverSetState::Entry::OnDisconnect(uint32_t custom_reason_code, const std::string& description) { WillDispatch(); diff --git a/chromium/mojo/public/cpp/bindings/receiver_set.h b/chromium/mojo/public/cpp/bindings/receiver_set.h index 3121f58bd6b..16de0e656b7 100644 --- a/chromium/mojo/public/cpp/bindings/receiver_set.h +++ b/chromium/mojo/public/cpp/bindings/receiver_set.h @@ -80,6 +80,7 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) ReceiverSetState { class DispatchFilter; void WillDispatch(); + void DidDispatchOrReject(); void OnDisconnect(uint32_t custom_reason_code, const std::string& description); @@ -321,7 +322,7 @@ class ReceiverSetBase { return nullptr; ReceiverEntry& entry = static_cast<ReceiverEntry&>(it->second->receiver()); - return entry.SwapImplForTesting(new_impl); + return entry.SwapImplForTesting(std::move(new_impl)); } private: @@ -355,7 +356,7 @@ class ReceiverSetBase { void FlushForTesting() override { receiver_.FlushForTesting(); } ImplPointerType SwapImplForTesting(ImplPointerType new_impl) { - return receiver_.SwapImplForTesting(new_impl); + return receiver_.SwapImplForTesting(std::move(new_impl)); } PendingType Unbind() { return receiver_.Unbind(); } diff --git a/chromium/mojo/public/cpp/bindings/remote_set.h b/chromium/mojo/public/cpp/bindings/remote_set.h index eb98f73ecaa..3426704c8fe 100644 --- a/chromium/mojo/public/cpp/bindings/remote_set.h +++ b/chromium/mojo/public/cpp/bindings/remote_set.h @@ -15,7 +15,7 @@ #include "base/macros.h" #include "base/memory/scoped_refptr.h" #include "base/sequenced_task_runner.h" -#include "base/util/type_safety/id_type.h" +#include "base/types/id_type.h" #include "mojo/public/cpp/bindings/associated_remote.h" #include "mojo/public/cpp/bindings/pending_associated_remote.h" #include "mojo/public/cpp/bindings/pending_remote.h" @@ -27,7 +27,7 @@ namespace internal { struct RemoteSetElementIdTypeTag {}; } // namespace internal -using RemoteSetElementId = util::IdTypeU32<internal::RemoteSetElementIdTypeTag>; +using RemoteSetElementId = base::IdTypeU32<internal::RemoteSetElementIdTypeTag>; // Shared implementation of a set of remotes, used by both RemoteSet and // AssociatedRemoteSet aliases (see below). @@ -122,7 +122,7 @@ class RemoteSetImpl { // Returns an `Interface*` for the given ID, that can be used to issue // interface calls. Interface* Get(RemoteSetElementId id) { - auto* it = storage_.find(id); + auto it = storage_.find(id); if (it == storage_.end()) return nullptr; return it->second.get(); @@ -148,7 +148,7 @@ class RemoteSetImpl { void FlushForTesting() { for (auto& it : storage_) { - it.second.FlushForTesting(); + it.second.FlushForTesting(); } } diff --git a/chromium/mojo/public/cpp/bindings/shared_associated_remote.h b/chromium/mojo/public/cpp/bindings/shared_associated_remote.h index 1b592831045..dc1df56c15e 100644 --- a/chromium/mojo/public/cpp/bindings/shared_associated_remote.h +++ b/chromium/mojo/public/cpp/bindings/shared_associated_remote.h @@ -5,6 +5,7 @@ #ifndef MOJO_PUBLIC_CPP_BINDINGS_SHARED_ASSOCIATED_REMOTE_H_ #define MOJO_PUBLIC_CPP_BINDINGS_SHARED_ASSOCIATED_REMOTE_H_ +#include "base/memory/ref_counted.h" #include "base/threading/sequenced_task_runner_handle.h" #include "mojo/public/cpp/bindings/associated_remote.h" #include "mojo/public/cpp/bindings/pending_associated_remote.h" @@ -12,6 +13,17 @@ namespace mojo { +namespace internal { + +template <typename Interface> +struct SharedRemoteTraits<AssociatedRemote<Interface>> { + static void BindDisconnected(AssociatedRemote<Interface>& remote) { + ignore_result(remote.BindNewEndpointAndPassDedicatedReceiver()); + } +}; + +} // namespace internal + // SharedAssociatedRemote wraps a non-thread-safe AssociatedRemote and proxies // messages to it. Unlike normal AssociatedRemote objects, // SharedAssociatedRemote is copyable and usable from any thread, but has some @@ -32,12 +44,10 @@ class SharedAssociatedRemote { explicit SharedAssociatedRemote( PendingAssociatedRemote<Interface> pending_remote, scoped_refptr<base::SequencedTaskRunner> bind_task_runner = - base::SequencedTaskRunnerHandle::Get()) - : remote_(pending_remote.is_valid() - ? SharedRemoteBase<AssociatedRemote<Interface>>::Create( - std::move(pending_remote), - std::move(bind_task_runner)) - : nullptr) {} + base::SequencedTaskRunnerHandle::Get()) { + if (pending_remote.is_valid()) + Bind(std::move(pending_remote), std::move(bind_task_runner)); + } bool is_bound() const { return remote_ != nullptr; } explicit operator bool() const { return is_bound(); } @@ -46,11 +56,47 @@ class SharedAssociatedRemote { Interface* operator->() const { return get(); } Interface& operator*() const { return *get(); } + void set_disconnect_handler( + base::OnceClosure handler, + scoped_refptr<base::SequencedTaskRunner> handler_task_runner) { + remote_->set_disconnect_handler(std::move(handler), + std::move(handler_task_runner)); + } + // Clears this SharedAssociatedRemote. Note that this does *not* necessarily // close the remote's endpoint as other SharedAssociatedRemote instances may // reference the same underlying endpoint. void reset() { remote_.reset(); } + // Disconnects the SharedAssociatedRemote. This leaves the object in a usable + // state -- i.e. it's still safe to dereference and make calls -- but severs + // the underlying connection so that no new replies will be received and all + // outgoing messages will be discarded. This is useful when you want to force + // a disconnection like with reset(), but you don't want the + // SharedAssociatedRemote to become unbound. + void Disconnect() { remote_->Disconnect(); } + + // Creates a new pair of endpoints and binds this SharedAssociatedRemote to + // one of them, on `task_runner`. The other is returned as a receiver. + mojo::PendingAssociatedReceiver<Interface> BindNewEndpointAndPassReceiver( + scoped_refptr<base::SequencedTaskRunner> bind_task_runner = + base::SequencedTaskRunnerHandle::Get()) { + mojo::PendingAssociatedRemote<Interface> remote; + auto receiver = remote.InitWithNewEndpointAndPassReceiver(); + Bind(std::move(remote), std::move(bind_task_runner)); + return receiver; + } + + // Binds to `pending_remote` on `bind_task_runner`. + void Bind(PendingAssociatedRemote<Interface> pending_remote, + scoped_refptr<base::SequencedTaskRunner> bind_task_runner = + base::SequencedTaskRunnerHandle::Get()) { + DCHECK(!remote_); + DCHECK(pending_remote.is_valid()); + remote_ = SharedRemoteBase<AssociatedRemote<Interface>>::Create( + std::move(pending_remote), std::move(bind_task_runner)); + } + private: scoped_refptr<SharedRemoteBase<AssociatedRemote<Interface>>> remote_; }; diff --git a/chromium/mojo/public/cpp/bindings/shared_remote.h b/chromium/mojo/public/cpp/bindings/shared_remote.h index fb1221ddac6..15f5ff397e9 100644 --- a/chromium/mojo/public/cpp/bindings/shared_remote.h +++ b/chromium/mojo/public/cpp/bindings/shared_remote.h @@ -11,7 +11,6 @@ #include "base/macros.h" #include "base/memory/ptr_util.h" #include "base/memory/ref_counted.h" -#include "base/stl_util.h" #include "base/synchronization/waitable_event.h" #include "base/task_runner.h" #include "base/threading/sequenced_task_runner_handle.h" @@ -24,6 +23,20 @@ namespace mojo { +namespace internal { + +template <typename RemoteType> +struct SharedRemoteTraits; + +template <typename Interface> +struct SharedRemoteTraits<Remote<Interface>> { + static void BindDisconnected(Remote<Interface>& remote) { + ignore_result(remote.BindNewPipeAndPassReceiver()); + } +}; + +} // namespace internal + // Helper that may be used from any sequence to serialize |Interface| messages // and forward them elsewhere. In general, prefer `SharedRemote`, but this type // may be useful when it's necessary to manually manage the lifetime of the @@ -72,6 +85,8 @@ class SharedRemoteBase std::move(handler_task_runner)); } + void Disconnect() { wrapper_->Disconnect(); } + private: friend class base::RefCountedThreadSafe<SharedRemoteBase<RemoteType>>; template <typename Interface> @@ -126,6 +141,19 @@ class SharedRemoteBase remote_.set_disconnect_handler(std::move(wrapped_handler)); } + void Disconnect() { + if (!task_runner_->RunsTasksInCurrentSequence()) { + task_runner_->PostTask( + FROM_HERE, base::BindOnce(&RemoteWrapper::Disconnect, this)); + return; + } + + // Reset the remote and rebind it in a disconnected state, so that it's + // usable but discards all messages. + remote_.reset(); + internal::SharedRemoteTraits<RemoteType>::BindDisconnected(remote_); + } + private: friend struct RemoteWrapperDeleter; @@ -253,6 +281,14 @@ class SharedRemote { // underlying endpoint. void reset() { remote_.reset(); } + // Disconnects the SharedRemote. This leaves the object in a usable state -- + // i.e. it's still safe to dereference and make calls -- but severs the + // underlying connection so that no new replies will be received and all + // outgoing messages will be discarded. This is useful when you want to force + // a disconnection like with reset(), but you don't want the SharedRemote to + // become unbound. + void Disconnect() { remote_->Disconnect(); } + // Binds this SharedRemote to `pending_remote` on the sequence given by // `bind_task_runner`, or the calling sequence if `bind_task_runner` is null. // Once bound, the SharedRemote may be used to send messages on the underlying @@ -281,6 +317,16 @@ class SharedRemote { } } + // Creates a new pipe, binding this SharedRemote to one end on + // `bind_task_runner` and returning the other end as a PendingReceiver. + PendingReceiver<Interface> BindNewPipeAndPassReceiver( + scoped_refptr<base::SequencedTaskRunner> bind_task_runner = nullptr) { + PendingRemote<Interface> remote; + auto receiver = remote.InitWithNewPipeAndPassReceiver(); + Bind(std::move(remote), std::move(bind_task_runner)); + return receiver; + } + private: scoped_refptr<SharedRemoteBase<Remote<Interface>>> remote_; }; diff --git a/chromium/mojo/public/cpp/bindings/struct_ptr.h b/chromium/mojo/public/cpp/bindings/struct_ptr.h index 6666fee51a0..5d8875c8933 100644 --- a/chromium/mojo/public/cpp/bindings/struct_ptr.h +++ b/chromium/mojo/public/cpp/bindings/struct_ptr.h @@ -15,7 +15,6 @@ #include "base/template_util.h" #include "mojo/public/cpp/bindings/lib/hash_util.h" #include "mojo/public/cpp/bindings/type_converter.h" -#include "third_party/abseil-cpp/absl/types/optional.h" #include "third_party/perfetto/include/perfetto/tracing/traced_value_forward.h" namespace mojo { diff --git a/chromium/mojo/public/cpp/bindings/struct_traits.h b/chromium/mojo/public/cpp/bindings/struct_traits.h index a0f226adf27..ca6304256ba 100644 --- a/chromium/mojo/public/cpp/bindings/struct_traits.h +++ b/chromium/mojo/public/cpp/bindings/struct_traits.h @@ -98,22 +98,6 @@ namespace mojo { // that case, an incoming null value is considered invalid and causes the // message pipe to be disconnected. // -// 4. [Optional] As mentioned above, getters for string/struct/array/map/union -// fields are called multiple times (twice to be exact). If you need to do -// some expensive calculation/conversion, you probably want to cache the -// result across multiple calls. You can introduce an arbitrary context -// object by adding two optional methods: -// static void* SetUpContext(const T& input); -// static void TearDownContext(const T& input, void* context); -// -// And then you append a second parameter, void* context, to getters: -// static <return type> <field name>(const T& input, void* context); -// -// If a T instance is not null, the serialization code will call -// SetUpContext() at the beginning, and pass the resulting context pointer -// to getters. After serialization is done, it calls TearDownContext() so -// that you can do any necessary cleanup. -// // In the description above, methods having an |input| parameter define it as // const reference of T. Actually, it can be a non-const reference of T too. // E.g., if T contains Mojo handles or interfaces whose ownership needs to be diff --git a/chromium/mojo/public/cpp/bindings/sync_call_restrictions.h b/chromium/mojo/public/cpp/bindings/sync_call_restrictions.h index fd962645ec6..fc4142741f7 100644 --- a/chromium/mojo/public/cpp/bindings/sync_call_restrictions.h +++ b/chromium/mojo/public/cpp/bindings/sync_call_restrictions.h @@ -8,6 +8,7 @@ #include "base/component_export.h" #include "base/macros.h" #include "base/threading/thread_restrictions.h" +#include "build/build_config.h" #if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)) #define ENABLE_SYNC_CALL_RESTRICTIONS 1 @@ -23,6 +24,9 @@ namespace content { class AndroidOverlaySyncHelper; class DesktopCapturerLacros; class StreamTextureFactory; +#if defined(OS_WIN) +class DCOMPTextureFactory; +#endif } // namespace content namespace crosapi { @@ -42,7 +46,7 @@ namespace viz { class GpuHostImpl; class HostFrameSinkManager; class HostGpuMemoryBufferManager; -} +} // namespace viz namespace mojo { class ScopedAllowSyncCallForTesting; @@ -109,6 +113,9 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) SyncCallRestrictions { friend class gpu::GpuChannelHost; friend class gpu::CommandBufferProxyImpl; friend class content::StreamTextureFactory; +#if defined(OS_WIN) + friend class content::DCOMPTextureFactory; +#endif // END ALLOWED USAGE. #if ENABLE_SYNC_CALL_RESTRICTIONS diff --git a/chromium/mojo/public/cpp/bindings/sync_handle_registry.h b/chromium/mojo/public/cpp/bindings/sync_handle_registry.h index 69ab2d94464..6069cd91096 100644 --- a/chromium/mojo/public/cpp/bindings/sync_handle_registry.h +++ b/chromium/mojo/public/cpp/bindings/sync_handle_registry.h @@ -16,6 +16,7 @@ #include "base/memory/ref_counted.h" #include "base/sequence_checker.h" #include "base/synchronization/waitable_event.h" +#include "base/types/pass_key.h" #include "mojo/public/cpp/system/core.h" #include "mojo/public/cpp/system/wait_set.h" @@ -53,6 +54,12 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) SyncHandleRegistry // Returns a sequence-local object. static scoped_refptr<SyncHandleRegistry> current(); + // Exposed for base::MakeRefCounted. + explicit SyncHandleRegistry(base::PassKey<SyncHandleRegistry>); + + SyncHandleRegistry(const SyncHandleRegistry&) = delete; + SyncHandleRegistry& operator=(const SyncHandleRegistry&) = delete; + // Registers a |Handle| to be watched for |handle_signals|. If any such // signals are satisfied during a Wait(), the Wait() is woken up and // |callback| is run. @@ -79,7 +86,6 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) SyncHandleRegistry private: friend class base::RefCounted<SyncHandleRegistry>; - SyncHandleRegistry(); ~SyncHandleRegistry(); WaitSet wait_set_; @@ -92,8 +98,6 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) SyncHandleRegistry bool in_nested_wait_ = false; SEQUENCE_CHECKER(sequence_checker_); - - DISALLOW_COPY_AND_ASSIGN(SyncHandleRegistry); }; } // namespace mojo diff --git a/chromium/mojo/public/cpp/bindings/union_traits.h b/chromium/mojo/public/cpp/bindings/union_traits.h index 243addd7ed9..97aa14ba61a 100644 --- a/chromium/mojo/public/cpp/bindings/union_traits.h +++ b/chromium/mojo/public/cpp/bindings/union_traits.h @@ -21,11 +21,10 @@ namespace mojo { // 1. Getters for each field in the Mojom type. // 2. Read() method. // 3. [Optional] IsNull() and SetToNull(). -// 4. [Optional] SetUpContext() and TearDownContext(). // Please see the documentation of StructTraits for details of these methods. // // Unlike StructTraits, there is one more method to implement: -// 5. A static GetTag() method indicating which field is the current active +// 4. A static GetTag() method indicating which field is the current active // field for serialization: // // static DataViewType::Tag GetTag(const T& input); diff --git a/chromium/mojo/public/cpp/system/file_data_source.h b/chromium/mojo/public/cpp/system/file_data_source.h index ee612a5f7cb..02a7bf3fda5 100644 --- a/chromium/mojo/public/cpp/system/file_data_source.h +++ b/chromium/mojo/public/cpp/system/file_data_source.h @@ -10,7 +10,6 @@ #include "base/macros.h" #include "mojo/public/cpp/system/data_pipe_producer.h" #include "mojo/public/cpp/system/system_export.h" -#include "third_party/abseil-cpp/absl/types/optional.h" namespace mojo { diff --git a/chromium/mojo/public/cpp/system/invitation.h b/chromium/mojo/public/cpp/system/invitation.h index 6fb413d7b23..5191a097a44 100644 --- a/chromium/mojo/public/cpp/system/invitation.h +++ b/chromium/mojo/public/cpp/system/invitation.h @@ -163,6 +163,8 @@ class MOJO_CPP_SYSTEM_EXPORT IncomingInvitation { IncomingInvitation& operator=(IncomingInvitation&& other); + bool is_valid() const { return handle_.is_valid(); } + // Accepts an incoming invitation from |channel_endpoint|. If the invitation // was sent using one end of a |PlatformChannel|, |channel_endpoint| should be // the other end of that channel. If the invitation was sent using a diff --git a/chromium/mojo/public/java/system/core_impl.cc b/chromium/mojo/public/java/system/core_impl.cc index 7958387555f..0e00fbb0b48 100644 --- a/chromium/mojo/public/java/system/core_impl.cc +++ b/chromium/mojo/public/java/system/core_impl.cc @@ -214,6 +214,7 @@ static ScopedJavaLocalRef<jobject> JNI_CoreImpl_BeginReadData( if (result == MOJO_RESULT_OK) { ScopedJavaLocalRef<jobject> byte_buffer( env, env->NewDirectByteBuffer(const_cast<void*>(buffer), buffer_size)); + base::android::CheckException(env); return Java_CoreImpl_newResultAndBuffer(env, result, byte_buffer); } else { return Java_CoreImpl_newResultAndBuffer(env, result, nullptr); @@ -264,6 +265,7 @@ static ScopedJavaLocalRef<jobject> JNI_CoreImpl_BeginWriteData( if (result == MOJO_RESULT_OK) { ScopedJavaLocalRef<jobject> byte_buffer( env, env->NewDirectByteBuffer(buffer, buffer_size)); + base::android::CheckException(env); return Java_CoreImpl_newResultAndBuffer(env, result, byte_buffer); } else { return Java_CoreImpl_newResultAndBuffer(env, result, nullptr); @@ -313,6 +315,7 @@ static ScopedJavaLocalRef<jobject> JNI_CoreImpl_Map( if (result == MOJO_RESULT_OK) { ScopedJavaLocalRef<jobject> byte_buffer( env, env->NewDirectByteBuffer(buffer, num_bytes)); + base::android::CheckException(env); return Java_CoreImpl_newResultAndBuffer(env, result, byte_buffer); } else { return Java_CoreImpl_newResultAndBuffer(env, result, nullptr); diff --git a/chromium/mojo/public/java/system/javatests/validation_test_util.cc b/chromium/mojo/public/java/system/javatests/validation_test_util.cc index 7c3c0a64edd..4d739e62691 100644 --- a/chromium/mojo/public/java/system/javatests/validation_test_util.cc +++ b/chromium/mojo/public/java/system/javatests/validation_test_util.cc @@ -39,6 +39,7 @@ ScopedJavaLocalRef<jobject> JNI_ValidationTestUtil_ParseData( } ScopedJavaLocalRef<jobject> byte_buffer( env, env->NewDirectByteBuffer(data_ptr, data.size())); + base::android::CheckException(env); return Java_ValidationTestUtil_buildData(env, byte_buffer, num_handles, nullptr); } diff --git a/chromium/mojo/public/js/lib/codec.js b/chromium/mojo/public/js/lib/codec.js index 2925eb7a80e..d83af267d1b 100644 --- a/chromium/mojo/public/js/lib/codec.js +++ b/chromium/mojo/public/js/lib/codec.js @@ -876,7 +876,7 @@ Enum.prototype.decode = function(decoder) { let value = decoder.readInt32(); - return cls.toKnownEnumValue(value); + return this.cls.toKnownEnumValue(value); }; Enum.prototype.encode = function(encoder, val) { diff --git a/chromium/mojo/public/js/mojo_bindings_resources.grd b/chromium/mojo/public/js/mojo_bindings_resources.grd index 1f8e8bb6932..81073ddad62 100644 --- a/chromium/mojo/public/js/mojo_bindings_resources.grd +++ b/chromium/mojo/public/js/mojo_bindings_resources.grd @@ -90,6 +90,16 @@ use_base_dir="false" resource_path="mojo/mojo/public/mojom/base/text_direction.mojom-lite.js" type="BINDATA" /> + <include name="IDR_MOJO_TEXT_DIRECTION_MOJOM_WEBUI_JS" + file="${root_gen_dir}/mojom-webui/mojo/public/mojom/base/text_direction.mojom-webui.js" + use_base_dir="false" + resource_path="mojo/mojo/public/mojom/base/text_direction.mojom-webui.js" + type="BINDATA" /> + <include name="IDR_MOJO_TOKEN_MOJOM_WEBUI_JS" + file="${root_gen_dir}/mojom-webui/mojo/public/mojom/base/token.mojom-webui.js" + use_base_dir="false" + resource_path="mojo/mojo/public/mojom/base/token.mojom-webui.js" + type="BINDATA" /> <include name="IDR_MOJO_UNGUESSABLE_TOKEN_MOJOM_WEBUI_JS" file="${root_gen_dir}/mojom-webui/mojo/public/mojom/base/unguessable_token.mojom-webui.js" use_base_dir="false" diff --git a/chromium/mojo/public/mojom/base/BUILD.gn b/chromium/mojo/public/mojom/base/BUILD.gn index bc65d6e0007..a110e1ab4e7 100644 --- a/chromium/mojo/public/mojom/base/BUILD.gn +++ b/chromium/mojo/public/mojom/base/BUILD.gn @@ -16,6 +16,7 @@ mojom_component("base") { "file_error.mojom", "file_info.mojom", "file_path.mojom", + "generic_pending_associated_receiver.mojom", "generic_pending_receiver.mojom", "memory_allocator_dump_cross_process_uid.mojom", "memory_pressure_level.mojom", @@ -153,6 +154,21 @@ mojom_component("base") { { types = [ { + mojom = "mojo_base.mojom.GenericPendingAssociatedReceiver" + cpp = "::mojo::GenericPendingAssociatedReceiver" + move_only = true + nullable_is_same_type = true + }, + ] + traits_headers = [ "//mojo/public/cpp/base/generic_pending_associated_receiver_mojom_traits.h" ] + traits_public_deps = [ + "//mojo/public/cpp/base:shared_typemap_traits", + "//mojo/public/cpp/bindings", + ] + }, + { + types = [ + { mojom = "mojo_base.mojom.GenericPendingReceiver" cpp = "::mojo::GenericPendingReceiver" move_only = true diff --git a/chromium/mojo/public/mojom/base/generic_pending_associated_receiver.mojom b/chromium/mojo/public/mojom/base/generic_pending_associated_receiver.mojom new file mode 100644 index 00000000000..df29965e0ab --- /dev/null +++ b/chromium/mojo/public/mojom/base/generic_pending_associated_receiver.mojom @@ -0,0 +1,25 @@ +// Copyright 2021 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. + +module mojo_base.mojom; + +// Convenience helper to wrap the pairing of a receiving associated interface +// endpoint and the name of the interface expected by the remote endpoint. +// +// This should be used sparingly, in cases where APIs need to dynamically pass +// different types of asspcoated receivers that cannot or should not be known at +// compile time. +struct GenericPendingAssociatedReceiver { + // The name of the interface which defines the messages to be received by + // `receiver`. + string interface_name; + + // A generic associated interface receiver which is actually expected to + // receive messages defined by the interface named by `interface_name` above. + pending_associated_receiver<GenericAssociatedInterface> receiver; +}; + +// A generic placeholder interface for the associated endpoint to be passed by a +// GenericPendingAssociatedReceiver. +interface GenericAssociatedInterface {}; diff --git a/chromium/mojo/public/mojom/base/shared_memory.mojom b/chromium/mojo/public/mojom/base/shared_memory.mojom index 45226ca367f..666e59af1d7 100644 --- a/chromium/mojo/public/mojom/base/shared_memory.mojom +++ b/chromium/mojo/public/mojom/base/shared_memory.mojom @@ -6,6 +6,7 @@ module mojo_base.mojom; // Wraps a shared memory handle with additional type information to convey that // the handle is only mappable to read-only memory. +[Stable] struct ReadOnlySharedMemoryRegion { handle<shared_buffer> buffer; }; diff --git a/chromium/mojo/public/tools/bindings/BUILD.gn b/chromium/mojo/public/tools/bindings/BUILD.gn index 3e242532749..b267e510697 100644 --- a/chromium/mojo/public/tools/bindings/BUILD.gn +++ b/chromium/mojo/public/tools/bindings/BUILD.gn @@ -2,12 +2,10 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -import("//build/config/python.gni") import("//mojo/public/tools/bindings/mojom.gni") import("//third_party/jinja2/jinja2.gni") -# TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. -python2_action("precompile_templates") { +action("precompile_templates") { sources = mojom_generator_sources sources += [ "$mojom_generator_root/generators/cpp_templates/enum_macros.tmpl", @@ -65,9 +63,6 @@ python2_action("precompile_templates") { "$mojom_generator_root/generators/java_templates/struct.java.tmpl", "$mojom_generator_root/generators/java_templates/union.java.tmpl", "$mojom_generator_root/generators/js_templates/enum_definition.tmpl", - "$mojom_generator_root/generators/js_templates/externs/interface_definition.tmpl", - "$mojom_generator_root/generators/js_templates/externs/module.externs.tmpl", - "$mojom_generator_root/generators/js_templates/externs/struct_definition.tmpl", "$mojom_generator_root/generators/js_templates/fuzzing.tmpl", "$mojom_generator_root/generators/js_templates/interface_definition.tmpl", "$mojom_generator_root/generators/js_templates/lite/enum_definition.tmpl", diff --git a/chromium/mojo/public/tools/bindings/README.md b/chromium/mojo/public/tools/bindings/README.md index 43882450228..da427e40e79 100644 --- a/chromium/mojo/public/tools/bindings/README.md +++ b/chromium/mojo/public/tools/bindings/README.md @@ -448,7 +448,22 @@ interesting attributes supported today. matching `value` in the list of `enabled_features`, the definition will be disabled. This is useful for mojom definitions that only make sense on one platform. Note that the `EnableIf` attribute can only be set once per - definition. + definition and cannot be set at the same time as `EnableIfNot`. + +* **`[EnableIfNot=value]`**: + The `EnableIfNot` attribute is used to conditionally enable definitions when + the mojom is parsed. If the `mojom` target in the GN file includes the + matching `value` in the list of `enabled_features`, the definition will be + disabled. This is useful for mojom definitions that only make sense on all but + one platform. Note that the `EnableIfNot` attribute can only be set once per + definition and cannot be set at the same time as `EnableIf`. + +* **`[ServiceSandbox=value]`**: + The `ServiceSandbox` attribute is used in Chromium to tag which sandbox a + service hosting an implementation of interface will be launched in. This only + applies to `C++` bindings. `value` should match a constant defined in an + imported `sandbox.mojom.Sandbox` enum (for Chromium this is + `//sandbox/policy/mojom/sandbox.mojom`), such as `kService`. ## Generated Code For Target Languages @@ -550,10 +565,16 @@ See the documentation for *** note **NOTE:** You don't need to worry about versioning if you don't care about -backwards compatibility. Specifically, all parts of Chrome are updated -atomically today and there is not yet any possibility of any two Chrome -processes communicating with two different versions of any given Mojom -interface. +backwards compatibility. Today, all parts of the Chrome browser are +updated atomically and there is not yet any possibility of any two +Chrome processes communicating with two different versions of any given Mojom +interface. On Chrome OS, there are several places where versioning is required. +For example, +[ARC++](https://developer.android.com/chrome-os/intro) +uses versioned mojo to send IPC to the Android container. +Likewise, the +[Lacros](/docs/lacros.md) +browser uses versioned mojo to talk to the ash system UI. *** Services extend their interfaces to support new features over time, and clients @@ -593,8 +614,8 @@ struct Employee { *** note **NOTE:** Mojo object or handle types added with a `MinVersion` **MUST** be -optional (nullable). See [Primitive Types](#Primitive-Types) for details on -nullable values. +optional (nullable) or primitive. See [Primitive Types](#Primitive-Types) for +details on nullable values. *** By default, fields belong to version 0. New fields must be appended to the diff --git a/chromium/mojo/public/tools/bindings/generate_type_mappings.py b/chromium/mojo/public/tools/bindings/generate_type_mappings.py index a009664945f..5eaad1e2cfa 100755 --- a/chromium/mojo/public/tools/bindings/generate_type_mappings.py +++ b/chromium/mojo/public/tools/bindings/generate_type_mappings.py @@ -82,6 +82,7 @@ def LoadCppTypemapConfig(path): for entry in config['types']: configs[entry['mojom']] = { 'typename': entry['cpp'], + 'forward_declaration': entry.get('forward_declaration', None), 'public_headers': config.get('traits_headers', []), 'traits_headers': config.get('traits_private_headers', []), 'copyable_pass_by_value': entry.get('copyable_pass_by_value', diff --git a/chromium/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl b/chromium/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl index 94c766c9580..c4dd04adb43 100644 --- a/chromium/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl +++ b/chromium/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl @@ -20,6 +20,10 @@ class {{export_attribute}} {{interface.name}} static constexpr base::Token Uuid_{ {{interface.uuid[0]}}ULL, {{interface.uuid[1]}}ULL }; {%- endif %} +{%- if interface.service_sandbox %} +{%- set sandbox_enum = "%s"|format(interface.service_sandbox|replace(".","::")) %} + static constexpr auto kServiceSandbox = {{ sandbox_enum }}; +{%- endif %} static constexpr uint32_t Version_ = {{interface.version}}; static constexpr bool PassesAssociatedKinds_ = {% if interface|passes_associated_kinds %}true{% else %}false{% endif %}; static constexpr bool HasSyncMethods_ = {% if interface|has_sync_methods %}true{% else %}false{% endif %}; diff --git a/chromium/mojo/public/tools/bindings/generators/cpp_templates/module-forward.h.tmpl b/chromium/mojo/public/tools/bindings/generators/cpp_templates/module-forward.h.tmpl index 807845a7084..06647c627f5 100644 --- a/chromium/mojo/public/tools/bindings/generators/cpp_templates/module-forward.h.tmpl +++ b/chromium/mojo/public/tools/bindings/generators/cpp_templates/module-forward.h.tmpl @@ -140,10 +140,12 @@ using {{interface.name}}InterfaceBase = {{interface.name}}InterfaceBase; {%- endfor %} {%- endif %} -{#--- Constants #} +{#--- Constants that do not rely on other headers (basic types) #} {%- for constant in module.constants %} +{%- if not constant.kind|is_enum_kind %} {{ kythe_annotation("%s.%s"|format(module_prefix, constant.name)) }} {{constant|format_constant_declaration}}; +{%- endif %} {%- endfor %} {#--- Struct Forward Declarations -#} diff --git a/chromium/mojo/public/tools/bindings/generators/cpp_templates/module-shared.h.tmpl b/chromium/mojo/public/tools/bindings/generators/cpp_templates/module-shared.h.tmpl index 62e0aecd7f6..8f9fad74bdc 100644 --- a/chromium/mojo/public/tools/bindings/generators/cpp_templates/module-shared.h.tmpl +++ b/chromium/mojo/public/tools/bindings/generators/cpp_templates/module-shared.h.tmpl @@ -44,7 +44,6 @@ namespace {{namespace}} { {%- if not contains_only_enums %} #include "base/compiler_specific.h" -#include "base/containers/flat_map.h" #include "mojo/public/cpp/bindings/array_data_view.h" #include "mojo/public/cpp/bindings/enum_traits.h" #include "mojo/public/cpp/bindings/interface_data_view.h" diff --git a/chromium/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl b/chromium/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl index 58bbb17d1a0..b16066e963b 100644 --- a/chromium/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl +++ b/chromium/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl @@ -120,6 +120,10 @@ namespace {{variant}} { #endif {%- endif %} +{%- for forward_declaration in typemap_forward_declarations %} +{{forward_declaration}} +{%- endfor %} + {#--- WTF enum hashing #} {%- from "enum_macros.tmpl" import enum_hash_blink%} {%- if for_blink %} @@ -130,6 +134,14 @@ namespace {{variant}} { {%- endfor %} {%- endif %} +{#--- Constants that need headers (enum types) (basic types go in forward.h) #} +{%- for constant in module.constants %} +{%- if constant.kind|is_enum_kind %} +{{ kythe_annotation("%s.%s"|format(module_prefix, constant.name)) }} +{{constant|format_enum_constant_declaration}}; +{%- endif %} +{%- endfor %} + {{namespace_begin()}} {%- set module_prefix = "%s"|format(namespaces_as_array|join(".")) %} diff --git a/chromium/mojo/public/tools/bindings/generators/js_templates/externs/interface_definition.tmpl b/chromium/mojo/public/tools/bindings/generators/js_templates/externs/interface_definition.tmpl deleted file mode 100644 index cbdac796087..00000000000 --- a/chromium/mojo/public/tools/bindings/generators/js_templates/externs/interface_definition.tmpl +++ /dev/null @@ -1,40 +0,0 @@ -{# Note that goog.provide is understood by the Closure Compiler even if the - Closure base library is unavailable. See https://crbug.com/898692 #} -goog.provide('{{module.namespace}}.{{interface.name}}'); -goog.provide('{{module.namespace}}.{{interface.name}}Impl'); -goog.provide('{{module.namespace}}.{{interface.name}}Ptr'); - -{% macro generateInterfaceClass() -%} -class { -{%- for method in interface.methods %} - /** -{%- for parameter in method.parameters %} - * @param { {{parameter.kind|closure_type_with_nullability}} } {{parameter.name|sanitize_identifier}} -{%- endfor -%} -{%- if method.response_parameters != None %} - * @return {Promise} -{%- endif %} - */ - {{method.name}}( -{%- for parameter in method.parameters -%} -{{parameter.name|sanitize_identifier}}{% if not loop.last %}, {% endif %} -{%- endfor -%} -) {} -{%- endfor %} -}; -{%- endmacro %} - -/** - * @const - * @type { !mojo.Interface }; - */ -{{module.namespace}}.{{interface.name}}; - -/** @interface */ -{{module.namespace}}.{{interface.name}}Impl = {{ generateInterfaceClass() }} - -/** - * @implements { mojo.InterfacePtr } - * @implements { {{module.namespace}}.{{interface.name}}Impl } - */ -{{module.namespace}}.{{interface.name}}Ptr = {{ generateInterfaceClass() }} diff --git a/chromium/mojo/public/tools/bindings/generators/js_templates/externs/module.externs.tmpl b/chromium/mojo/public/tools/bindings/generators/js_templates/externs/module.externs.tmpl deleted file mode 100644 index 938252f78f2..00000000000 --- a/chromium/mojo/public/tools/bindings/generators/js_templates/externs/module.externs.tmpl +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2018 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. - -{# Note that goog.provide is understood by the Closure Compiler even if the - Closure base library is unavailable. See https://crbug.com/898692 #} -{#--- Constant definitions #} -{%- for constant in module.constants %} -/** @type { {{constant.kind|closure_type_with_nullability }} } */ -goog.provide('{{module.namespace}}.{{constant.name}}'); -{%- endfor %} - -{#--- Enum definitions #} -{% for enum in enums %} -/** @enum {number} */ -{{module.namespace}}.{{enum.name}} = {}; -{%- for field in enum.fields %} -{{module.namespace}}.{{enum.name}}.{{field.name}}; -{%- endfor %} -{%- endfor %} - -{#--- Interface definitions #} -{%- for interface in interfaces -%} -{%- include "externs/interface_definition.tmpl" %} -{% endfor -%} - -{#--- Struct definitions #} -{%- for struct in structs -%} -{%- include "externs/struct_definition.tmpl" %} -{% endfor -%} diff --git a/chromium/mojo/public/tools/bindings/generators/js_templates/externs/struct_definition.tmpl b/chromium/mojo/public/tools/bindings/generators/js_templates/externs/struct_definition.tmpl deleted file mode 100644 index 9aeeb65e436..00000000000 --- a/chromium/mojo/public/tools/bindings/generators/js_templates/externs/struct_definition.tmpl +++ /dev/null @@ -1,12 +0,0 @@ -{# Note that goog.provide is understood by the Closure Compiler even if the - Closure base library is unavailable. See https://crbug.com/898692 #} -goog.provide('{{module.namespace}}.{{struct.name}}'); - -{{module.namespace}}.{{struct.name}} = class { - constructor() { -{%- for packed_field in struct.packed.packed_fields %} - /** @type { {{packed_field.field.kind|closure_type_with_nullability}} } */ - this.{{packed_field.field.name}}; -{%- endfor %} - } -}; diff --git a/chromium/mojo/public/tools/bindings/generators/js_templates/lite/interface_definition_for_module.tmpl b/chromium/mojo/public/tools/bindings/generators/js_templates/lite/interface_definition_for_module.tmpl index 5b6dfe19e42..e2236b2e6d3 100644 --- a/chromium/mojo/public/tools/bindings/generators/js_templates/lite/interface_definition_for_module.tmpl +++ b/chromium/mojo/public/tools/bindings/generators/js_templates/lite/interface_definition_for_module.tmpl @@ -20,7 +20,7 @@ /** * @implements {mojo.internal.interfaceSupport.PendingReceiver} */ -export const {{interface.name}}PendingReceiver = class { +export class {{interface.name}}PendingReceiver { /** * @param {!MojoHandle|!mojo.internal.interfaceSupport.Endpoint} handle */ @@ -34,10 +34,10 @@ export const {{interface.name}}PendingReceiver = class { mojo.internal.interfaceSupport.bind( this.handle, '{{mojom_namespace}}.{{interface.name}}', scope); } -}; +} /** @interface */ -export const {{interface.name}}Interface = class { +export class {{interface.name}}Interface { {%- for method in interface.methods %} {{generateMethodAnnotation(method)}} {{method.name}}( @@ -46,12 +46,12 @@ export const {{interface.name}}Interface = class { {%- endfor -%} ) {} {%- endfor %} -}; +} /** * @implements { {{interface.name}}Interface } */ -export const {{interface.name}}Remote = class { +export class {{interface.name}}Remote { /** @param {MojoHandle|mojo.internal.interfaceSupport.Endpoint=} handle */ constructor(handle = undefined) { /** @@ -99,14 +99,14 @@ export const {{interface.name}}Remote = class { ]); } {%- endfor %} -}; +} /** * An object which receives request messages for the {{interface.name}} * mojom interface. Must be constructed over an object which implements that * interface. */ -export const {{interface.name}}Receiver = class { +export class {{interface.name}}Receiver { /** * @param {!{{interface.name}}Interface } impl */ @@ -136,9 +136,9 @@ export const {{interface.name}}Receiver = class { /** @public {!mojo.internal.interfaceSupport.ConnectionErrorEventRouter} */ this.onConnectionError = this.helper_internal_.getConnectionErrorEventRouter(); } -}; +} -export const {{interface.name}} = class { +export class {{interface.name}} { /** * @return {!string} */ @@ -158,7 +158,7 @@ export const {{interface.name}} = class { remote.$.bindNewPipeAndPassReceiver().bindInBrowser(); return remote; } -}; +} {#--- Enums #} {% from "lite/enum_definition_for_module.tmpl" import enum_def with context %} @@ -172,7 +172,7 @@ export const {{interface.name}} = class { * on this object for each message defined in the mojom interface, and each * receiver can have any number of listeners added to it. */ -export const {{interface.name}}CallbackRouter = class { +export class {{interface.name}}CallbackRouter { constructor() { this.helper_internal_ = new mojo.internal.interfaceSupport.InterfaceReceiverHelperInternal( {{interface.name}}Remote); @@ -215,4 +215,4 @@ export const {{interface.name}}CallbackRouter = class { removeListener(id) { return this.router_.removeListener(id); } -}; +} diff --git a/chromium/mojo/public/tools/bindings/generators/js_templates/lite/mojom-lite.js.tmpl b/chromium/mojo/public/tools/bindings/generators/js_templates/lite/mojom-lite.js.tmpl index e057457d615..64d28589f49 100644 --- a/chromium/mojo/public/tools/bindings/generators/js_templates/lite/mojom-lite.js.tmpl +++ b/chromium/mojo/public/tools/bindings/generators/js_templates/lite/mojom-lite.js.tmpl @@ -12,7 +12,7 @@ goog.require('mojo.internal'); {%- if interfaces %} goog.require('mojo.internal.interfaceSupport'); {%- endif %} -{% for kind in module.imported_kinds.values() %} +{% for kind in module.imported_kinds.values()|sort %} goog.require('{{kind|lite_js_import_name}}'); {%- endfor %} {% elif not for_bindings_internals %} diff --git a/chromium/mojo/public/tools/bindings/generators/js_templates/lite/mojom.m.js.tmpl b/chromium/mojo/public/tools/bindings/generators/js_templates/lite/mojom.m.js.tmpl index 659d6480c9e..ffdcc5d4dec 100644 --- a/chromium/mojo/public/tools/bindings/generators/js_templates/lite/mojom.m.js.tmpl +++ b/chromium/mojo/public/tools/bindings/generators/js_templates/lite/mojom.m.js.tmpl @@ -6,9 +6,9 @@ import {mojo} from '{{bindings_library_path}}'; {%- endif %} -{% for path, kinds in js_module_imports.items() -%} +{% for path, kinds in js_module_imports.items()|sort -%} import { -{%- for kind in kinds %} +{%- for kind in kinds|sort %} {%- for item in kind|imports_for_kind %} {{item.name}} as {{item.alias}} {%- if not loop.last -%},{% endif -%} diff --git a/chromium/mojo/public/tools/bindings/generators/js_templates/lite/struct_definition_for_module.tmpl b/chromium/mojo/public/tools/bindings/generators/js_templates/lite/struct_definition_for_module.tmpl index 05fb3278386..f9d52414867 100644 --- a/chromium/mojo/public/tools/bindings/generators/js_templates/lite/struct_definition_for_module.tmpl +++ b/chromium/mojo/public/tools/bindings/generators/js_templates/lite/struct_definition_for_module.tmpl @@ -44,11 +44,11 @@ export const {{struct.name}}_Deserialize = /** * @record */ -export const {{struct.name}} = class { +export class {{struct.name}} { constructor() { {%- for packed_field in struct.packed.packed_fields %} /** @type { {{packed_field.field.kind|field_type_in_js_module}} } */ this.{{packed_field.field.name}}; {%- endfor %} } -}; +} diff --git a/chromium/mojo/public/tools/bindings/generators/mojolpm_templates/mojolpm_to_proto_macros.tmpl b/chromium/mojo/public/tools/bindings/generators/mojolpm_templates/mojolpm_to_proto_macros.tmpl index 38c68ea2dbb..f50b9efff0c 100644 --- a/chromium/mojo/public/tools/bindings/generators/mojolpm_templates/mojolpm_to_proto_macros.tmpl +++ b/chromium/mojo/public/tools/bindings/generators/mojolpm_templates/mojolpm_to_proto_macros.tmpl @@ -83,7 +83,7 @@ bool ToProto( {{type}}& output) { bool result = true; - for ({{maybe_const}}auto& in_value : input) { + for (auto&& in_value : input) { {%- if kind.kind|is_nullable_kind %} {{type}}Entry* out_value = output.mutable_values()->Add(); if ({{util.not_null(kind.kind, 'in_value')}}) { diff --git a/chromium/mojo/public/tools/bindings/generators/mojom_cpp_generator.py b/chromium/mojo/public/tools/bindings/generators/mojom_cpp_generator.py index 74bafc943f8..95c8f81bed8 100644 --- a/chromium/mojo/public/tools/bindings/generators/mojom_cpp_generator.py +++ b/chromium/mojo/public/tools/bindings/generators/mojom_cpp_generator.py @@ -331,6 +331,12 @@ class Generator(generator.Generator): for interface in self.module.interfaces: all_enums.extend(interface.enums) + typemap_forward_declarations = [] + for kind in self.module.imported_kinds.values(): + forward_declaration = self._GetTypemappedForwardDeclaration(kind) + if forward_declaration: + typemap_forward_declarations.append(forward_declaration) + return { "all_enums": all_enums, "contains_only_enums": self._ContainsOnlyEnums(), @@ -344,6 +350,7 @@ class Generator(generator.Generator): "extra_traits_headers": self._GetExtraTraitsHeaders(), "for_blink": self.for_blink, "imports": self.module.imports, + "typemap_forward_declarations": typemap_forward_declarations, "interfaces": self.module.interfaces, "kinds": self.module.kinds, "module": self.module, @@ -382,6 +389,7 @@ class Generator(generator.Generator): "default_value": self._DefaultValue, "expression_to_text": self._ExpressionToText, "format_constant_declaration": self._FormatConstantDeclaration, + "format_enum_constant_declaration": self._FormatEnumConstantDeclaration, "get_container_validate_params_ctor_args": self._GetContainerValidateParamsCtorArgs, "get_full_mojom_name_for_kind": self._GetFullMojomNameForKind, @@ -566,6 +574,12 @@ class Generator(generator.Generator): return hasattr(kind, "name") and \ self._GetFullMojomNameForKind(kind) in self.typemap + def _GetTypemappedForwardDeclaration(self, kind): + if not self._IsTypemappedKind(kind): + return None + return self.typemap[self._GetFullMojomNameForKind( + kind)]["forward_declaration"] + def _IsHashableKind(self, kind): """Check if the kind can be hashed. @@ -612,6 +626,7 @@ class Generator(generator.Generator): return self.typemap[self._GetFullMojomNameForKind(typemapped_kind)][ "typename"] + # Constants that go in module-forward.h. def _FormatConstantDeclaration(self, constant, nested=False): if mojom.IsStringKind(constant.kind): if nested: @@ -623,6 +638,12 @@ class Generator(generator.Generator): GetCppPodType(constant.kind), constant.name, self._ConstantValue(constant)) + # Constants that go in module.h. + def _FormatEnumConstantDeclaration(self, constant): + if mojom.IsEnumKind(constant.kind): + return "constexpr %s %s = %s" % (self._GetNameForKind( + constant.kind), constant.name, self._ConstantValue(constant)) + def _GetCppWrapperType(self, kind, add_same_module_namespaces=False, @@ -727,32 +748,46 @@ class Generator(generator.Generator): def _IsFullHeaderRequiredForImport(self, imported_module): """Determines whether a given import module requires a full header include, - or if the forward header is sufficient. The full header is required if any - imported structs, unions, or typemapped types are referenced by - the module we're generating bindings for; or if an imported enum is used as - a map key.""" - - def requires_full_header(kind): - if (mojom.IsUnionKind(kind) or mojom.IsStructKind(kind) - or self._IsTypemappedKind(kind)): - return True + or if the forward header is sufficient.""" + + # Type-mapped kinds may not have forward declarations, and nested kinds + # cannot be forward declared. + if any(kind.module == imported_module and ( + (self._IsTypemappedKind(kind) and not self. + _GetTypemappedForwardDeclaration(kind)) or kind.parent_kind != None) + for kind in self.module.imported_kinds.values()): + return True - if mojom.IsEnumKind(kind): - # Blink bindings need the full header for an enum used as a map key. - # This is uncommon enough that we set the requirement generically for - # Blink and non-Blink bindings. - return any( - mojom.IsMapKind(k) and k.key_kind == kind - for k in self.module.kinds.values()) - return False + # For most kinds, whether or not a full definition is needed depends on how + # the kind is used. + for kind in self.module.structs + self.module.unions: + for field in kind.fields: - for spec, kind in imported_module.kinds.items(): - if kind.module != imported_module: - # If it wasn't defined directly in the imported module, it doesn't - # affect whether we need the module or not. - continue - if spec in self.module.imported_kinds and requires_full_header(kind): + # Peel array kinds. + kind = field.kind + while mojom.IsArrayKind(kind): + kind = kind.kind + + if kind.module == imported_module: + # Need full def for struct/union fields, even when not inlined. + if mojom.IsStructKind(kind) or mojom.IsUnionKind(kind): + return True + + for kind in self.module.kinds.values(): + if mojom.IsMapKind(kind): + if kind.key_kind.module == imported_module: + # Map keys need the full definition. + return True + if self.for_blink and kind.value_kind.module == imported_module: + # For Blink, map values need the full definition for tracing. + return True + + for constant in self.module.constants: + # Constants referencing enums need the full definition. + if mojom.IsEnumKind( + constant.kind) and constant.value.module == imported_module: return True + return False def _IsReceiverKind(self, kind): diff --git a/chromium/mojo/public/tools/bindings/generators/mojom_js_generator.py b/chromium/mojo/public/tools/bindings/generators/mojom_js_generator.py index 0cb136e4a77..5b831bbb019 100644 --- a/chromium/mojo/public/tools/bindings/generators/mojom_js_generator.py +++ b/chromium/mojo/public/tools/bindings/generators/mojom_js_generator.py @@ -400,10 +400,6 @@ class Generator(generator.Generator): def _GenerateAMDModule(self): return self._GetParameters() - @UseJinja("externs/module.externs.tmpl") - def _GenerateExterns(self): - return self._GetParameters() - @UseJinja("lite/mojom.html.tmpl") def _GenerateLiteHtml(self): return self._GetParameters() @@ -436,8 +432,6 @@ class Generator(generator.Generator): self._SetUniqueNameForImports() self.WriteWithComment(self._GenerateAMDModule(), "%s.js" % self.module.path) - self.WriteWithComment(self._GenerateExterns(), - "%s.externs.js" % self.module.path) if self.js_bindings_mode == "new": self.WriteWithComment(self._GenerateLiteHtml(), "%s.html" % self.module.path) diff --git a/chromium/mojo/public/tools/bindings/mojom.gni b/chromium/mojo/public/tools/bindings/mojom.gni index fe2a1da3750..687534669ba 100644 --- a/chromium/mojo/public/tools/bindings/mojom.gni +++ b/chromium/mojo/public/tools/bindings/mojom.gni @@ -2,7 +2,6 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -import("//build/config/python.gni") import("//third_party/closure_compiler/closure_args.gni") import("//third_party/closure_compiler/compile_js.gni") import("//third_party/protobuf/proto_library.gni") @@ -402,6 +401,12 @@ if (enable_scrambled_message_ids) { # should be mapped in generated bindings. This is a string like # "::base::Value" or "std::vector<::base::Value>". # +# forward_declaration (optional) +# A forward declaration of the C++ type, which bindings that don't +# need the full type definition can use to reduce the size of +# the generated code. This is a string like +# "namespace base { class Value; }". +# # move_only (optional) # A boolean value (default false) which indicates whether the C++ # type is move-only. If true, generated bindings will pass the type @@ -683,8 +688,7 @@ template("mojom") { enabled_features += [ "is_win" ] } - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(parser_target_name) { + action(parser_target_name) { script = mojom_parser_script inputs = mojom_parser_sources + [ build_metadata_filename ] sources = sources_list @@ -819,8 +823,7 @@ template("mojom") { } } - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(generator_cpp_message_ids_target_name) { + action(generator_cpp_message_ids_target_name) { script = mojom_generator_script inputs = mojom_generator_sources + jinja2_sources sources = sources_list @@ -860,8 +863,7 @@ template("mojom") { generator_shared_target_name = "${target_name}_shared__generator" - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(generator_shared_target_name) { + action(generator_shared_target_name) { visibility = [ ":*" ] script = mojom_generator_script inputs = mojom_generator_sources + jinja2_sources @@ -981,8 +983,7 @@ template("mojom") { generator_mojolpm_proto_target_name = "${target_name}_mojolpm_proto_generator" - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(generator_mojolpm_proto_target_name) { + action(generator_mojolpm_proto_target_name) { script = mojom_generator_script inputs = mojom_generator_sources + jinja2_sources sources = invoker.sources @@ -1207,7 +1208,7 @@ template("mojom") { generator_target_name = "${target_name}${variant_suffix}__generator" # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(generator_target_name) { + action(generator_target_name) { visibility = [ ":*" ] script = mojom_generator_script inputs = mojom_generator_sources + jinja2_sources @@ -1389,8 +1390,7 @@ template("mojom") { write_file(_typemap_config_filename, _rebased_typemap_configs, "json") _mojom_target_name = target_name - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(_typemap_validator_target_name) { + action(_typemap_validator_target_name) { script = "$mojom_generator_root/validate_typemap_config.py" inputs = [ _typemap_config_filename ] outputs = [ _typemap_stamp_filename ] @@ -1401,8 +1401,7 @@ template("mojom") { ] } - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(type_mappings_target_name) { + action(type_mappings_target_name) { inputs = mojom_generator_sources + jinja2_sources + [ _typemap_stamp_filename ] outputs = [ type_mappings_path ] @@ -1583,8 +1582,7 @@ template("mojom") { java_generator_target_name = target_name + "_java__generator" if (sources_list != []) { - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(java_generator_target_name) { + action(java_generator_target_name) { script = mojom_generator_script inputs = mojom_generator_sources + jinja2_sources sources = sources_list @@ -1624,8 +1622,7 @@ template("mojom") { java_srcjar_target_name = target_name + "_java_sources" - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(java_srcjar_target_name) { + action(java_srcjar_target_name) { script = "//build/android/gyp/zip.py" inputs = [] if (output_file_base_paths != []) { @@ -1686,8 +1683,7 @@ template("mojom") { if (sources_list != []) { generator_js_target_name = "${target_name}_js__generator" - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(generator_js_target_name) { + action(generator_js_target_name) { script = mojom_generator_script inputs = mojom_generator_sources + jinja2_sources sources = sources_list @@ -1707,7 +1703,6 @@ template("mojom") { foreach(base_path, output_file_base_paths) { outputs += [ "$root_gen_dir/$base_path.js", - "$root_gen_dir/$base_path.externs.js", "$root_gen_dir/$base_path.m.js", "$root_gen_dir/$base_path-lite.js", "$root_gen_dir/$base_path.html", @@ -1928,8 +1923,7 @@ template("mojom") { generator_ts_target_name = "${target_name}_${dependency_type.name}__ts__generator" - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(generator_ts_target_name) { + action(generator_ts_target_name) { script = mojom_generator_script inputs = mojom_generator_sources + jinja2_sources sources = sources_list @@ -1996,8 +1990,7 @@ template("mojom") { "${target_name}_${dependency_type.name}__js__generator" generator_js_target_names += [ generator_js_target_name ] - # TODO(crbug.com/1194274): Investigate nondeterminism in Py3 builds. - python2_action(generator_js_target_name) { + action(generator_js_target_name) { script = "$mojom_generator_root/compile_typescript.py" sources = ts_outputs outputs = js_outputs diff --git a/chromium/mojo/public/tools/bindings/validate_typemap_config.py b/chromium/mojo/public/tools/bindings/validate_typemap_config.py index f1783d59b95..4cb714a5f5e 100755 --- a/chromium/mojo/public/tools/bindings/validate_typemap_config.py +++ b/chromium/mojo/public/tools/bindings/validate_typemap_config.py @@ -17,7 +17,7 @@ def CheckCppTypemapConfigs(target_name, config_filename, out_filename): ]) _SUPPORTED_TYPE_KEYS = set([ 'mojom', 'cpp', 'copyable_pass_by_value', 'force_serialize', 'hashable', - 'move_only', 'nullable_is_same_type' + 'move_only', 'nullable_is_same_type', 'forward_declaration' ]) with open(config_filename, 'r') as f: for config in json.load(f): diff --git a/chromium/mojo/public/tools/mojom/mojom/generate/module.py b/chromium/mojo/public/tools/mojom/mojom/generate/module.py index 9bdb28e05f6..3bba7dd2858 100644 --- a/chromium/mojo/public/tools/mojom/mojom/generate/module.py +++ b/chromium/mojo/public/tools/mojom/mojom/generate/module.py @@ -294,6 +294,7 @@ ATTRIBUTE_STABLE = 'Stable' ATTRIBUTE_SYNC = 'Sync' ATTRIBUTE_UNLIMITED_SIZE = 'UnlimitedSize' ATTRIBUTE_UUID = 'Uuid' +ATTRIBUTE_SERVICE_SANDBOX = 'ServiceSandbox' class NamedValue(object): @@ -574,11 +575,18 @@ class Struct(ReferenceKind): prefix = self.module.GetNamespacePrefix() return '%s%s' % (prefix, self.mojom_name) + def _tuple(self): + return (self.mojom_name, self.native_only, self.fields, self.constants, + self.attributes) + def __eq__(self, rhs): - return (isinstance(rhs, Struct) and - (self.mojom_name, self.native_only, self.fields, self.constants, - self.attributes) == (rhs.mojom_name, rhs.native_only, rhs.fields, - rhs.constants, rhs.attributes)) + return isinstance(rhs, Struct) and self._tuple() == rhs._tuple() + + def __lt__(self, rhs): + if not isinstance(self, type(rhs)): + return str(type(self)) < str(type(rhs)) + + return self._tuple() < rhs._tuple() def __hash__(self): return id(self) @@ -690,10 +698,17 @@ class Union(ReferenceKind): prefix = self.module.GetNamespacePrefix() return '%s%s' % (prefix, self.mojom_name) + def _tuple(self): + return (self.mojom_name, self.fields, self.attributes) + def __eq__(self, rhs): - return (isinstance(rhs, Union) and - (self.mojom_name, self.fields, - self.attributes) == (rhs.mojom_name, rhs.fields, rhs.attributes)) + return isinstance(rhs, Union) and self._tuple() == rhs._tuple() + + def __lt__(self, rhs): + if not isinstance(self, type(rhs)): + return str(type(self)) < str(type(rhs)) + + return self._tuple() < rhs._tuple() def __hash__(self): return id(self) @@ -1061,12 +1076,18 @@ class Method(object): return self.attributes.get(ATTRIBUTE_UNLIMITED_SIZE) \ if self.attributes else False + def _tuple(self): + return (self.mojom_name, self.ordinal, self.parameters, + self.response_parameters, self.attributes) + def __eq__(self, rhs): - return (isinstance(rhs, Method) and - (self.mojom_name, self.ordinal, self.parameters, - self.response_parameters, - self.attributes) == (rhs.mojom_name, rhs.ordinal, rhs.parameters, - rhs.response_parameters, rhs.attributes)) + return isinstance(rhs, Method) and self._tuple() == rhs._tuple() + + def __lt__(self, rhs): + if not isinstance(self, type(rhs)): + return str(type(self)) < str(type(rhs)) + + return self._tuple() < rhs._tuple() class Interface(ReferenceKind): @@ -1187,6 +1208,12 @@ class Interface(ReferenceKind): return True @property + def service_sandbox(self): + if not self.attributes: + return None + return self.attributes.get(ATTRIBUTE_SERVICE_SANDBOX, None) + + @property def stable(self): return self.attributes.get(ATTRIBUTE_STABLE, False) \ if self.attributes else False @@ -1199,11 +1226,18 @@ class Interface(ReferenceKind): prefix = self.module.GetNamespacePrefix() return '%s%s' % (prefix, self.mojom_name) + def _tuple(self): + return (self.mojom_name, self.methods, self.enums, self.constants, + self.attributes) + def __eq__(self, rhs): - return (isinstance(rhs, Interface) - and (self.mojom_name, self.methods, self.enums, self.constants, - self.attributes) == (rhs.mojom_name, rhs.methods, rhs.enums, - rhs.constants, rhs.attributes)) + return isinstance(rhs, Interface) and self._tuple() == rhs._tuple() + + def __lt__(self, rhs): + if not isinstance(self, type(rhs)): + return str(type(self)) < str(type(rhs)) + + return self._tuple() < rhs._tuple() @property def uuid(self): @@ -1358,13 +1392,18 @@ class Enum(Kind): return True + def _tuple(self): + return (self.mojom_name, self.native_only, self.fields, self.attributes, + self.min_value, self.max_value, self.default_field) + def __eq__(self, rhs): - return (isinstance(rhs, Enum) and - (self.mojom_name, self.native_only, self.fields, self.attributes, - self.min_value, self.max_value, - self.default_field) == (rhs.mojom_name, rhs.native_only, - rhs.fields, rhs.attributes, rhs.min_value, - rhs.max_value, rhs.default_field)) + return isinstance(rhs, Enum) and self._tuple() == rhs._tuple() + + def __lt__(self, rhs): + if not isinstance(self, type(rhs)): + return str(type(self)) < str(type(rhs)) + + return self._tuple() < rhs._tuple() def __hash__(self): return id(self) diff --git a/chromium/mojo/public/tools/mojom/mojom/generate/template_expander.py b/chromium/mojo/public/tools/mojom/mojom/generate/template_expander.py index 8d9e26fb7f6..0da900585ce 100644 --- a/chromium/mojo/public/tools/mojom/mojom/generate/template_expander.py +++ b/chromium/mojo/public/tools/mojom/mojom/generate/template_expander.py @@ -79,5 +79,4 @@ def PrecompileTemplates(generator_modules, output_dir): output_dir, "%s.zip" % generator.GetTemplatePrefix()), extensions=["tmpl"], zip="stored", - py_compile=sys.version_info.major < 3, ignore_errors=False) diff --git a/chromium/mojo/public/tools/mojom/mojom/parse/conditional_features.py b/chromium/mojo/public/tools/mojom/mojom/parse/conditional_features.py index 3cb73c5d708..ea253c31e35 100644 --- a/chromium/mojo/public/tools/mojom/mojom/parse/conditional_features.py +++ b/chromium/mojo/public/tools/mojom/mojom/parse/conditional_features.py @@ -17,8 +17,10 @@ class EnableIfError(Error): def _IsEnabled(definition, enabled_features): """Returns true if a definition is enabled. - A definition is enabled if it has no EnableIf attribute, or if the value of - the EnableIf attribute is in enabled_features. + A definition is enabled if it has no EnableIf/EnableIfNot attribute. + It is retained if it has an EnableIf attribute and the attribute is in + enabled_features. It is retained if it has an EnableIfNot attribute and the + attribute is not in enabled features. """ if not hasattr(definition, "attribute_list"): return True @@ -27,17 +29,19 @@ def _IsEnabled(definition, enabled_features): already_defined = False for a in definition.attribute_list: - if a.key == 'EnableIf': + if a.key == 'EnableIf' or a.key == 'EnableIfNot': if already_defined: raise EnableIfError( definition.filename, - "EnableIf attribute may only be defined once per field.", + "EnableIf/EnableIfNot attribute may only be set once per field.", definition.lineno) already_defined = True for attribute in definition.attribute_list: if attribute.key == 'EnableIf' and attribute.value not in enabled_features: return False + if attribute.key == 'EnableIfNot' and attribute.value in enabled_features: + return False return True diff --git a/chromium/mojo/public/tools/mojom/mojom/parse/conditional_features_unittest.py b/chromium/mojo/public/tools/mojom/mojom/parse/conditional_features_unittest.py index aa609be7383..de5d48f384f 100644 --- a/chromium/mojo/public/tools/mojom/mojom/parse/conditional_features_unittest.py +++ b/chromium/mojo/public/tools/mojom/mojom/parse/conditional_features_unittest.py @@ -55,6 +55,48 @@ class ConditionalFeaturesTest(unittest.TestCase): """ self.parseAndAssertEqual(const_source, expected_source) + def testFilterIfNotConst(self): + """Test that Consts are correctly filtered.""" + const_source = """ + [EnableIfNot=blue] + const int kMyConst1 = 1; + [EnableIfNot=orange] + const double kMyConst2 = 2; + [EnableIf=blue,orange] + const int kMyConst3 = 3; + [EnableIfNot=blue,orange] + const int kMyConst4 = 4; + [EnableIfNot=purple,orange] + const int kMyConst5 = 5; + """ + expected_source = """ + [EnableIfNot=orange] + const double kMyConst2 = 2; + [EnableIf=blue,orange] + const int kMyConst3 = 3; + [EnableIfNot=purple,orange] + const int kMyConst5 = 5; + """ + self.parseAndAssertEqual(const_source, expected_source) + + def testFilterIfNotMultipleConst(self): + """Test that Consts are correctly filtered.""" + const_source = """ + [EnableIfNot=blue] + const int kMyConst1 = 1; + [EnableIfNot=orange,blue] + const double kMyConst2 = 2; + [EnableIfNot=orange,purple] + const int kMyConst3 = 3; + """ + expected_source = """ + [EnableIfNot=orange,blue] + const double kMyConst2 = 2; + [EnableIfNot=orange,purple] + const int kMyConst3 = 3; + """ + self.parseAndAssertEqual(const_source, expected_source) + def testFilterEnum(self): """Test that EnumValues are correctly filtered from an Enum.""" enum_source = """ @@ -91,6 +133,24 @@ class ConditionalFeaturesTest(unittest.TestCase): """ self.parseAndAssertEqual(import_source, expected_source) + def testFilterIfNotImport(self): + """Test that imports are correctly filtered from a Mojom.""" + import_source = """ + [EnableIf=blue] + import "foo.mojom"; + [EnableIfNot=purple] + import "bar.mojom"; + [EnableIfNot=green] + import "baz.mojom"; + """ + expected_source = """ + [EnableIf=blue] + import "foo.mojom"; + [EnableIfNot=purple] + import "bar.mojom"; + """ + self.parseAndAssertEqual(import_source, expected_source) + def testFilterInterface(self): """Test that definitions are correctly filtered from an Interface.""" interface_source = """ @@ -175,6 +235,50 @@ class ConditionalFeaturesTest(unittest.TestCase): """ self.parseAndAssertEqual(struct_source, expected_source) + def testFilterIfNotStruct(self): + """Test that definitions are correctly filtered from a Struct.""" + struct_source = """ + struct MyStruct { + [EnableIf=blue] + enum MyEnum { + VALUE1, + [EnableIfNot=red] + VALUE2, + }; + [EnableIfNot=yellow] + const double kMyConst = 1.23; + [EnableIf=green] + int32 a; + double b; + [EnableIfNot=purple] + int32 c; + [EnableIf=blue] + double d; + int32 e; + [EnableIfNot=red] + double f; + }; + """ + expected_source = """ + struct MyStruct { + [EnableIf=blue] + enum MyEnum { + VALUE1, + }; + [EnableIfNot=yellow] + const double kMyConst = 1.23; + [EnableIf=green] + int32 a; + double b; + [EnableIfNot=purple] + int32 c; + [EnableIf=blue] + double d; + int32 e; + }; + """ + self.parseAndAssertEqual(struct_source, expected_source) + def testFilterUnion(self): """Test that UnionFields are correctly filtered from a Union.""" union_source = """ @@ -228,6 +332,30 @@ class ConditionalFeaturesTest(unittest.TestCase): conditional_features.RemoveDisabledDefinitions, definition, ENABLED_FEATURES) + def testMultipleEnableIfs(self): + source = """ + enum Foo { + [EnableIf=red,EnableIfNot=yellow] + kBarValue = 5, + }; + """ + definition = parser.Parse(source, "my_file.mojom") + self.assertRaises(conditional_features.EnableIfError, + conditional_features.RemoveDisabledDefinitions, + definition, ENABLED_FEATURES) + + def testMultipleEnableIfs(self): + source = """ + enum Foo { + [EnableIfNot=red,EnableIfNot=yellow] + kBarValue = 5, + }; + """ + definition = parser.Parse(source, "my_file.mojom") + self.assertRaises(conditional_features.EnableIfError, + conditional_features.RemoveDisabledDefinitions, + definition, ENABLED_FEATURES) + if __name__ == '__main__': unittest.main() diff --git a/chromium/mojo/public/tools/mojom/mojom/parse/parser.py b/chromium/mojo/public/tools/mojom/mojom/parse/parser.py index b3b803d6f33..cfc863778a6 100644 --- a/chromium/mojo/public/tools/mojom/mojom/parse/parser.py +++ b/chromium/mojo/public/tools/mojom/mojom/parse/parser.py @@ -140,11 +140,18 @@ class Parser(object): p[0].Append(p[3]) def p_attribute_1(self, p): + """attribute : NAME EQUALS identifier_wrapped""" + p[0] = ast.Attribute(p[1], + p[3][1], + filename=self.filename, + lineno=p.lineno(1)) + + def p_attribute_2(self, p): """attribute : NAME EQUALS evaled_literal | NAME EQUALS NAME""" p[0] = ast.Attribute(p[1], p[3], filename=self.filename, lineno=p.lineno(1)) - def p_attribute_2(self, p): + def p_attribute_3(self, p): """attribute : NAME""" p[0] = ast.Attribute(p[1], True, filename=self.filename, lineno=p.lineno(1)) diff --git a/chromium/mojo/public/tools/mojom/mojom_parser.py b/chromium/mojo/public/tools/mojom/mojom_parser.py index eb90c825f9b..74beb077dab 100755 --- a/chromium/mojo/public/tools/mojom/mojom_parser.py +++ b/chromium/mojo/public/tools/mojom/mojom_parser.py @@ -11,6 +11,7 @@ generate usable language bindings. """ import argparse +import builtins import codecs import errno import json @@ -19,6 +20,7 @@ import multiprocessing import os import os.path import sys +import traceback from collections import defaultdict from mojom.generate import module @@ -139,7 +141,7 @@ def _EnsureInputLoaded(mojom_abspath, module_path, abs_paths, asts, # Already done. return - for dep_abspath, dep_path in dependencies[mojom_abspath]: + for dep_abspath, dep_path in sorted(dependencies[mojom_abspath]): if dep_abspath not in loaded_modules: _EnsureInputLoaded(dep_abspath, dep_path, abs_paths, asts, dependencies, loaded_modules, module_metadata) @@ -172,8 +174,7 @@ def _CollectAllowedImportsFromBuildMetadata(build_metadata_filename): # multiprocessing helper. -def _ParseAstHelper(args): - mojom_abspath, enabled_features = args +def _ParseAstHelper(mojom_abspath, enabled_features): with codecs.open(mojom_abspath, encoding='utf-8') as f: ast = parser.Parse(f.read(), mojom_abspath) conditional_features.RemoveDisabledDefinitions(ast, enabled_features) @@ -181,8 +182,7 @@ def _ParseAstHelper(args): # multiprocessing helper. -def _SerializeHelper(args): - mojom_abspath, mojom_path = args +def _SerializeHelper(mojom_abspath, mojom_path): module_path = os.path.join(_SerializeHelper.output_root_path, _GetModuleFilename(mojom_path)) module_dir = os.path.dirname(module_path) @@ -199,12 +199,33 @@ def _SerializeHelper(args): _SerializeHelper.loaded_modules[mojom_abspath].Dump(f) -def _Shard(target_func, args, processes=None): - args = list(args) +class _ExceptionWrapper: + def __init__(self): + # Do not capture exception object to ensure pickling works. + self.formatted_trace = traceback.format_exc() + + +class _FuncWrapper: + """Marshals exceptions and spreads args.""" + + def __init__(self, func): + self._func = func + + def __call__(self, args): + # multiprocessing does not gracefully handle excptions. + # https://crbug.com/1219044 + try: + return self._func(*args) + except: # pylint: disable=bare-except + return _ExceptionWrapper() + + +def _Shard(target_func, arg_list, processes=None): + arg_list = list(arg_list) if processes is None: processes = multiprocessing.cpu_count() # Seems optimal to have each process perform at least 2 tasks. - processes = min(processes, len(args) // 2) + processes = min(processes, len(arg_list) // 2) if sys.platform == 'win32': # TODO(crbug.com/1190269) - we can't use more than 56 @@ -213,13 +234,17 @@ def _Shard(target_func, args, processes=None): # Don't spin up processes unless there is enough work to merit doing so. if not _ENABLE_MULTIPROCESSING or processes < 2: - for result in map(target_func, args): - yield result + for arg_tuple in arg_list: + yield target_func(*arg_tuple) return pool = multiprocessing.Pool(processes=processes) try: - for result in pool.imap_unordered(target_func, args): + wrapped_func = _FuncWrapper(target_func) + for result in pool.imap_unordered(wrapped_func, arg_list): + if isinstance(result, _ExceptionWrapper): + sys.stderr.write(result.formatted_trace) + sys.exit(1) yield result finally: pool.close() @@ -230,6 +255,7 @@ def _Shard(target_func, args, processes=None): def _ParseMojoms(mojom_files, input_root_paths, output_root_path, + module_root_paths, enabled_features, module_metadata, allowed_imports=None): @@ -245,8 +271,10 @@ def _ParseMojoms(mojom_files, are based on the mojom's relative path, rebased onto this path. Additionally, the script expects this root to contain already-generated modules for any transitive dependencies not listed in mojom_files. + module_root_paths: A list of absolute filesystem paths which contain + already-generated modules for any non-transitive dependencies. enabled_features: A list of enabled feature names, controlling which AST - nodes are filtered by [EnableIf] attributes. + nodes are filtered by [EnableIf] or [EnableIfNot] attributes. module_metadata: A list of 2-tuples representing metadata key-value pairs to attach to each compiled module output. @@ -274,7 +302,7 @@ def _ParseMojoms(mojom_files, loaded_mojom_asts[mojom_abspath] = ast logging.info('Processing dependencies') - for mojom_abspath, ast in loaded_mojom_asts.items(): + for mojom_abspath, ast in sorted(loaded_mojom_asts.items()): invalid_imports = [] for imp in ast.import_list: import_abspath = _ResolveRelativeImportPath(imp.import_filename, @@ -295,8 +323,8 @@ def _ParseMojoms(mojom_files, # be parsed and have a module file sitting in a corresponding output # location. module_path = _GetModuleFilename(imp.import_filename) - module_abspath = _ResolveRelativeImportPath(module_path, - [output_root_path]) + module_abspath = _ResolveRelativeImportPath( + module_path, module_root_paths + [output_root_path]) with open(module_abspath, 'rb') as module_file: loaded_modules[import_abspath] = module.Module.Load(module_file) @@ -371,6 +399,15 @@ already present in the provided output root.""") 'ROOT is also searched for existing modules of any transitive imports ' 'which were not included in the set of inputs.') arg_parser.add_argument( + '--module-root', + default=[], + action='append', + metavar='ROOT', + dest='module_root_paths', + help='Adds ROOT to the set of root paths to search for existing modules ' + 'of non-transitive imports. Provided root paths are always searched in ' + 'order from longest absolute path to shortest.') + arg_parser.add_argument( '--mojoms', nargs='+', dest='mojom_files', @@ -396,9 +433,9 @@ already present in the provided output root.""") help='Enables a named feature when parsing the given mojoms. Features ' 'are identified by arbitrary string values. Specifying this flag with a ' 'given FEATURE name will cause the parser to process any syntax elements ' - 'tagged with an [EnableIf=FEATURE] attribute. If this flag is not ' - 'provided for a given FEATURE, such tagged elements are discarded by the ' - 'parser and will not be present in the compiled output.') + 'tagged with an [EnableIf=FEATURE] or [EnableIfNot] attribute. If this ' + 'flag is not provided for a given FEATURE, such tagged elements are ' + 'discarded by the parser and will not be present in the compiled output.') arg_parser.add_argument( '--check-imports', dest='build_metadata_filename', @@ -436,6 +473,7 @@ already present in the provided output root.""") mojom_files = list(map(os.path.abspath, args.mojom_files)) input_roots = list(map(os.path.abspath, args.input_root_paths)) output_root = os.path.abspath(args.output_root_path) + module_roots = list(map(os.path.abspath, args.module_root_paths)) if args.build_metadata_filename: allowed_imports = _CollectAllowedImportsFromBuildMetadata( @@ -445,8 +483,8 @@ already present in the provided output root.""") module_metadata = list( map(lambda kvp: tuple(kvp.split('=')), args.module_metadata)) - _ParseMojoms(mojom_files, input_roots, output_root, args.enabled_features, - module_metadata, allowed_imports) + _ParseMojoms(mojom_files, input_roots, output_root, module_roots, + args.enabled_features, module_metadata, allowed_imports) logging.info('Finished') # Exit without running GC, which can save multiple seconds due the large # number of object created. |