diff options
author | rockot <rockot@chromium.org> | 2015-08-22 16:41:00 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-08-22 23:41:36 +0000 |
commit | 0bfe154baad9ddfb1f0c6ecc7cace36e8f430e75 (patch) | |
tree | 10034145e76f1a37c0bb02e3288a299b2b84754c | |
parent | 0eb6b59c463a0aa9dd01956b5542b8cf158e835a (diff) | |
download | chromium_src-0bfe154baad9ddfb1f0c6ecc7cace36e8f430e75.zip chromium_src-0bfe154baad9ddfb1f0c6ecc7cace36e8f430e75.tar.gz chromium_src-0bfe154baad9ddfb1f0c6ecc7cace36e8f430e75.tar.bz2 |
Update mojo sdk to rev c02a28868825edfa57ab77947b8cb15e741c5598
Re-land. Previously conflicted with another CL that landed
just before the roll.
BUG=None
TBR=jam@chromium.org,ben@chromium.org
TBR=mfoltz@chromium.org
TBR=mek@chromium.org
TBR=xhwang@chromium.org
TBR=shess@chromium.org
Review URL: https://codereview.chromium.org/1310103002
Cr-Commit-Position: refs/heads/master@{#344994}
138 files changed, 2418 insertions, 981 deletions
diff --git a/build/module_args/dart.gni b/build/module_args/dart.gni new file mode 100644 index 0000000..be5e83b --- /dev/null +++ b/build/module_args/dart.gni @@ -0,0 +1,6 @@ +# Copyright 2015 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +# Mojo needs this file to exist. We don't currently need it to define anything. + diff --git a/chrome/browser/media/router/media_router_mojo_impl.h b/chrome/browser/media/router/media_router_mojo_impl.h index c9053af..5c28a5a 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.h +++ b/chrome/browser/media/router/media_router_mojo_impl.h @@ -23,6 +23,7 @@ #include "chrome/browser/media/router/issue_manager.h" #include "chrome/browser/media/router/media_router.h" #include "chrome/browser/media/router/media_router.mojom.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" namespace content { class BrowserContext; diff --git a/chrome/browser/media/router/media_router_mojo_test.h b/chrome/browser/media/router/media_router_mojo_test.h index 0a2e933..69db798 100644 --- a/chrome/browser/media/router/media_router_mojo_test.h +++ b/chrome/browser/media/router/media_router_mojo_test.h @@ -15,6 +15,7 @@ #include "mojo/message_pump/message_pump_mojo.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" namespace media_router { diff --git a/components/devtools_service/devtools_http_server.h b/components/devtools_service/devtools_http_server.h index a4f4eed..6906db6 100644 --- a/components/devtools_service/devtools_http_server.h +++ b/components/devtools_service/devtools_http_server.h @@ -12,6 +12,7 @@ #include "mojo/services/network/public/interfaces/http_connection.mojom.h" #include "mojo/services/network/public/interfaces/http_message.mojom.h" #include "mojo/services/network/public/interfaces/http_server.mojom.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" namespace devtools_service { diff --git a/components/filesystem/files_test_base.h b/components/filesystem/files_test_base.h index 5388a9e..43ffccf 100644 --- a/components/filesystem/files_test_base.h +++ b/components/filesystem/files_test_base.h @@ -8,6 +8,7 @@ #include "base/macros.h" #include "components/filesystem/public/interfaces/file_system.mojom.h" #include "mojo/application/public/cpp/application_test_base.h" +#include "mojo/public/cpp/bindings/binding.h" namespace filesystem { diff --git a/components/html_viewer/document_resource_waiter.h b/components/html_viewer/document_resource_waiter.h index c0a2f62e..a2312e4 100644 --- a/components/html_viewer/document_resource_waiter.h +++ b/components/html_viewer/document_resource_waiter.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "mandoline/tab/public/interfaces/frame_tree.mojom.h" #include "mojo/services/network/public/interfaces/url_loader.mojom.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" namespace mojo { class View; diff --git a/components/html_viewer/html_frame.cc b/components/html_viewer/html_frame.cc index 5cb3223..7699a33 100644 --- a/components/html_viewer/html_frame.cc +++ b/components/html_viewer/html_frame.cc @@ -73,7 +73,6 @@ using mojo::Rect; using mojo::ServiceProviderPtr; using mojo::URLResponsePtr; using mojo::View; -using mojo::WeakBindToRequest; namespace html_viewer { namespace { diff --git a/components/html_viewer/html_frame.h b/components/html_viewer/html_frame.h index 7b47bdc..8a82e15 100644 --- a/components/html_viewer/html_frame.h +++ b/components/html_viewer/html_frame.h @@ -20,6 +20,7 @@ #include "third_party/WebKit/public/web/WebSandboxFlags.h" #include "third_party/WebKit/public/web/WebTextInputInfo.h" #include "third_party/WebKit/public/web/WebViewClient.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" namespace blink { class WebFrame; diff --git a/components/html_viewer/web_socket_handle_impl.cc b/components/html_viewer/web_socket_handle_impl.cc index fa103d4..ef08b04 100644 --- a/components/html_viewer/web_socket_handle_impl.cc +++ b/components/html_viewer/web_socket_handle_impl.cc @@ -17,6 +17,7 @@ #include "third_party/WebKit/public/platform/WebString.h" #include "third_party/WebKit/public/platform/WebURL.h" #include "third_party/WebKit/public/platform/WebVector.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" using blink::WebSecurityOrigin; using blink::WebSocketHandle; diff --git a/content/browser/presentation/presentation_service_impl.cc b/content/browser/presentation/presentation_service_impl.cc index 97b5ddd..80e2aea 100644 --- a/content/browser/presentation/presentation_service_impl.cc +++ b/content/browser/presentation/presentation_service_impl.cc @@ -169,12 +169,10 @@ void PresentationServiceImpl::Bind( mojo::InterfaceRequest<presentation::PresentationService> request) { binding_.reset(new mojo::Binding<presentation::PresentationService>( this, request.Pass())); - binding_->set_error_handler(this); -} - -void PresentationServiceImpl::OnConnectionError() { - DVLOG(1) << "OnConnectionError"; - delete this; + binding_->set_connection_error_handler([this]() { + DVLOG(1) << "Connection error"; + delete this; + }); } void PresentationServiceImpl::SetClient( diff --git a/content/browser/presentation/presentation_service_impl.h b/content/browser/presentation/presentation_service_impl.h index 2e991fa..0a3fe6f 100644 --- a/content/browser/presentation/presentation_service_impl.h +++ b/content/browser/presentation/presentation_service_impl.h @@ -26,6 +26,7 @@ #include "content/public/browser/presentation_service_delegate.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/common/frame_navigate_params.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" namespace content { @@ -48,7 +49,6 @@ using NewSessionMojoCallback = mojo::Callback< // from the renderer when the first presentation API request is handled. class CONTENT_EXPORT PresentationServiceImpl : public NON_EXPORTED_BASE(presentation::PresentationService), - public mojo::ErrorHandler, public WebContentsObserver, public PresentationServiceDelegate::Observer { public: @@ -198,10 +198,6 @@ class CONTENT_EXPORT PresentationServiceImpl // Creates a binding between this object and |request|. void Bind(mojo::InterfaceRequest<presentation::PresentationService> request); - // mojo::ErrorHandler override. - // Note that this is called when the RenderFrameHost is deleted. - void OnConnectionError() override; - // WebContentsObserver override. void DidNavigateAnyFrame( content::RenderFrameHost* render_frame_host, diff --git a/content/child/navigator_connect/service_port_provider.h b/content/child/navigator_connect/service_port_provider.h index 7764be0..e99c9be 100644 --- a/content/child/navigator_connect/service_port_provider.h +++ b/content/child/navigator_connect/service_port_provider.h @@ -11,6 +11,7 @@ #include "content/child/worker_task_runner.h" #include "content/common/service_port_service.mojom.h" #include "third_party/WebKit/public/platform/modules/navigator_services/WebServicePortProvider.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" class GURL; diff --git a/content/renderer/presentation/presentation_dispatcher.h b/content/renderer/presentation/presentation_dispatcher.h index e332a46..71bbd0e 100644 --- a/content/renderer/presentation/presentation_dispatcher.h +++ b/content/renderer/presentation/presentation_dispatcher.h @@ -5,6 +5,8 @@ #ifndef CONTENT_RENDERER_PRESENTATION_PRESENTATION_DISPATCHER_H_ #define CONTENT_RENDERER_PRESENTATION_PRESENTATION_DISPATCHER_H_ +#include <queue> + #include "base/compiler_specific.h" #include "base/containers/scoped_ptr_map.h" #include "base/id_map.h" @@ -14,6 +16,7 @@ #include "content/common/presentation/presentation_service.mojom.h" #include "content/public/renderer/render_frame_observer.h" #include "third_party/WebKit/public/platform/modules/presentation/WebPresentationClient.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" namespace blink { class WebPresentationAvailabilityObserver; diff --git a/content/renderer/usb/web_usb_client_impl.cc b/content/renderer/usb/web_usb_client_impl.cc index 84062d9..ac4f681 100644 --- a/content/renderer/usb/web_usb_client_impl.cc +++ b/content/renderer/usb/web_usb_client_impl.cc @@ -22,7 +22,6 @@ #include "third_party/WebKit/public/platform/modules/webusb/WebUSBDeviceRequestOptions.h" #include "third_party/WebKit/public/platform/modules/webusb/WebUSBError.h" #include "third_party/mojo/src/mojo/public/cpp/bindings/array.h" -#include "third_party/mojo/src/mojo/public/cpp/bindings/error_handler.h" #include "third_party/mojo/src/mojo/public/cpp/bindings/interface_request.h" namespace content { diff --git a/content/renderer/usb/web_usb_device_impl.cc b/content/renderer/usb/web_usb_device_impl.cc index e59d5bf..d9b1dd8 100644 --- a/content/renderer/usb/web_usb_device_impl.cc +++ b/content/renderer/usb/web_usb_device_impl.cc @@ -106,8 +106,4 @@ void WebUSBDeviceImpl::reset(blink::WebUSBDeviceResetCallbacks* callbacks) { RejectAsNotImplemented(callbacks); } -void WebUSBDeviceImpl::OnConnectionError() { - device_.reset(); -} - } // namespace content diff --git a/content/renderer/usb/web_usb_device_impl.h b/content/renderer/usb/web_usb_device_impl.h index 7f01d8d..3f6eab5 100644 --- a/content/renderer/usb/web_usb_device_impl.h +++ b/content/renderer/usb/web_usb_device_impl.h @@ -14,7 +14,6 @@ #include "third_party/WebKit/public/platform/modules/webusb/WebUSBDevice.h" #include "third_party/WebKit/public/platform/modules/webusb/WebUSBDeviceInfo.h" #include "third_party/WebKit/public/platform/modules/webusb/WebUSBError.h" -#include "third_party/mojo/src/mojo/public/cpp/bindings/error_handler.h" namespace mojo { class Shell; @@ -22,7 +21,7 @@ class Shell; namespace content { -class WebUSBDeviceImpl : public blink::WebUSBDevice, public mojo::ErrorHandler { +class WebUSBDeviceImpl : public blink::WebUSBDevice { public: WebUSBDeviceImpl(device::usb::DeviceManagerPtr device_manager, const blink::WebUSBDeviceInfo& device_info); @@ -62,9 +61,6 @@ class WebUSBDeviceImpl : public blink::WebUSBDevice, public mojo::ErrorHandler { blink::WebUSBDeviceBulkTransferCallbacks* callbacks) override; void reset(blink::WebUSBDeviceResetCallbacks* callbacks) override; - // mojo::ErrorHandler implementation: - void OnConnectionError() override; - device::usb::DeviceManagerPtr device_manager_; blink::WebUSBDeviceInfo device_info_; diff --git a/device/serial/data_receiver.h b/device/serial/data_receiver.h index 774d395..0eba0a2 100644 --- a/device/serial/data_receiver.h +++ b/device/serial/data_receiver.h @@ -13,6 +13,7 @@ #include "base/memory/weak_ptr.h" #include "device/serial/buffer.h" #include "device/serial/data_stream.mojom.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" #include "third_party/mojo/src/mojo/public/cpp/system/data_pipe.h" namespace device { diff --git a/device/serial/data_sink_receiver.h b/device/serial/data_sink_receiver.h index 8bb306e..733b0a1 100644 --- a/device/serial/data_sink_receiver.h +++ b/device/serial/data_sink_receiver.h @@ -13,6 +13,7 @@ #include "base/memory/weak_ptr.h" #include "device/serial/buffer.h" #include "device/serial/data_stream.mojom.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" #include "third_party/mojo/src/mojo/public/cpp/system/data_pipe.h" namespace device { diff --git a/device/serial/data_source_sender.h b/device/serial/data_source_sender.h index 2de7ce6..160dec9 100644 --- a/device/serial/data_source_sender.h +++ b/device/serial/data_source_sender.h @@ -12,6 +12,7 @@ #include "base/memory/weak_ptr.h" #include "device/serial/buffer.h" #include "device/serial/data_stream.mojom.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" #include "third_party/mojo/src/mojo/public/cpp/system/data_pipe.h" namespace device { diff --git a/extensions/browser/mojo/stash_backend_unittest.cc b/extensions/browser/mojo/stash_backend_unittest.cc index ae14208..be76bac 100644 --- a/extensions/browser/mojo/stash_backend_unittest.cc +++ b/extensions/browser/mojo/stash_backend_unittest.cc @@ -33,7 +33,7 @@ mojo::ScopedHandle CreateReadableHandle() { } // namespace -class StashServiceTest : public testing::Test, public mojo::ErrorHandler { +class StashServiceTest : public testing::Test { public: enum Event { EVENT_NONE, @@ -49,11 +49,11 @@ class StashServiceTest : public testing::Test, public mojo::ErrorHandler { stash_backend_.reset(new StashBackend(base::Bind( &StashServiceTest::OnHandleReadyToRead, base::Unretained(this)))); stash_backend_->BindToRequest(mojo::GetProxy(&stash_service_)); - stash_service_.set_error_handler(this); + stash_service_.set_connection_error_handler(base::Bind(&OnConnectionError)); handles_ready_ = 0; } - void OnConnectionError() override { FAIL() << "Unexpected connection error"; } + static void OnConnectionError() { FAIL() << "Unexpected connection error"; } mojo::Array<StashedObjectPtr> RetrieveStash() { mojo::Array<StashedObjectPtr> stash; @@ -271,7 +271,7 @@ TEST_F(StashServiceTest, NotifyOncePerStashOnReadableHandles) { // exists. TEST_F(StashServiceTest, ServiceWithDeletedBackend) { stash_backend_.reset(); - stash_service_.set_error_handler(this); + stash_service_.set_connection_error_handler(base::Bind(&OnConnectionError)); mojo::Array<StashedObjectPtr> stashed_objects; StashedObjectPtr stashed_object(StashedObject::New()); diff --git a/ipc/mojo/ipc_channel_mojo.cc b/ipc/mojo/ipc_channel_mojo.cc index 08c3458..ff20719 100644 --- a/ipc/mojo/ipc_channel_mojo.cc +++ b/ipc/mojo/ipc_channel_mojo.cc @@ -16,6 +16,7 @@ #include "ipc/mojo/ipc_mojo_bootstrap.h" #include "ipc/mojo/ipc_mojo_handle_attachment.h" #include "third_party/mojo/src/mojo/edk/embedder/embedder.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" #if defined(OS_POSIX) && !defined(OS_NACL) #include "ipc/ipc_platform_file_attachment_posix.h" diff --git a/mandoline/ui/aura/surface_binding.cc b/mandoline/ui/aura/surface_binding.cc index 0307112..5c20cca 100644 --- a/mandoline/ui/aura/surface_binding.cc +++ b/mandoline/ui/aura/surface_binding.cc @@ -24,6 +24,7 @@ #include "mojo/cc/context_provider_mojo.h" #include "mojo/converters/geometry/geometry_type_converters.h" #include "mojo/converters/surfaces/surfaces_type_converters.h" +#include "mojo/public/cpp/bindings/binding.h" namespace mandoline { namespace { diff --git a/media/mojo/services/mojo_cdm.h b/media/mojo/services/mojo_cdm.h index ea4e8a4..96b4532 100644 --- a/media/mojo/services/mojo_cdm.h +++ b/media/mojo/services/mojo_cdm.h @@ -14,6 +14,7 @@ #include "media/base/media_keys.h" #include "media/mojo/interfaces/content_decryption_module.mojom.h" #include "media/mojo/services/mojo_type_trait.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" namespace mojo { class ServiceProvider; diff --git a/media/mojo/services/mojo_renderer_impl.h b/media/mojo/services/mojo_renderer_impl.h index f25fb4f..e574b15 100644 --- a/media/mojo/services/mojo_renderer_impl.h +++ b/media/mojo/services/mojo_renderer_impl.h @@ -9,6 +9,7 @@ #include "base/memory/weak_ptr.h" #include "media/base/renderer.h" #include "media/mojo/interfaces/renderer.mojom.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" namespace base { class SingleThreadTaskRunner; diff --git a/mojo/application/public/cpp/application_impl.h b/mojo/application/public/cpp/application_impl.h index 9b587b2..01088f8 100644 --- a/mojo/application/public/cpp/application_impl.h +++ b/mojo/application/public/cpp/application_impl.h @@ -15,6 +15,7 @@ #include "mojo/application/public/cpp/lib/service_registry.h" #include "mojo/application/public/interfaces/application.mojom.h" #include "mojo/application/public/interfaces/shell.mojom.h" +#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/callback.h" #include "mojo/public/cpp/system/core.h" diff --git a/mojo/application/public/cpp/lib/service_registry.h b/mojo/application/public/cpp/lib/service_registry.h index 7e4d604..8cb1d1b 100644 --- a/mojo/application/public/cpp/lib/service_registry.h +++ b/mojo/application/public/cpp/lib/service_registry.h @@ -11,6 +11,7 @@ #include "mojo/application/public/cpp/application_connection.h" #include "mojo/application/public/cpp/lib/service_connector_registry.h" #include "mojo/application/public/interfaces/service_provider.mojom.h" +#include "mojo/public/cpp/bindings/binding.h" namespace mojo { namespace internal { diff --git a/mojo/gles2/command_buffer_client_impl.h b/mojo/gles2/command_buffer_client_impl.h index a01c12c..1ec0f1f 100644 --- a/mojo/gles2/command_buffer_client_impl.h +++ b/mojo/gles2/command_buffer_client_impl.h @@ -13,6 +13,7 @@ #include "gpu/command_buffer/client/gpu_control.h" #include "gpu/command_buffer/common/command_buffer.h" #include "gpu/command_buffer/common/command_buffer_shared.h" +#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h" namespace base { class RunLoop; diff --git a/mojo/runner/child_process.cc b/mojo/runner/child_process.cc index 85ce4b3..8c62fdb 100644 --- a/mojo/runner/child_process.cc +++ b/mojo/runner/child_process.cc @@ -26,6 +26,7 @@ #include "mojo/edk/embedder/scoped_platform_handle.h" #include "mojo/edk/embedder/simple_platform_support.h" #include "mojo/message_pump/message_pump_mojo.h" +#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/system/core.h" #include "mojo/runner/child_process.mojom.h" #include "mojo/runner/native_application_support.h" diff --git a/mojo/tools/rev_sdk.py b/mojo/tools/rev_sdk.py index d3d14c7..fa222d8 100755 --- a/mojo/tools/rev_sdk.py +++ b/mojo/tools/rev_sdk.py @@ -33,6 +33,7 @@ sdk_dirs_to_not_clone = [ preserved_chromium_files = [ 'mojo/edk/DEPS', 'mojo/public/DEPS', + 'mojo/public/c/gpu/DEPS', 'mojo/public/platform/nacl/DEPS', 'nacl_bindings/DEPS', ] diff --git a/sql/mojo/sql_test_base.h b/sql/mojo/sql_test_base.h index 7a7ccca..d39198d 100644 --- a/sql/mojo/sql_test_base.h +++ b/sql/mojo/sql_test_base.h @@ -9,6 +9,7 @@ #include "base/memory/scoped_ptr.h" #include "components/filesystem/public/interfaces/file_system.mojom.h" #include "mojo/application/public/cpp/application_test_base.h" +#include "mojo/public/cpp/bindings/binding.h" #include "sql/connection.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/third_party/mojo/mojo_edk.gyp b/third_party/mojo/mojo_edk.gyp index 8f60ae6..0b5b475 100644 --- a/third_party/mojo/mojo_edk.gyp +++ b/third_party/mojo/mojo_edk.gyp @@ -24,7 +24,6 @@ 'dependencies': [ '../../base/base.gyp:base', '../../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations', - '../../crypto/crypto.gyp:crypto', ], 'includes': [ 'mojo_edk_system_impl.gypi', @@ -141,7 +140,6 @@ 'dependencies': [ '../../base/base.gyp:base_win64', '../../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations_win64', - '../../crypto/crypto.gyp:crypto_nacl_win64', ], 'includes': [ 'mojo_edk_system_impl.gypi', diff --git a/third_party/mojo/mojo_edk_system_impl.gypi b/third_party/mojo/mojo_edk_system_impl.gypi index 5bac8f3..fc455ed 100644 --- a/third_party/mojo/mojo_edk_system_impl.gypi +++ b/third_party/mojo/mojo_edk_system_impl.gypi @@ -60,6 +60,7 @@ 'src/mojo/edk/system/connection_identifier.h', 'src/mojo/edk/system/connection_manager.cc', 'src/mojo/edk/system/connection_manager.h', + 'src/mojo/edk/system/connection_manager_messages.h', 'src/mojo/edk/system/core.cc', 'src/mojo/edk/system/core.h', 'src/mojo/edk/system/data_pipe.cc', @@ -101,6 +102,8 @@ 'src/mojo/edk/system/message_pipe_dispatcher.h', 'src/mojo/edk/system/message_pipe_endpoint.cc', 'src/mojo/edk/system/message_pipe_endpoint.h', + 'src/mojo/edk/system/mutex.cc', + 'src/mojo/edk/system/mutex.h', 'src/mojo/edk/system/options_validation.h', 'src/mojo/edk/system/platform_handle_dispatcher.cc', 'src/mojo/edk/system/platform_handle_dispatcher.h', @@ -122,6 +125,7 @@ 'src/mojo/edk/system/simple_dispatcher.h', 'src/mojo/edk/system/slave_connection_manager.cc', 'src/mojo/edk/system/slave_connection_manager.h', + 'src/mojo/edk/system/thread_annotations.h', 'src/mojo/edk/system/transport_data.cc', 'src/mojo/edk/system/transport_data.h', 'src/mojo/edk/system/unique_identifier.cc', diff --git a/third_party/mojo/mojo_edk_tests.gyp b/third_party/mojo/mojo_edk_tests.gyp index db1fc36..2e47591 100644 --- a/third_party/mojo/mojo_edk_tests.gyp +++ b/third_party/mojo/mojo_edk_tests.gyp @@ -17,6 +17,7 @@ # target on iOS due to the presence of the js targets, which cause v8 # to be built. 'mojo_message_pipe_perftests', + 'mojo_public_bindings_perftests', 'mojo_public_bindings_unittests', 'mojo_public_environment_unittests', 'mojo_public_system_perftests', @@ -52,6 +53,8 @@ 'src/mojo/public/cpp/bindings/tests/handle_passing_unittest.cc', 'src/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc', 'src/mojo/public/cpp/bindings/tests/map_unittest.cc', + 'src/mojo/public/cpp/bindings/tests/message_queue.cc', + 'src/mojo/public/cpp/bindings/tests/message_queue.h', 'src/mojo/public/cpp/bindings/tests/request_response_unittest.cc', 'src/mojo/public/cpp/bindings/tests/router_unittest.cc', 'src/mojo/public/cpp/bindings/tests/sample_service_unittest.cc', @@ -63,6 +66,24 @@ ], }, { + # GN version: //mojo/edk/test:mojo_public_bindings_perftests + 'target_name': 'mojo_public_bindings_perftests', + 'type': 'executable', + 'dependencies': [ + '../../testing/gtest.gyp:gtest', + 'mojo_edk.gyp:mojo_run_all_unittests', + 'mojo_public.gyp:mojo_cpp_bindings', + 'mojo_public.gyp:mojo_environment_standalone', + 'mojo_public.gyp:mojo_public_bindings_test_utils', + 'mojo_public.gyp:mojo_public_test_interfaces', + 'mojo_public.gyp:mojo_public_test_utils', + 'mojo_public.gyp:mojo_utility', + ], + 'sources': [ + 'src/mojo/public/cpp/bindings/tests/bindings_perftest.cc', + ], + }, + { # GN version: //mojo/edk/test:mojo_public_environment_unittests 'target_name': 'mojo_public_environment_unittests', 'type': 'executable', @@ -175,6 +196,7 @@ 'src/mojo/edk/system/message_pipe_test_utils.h', 'src/mojo/edk/system/message_pipe_unittest.cc', 'src/mojo/edk/system/multiprocess_message_pipe_unittest.cc', + 'src/mojo/edk/system/mutex_unittest.cc', 'src/mojo/edk/system/options_validation_unittest.cc', 'src/mojo/edk/system/platform_handle_dispatcher_unittest.cc', 'src/mojo/edk/system/raw_channel_unittest.cc', @@ -187,6 +209,7 @@ 'src/mojo/edk/system/test_channel_endpoint_client.h', 'src/mojo/edk/system/test_utils.cc', 'src/mojo/edk/system/test_utils.h', + 'src/mojo/edk/system/thread_annotations_unittest.cc', 'src/mojo/edk/system/unique_identifier_unittest.cc', 'src/mojo/edk/system/waiter_test_utils.cc', 'src/mojo/edk/system/waiter_test_utils.h', diff --git a/third_party/mojo/mojo_public.gyp b/third_party/mojo/mojo_public.gyp index 3f16424..f5b8265 100644 --- a/third_party/mojo/mojo_public.gyp +++ b/third_party/mojo/mojo_public.gyp @@ -104,7 +104,6 @@ 'src/mojo/public/cpp/bindings/array.h', 'src/mojo/public/cpp/bindings/binding.h', 'src/mojo/public/cpp/bindings/callback.h', - 'src/mojo/public/cpp/bindings/error_handler.h', 'src/mojo/public/cpp/bindings/interface_impl.h', 'src/mojo/public/cpp/bindings/interface_ptr.h', 'src/mojo/public/cpp/bindings/interface_request.h', @@ -145,8 +144,6 @@ 'src/mojo/public/cpp/bindings/lib/message_header_validator.cc', 'src/mojo/public/cpp/bindings/lib/message_header_validator.h', 'src/mojo/public/cpp/bindings/lib/message_internal.h', - 'src/mojo/public/cpp/bindings/lib/message_queue.cc', - 'src/mojo/public/cpp/bindings/lib/message_queue.h', 'src/mojo/public/cpp/bindings/lib/no_interface.cc', 'src/mojo/public/cpp/bindings/lib/router.cc', 'src/mojo/public/cpp/bindings/lib/router.h', @@ -333,6 +330,7 @@ 'mojom_files': [ 'src/mojo/public/interfaces/bindings/tests/math_calculator.mojom', 'src/mojo/public/interfaces/bindings/tests/no_module.mojom', + 'src/mojo/public/interfaces/bindings/tests/ping_service.mojom', 'src/mojo/public/interfaces/bindings/tests/rect.mojom', 'src/mojo/public/interfaces/bindings/tests/regression_tests.mojom', 'src/mojo/public/interfaces/bindings/tests/sample_factory.mojom', diff --git a/third_party/mojo/src/mojo/edk/embedder/BUILD.gn b/third_party/mojo/src/mojo/edk/embedder/BUILD.gn index c3e0d53..b3144f0 100644 --- a/third_party/mojo/src/mojo/edk/embedder/BUILD.gn +++ b/third_party/mojo/src/mojo/edk/embedder/BUILD.gn @@ -87,7 +87,6 @@ mojo_edk_source_set("platform") { deps = [ "//base", - "//crypto", ] if (is_android) { diff --git a/third_party/mojo/src/mojo/edk/embedder/embedder_unittest.cc b/third_party/mojo/src/mojo/edk/embedder/embedder_unittest.cc index 0d1f3d9..e9735b1 100644 --- a/third_party/mojo/src/mojo/edk/embedder/embedder_unittest.cc +++ b/third_party/mojo/src/mojo/edk/embedder/embedder_unittest.cc @@ -16,6 +16,7 @@ #include "base/test/test_timeouts.h" #include "mojo/edk/embedder/platform_channel_pair.h" #include "mojo/edk/embedder/test_embedder.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/test_utils.h" #include "mojo/edk/test/multiprocess_test_helper.h" #include "mojo/edk/test/scoped_ipc_support.h" @@ -180,7 +181,7 @@ class TestAsyncWaiter { TestAsyncWaiter() : event_(true, false), wait_result_(MOJO_RESULT_UNKNOWN) {} void Awake(MojoResult result) { - base::AutoLock l(wait_result_lock_); + system::MutexLocker l(&wait_result_mutex_); wait_result_ = result; event_.Signal(); } @@ -188,15 +189,15 @@ class TestAsyncWaiter { bool TryWait() { return event_.TimedWait(TestTimeouts::action_timeout()); } MojoResult wait_result() const { - base::AutoLock l(wait_result_lock_); + system::MutexLocker l(&wait_result_mutex_); return wait_result_; } private: base::WaitableEvent event_; - mutable base::Lock wait_result_lock_; - MojoResult wait_result_; + mutable system::Mutex wait_result_mutex_; + MojoResult wait_result_ MOJO_GUARDED_BY(wait_result_mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(TestAsyncWaiter); }; diff --git a/third_party/mojo/src/mojo/edk/embedder/platform_channel_pair_posix_unittest.cc b/third_party/mojo/src/mojo/edk/embedder/platform_channel_pair_posix_unittest.cc index 897929f..c7b49c2 100644 --- a/third_party/mojo/src/mojo/edk/embedder/platform_channel_pair_posix_unittest.cc +++ b/third_party/mojo/src/mojo/edk/embedder/platform_channel_pair_posix_unittest.cc @@ -15,10 +15,7 @@ #include <deque> -#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 "mojo/edk/embedder/platform_channel_utils_posix.h" #include "mojo/edk/embedder/platform_handle.h" @@ -128,9 +125,6 @@ TEST_F(PlatformChannelPairPosixTest, SendReceiveData) { } TEST_F(PlatformChannelPairPosixTest, SendReceiveFDs) { - base::ScopedTempDir temp_dir; - ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); - static const char kHello[] = "hello"; PlatformChannelPair channel_pair; @@ -150,9 +144,7 @@ TEST_F(PlatformChannelPairPosixTest, SendReceiveFDs) { const char c = '0' + (i % 10); ScopedPlatformHandleVectorPtr platform_handles(new PlatformHandleVector); for (size_t j = 1; j <= i; j++) { - base::FilePath unused; - base::ScopedFILE fp( - base::CreateAndOpenTemporaryFileInDir(temp_dir.path(), &unused)); + base::ScopedFILE fp(tmpfile()); ASSERT_TRUE(fp); ASSERT_EQ(j, fwrite(std::string(j, c).data(), 1, j, fp.get())); platform_handles->push_back( @@ -194,9 +186,6 @@ TEST_F(PlatformChannelPairPosixTest, SendReceiveFDs) { } TEST_F(PlatformChannelPairPosixTest, AppendReceivedFDs) { - base::ScopedTempDir temp_dir; - ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); - static const char kHello[] = "hello"; PlatformChannelPair channel_pair; @@ -206,9 +195,7 @@ TEST_F(PlatformChannelPairPosixTest, AppendReceivedFDs) { const std::string file_contents("hello world"); { - base::FilePath unused; - base::ScopedFILE fp( - base::CreateAndOpenTemporaryFileInDir(temp_dir.path(), &unused)); + base::ScopedFILE fp(tmpfile()); ASSERT_TRUE(fp); ASSERT_EQ(file_contents.size(), fwrite(file_contents.data(), 1, file_contents.size(), fp.get())); diff --git a/third_party/mojo/src/mojo/edk/embedder/simple_platform_support.cc b/third_party/mojo/src/mojo/edk/embedder/simple_platform_support.cc index 5024087..4207120 100644 --- a/third_party/mojo/src/mojo/edk/embedder/simple_platform_support.cc +++ b/third_party/mojo/src/mojo/edk/embedder/simple_platform_support.cc @@ -4,7 +4,7 @@ #include "mojo/edk/embedder/simple_platform_support.h" -#include "crypto/random.h" +#include "base/rand_util.h" #include "mojo/edk/embedder/simple_platform_shared_buffer.h" namespace mojo { @@ -12,7 +12,7 @@ namespace embedder { void SimplePlatformSupport::GetCryptoRandomBytes(void* bytes, size_t num_bytes) { - crypto::RandBytes(bytes, num_bytes); + base::RandBytes(bytes, num_bytes); } PlatformSharedBuffer* SimplePlatformSupport::CreateSharedBuffer( diff --git a/third_party/mojo/src/mojo/edk/js/tests/js_to_cpp_tests.cc b/third_party/mojo/src/mojo/edk/js/tests/js_to_cpp_tests.cc index 3675f97..c012daf 100644 --- a/third_party/mojo/src/mojo/edk/js/tests/js_to_cpp_tests.cc +++ b/third_party/mojo/src/mojo/edk/js/tests/js_to_cpp_tests.cc @@ -14,6 +14,7 @@ #include "mojo/edk/js/mojo_runner_delegate.h" #include "mojo/edk/js/tests/js_to_cpp.mojom.h" #include "mojo/edk/test/test_utils.h" +#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/system/core.h" #include "mojo/public/cpp/system/macros.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/third_party/mojo/src/mojo/edk/mojo_edk.gni b/third_party/mojo/src/mojo/edk/mojo_edk.gni index b90462d..21ad315 100644 --- a/third_party/mojo/src/mojo/edk/mojo_edk.gni +++ b/third_party/mojo/src/mojo/edk/mojo_edk.gni @@ -95,3 +95,9 @@ template("mojo_edk_source_set") { } } } + +# Build EDK things with static thread annotation analysis enabled. +# TODO(vtl): Should we set this at a higher level? +if (is_clang) { + cflags = [ "-Wthread-safety" ] +} diff --git a/third_party/mojo/src/mojo/edk/system/BUILD.gn b/third_party/mojo/src/mojo/edk/system/BUILD.gn index 95ee992..5dd2682 100644 --- a/third_party/mojo/src/mojo/edk/system/BUILD.gn +++ b/third_party/mojo/src/mojo/edk/system/BUILD.gn @@ -82,6 +82,8 @@ component("system") { "message_pipe_dispatcher.h", "message_pipe_endpoint.cc", "message_pipe_endpoint.h", + "mutex.cc", + "mutex.h", "options_validation.h", "platform_handle_dispatcher.cc", "platform_handle_dispatcher.h", @@ -103,6 +105,7 @@ component("system") { "simple_dispatcher.h", "slave_connection_manager.cc", "slave_connection_manager.h", + "thread_annotations.h", "transport_data.cc", "transport_data.h", "unique_identifier.cc", @@ -189,6 +192,7 @@ test("mojo_system_unittests") { "message_pipe_test_utils.h", "message_pipe_unittest.cc", "multiprocess_message_pipe_unittest.cc", + "mutex_unittest.cc", "options_validation_unittest.cc", "platform_handle_dispatcher_unittest.cc", "raw_channel_unittest.cc", @@ -199,6 +203,7 @@ test("mojo_system_unittests") { "simple_dispatcher_unittest.cc", "test_channel_endpoint_client.cc", "test_channel_endpoint_client.h", + "thread_annotations_unittest.cc", "unique_identifier_unittest.cc", "waiter_test_utils.cc", "waiter_test_utils.h", diff --git a/third_party/mojo/src/mojo/edk/system/channel.cc b/third_party/mojo/src/mojo/edk/system/channel.cc index 470956f..601ddb0 100644 --- a/third_party/mojo/src/mojo/edk/system/channel.cc +++ b/third_party/mojo/src/mojo/edk/system/channel.cc @@ -39,7 +39,7 @@ void Channel::Init(scoped_ptr<RawChannel> raw_channel) { DCHECK(creation_thread_checker_.CalledOnValidThread()); DCHECK(raw_channel); - // No need to take |lock_|, since this must be called before this object + // No need to take |mutex_|, since this must be called before this object // becomes thread-safe. DCHECK(!is_running_); raw_channel_ = raw_channel.Pass(); @@ -50,7 +50,7 @@ void Channel::Init(scoped_ptr<RawChannel> raw_channel) { void Channel::SetChannelManager(ChannelManager* channel_manager) { DCHECK(channel_manager); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); DCHECK(!is_shutting_down_); DCHECK(!channel_manager_); channel_manager_ = channel_manager; @@ -61,7 +61,7 @@ void Channel::Shutdown() { IdToEndpointMap to_destroy; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (!is_running_) return; @@ -91,7 +91,7 @@ void Channel::Shutdown() { } void Channel::WillShutdownSoon() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); is_shutting_down_ = true; channel_manager_ = nullptr; } @@ -109,7 +109,7 @@ void Channel::SetBootstrapEndpointWithIds( DCHECK(endpoint); { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); DLOG_IF(WARNING, is_shutting_down_) << "SetBootstrapEndpoint() while shutting down"; @@ -125,7 +125,7 @@ void Channel::SetBootstrapEndpointWithIds( } bool Channel::WriteMessage(scoped_ptr<MessageInTransit> message) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (!is_running_) { // TODO(vtl): I think this is probably not an error condition, but I should // think about it (and the shutdown sequence) more carefully. @@ -138,7 +138,7 @@ bool Channel::WriteMessage(scoped_ptr<MessageInTransit> message) { } bool Channel::IsWriteBufferEmpty() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (!is_running_) return true; return raw_channel_->IsWriteBufferEmpty(); @@ -154,7 +154,7 @@ void Channel::DetachEndpoint(ChannelEndpoint* endpoint, return; // Nothing to do. { - base::AutoLock locker_(lock_); + MutexLocker locker_(&mutex_); if (!is_running_) return; @@ -247,7 +247,7 @@ scoped_refptr<IncomingEndpoint> Channel::DeserializeEndpoint( DVLOG_IF(2, !local_id.is_valid() || !local_id.is_remote()) << "Attempt to get incoming endpoint for invalid ID " << local_id; - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); auto it = incoming_endpoints_.find(local_id); if (it == incoming_endpoints_.end()) { @@ -264,6 +264,9 @@ scoped_refptr<IncomingEndpoint> Channel::DeserializeEndpoint( } size_t Channel::GetSerializedPlatformHandleSize() const { + // TODO(vtl): Having to lock |mutex_| here is a bit unfortunate. Maybe we + // should get the size in |Init()| and cache it? + MutexLocker locker(&mutex_); return raw_channel_->GetSerializedPlatformHandleSize(); } @@ -303,7 +306,7 @@ void Channel::OnError(Error error) { DVLOG(1) << "RawChannel read error (shutdown)"; break; case ERROR_READ_BROKEN: { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); LOG_IF(ERROR, !is_shutting_down_) << "RawChannel read error (connection broken)"; break; @@ -340,7 +343,7 @@ void Channel::OnReadMessageForEndpoint( scoped_refptr<ChannelEndpoint> endpoint; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); // Since we own |raw_channel_|, and this method and |Shutdown()| should only // be called from the creation thread, |raw_channel_| should never be null @@ -459,7 +462,7 @@ bool Channel::OnAttachAndRunEndpoint(ChannelEndpointId local_id, bool success = true; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (local_id_to_endpoint_map_.find(local_id) == local_id_to_endpoint_map_.end()) { @@ -490,7 +493,7 @@ bool Channel::OnRemoveEndpoint(ChannelEndpointId local_id, scoped_refptr<ChannelEndpoint> endpoint; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); IdToEndpointMap::iterator it = local_id_to_endpoint_map_.find(local_id); if (it == local_id_to_endpoint_map_.end()) { @@ -526,7 +529,7 @@ bool Channel::OnRemoveEndpoint(ChannelEndpointId local_id, bool Channel::OnRemoveEndpointAck(ChannelEndpointId local_id) { DCHECK(creation_thread_checker_.CalledOnValidThread()); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); IdToEndpointMap::iterator it = local_id_to_endpoint_map_.find(local_id); if (it == local_id_to_endpoint_map_.end()) { @@ -569,7 +572,7 @@ ChannelEndpointId Channel::AttachAndRunEndpoint( ChannelEndpointId local_id; ChannelEndpointId remote_id; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); DLOG_IF(WARNING, is_shutting_down_) << "AttachAndRunEndpoint() while shutting down"; diff --git a/third_party/mojo/src/mojo/edk/system/channel.h b/third_party/mojo/src/mojo/edk/system/channel.h index 015fa73..9b7c3ae 100644 --- a/third_party/mojo/src/mojo/edk/system/channel.h +++ b/third_party/mojo/src/mojo/edk/system/channel.h @@ -10,13 +10,13 @@ #include "base/containers/hash_tables.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/synchronization/lock.h" #include "base/threading/thread_checker.h" #include "mojo/edk/embedder/scoped_platform_handle.h" #include "mojo/edk/system/channel_endpoint.h" #include "mojo/edk/system/channel_endpoint_id.h" #include "mojo/edk/system/incoming_endpoint.h" #include "mojo/edk/system/message_in_transit.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/raw_channel.h" #include "mojo/edk/system/system_impl_export.h" #include "mojo/public/c/system/types.h" @@ -59,7 +59,7 @@ class MOJO_SYSTEM_IMPL_EXPORT Channel final // This must be called on the creation thread before any other methods are // called, and before references to this object are given to any other // threads. |raw_channel| should be uninitialized. - void Init(scoped_ptr<RawChannel> raw_channel); + void Init(scoped_ptr<RawChannel> raw_channel) MOJO_NOT_THREAD_SAFE; // Sets the channel manager associated with this channel. This should be set // at most once and only called before |WillShutdownSoon()| (and @@ -220,47 +220,51 @@ class MOJO_SYSTEM_IMPL_EXPORT Channel final ChannelEndpointId AttachAndRunEndpoint( scoped_refptr<ChannelEndpoint> endpoint); - // Helper to send channel control messages. Returns true on success. Should be - // called *without* |lock_| held. Callable from any thread. + // Helper to send channel control messages. Returns true on success. Callable + // from any thread. bool SendControlMessage(MessageInTransit::Subtype subtype, ChannelEndpointId source_id, - ChannelEndpointId destination_id); + ChannelEndpointId destination_id) + MOJO_LOCKS_EXCLUDED(mutex_); base::ThreadChecker creation_thread_checker_; embedder::PlatformSupport* const platform_support_; // Note: |ChannelEndpointClient|s (in particular, |MessagePipe|s) MUST NOT be - // used under |lock_|. E.g., |lock_| can only be acquired after + // used under |mutex_|. E.g., |mutex_| can only be acquired after // |MessagePipe::lock_|, never before. Thus to call into a // |ChannelEndpointClient|, a reference should be acquired from - // |local_id_to_endpoint_map_| under |lock_| and then the lock released. - base::Lock lock_; // Protects the members below. - - scoped_ptr<RawChannel> raw_channel_; - bool is_running_; + // |local_id_to_endpoint_map_| under |mutex_| and then the lock released. + // TODO(vtl): Annotate the above rule using |MOJO_ACQUIRED_{BEFORE,AFTER}()|, + // once clang actually checks such annotations. + // https://github.com/domokit/mojo/issues/313 + mutable Mutex mutex_; + + scoped_ptr<RawChannel> raw_channel_ MOJO_GUARDED_BY(mutex_); + bool is_running_ MOJO_GUARDED_BY(mutex_); // Set when |WillShutdownSoon()| is called. - bool is_shutting_down_; + bool is_shutting_down_ MOJO_GUARDED_BY(mutex_); // Has a reference to us. - ChannelManager* channel_manager_; + ChannelManager* channel_manager_ MOJO_GUARDED_BY(mutex_); using IdToEndpointMap = base::hash_map<ChannelEndpointId, scoped_refptr<ChannelEndpoint>>; // Map from local IDs to endpoints. If the endpoint is null, this means that // we're just waiting for the remove ack before removing the entry. - IdToEndpointMap local_id_to_endpoint_map_; + IdToEndpointMap local_id_to_endpoint_map_ MOJO_GUARDED_BY(mutex_); // Note: The IDs generated by this should be checked for existence before use. - LocalChannelEndpointIdGenerator local_id_generator_; + LocalChannelEndpointIdGenerator local_id_generator_ MOJO_GUARDED_BY(mutex_); using IdToIncomingEndpointMap = base::hash_map<ChannelEndpointId, scoped_refptr<IncomingEndpoint>>; // Map from local IDs to incoming endpoints (i.e., those received inside other // messages, but not yet claimed via |DeserializeEndpoint()|). - IdToIncomingEndpointMap incoming_endpoints_; + IdToIncomingEndpointMap incoming_endpoints_ MOJO_GUARDED_BY(mutex_); // TODO(vtl): We need to keep track of remote IDs (so that we don't collide // if/when we wrap). - RemoteChannelEndpointIdGenerator remote_id_generator_; + RemoteChannelEndpointIdGenerator remote_id_generator_ MOJO_GUARDED_BY(mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(Channel); }; diff --git a/third_party/mojo/src/mojo/edk/system/channel_endpoint.cc b/third_party/mojo/src/mojo/edk/system/channel_endpoint.cc index 113bbd58..038de8d 100644 --- a/third_party/mojo/src/mojo/edk/system/channel_endpoint.cc +++ b/third_party/mojo/src/mojo/edk/system/channel_endpoint.cc @@ -29,7 +29,7 @@ ChannelEndpoint::ChannelEndpoint(ChannelEndpointClient* client, bool ChannelEndpoint::EnqueueMessage(scoped_ptr<MessageInTransit> message) { DCHECK(message); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); switch (channel_state_) { case ChannelState::NOT_YET_ATTACHED: @@ -53,7 +53,7 @@ bool ChannelEndpoint::ReplaceClient(ChannelEndpointClient* client, unsigned client_port) { DCHECK(client); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); DCHECK(client_); DCHECK(client != client_.get() || client_port != client_port_); client_ = client; @@ -62,7 +62,7 @@ bool ChannelEndpoint::ReplaceClient(ChannelEndpointClient* client, } void ChannelEndpoint::DetachFromClient() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); DCHECK(client_); client_ = nullptr; @@ -79,7 +79,7 @@ void ChannelEndpoint::AttachAndRun(Channel* channel, DCHECK(local_id.is_valid()); DCHECK(remote_id.is_valid()); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); DCHECK(channel_state_ == ChannelState::NOT_YET_ATTACHED); DCHECK(!channel_); DCHECK(!local_id_.is_valid()); @@ -119,7 +119,7 @@ void ChannelEndpoint::DetachFromChannel() { scoped_refptr<ChannelEndpointClient> client; unsigned client_port = 0; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (client_) { // Take a ref, and call |OnDetachFromChannel()| outside the lock. @@ -155,7 +155,7 @@ ChannelEndpoint::~ChannelEndpoint() { bool ChannelEndpoint::WriteMessageNoLock(scoped_ptr<MessageInTransit> message) { DCHECK(message); - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(channel_); DCHECK(local_id_.is_valid()); @@ -187,7 +187,7 @@ void ChannelEndpoint::OnReadMessageForClient( // -- impose significant cost in the common case.) for (;;) { { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (!channel_ || !client_) { // This isn't a failure per se. (It just means that, e.g., the other end // of the message point closed first.) diff --git a/third_party/mojo/src/mojo/edk/system/channel_endpoint.h b/third_party/mojo/src/mojo/edk/system/channel_endpoint.h index 807b1a8b..c70e66e 100644 --- a/third_party/mojo/src/mojo/edk/system/channel_endpoint.h +++ b/third_party/mojo/src/mojo/edk/system/channel_endpoint.h @@ -7,9 +7,9 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/synchronization/lock.h" #include "mojo/edk/system/channel_endpoint_id.h" #include "mojo/edk/system/message_in_transit_queue.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/system_impl_export.h" #include "mojo/public/cpp/system/macros.h" @@ -85,7 +85,7 @@ class MessageInTransit; // shouldn't receive any more messages for that message pipe endpoint // (local ID), but it must wait for the endpoint to detach. (It can't // do so without a race, since it can't call into the message pipe -// under |lock_|.) [TODO(vtl): When I add remotely-allocated IDs, +// under |mutex_|.) [TODO(vtl): When I add remotely-allocated IDs, // we'll have to remove the |EndpointInfo| from // |local_id_to_endpoint_info_map_| -- i.e., remove the local ID, // since it's no longer valid and may be reused by the remote side -- @@ -162,19 +162,18 @@ class MOJO_SYSTEM_IMPL_EXPORT ChannelEndpoint final friend class base::RefCountedThreadSafe<ChannelEndpoint>; ~ChannelEndpoint(); - // Must be called with |lock_| held. - bool WriteMessageNoLock(scoped_ptr<MessageInTransit> message); + bool WriteMessageNoLock(scoped_ptr<MessageInTransit> message) + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); // Helper for |OnReadMessage()|, handling messages for the client. void OnReadMessageForClient(scoped_ptr<MessageInTransit> message); // Resets |channel_| to null (and sets |channel_state_| to // |ChannelState::DETACHED|). This may only be called if |channel_| is - // non-null. Must be called with |lock_| held. - void ResetChannelNoLock(); + // non-null. + void ResetChannelNoLock() MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); - // Protects the members below. - base::Lock lock_; + Mutex mutex_; // |client_| must be valid whenever it is non-null. Before |*client_| gives up // its reference to this object, it must call |DetachFromClient()|. @@ -182,14 +181,17 @@ class MOJO_SYSTEM_IMPL_EXPORT ChannelEndpoint final // |Channel| needs to keep the |MessagePipe| alive for the "proxy-proxy" case. // Possibly we'll be able to eliminate that case when we have full // multiprocess support. - // WARNING: |ChannelEndpointClient| methods must not be called under |lock_|. - // Thus to make such a call, a reference must first be taken under |lock_| and - // the lock released. + // WARNING: |ChannelEndpointClient| methods must not be called under |mutex_|. + // Thus to make such a call, a reference must first be taken under |mutex_| + // and the lock released. + // TODO(vtl): Annotate the above rule using |MOJO_ACQUIRED_{BEFORE,AFTER}()|, + // once clang actually checks such annotations. + // https://github.com/domokit/mojo/issues/313 // WARNING: Beware of interactions with |ReplaceClient()|. By the time the // call is made, the client may have changed. This must be detected and dealt // with. - scoped_refptr<ChannelEndpointClient> client_; - unsigned client_port_; + scoped_refptr<ChannelEndpointClient> client_ MOJO_GUARDED_BY(mutex_); + unsigned client_port_ MOJO_GUARDED_BY(mutex_); // State with respect to interaction with the |Channel|. enum class ChannelState { @@ -201,17 +203,17 @@ class MOJO_SYSTEM_IMPL_EXPORT ChannelEndpoint final // |DetachFromChannel()| has been called (|channel_| is null). DETACHED }; - ChannelState channel_state_; + ChannelState channel_state_ MOJO_GUARDED_BY(mutex_); // |channel_| must be valid whenever it is non-null. Before |*channel_| gives // up its reference to this object, it must call |DetachFromChannel()|. // |local_id_| and |remote_id_| are valid if and only |channel_| is non-null. - Channel* channel_; - ChannelEndpointId local_id_; - ChannelEndpointId remote_id_; + Channel* channel_ MOJO_GUARDED_BY(mutex_); + ChannelEndpointId local_id_ MOJO_GUARDED_BY(mutex_); + ChannelEndpointId remote_id_ MOJO_GUARDED_BY(mutex_); // This queue is used before we're running on a channel and ready to send // messages to the channel. - MessageInTransitQueue channel_message_queue_; + MessageInTransitQueue channel_message_queue_ MOJO_GUARDED_BY(mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(ChannelEndpoint); }; diff --git a/third_party/mojo/src/mojo/edk/system/channel_manager.cc b/third_party/mojo/src/mojo/edk/system/channel_manager.cc index 711b4da..a2e4bf9 100644 --- a/third_party/mojo/src/mojo/edk/system/channel_manager.cc +++ b/third_party/mojo/src/mojo/edk/system/channel_manager.cc @@ -57,7 +57,7 @@ void ChannelManager::ShutdownOnIOThread() { // consistency. ChannelIdToChannelMap channels; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); channels.swap(channels_); } @@ -110,7 +110,7 @@ scoped_refptr<MessagePipeDispatcher> ChannelManager::CreateChannel( } scoped_refptr<Channel> ChannelManager::GetChannel(ChannelId channel_id) const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); auto it = channels_.find(channel_id); DCHECK(it != channels_.end()); return it->second; @@ -123,7 +123,7 @@ void ChannelManager::WillShutdownChannel(ChannelId channel_id) { void ChannelManager::ShutdownChannelOnIOThread(ChannelId channel_id) { scoped_refptr<Channel> channel; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); auto it = channels_.find(channel_id); DCHECK(it != channels_.end()); channel.swap(it->second); @@ -138,7 +138,7 @@ void ChannelManager::ShutdownChannel( scoped_refptr<base::TaskRunner> callback_thread_task_runner) { scoped_refptr<Channel> channel; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); auto it = channels_.find(channel_id); DCHECK(it != channels_.end()); channel.swap(it->second); @@ -178,7 +178,7 @@ void ChannelManager::CreateChannelOnIOThreadHelper( channel->SetBootstrapEndpoint(bootstrap_channel_endpoint); { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); CHECK(channels_.find(channel_id) == channels_.end()); channels_[channel_id] = channel; } diff --git a/third_party/mojo/src/mojo/edk/system/channel_manager.h b/third_party/mojo/src/mojo/edk/system/channel_manager.h index 9a9c0e8..014c1b3 100644 --- a/third_party/mojo/src/mojo/edk/system/channel_manager.h +++ b/third_party/mojo/src/mojo/edk/system/channel_manager.h @@ -10,9 +10,9 @@ #include "base/callback_forward.h" #include "base/containers/hash_tables.h" #include "base/memory/ref_counted.h" -#include "base/synchronization/lock.h" #include "mojo/edk/embedder/scoped_platform_handle.h" #include "mojo/edk/system/channel_id.h" +#include "mojo/edk/system/mutex.h" #include "mojo/public/cpp/system/macros.h" namespace base { @@ -135,12 +135,15 @@ class MOJO_SYSTEM_IMPL_EXPORT ChannelManager { const scoped_refptr<base::TaskRunner> io_thread_task_runner_; ConnectionManager* const connection_manager_; - // Note: |Channel| methods should not be called under |lock_|. - mutable base::Lock lock_; // Protects the members below. + // Note: |Channel| methods should not be called under |mutex_|. + // TODO(vtl): Annotate the above rule using |MOJO_ACQUIRED_{BEFORE,AFTER}()|, + // once clang actually checks such annotations. + // https://github.com/domokit/mojo/issues/313 + mutable Mutex mutex_; using ChannelIdToChannelMap = base::hash_map<ChannelId, scoped_refptr<Channel>>; - ChannelIdToChannelMap channels_; + ChannelIdToChannelMap channels_ MOJO_GUARDED_BY(mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(ChannelManager); }; diff --git a/third_party/mojo/src/mojo/edk/system/connection_manager.h b/third_party/mojo/src/mojo/edk/system/connection_manager.h index c92a121..5923c75 100644 --- a/third_party/mojo/src/mojo/edk/system/connection_manager.h +++ b/third_party/mojo/src/mojo/edk/system/connection_manager.h @@ -5,9 +5,12 @@ #ifndef MOJO_EDK_SYSTEM_CONNECTION_MANAGER_H_ #define MOJO_EDK_SYSTEM_CONNECTION_MANAGER_H_ +#include <ostream> + #include "mojo/edk/system/connection_identifier.h" #include "mojo/edk/system/process_identifier.h" #include "mojo/edk/system/system_impl_export.h" +#include "mojo/edk/system/thread_annotations.h" #include "mojo/public/cpp/system/macros.h" namespace mojo { @@ -59,13 +62,23 @@ namespace system { // slave). class MOJO_SYSTEM_IMPL_EXPORT ConnectionManager { public: + enum class Result { + FAILURE = 0, + SUCCESS, + // These results are used for |Connect()| (which also uses |FAILURE|, but + // not |SUCCESS|). + SUCCESS_CONNECT_SAME_PROCESS, + SUCCESS_CONNECT_NEW_CONNECTION, + SUCCESS_CONNECT_REUSE_CONNECTION + }; + virtual ~ConnectionManager() {} ConnectionIdentifier GenerateConnectionIdentifier(); // Shuts down this connection manager. No other methods may be called after // this is (or while it is being) called. - virtual void Shutdown() = 0; + virtual void Shutdown() MOJO_NOT_THREAD_SAFE = 0; // TODO(vtl): Add a "get my own process identifier" method? @@ -85,14 +98,14 @@ class MOJO_SYSTEM_IMPL_EXPORT ConnectionManager { virtual bool CancelConnect(const ConnectionIdentifier& connection_id) = 0; // Connects a pending connection; to be called only after both parties have - // called |AllowConnect()|. On success, |peer_process_identifier| is set to an - // unique identifier for the peer process, and if the peer process is not the - // same as the calling process then |*platform_handle| is set to a suitable - // native handle connecting the two parties (if the two parties are the same - // process, then |*platform_handle| is reset to be invalid). - virtual bool Connect(const ConnectionIdentifier& connection_id, - ProcessIdentifier* peer_process_identifier, - embedder::ScopedPlatformHandle* platform_handle) = 0; + // called |AllowConnect()|. On success, |Result::SUCCESS_CONNECT_...| is + // returned and |peer_process_identifier| is set to an unique identifier for + // the peer process. In the case of |SUCCESS_CONNECT_SAME_PROCESS|, + // |*platform_handle| is set to a suitable native handle connecting the two + // parties. + virtual Result Connect(const ConnectionIdentifier& connection_id, + ProcessIdentifier* peer_process_identifier, + embedder::ScopedPlatformHandle* platform_handle) = 0; protected: // |platform_support| must be valid and remain alive until after |Shutdown()| @@ -106,6 +119,13 @@ class MOJO_SYSTEM_IMPL_EXPORT ConnectionManager { MOJO_DISALLOW_COPY_AND_ASSIGN(ConnectionManager); }; +// So logging macros and |DCHECK_EQ()|, etc. work. +MOJO_SYSTEM_IMPL_EXPORT inline std::ostream& operator<<( + std::ostream& out, + ConnectionManager::Result result) { + return out << static_cast<int>(result); +} + } // namespace system } // namespace mojo diff --git a/third_party/mojo/src/mojo/edk/system/connection_manager_unittest.cc b/third_party/mojo/src/mojo/edk/system/connection_manager_unittest.cc index cb22ef3..20fa73e 100644 --- a/third_party/mojo/src/mojo/edk/system/connection_manager_unittest.cc +++ b/third_party/mojo/src/mojo/edk/system/connection_manager_unittest.cc @@ -226,12 +226,14 @@ TEST_F(ConnectionManagerTest, BasicConnectSlaves) { ProcessIdentifier peer1 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h1; - EXPECT_TRUE(slave1.Connect(connection_id, &peer1, &h1)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, + slave1.Connect(connection_id, &peer1, &h1)); EXPECT_EQ(slave2_id, peer1); EXPECT_TRUE(h1.is_valid()); ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_TRUE(slave2.Connect(connection_id, &peer2, &h2)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, + slave2.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(slave1_id, peer2); EXPECT_TRUE(h2.is_valid()); @@ -319,7 +321,8 @@ TEST_F(ConnectionManagerTest, SlaveCancelConnect) { EXPECT_TRUE(slave1.CancelConnect(connection_id)); ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_FALSE(slave2.Connect(connection_id, &peer2, &h2)); + EXPECT_EQ(ConnectionManager::Result::FAILURE, + slave2.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(kInvalidProcessIdentifier, peer2); EXPECT_FALSE(h2.is_valid()); @@ -361,7 +364,8 @@ TEST_F(ConnectionManagerTest, ErrorRemovePending) { ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_FALSE(slave2.Connect(connection_id, &peer2, &h2)); + EXPECT_EQ(ConnectionManager::Result::FAILURE, + slave2.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(kInvalidProcessIdentifier, peer2); EXPECT_FALSE(h2.is_valid()); @@ -384,16 +388,16 @@ TEST_F(ConnectionManagerTest, ConnectSlaveToSelf) { EXPECT_TRUE(slave.AllowConnect(connection_id)); EXPECT_TRUE(slave.AllowConnect(connection_id)); - // Currently, the connect-to-self case is signalled by the master not sending - // back a handle. ProcessIdentifier peer1 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h1; - EXPECT_TRUE(slave.Connect(connection_id, &peer1, &h1)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_SAME_PROCESS, + slave.Connect(connection_id, &peer1, &h1)); EXPECT_EQ(slave_id, peer1); EXPECT_FALSE(h1.is_valid()); ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_TRUE(slave.Connect(connection_id, &peer2, &h2)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_SAME_PROCESS, + slave.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(slave_id, peer2); EXPECT_FALSE(h2.is_valid()); @@ -425,30 +429,36 @@ TEST_F(ConnectionManagerTest, ConnectSlavesTwice) { ProcessIdentifier peer1 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h1; - EXPECT_TRUE(slave1.Connect(connection_id, &peer1, &h1)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, + slave1.Connect(connection_id, &peer1, &h1)); EXPECT_EQ(slave2_id, peer1); ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_TRUE(slave2.Connect(connection_id, &peer2, &h2)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, + slave2.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(slave1_id, peer2); EXPECT_TRUE(ArePlatformHandlesConnected(h1.get(), h2.get())); - // Currently, the master doesn't detect the case of connecting a pair of - // slaves that are already connected. (Doing so would require more careful - // tracking and is prone to races -- especially if we want slaves to be able - // to tear down no-longer-needed connections.) But the slaves should be able - // to do the tracking themselves (using the peer process identifiers). + // TODO(vtl): Currently, the master doesn't detect the case of connecting a + // pair of slaves that are already connected. (Doing so would require more + // careful tracking and is prone to races -- especially if we want slaves to + // be able to tear down no-longer-needed connections.) But the slaves should + // be able to do the tracking themselves (using the peer process identifiers). connection_id = master.GenerateConnectionIdentifier(); EXPECT_TRUE(slave1.AllowConnect(connection_id)); EXPECT_TRUE(slave2.AllowConnect(connection_id)); h1.reset(); h2.reset(); + // TODO(vtl): FIXME -- this will break when I implemented + // SUCCESS_CONNECT_REUSE_CONNECTION. ProcessIdentifier second_peer2 = kInvalidProcessIdentifier; - EXPECT_TRUE(slave2.Connect(connection_id, &second_peer2, &h2)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, + slave2.Connect(connection_id, &second_peer2, &h2)); ProcessIdentifier second_peer1 = kInvalidProcessIdentifier; - EXPECT_TRUE(slave1.Connect(connection_id, &second_peer1, &h1)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, + slave1.Connect(connection_id, &second_peer1, &h1)); EXPECT_EQ(peer1, second_peer1); EXPECT_EQ(peer2, second_peer2); @@ -476,12 +486,14 @@ TEST_F(ConnectionManagerTest, ConnectMasterToSlave) { ProcessIdentifier master_peer = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle master_h; - EXPECT_TRUE(master.Connect(connection_id, &master_peer, &master_h)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, + master.Connect(connection_id, &master_peer, &master_h)); EXPECT_EQ(slave_id, master_peer); EXPECT_TRUE(master_h.is_valid()); ProcessIdentifier slave_peer = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle slave_h; - EXPECT_TRUE(slave.Connect(connection_id, &slave_peer, &slave_h)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, + slave.Connect(connection_id, &slave_peer, &slave_h)); EXPECT_EQ(kMasterProcessIdentifier, slave_peer); EXPECT_TRUE(slave_h.is_valid()); @@ -500,16 +512,16 @@ TEST_F(ConnectionManagerTest, ConnectMasterToSelf) { EXPECT_TRUE(master.AllowConnect(connection_id)); EXPECT_TRUE(master.AllowConnect(connection_id)); - // Currently, the connect-to-self case is signalled by the master not sending - // back a handle. ProcessIdentifier peer1 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h1; - EXPECT_TRUE(master.Connect(connection_id, &peer1, &h1)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_SAME_PROCESS, + master.Connect(connection_id, &peer1, &h1)); EXPECT_EQ(kMasterProcessIdentifier, peer1); EXPECT_FALSE(h1.is_valid()); ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_TRUE(master.Connect(connection_id, &peer2, &h2)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_SAME_PROCESS, + master.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(kMasterProcessIdentifier, peer2); EXPECT_FALSE(h2.is_valid()); @@ -536,7 +548,8 @@ TEST_F(ConnectionManagerTest, MasterCancelConnect) { EXPECT_TRUE(master.CancelConnect(connection_id)); ProcessIdentifier peer = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h; - EXPECT_FALSE(slave.Connect(connection_id, &peer, &h)); + EXPECT_EQ(ConnectionManager::Result::FAILURE, + slave.Connect(connection_id, &peer, &h)); EXPECT_EQ(kInvalidProcessIdentifier, peer); EXPECT_FALSE(h.is_valid()); @@ -573,7 +586,8 @@ TEST_F(ConnectionManagerTest, AddSlaveAndBootstrap) { embedder::ScopedPlatformHandle h1; ProcessIdentifier master_peer = kInvalidProcessIdentifier; - EXPECT_TRUE(master.Connect(connection_id, &master_peer, &h1)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, + master.Connect(connection_id, &master_peer, &h1)); EXPECT_EQ(slave_id, master_peer); EXPECT_TRUE(h1.is_valid()); @@ -585,7 +599,8 @@ TEST_F(ConnectionManagerTest, AddSlaveAndBootstrap) { ProcessIdentifier slave_peer = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_TRUE(slave.Connect(connection_id, &slave_peer, &h2)); + EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, + slave.Connect(connection_id, &slave_peer, &h2)); EXPECT_EQ(kMasterProcessIdentifier, slave_peer); EXPECT_TRUE(ArePlatformHandlesConnected(h1.get(), h2.get())); diff --git a/third_party/mojo/src/mojo/edk/system/core.cc b/third_party/mojo/src/mojo/edk/system/core.cc index 35d6614..fbb6cf1 100644 --- a/third_party/mojo/src/mojo/edk/system/core.cc +++ b/third_party/mojo/src/mojo/edk/system/core.cc @@ -86,7 +86,7 @@ Core::~Core() { } MojoHandle Core::AddDispatcher(const scoped_refptr<Dispatcher>& dispatcher) { - base::AutoLock locker(handle_table_lock_); + MutexLocker locker(&handle_table_mutex_); return handle_table_.AddDispatcher(dispatcher); } @@ -94,7 +94,7 @@ scoped_refptr<Dispatcher> Core::GetDispatcher(MojoHandle handle) { if (handle == MOJO_HANDLE_INVALID) return nullptr; - base::AutoLock locker(handle_table_lock_); + MutexLocker locker(&handle_table_mutex_); return handle_table_.GetDispatcher(handle); } @@ -103,7 +103,7 @@ MojoResult Core::GetAndRemoveDispatcher(MojoHandle handle, if (handle == MOJO_HANDLE_INVALID) return MOJO_RESULT_INVALID_ARGUMENT; - base::AutoLock locker(handle_table_lock_); + MutexLocker locker(&handle_table_mutex_); return handle_table_.GetAndRemoveDispatcher(handle, dispatcher); } @@ -130,7 +130,7 @@ MojoResult Core::Close(MojoHandle handle) { scoped_refptr<Dispatcher> dispatcher; { - base::AutoLock locker(handle_table_lock_); + MutexLocker locker(&handle_table_mutex_); MojoResult result = handle_table_.GetAndRemoveDispatcher(handle, &dispatcher); if (result != MOJO_RESULT_OK) @@ -138,7 +138,7 @@ MojoResult Core::Close(MojoHandle handle) { } // The dispatcher doesn't have a say in being closed, but gets notified of it. - // Note: This is done outside of |handle_table_lock_|. As a result, there's a + // Note: This is done outside of |handle_table_mutex_|. As a result, there's a // race condition that the dispatcher must handle; see the comment in // |Dispatcher| in dispatcher.h. return dispatcher->Close(); @@ -211,7 +211,7 @@ MojoResult Core::CreateMessagePipe( std::pair<MojoHandle, MojoHandle> handle_pair; { - base::AutoLock locker(handle_table_lock_); + MutexLocker locker(&handle_table_mutex_); handle_pair = handle_table_.AddDispatcherPair(dispatcher0, dispatcher1); } if (handle_pair.first == MOJO_HANDLE_INVALID) { @@ -276,7 +276,7 @@ MojoResult Core::WriteMessage(MojoHandle message_pipe_handle, // and mark the handles as busy. If the call succeeds, we then remove the // handles from the handle table. { - base::AutoLock locker(handle_table_lock_); + MutexLocker locker(&handle_table_mutex_); MojoResult result = handle_table_.MarkBusyAndStartTransport( message_pipe_handle, handles_reader.GetPointer(), num_handles, &transports); @@ -293,7 +293,7 @@ MojoResult Core::WriteMessage(MojoHandle message_pipe_handle, transports[i].End(); { - base::AutoLock locker(handle_table_lock_); + MutexLocker locker(&handle_table_mutex_); if (rv == MOJO_RESULT_OK) { handle_table_.RemoveBusyHandles(handles_reader.GetPointer(), num_handles); } else { @@ -335,7 +335,7 @@ MojoResult Core::ReadMessage(MojoHandle message_pipe_handle, UserPointer<MojoHandle>::Writer handles_writer(handles, dispatchers.size()); { - base::AutoLock locker(handle_table_lock_); + MutexLocker locker(&handle_table_mutex_); success = handle_table_.AddDispatcherVector( dispatchers, handles_writer.GetPointer()); } @@ -377,7 +377,7 @@ MojoResult Core::CreateDataPipe( std::pair<MojoHandle, MojoHandle> handle_pair; { - base::AutoLock locker(handle_table_lock_); + MutexLocker locker(&handle_table_mutex_); handle_pair = handle_table_.AddDispatcherPair(producer_dispatcher, consumer_dispatcher); } @@ -539,7 +539,7 @@ MojoResult Core::MapBuffer(MojoHandle buffer_handle, DCHECK(mapping); void* address = mapping->GetBase(); { - base::AutoLock locker(mapping_table_lock_); + MutexLocker locker(&mapping_table_mutex_); result = mapping_table_.AddMapping(mapping.Pass()); } if (result != MOJO_RESULT_OK) @@ -550,7 +550,7 @@ MojoResult Core::MapBuffer(MojoHandle buffer_handle, } MojoResult Core::UnmapBuffer(UserPointer<void> buffer) { - base::AutoLock locker(mapping_table_lock_); + MutexLocker locker(&mapping_table_mutex_); return mapping_table_.RemoveMapping(buffer.GetPointerValue()); } diff --git a/third_party/mojo/src/mojo/edk/system/core.h b/third_party/mojo/src/mojo/edk/system/core.h index 407a064..4ed5ede 100644 --- a/third_party/mojo/src/mojo/edk/system/core.h +++ b/third_party/mojo/src/mojo/edk/system/core.h @@ -10,10 +10,10 @@ #include "base/callback.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/synchronization/lock.h" #include "mojo/edk/system/handle_table.h" #include "mojo/edk/system/mapping_table.h" #include "mojo/edk/system/memory.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/system_impl_export.h" #include "mojo/public/c/system/buffer.h" #include "mojo/public/c/system/data_pipe.h" @@ -175,13 +175,13 @@ class MOJO_SYSTEM_IMPL_EXPORT Core { embedder::PlatformSupport* const platform_support_; - // TODO(vtl): |handle_table_lock_| should be a reader-writer lock (if only we + // TODO(vtl): |handle_table_mutex_| should be a reader-writer lock (if only we // had them). - base::Lock handle_table_lock_; // Protects |handle_table_|. - HandleTable handle_table_; + Mutex handle_table_mutex_; + HandleTable handle_table_ MOJO_GUARDED_BY(handle_table_mutex_); - base::Lock mapping_table_lock_; // Protects |mapping_table_|. - MappingTable mapping_table_; + Mutex mapping_table_mutex_; + MappingTable mapping_table_ MOJO_GUARDED_BY(mapping_table_mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(Core); }; diff --git a/third_party/mojo/src/mojo/edk/system/core_test_base.cc b/third_party/mojo/src/mojo/edk/system/core_test_base.cc index bfdcb1b..2552fd2 100644 --- a/third_party/mojo/src/mojo/edk/system/core_test_base.cc +++ b/third_party/mojo/src/mojo/edk/system/core_test_base.cc @@ -43,7 +43,7 @@ class MockDispatcher : public Dispatcher { // |Dispatcher| protected methods: void CloseImplNoLock() override { info_->IncrementCloseCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); } MojoResult WriteMessageImplNoLock( @@ -52,7 +52,7 @@ class MockDispatcher : public Dispatcher { std::vector<DispatcherTransport>* transports, MojoWriteMessageFlags /*flags*/) override { info_->IncrementWriteMessageCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); if (num_bytes > GetConfiguration().max_message_num_bytes) return MOJO_RESULT_RESOURCE_EXHAUSTED; @@ -69,7 +69,7 @@ class MockDispatcher : public Dispatcher { uint32_t* num_dispatchers, MojoReadMessageFlags /*flags*/) override { info_->IncrementReadMessageCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); if (num_dispatchers) { *num_dispatchers = 1; @@ -86,7 +86,7 @@ class MockDispatcher : public Dispatcher { UserPointer<uint32_t> /*num_bytes*/, MojoWriteDataFlags /*flags*/) override { info_->IncrementWriteDataCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); return MOJO_RESULT_UNIMPLEMENTED; } @@ -95,13 +95,13 @@ class MockDispatcher : public Dispatcher { UserPointer<uint32_t> /*buffer_num_bytes*/, MojoWriteDataFlags /*flags*/) override { info_->IncrementBeginWriteDataCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); return MOJO_RESULT_UNIMPLEMENTED; } MojoResult EndWriteDataImplNoLock(uint32_t /*num_bytes_written*/) override { info_->IncrementEndWriteDataCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); return MOJO_RESULT_UNIMPLEMENTED; } @@ -109,7 +109,7 @@ class MockDispatcher : public Dispatcher { UserPointer<uint32_t> /*num_bytes*/, MojoReadDataFlags /*flags*/) override { info_->IncrementReadDataCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); return MOJO_RESULT_UNIMPLEMENTED; } @@ -117,13 +117,13 @@ class MockDispatcher : public Dispatcher { UserPointer<uint32_t> /*buffer_num_bytes*/, MojoReadDataFlags /*flags*/) override { info_->IncrementBeginReadDataCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); return MOJO_RESULT_UNIMPLEMENTED; } MojoResult EndReadDataImplNoLock(uint32_t /*num_bytes_read*/) override { info_->IncrementEndReadDataCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); return MOJO_RESULT_UNIMPLEMENTED; } @@ -132,7 +132,7 @@ class MockDispatcher : public Dispatcher { uint32_t /*context*/, HandleSignalsState* signals_state) override { info_->IncrementAddAwakableCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); if (signals_state) *signals_state = HandleSignalsState(); if (info_->IsAddAwakableAllowed()) { @@ -146,14 +146,14 @@ class MockDispatcher : public Dispatcher { void RemoveAwakableImplNoLock(Awakable* /*awakable*/, HandleSignalsState* signals_state) override { info_->IncrementRemoveAwakableCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); if (signals_state) *signals_state = HandleSignalsState(); } void CancelAllAwakablesNoLock() override { info_->IncrementCancelAllAwakablesCallCount(); - lock().AssertAcquired(); + mutex().AssertHeld(); } scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndCloseImplNoLock() @@ -215,167 +215,167 @@ CoreTestBase_MockHandleInfo::~CoreTestBase_MockHandleInfo() { } unsigned CoreTestBase_MockHandleInfo::GetCtorCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return ctor_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetDtorCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return dtor_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetCloseCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return close_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetWriteMessageCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return write_message_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetReadMessageCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return read_message_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetWriteDataCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return write_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetBeginWriteDataCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return begin_write_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetEndWriteDataCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return end_write_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetReadDataCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return read_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetBeginReadDataCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return begin_read_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetEndReadDataCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return end_read_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetAddAwakableCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return add_awakable_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetRemoveAwakableCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return remove_awakable_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetCancelAllAwakablesCallCount() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return cancel_all_awakables_call_count_; } size_t CoreTestBase_MockHandleInfo::GetAddedAwakableSize() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return added_awakables_.size(); } Awakable* CoreTestBase_MockHandleInfo::GetAddedAwakableAt(unsigned i) const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return added_awakables_[i]; } void CoreTestBase_MockHandleInfo::IncrementCtorCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); ctor_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementDtorCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); dtor_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementCloseCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); close_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementWriteMessageCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); write_message_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementReadMessageCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); read_message_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementWriteDataCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); write_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementBeginWriteDataCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); begin_write_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementEndWriteDataCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); end_write_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementReadDataCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); read_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementBeginReadDataCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); begin_read_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementEndReadDataCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); end_read_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementAddAwakableCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); add_awakable_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementRemoveAwakableCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); remove_awakable_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementCancelAllAwakablesCallCount() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); cancel_all_awakables_call_count_++; } void CoreTestBase_MockHandleInfo::AllowAddAwakable(bool alllow) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); add_awakable_allowed_ = alllow; } bool CoreTestBase_MockHandleInfo::IsAddAwakableAllowed() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return add_awakable_allowed_; } void CoreTestBase_MockHandleInfo::AwakableWasAdded(Awakable* awakable) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); added_awakables_.push_back(awakable); } diff --git a/third_party/mojo/src/mojo/edk/system/core_test_base.h b/third_party/mojo/src/mojo/edk/system/core_test_base.h index ce32918..e2375cd 100644 --- a/third_party/mojo/src/mojo/edk/system/core_test_base.h +++ b/third_party/mojo/src/mojo/edk/system/core_test_base.h @@ -5,8 +5,8 @@ #ifndef MOJO_EDK_SYSTEM_CORE_TEST_BASE_H_ #define MOJO_EDK_SYSTEM_CORE_TEST_BASE_H_ -#include "base/synchronization/lock.h" #include "mojo/edk/embedder/simple_platform_support.h" +#include "mojo/edk/system/mutex.h" #include "mojo/public/c/system/types.h" #include "mojo/public/cpp/system/macros.h" #include "testing/gtest/include/gtest/gtest.h" @@ -88,24 +88,24 @@ class CoreTestBase_MockHandleInfo { void AwakableWasAdded(Awakable*); private: - mutable base::Lock lock_; // Protects the following members. - unsigned ctor_call_count_; - unsigned dtor_call_count_; - unsigned close_call_count_; - unsigned write_message_call_count_; - unsigned read_message_call_count_; - unsigned write_data_call_count_; - unsigned begin_write_data_call_count_; - unsigned end_write_data_call_count_; - unsigned read_data_call_count_; - unsigned begin_read_data_call_count_; - unsigned end_read_data_call_count_; - unsigned add_awakable_call_count_; - unsigned remove_awakable_call_count_; - unsigned cancel_all_awakables_call_count_; - - bool add_awakable_allowed_; - std::vector<Awakable*> added_awakables_; + mutable Mutex mutex_; + unsigned ctor_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned dtor_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned close_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned write_message_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned read_message_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned write_data_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned begin_write_data_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned end_write_data_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned read_data_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned begin_read_data_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned end_read_data_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned add_awakable_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned remove_awakable_call_count_ MOJO_GUARDED_BY(mutex_); + unsigned cancel_all_awakables_call_count_ MOJO_GUARDED_BY(mutex_); + + bool add_awakable_allowed_ MOJO_GUARDED_BY(mutex_); + std::vector<Awakable*> added_awakables_ MOJO_GUARDED_BY(mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(CoreTestBase_MockHandleInfo); }; diff --git a/third_party/mojo/src/mojo/edk/system/data_pipe_consumer_dispatcher.cc b/third_party/mojo/src/mojo/edk/system/data_pipe_consumer_dispatcher.cc index ac917d9..5a1bafb 100644 --- a/third_party/mojo/src/mojo/edk/system/data_pipe_consumer_dispatcher.cc +++ b/third_party/mojo/src/mojo/edk/system/data_pipe_consumer_dispatcher.cc @@ -35,6 +35,11 @@ DataPipeConsumerDispatcher::Deserialize(Channel* channel, return dispatcher; } +DataPipe* DataPipeConsumerDispatcher::GetDataPipeForTest() { + MutexLocker locker(&mutex()); + return data_pipe_.get(); +} + DataPipeConsumerDispatcher::DataPipeConsumerDispatcher() { } @@ -44,19 +49,19 @@ DataPipeConsumerDispatcher::~DataPipeConsumerDispatcher() { } void DataPipeConsumerDispatcher::CancelAllAwakablesNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); data_pipe_->ConsumerCancelAllAwakables(); } void DataPipeConsumerDispatcher::CloseImplNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); data_pipe_->ConsumerClose(); data_pipe_ = nullptr; } scoped_refptr<Dispatcher> DataPipeConsumerDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); scoped_refptr<DataPipeConsumerDispatcher> rv = Create(); rv->Init(data_pipe_); @@ -68,7 +73,7 @@ MojoResult DataPipeConsumerDispatcher::ReadDataImplNoLock( UserPointer<void> elements, UserPointer<uint32_t> num_bytes, MojoReadDataFlags flags) { - lock().AssertAcquired(); + mutex().AssertHeld(); if ((flags & MOJO_READ_DATA_FLAG_DISCARD)) { // These flags are mutally exclusive. @@ -99,7 +104,7 @@ MojoResult DataPipeConsumerDispatcher::BeginReadDataImplNoLock( UserPointer<const void*> buffer, UserPointer<uint32_t> buffer_num_bytes, MojoReadDataFlags flags) { - lock().AssertAcquired(); + mutex().AssertHeld(); // These flags may not be used in two-phase mode. if ((flags & MOJO_READ_DATA_FLAG_DISCARD) || @@ -112,14 +117,14 @@ MojoResult DataPipeConsumerDispatcher::BeginReadDataImplNoLock( MojoResult DataPipeConsumerDispatcher::EndReadDataImplNoLock( uint32_t num_bytes_read) { - lock().AssertAcquired(); + mutex().AssertHeld(); return data_pipe_->ConsumerEndReadData(num_bytes_read); } HandleSignalsState DataPipeConsumerDispatcher::GetHandleSignalsStateImplNoLock() const { - lock().AssertAcquired(); + mutex().AssertHeld(); return data_pipe_->ConsumerGetHandleSignalsState(); } @@ -128,7 +133,7 @@ MojoResult DataPipeConsumerDispatcher::AddAwakableImplNoLock( MojoHandleSignals signals, uint32_t context, HandleSignalsState* signals_state) { - lock().AssertAcquired(); + mutex().AssertHeld(); return data_pipe_->ConsumerAddAwakable(awakable, signals, context, signals_state); } @@ -136,7 +141,7 @@ MojoResult DataPipeConsumerDispatcher::AddAwakableImplNoLock( void DataPipeConsumerDispatcher::RemoveAwakableImplNoLock( Awakable* awakable, HandleSignalsState* signals_state) { - lock().AssertAcquired(); + mutex().AssertHeld(); data_pipe_->ConsumerRemoveAwakable(awakable, signals_state); } @@ -162,7 +167,7 @@ bool DataPipeConsumerDispatcher::EndSerializeAndCloseImplNoLock( } bool DataPipeConsumerDispatcher::IsBusyNoLock() const { - lock().AssertAcquired(); + mutex().AssertHeld(); return data_pipe_->ConsumerIsBusy(); } diff --git a/third_party/mojo/src/mojo/edk/system/data_pipe_consumer_dispatcher.h b/third_party/mojo/src/mojo/edk/system/data_pipe_consumer_dispatcher.h index 7d8b5ee..29df228e 100644 --- a/third_party/mojo/src/mojo/edk/system/data_pipe_consumer_dispatcher.h +++ b/third_party/mojo/src/mojo/edk/system/data_pipe_consumer_dispatcher.h @@ -26,7 +26,7 @@ class MOJO_SYSTEM_IMPL_EXPORT DataPipeConsumerDispatcher final } // Must be called before any other methods. - void Init(scoped_refptr<DataPipe> data_pipe); + void Init(scoped_refptr<DataPipe> data_pipe) MOJO_NOT_THREAD_SAFE; // |Dispatcher| public methods: Type GetType() const override; @@ -37,7 +37,7 @@ class MOJO_SYSTEM_IMPL_EXPORT DataPipeConsumerDispatcher final Deserialize(Channel* channel, const void* source, size_t size); // Get access to the |DataPipe| for testing. - DataPipe* GetDataPipeForTest() { return data_pipe_.get(); } + DataPipe* GetDataPipeForTest(); private: DataPipeConsumerDispatcher(); @@ -64,16 +64,18 @@ class MOJO_SYSTEM_IMPL_EXPORT DataPipeConsumerDispatcher final HandleSignalsState* signals_state) override; void StartSerializeImplNoLock(Channel* channel, size_t* max_size, - size_t* max_platform_handles) override; + size_t* max_platform_handles) override + MOJO_NOT_THREAD_SAFE; bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) override; + embedder::PlatformHandleVector* platform_handles) override + MOJO_NOT_THREAD_SAFE; bool IsBusyNoLock() const override; - // Protected by |lock()|: - scoped_refptr<DataPipe> data_pipe_; // This will be null if closed. + // This will be null if closed. + scoped_refptr<DataPipe> data_pipe_ MOJO_GUARDED_BY(mutex()); MOJO_DISALLOW_COPY_AND_ASSIGN(DataPipeConsumerDispatcher); }; diff --git a/third_party/mojo/src/mojo/edk/system/data_pipe_impl_unittest.cc b/third_party/mojo/src/mojo/edk/system/data_pipe_impl_unittest.cc index 2846f8d..3b0470b 100644 --- a/third_party/mojo/src/mojo/edk/system/data_pipe_impl_unittest.cc +++ b/third_party/mojo/src/mojo/edk/system/data_pipe_impl_unittest.cc @@ -1647,7 +1647,7 @@ TYPED_TEST(DataPipeImplTest, AllOrNone) { this->ConsumerClose(); } -TYPED_TEST(DataPipeImplTest, DISABLED_TwoPhaseAllOrNone) { +TYPED_TEST(DataPipeImplTest, TwoPhaseAllOrNone) { const MojoCreateDataPipeOptions options = { kSizeOfOptions, // |struct_size|. MOJO_CREATE_DATA_PIPE_OPTIONS_FLAG_NONE, // |flags|. diff --git a/third_party/mojo/src/mojo/edk/system/data_pipe_producer_dispatcher.cc b/third_party/mojo/src/mojo/edk/system/data_pipe_producer_dispatcher.cc index 88190cf..ddb8b9f 100644 --- a/third_party/mojo/src/mojo/edk/system/data_pipe_producer_dispatcher.cc +++ b/third_party/mojo/src/mojo/edk/system/data_pipe_producer_dispatcher.cc @@ -35,6 +35,11 @@ DataPipeProducerDispatcher::Deserialize(Channel* channel, return dispatcher; } +DataPipe* DataPipeProducerDispatcher::GetDataPipeForTest() { + MutexLocker locker(&mutex()); + return data_pipe_.get(); +} + DataPipeProducerDispatcher::DataPipeProducerDispatcher() { } @@ -44,19 +49,19 @@ DataPipeProducerDispatcher::~DataPipeProducerDispatcher() { } void DataPipeProducerDispatcher::CancelAllAwakablesNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); data_pipe_->ProducerCancelAllAwakables(); } void DataPipeProducerDispatcher::CloseImplNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); data_pipe_->ProducerClose(); data_pipe_ = nullptr; } scoped_refptr<Dispatcher> DataPipeProducerDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); scoped_refptr<DataPipeProducerDispatcher> rv = Create(); rv->Init(data_pipe_); @@ -68,7 +73,7 @@ MojoResult DataPipeProducerDispatcher::WriteDataImplNoLock( UserPointer<const void> elements, UserPointer<uint32_t> num_bytes, MojoWriteDataFlags flags) { - lock().AssertAcquired(); + mutex().AssertHeld(); return data_pipe_->ProducerWriteData( elements, num_bytes, (flags & MOJO_WRITE_DATA_FLAG_ALL_OR_NONE)); } @@ -77,7 +82,7 @@ MojoResult DataPipeProducerDispatcher::BeginWriteDataImplNoLock( UserPointer<void*> buffer, UserPointer<uint32_t> buffer_num_bytes, MojoWriteDataFlags flags) { - lock().AssertAcquired(); + mutex().AssertHeld(); return data_pipe_->ProducerBeginWriteData( buffer, buffer_num_bytes, (flags & MOJO_WRITE_DATA_FLAG_ALL_OR_NONE)); @@ -85,14 +90,14 @@ MojoResult DataPipeProducerDispatcher::BeginWriteDataImplNoLock( MojoResult DataPipeProducerDispatcher::EndWriteDataImplNoLock( uint32_t num_bytes_written) { - lock().AssertAcquired(); + mutex().AssertHeld(); return data_pipe_->ProducerEndWriteData(num_bytes_written); } HandleSignalsState DataPipeProducerDispatcher::GetHandleSignalsStateImplNoLock() const { - lock().AssertAcquired(); + mutex().AssertHeld(); return data_pipe_->ProducerGetHandleSignalsState(); } @@ -101,7 +106,7 @@ MojoResult DataPipeProducerDispatcher::AddAwakableImplNoLock( MojoHandleSignals signals, uint32_t context, HandleSignalsState* signals_state) { - lock().AssertAcquired(); + mutex().AssertHeld(); return data_pipe_->ProducerAddAwakable(awakable, signals, context, signals_state); } @@ -109,7 +114,7 @@ MojoResult DataPipeProducerDispatcher::AddAwakableImplNoLock( void DataPipeProducerDispatcher::RemoveAwakableImplNoLock( Awakable* awakable, HandleSignalsState* signals_state) { - lock().AssertAcquired(); + mutex().AssertHeld(); data_pipe_->ProducerRemoveAwakable(awakable, signals_state); } @@ -135,7 +140,7 @@ bool DataPipeProducerDispatcher::EndSerializeAndCloseImplNoLock( } bool DataPipeProducerDispatcher::IsBusyNoLock() const { - lock().AssertAcquired(); + mutex().AssertHeld(); return data_pipe_->ProducerIsBusy(); } diff --git a/third_party/mojo/src/mojo/edk/system/data_pipe_producer_dispatcher.h b/third_party/mojo/src/mojo/edk/system/data_pipe_producer_dispatcher.h index 86697fe..7ce5156 100644 --- a/third_party/mojo/src/mojo/edk/system/data_pipe_producer_dispatcher.h +++ b/third_party/mojo/src/mojo/edk/system/data_pipe_producer_dispatcher.h @@ -26,7 +26,7 @@ class MOJO_SYSTEM_IMPL_EXPORT DataPipeProducerDispatcher final } // Must be called before any other methods. - void Init(scoped_refptr<DataPipe> data_pipe); + void Init(scoped_refptr<DataPipe> data_pipe) MOJO_NOT_THREAD_SAFE; // |Dispatcher| public methods: Type GetType() const override; @@ -37,7 +37,7 @@ class MOJO_SYSTEM_IMPL_EXPORT DataPipeProducerDispatcher final Deserialize(Channel* channel, const void* source, size_t size); // Get access to the |DataPipe| for testing. - DataPipe* GetDataPipeForTest() { return data_pipe_.get(); } + DataPipe* GetDataPipeForTest(); private: DataPipeProducerDispatcher(); @@ -64,16 +64,18 @@ class MOJO_SYSTEM_IMPL_EXPORT DataPipeProducerDispatcher final HandleSignalsState* signals_state) override; void StartSerializeImplNoLock(Channel* channel, size_t* max_size, - size_t* max_platform_handles) override; + size_t* max_platform_handles) override + MOJO_NOT_THREAD_SAFE; bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) override; + embedder::PlatformHandleVector* platform_handles) override + MOJO_NOT_THREAD_SAFE; bool IsBusyNoLock() const override; - // Protected by |lock()|: - scoped_refptr<DataPipe> data_pipe_; // This will be null if closed. + // This will be null if closed. + scoped_refptr<DataPipe> data_pipe_ MOJO_GUARDED_BY(mutex()); MOJO_DISALLOW_COPY_AND_ASSIGN(DataPipeProducerDispatcher); }; diff --git a/third_party/mojo/src/mojo/edk/system/dispatcher.cc b/third_party/mojo/src/mojo/edk/system/dispatcher.cc index 24f2c26..ad362fe 100644 --- a/third_party/mojo/src/mojo/edk/system/dispatcher.cc +++ b/third_party/mojo/src/mojo/edk/system/dispatcher.cc @@ -26,16 +26,18 @@ DispatcherTransport DispatcherTryStartTransport(Dispatcher* dispatcher) { // Dispatcher ------------------------------------------------------------------ +// TODO(vtl): The thread-safety analyzer isn't smart enough to deal with the +// fact that we give up if |TryLock()| fails. // static DispatcherTransport Dispatcher::HandleTableAccess::TryStartTransport( - Dispatcher* dispatcher) { + Dispatcher* dispatcher) MOJO_NO_THREAD_SAFETY_ANALYSIS { DCHECK(dispatcher); - if (!dispatcher->lock_.Try()) + if (!dispatcher->mutex_.TryLock()) return DispatcherTransport(); // We shouldn't race with things that close dispatchers, since closing can - // only take place either under |handle_table_lock_| or when the handle is + // only take place either under |handle_table_mutex_| or when the handle is // marked as busy. DCHECK(!dispatcher->is_closed_); @@ -96,7 +98,7 @@ scoped_refptr<Dispatcher> Dispatcher::TransportDataAccess::Deserialize( } MojoResult Dispatcher::Close() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -113,7 +115,7 @@ MojoResult Dispatcher::WriteMessage( (transports->size() > 0 && transports->size() < GetConfiguration().max_message_num_handles)); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -128,7 +130,7 @@ MojoResult Dispatcher::ReadMessage(UserPointer<void> bytes, DCHECK(!num_dispatchers || *num_dispatchers == 0 || (dispatchers && dispatchers->empty())); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -139,7 +141,7 @@ MojoResult Dispatcher::ReadMessage(UserPointer<void> bytes, MojoResult Dispatcher::WriteData(UserPointer<const void> elements, UserPointer<uint32_t> num_bytes, MojoWriteDataFlags flags) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -149,7 +151,7 @@ MojoResult Dispatcher::WriteData(UserPointer<const void> elements, MojoResult Dispatcher::BeginWriteData(UserPointer<void*> buffer, UserPointer<uint32_t> buffer_num_bytes, MojoWriteDataFlags flags) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -157,7 +159,7 @@ MojoResult Dispatcher::BeginWriteData(UserPointer<void*> buffer, } MojoResult Dispatcher::EndWriteData(uint32_t num_bytes_written) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -167,7 +169,7 @@ MojoResult Dispatcher::EndWriteData(uint32_t num_bytes_written) { MojoResult Dispatcher::ReadData(UserPointer<void> elements, UserPointer<uint32_t> num_bytes, MojoReadDataFlags flags) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -177,7 +179,7 @@ MojoResult Dispatcher::ReadData(UserPointer<void> elements, MojoResult Dispatcher::BeginReadData(UserPointer<const void*> buffer, UserPointer<uint32_t> buffer_num_bytes, MojoReadDataFlags flags) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -185,7 +187,7 @@ MojoResult Dispatcher::BeginReadData(UserPointer<const void*> buffer, } MojoResult Dispatcher::EndReadData(uint32_t num_bytes_read) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -195,7 +197,7 @@ MojoResult Dispatcher::EndReadData(uint32_t num_bytes_read) { MojoResult Dispatcher::DuplicateBufferHandle( UserPointer<const MojoDuplicateBufferHandleOptions> options, scoped_refptr<Dispatcher>* new_dispatcher) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -207,7 +209,7 @@ MojoResult Dispatcher::MapBuffer( uint64_t num_bytes, MojoMapBufferFlags flags, scoped_ptr<embedder::PlatformSharedBufferMapping>* mapping) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -215,7 +217,7 @@ MojoResult Dispatcher::MapBuffer( } HandleSignalsState Dispatcher::GetHandleSignalsState() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) return HandleSignalsState(); @@ -226,7 +228,7 @@ MojoResult Dispatcher::AddAwakable(Awakable* awakable, MojoHandleSignals signals, uint32_t context, HandleSignalsState* signals_state) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) { if (signals_state) *signals_state = HandleSignalsState(); @@ -238,7 +240,7 @@ MojoResult Dispatcher::AddAwakable(Awakable* awakable, void Dispatcher::RemoveAwakable(Awakable* awakable, HandleSignalsState* handle_signals_state) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (is_closed_) { if (handle_signals_state) *handle_signals_state = HandleSignalsState(); @@ -257,14 +259,14 @@ Dispatcher::~Dispatcher() { } void Dispatcher::CancelAllAwakablesNoLock() { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(is_closed_); // By default, waiting isn't supported. Only dispatchers that can be waited on // will do something nontrivial. } void Dispatcher::CloseImplNoLock() { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(is_closed_); // This may not need to do anything. Dispatchers should override this to do // any actual close-time cleanup necessary. @@ -275,7 +277,7 @@ MojoResult Dispatcher::WriteMessageImplNoLock( uint32_t /*num_bytes*/, std::vector<DispatcherTransport>* /*transports*/, MojoWriteMessageFlags /*flags*/) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, not supported. Only needed for message pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -287,7 +289,7 @@ MojoResult Dispatcher::ReadMessageImplNoLock( DispatcherVector* /*dispatchers*/, uint32_t* /*num_dispatchers*/, MojoReadMessageFlags /*flags*/) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, not supported. Only needed for message pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -296,7 +298,7 @@ MojoResult Dispatcher::ReadMessageImplNoLock( MojoResult Dispatcher::WriteDataImplNoLock(UserPointer<const void> /*elements*/, UserPointer<uint32_t> /*num_bytes*/, MojoWriteDataFlags /*flags*/) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, not supported. Only needed for data pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -306,14 +308,14 @@ MojoResult Dispatcher::BeginWriteDataImplNoLock( UserPointer<void*> /*buffer*/, UserPointer<uint32_t> /*buffer_num_bytes*/, MojoWriteDataFlags /*flags*/) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, not supported. Only needed for data pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; } MojoResult Dispatcher::EndWriteDataImplNoLock(uint32_t /*num_bytes_written*/) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, not supported. Only needed for data pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -322,7 +324,7 @@ MojoResult Dispatcher::EndWriteDataImplNoLock(uint32_t /*num_bytes_written*/) { MojoResult Dispatcher::ReadDataImplNoLock(UserPointer<void> /*elements*/, UserPointer<uint32_t> /*num_bytes*/, MojoReadDataFlags /*flags*/) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, not supported. Only needed for data pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -332,14 +334,14 @@ MojoResult Dispatcher::BeginReadDataImplNoLock( UserPointer<const void*> /*buffer*/, UserPointer<uint32_t> /*buffer_num_bytes*/, MojoReadDataFlags /*flags*/) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, not supported. Only needed for data pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; } MojoResult Dispatcher::EndReadDataImplNoLock(uint32_t /*num_bytes_read*/) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, not supported. Only needed for data pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -348,7 +350,7 @@ MojoResult Dispatcher::EndReadDataImplNoLock(uint32_t /*num_bytes_read*/) { MojoResult Dispatcher::DuplicateBufferHandleImplNoLock( UserPointer<const MojoDuplicateBufferHandleOptions> /*options*/, scoped_refptr<Dispatcher>* /*new_dispatcher*/) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, not supported. Only needed for buffer dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -359,14 +361,14 @@ MojoResult Dispatcher::MapBufferImplNoLock( uint64_t /*num_bytes*/, MojoMapBufferFlags /*flags*/, scoped_ptr<embedder::PlatformSharedBufferMapping>* /*mapping*/) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, not supported. Only needed for buffer dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; } HandleSignalsState Dispatcher::GetHandleSignalsStateImplNoLock() const { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, waiting isn't supported. Only dispatchers that can be waited on // will do something nontrivial. @@ -378,7 +380,7 @@ MojoResult Dispatcher::AddAwakableImplNoLock( MojoHandleSignals /*signals*/, uint32_t /*context*/, HandleSignalsState* signals_state) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, waiting isn't supported. Only dispatchers that can be waited on // will do something nontrivial. @@ -389,7 +391,7 @@ MojoResult Dispatcher::AddAwakableImplNoLock( void Dispatcher::RemoveAwakableImplNoLock(Awakable* /*awakable*/, HandleSignalsState* signals_state) { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // By default, waiting isn't supported. Only dispatchers that can be waited on // will do something nontrivial. @@ -419,7 +421,7 @@ bool Dispatcher::EndSerializeAndCloseImplNoLock( } bool Dispatcher::IsBusyNoLock() const { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); // Most dispatchers support only "atomic" operations, so they are never busy // (in this sense). @@ -427,7 +429,7 @@ bool Dispatcher::IsBusyNoLock() const { } void Dispatcher::CloseNoLock() { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); is_closed_ = true; @@ -437,7 +439,7 @@ void Dispatcher::CloseNoLock() { scoped_refptr<Dispatcher> Dispatcher::CreateEquivalentDispatcherAndCloseNoLock() { - lock_.AssertAcquired(); + mutex_.AssertHeld(); DCHECK(!is_closed_); is_closed_ = true; @@ -475,7 +477,7 @@ bool Dispatcher::EndSerializeAndClose( // See the comment above |EndSerializeAndCloseImplNoLock()|. In brief: Locking // isn't actually needed, but we need to satisfy assertions (which we don't // want to remove or weaken). - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); #endif return EndSerializeAndCloseImplNoLock(channel, destination, actual_size, @@ -486,7 +488,7 @@ bool Dispatcher::EndSerializeAndClose( void DispatcherTransport::End() { DCHECK(dispatcher_); - dispatcher_->lock_.Release(); + dispatcher_->mutex_.Unlock(); dispatcher_ = nullptr; } diff --git a/third_party/mojo/src/mojo/edk/system/dispatcher.h b/third_party/mojo/src/mojo/edk/system/dispatcher.h index ed6999d..98b39f4 100644 --- a/third_party/mojo/src/mojo/edk/system/dispatcher.h +++ b/third_party/mojo/src/mojo/edk/system/dispatcher.h @@ -13,10 +13,10 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/synchronization/lock.h" #include "mojo/edk/embedder/platform_handle_vector.h" #include "mojo/edk/system/handle_signals_state.h" #include "mojo/edk/system/memory.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/system_impl_export.h" #include "mojo/public/c/system/buffer.h" #include "mojo/public/c/system/data_pipe.h" @@ -54,9 +54,9 @@ DispatcherTryStartTransport(Dispatcher* dispatcher); // A |Dispatcher| implements Mojo primitives that are "attached" to a particular // handle. This includes most (all?) primitives except for |MojoWait...()|. This -// object is thread-safe, with its state being protected by a single lock -// |lock_|, which is also made available to implementation subclasses (via the -// |lock()| method). +// object is thread-safe, with its state being protected by a single mutex +// |mutex_|, which is also made available to implementation subclasses (via the +// |mutex()| method). class MOJO_SYSTEM_IMPL_EXPORT Dispatcher : public base::RefCountedThreadSafe<Dispatcher> { public: @@ -73,9 +73,9 @@ class MOJO_SYSTEM_IMPL_EXPORT Dispatcher virtual Type GetType() const = 0; // These methods implement the various primitives named |Mojo...()|. These - // take |lock_| and handle races with |Close()|. Then they call out to - // subclasses' |...ImplNoLock()| methods (still under |lock_|), which actually - // implement the primitives. + // take |mutex_| and handle races with |Close()|. Then they call out to + // subclasses' |...ImplNoLock()| methods (still under |mutex_|), which + // actually implement the primitives. // NOTE(vtl): This puts a big lock around each dispatcher (i.e., handle), and // prevents the various |...ImplNoLock()|s from releasing the lock as soon as // possible. If this becomes an issue, we can rethink this. @@ -221,91 +221,100 @@ class MOJO_SYSTEM_IMPL_EXPORT Dispatcher virtual ~Dispatcher(); // These are to be overridden by subclasses (if necessary). They are called - // exactly once -- first |CancelAllAwakablesNoLock()|, then - // |CloseImplNoLock()|, - // when the dispatcher is being closed. They are called under |lock_|. - virtual void CancelAllAwakablesNoLock(); - virtual void CloseImplNoLock(); + // exactly once (first |CancelAllAwakablesNoLock()|, then |CloseImplNoLock()|) + // when the dispatcher is being closed. + virtual void CancelAllAwakablesNoLock() MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + virtual void CloseImplNoLock() MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + virtual scoped_refptr<Dispatcher> - CreateEquivalentDispatcherAndCloseImplNoLock() = 0; + CreateEquivalentDispatcherAndCloseImplNoLock() + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_) = 0; // These are to be overridden by subclasses (if necessary). They are never - // called after the dispatcher has been closed. They are called under |lock_|. - // See the descriptions of the methods without the "ImplNoLock" for more - // information. + // called after the dispatcher has been closed. See the descriptions of the + // methods without the "ImplNoLock" for more information. virtual MojoResult WriteMessageImplNoLock( UserPointer<const void> bytes, uint32_t num_bytes, std::vector<DispatcherTransport>* transports, - MojoWriteMessageFlags flags); + MojoWriteMessageFlags flags) MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); virtual MojoResult ReadMessageImplNoLock(UserPointer<void> bytes, UserPointer<uint32_t> num_bytes, DispatcherVector* dispatchers, uint32_t* num_dispatchers, - MojoReadMessageFlags flags); + MojoReadMessageFlags flags) + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); virtual MojoResult WriteDataImplNoLock(UserPointer<const void> elements, UserPointer<uint32_t> num_bytes, - MojoWriteDataFlags flags); + MojoWriteDataFlags flags) + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); virtual MojoResult BeginWriteDataImplNoLock( UserPointer<void*> buffer, UserPointer<uint32_t> buffer_num_bytes, - MojoWriteDataFlags flags); - virtual MojoResult EndWriteDataImplNoLock(uint32_t num_bytes_written); + MojoWriteDataFlags flags) MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + virtual MojoResult EndWriteDataImplNoLock(uint32_t num_bytes_written) + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); virtual MojoResult ReadDataImplNoLock(UserPointer<void> elements, UserPointer<uint32_t> num_bytes, - MojoReadDataFlags flags); + MojoReadDataFlags flags) + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); virtual MojoResult BeginReadDataImplNoLock( UserPointer<const void*> buffer, UserPointer<uint32_t> buffer_num_bytes, - MojoReadDataFlags flags); - virtual MojoResult EndReadDataImplNoLock(uint32_t num_bytes_read); + MojoReadDataFlags flags) MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + virtual MojoResult EndReadDataImplNoLock(uint32_t num_bytes_read) + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); virtual MojoResult DuplicateBufferHandleImplNoLock( UserPointer<const MojoDuplicateBufferHandleOptions> options, - scoped_refptr<Dispatcher>* new_dispatcher); + scoped_refptr<Dispatcher>* new_dispatcher) + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); virtual MojoResult MapBufferImplNoLock( uint64_t offset, uint64_t num_bytes, MojoMapBufferFlags flags, - scoped_ptr<embedder::PlatformSharedBufferMapping>* mapping); - virtual HandleSignalsState GetHandleSignalsStateImplNoLock() const; + scoped_ptr<embedder::PlatformSharedBufferMapping>* mapping) + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + virtual HandleSignalsState GetHandleSignalsStateImplNoLock() const + MOJO_SHARED_LOCKS_REQUIRED(mutex_); virtual MojoResult AddAwakableImplNoLock(Awakable* awakable, MojoHandleSignals signals, uint32_t context, - HandleSignalsState* signals_state); + HandleSignalsState* signals_state) + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); virtual void RemoveAwakableImplNoLock(Awakable* awakable, - HandleSignalsState* signals_state); + HandleSignalsState* signals_state) + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); // These implement the API used to serialize dispatchers to a |Channel| // (described below). They will only be called on a dispatcher that's attached // to and "owned" by a |MessageInTransit|. See the non-"impl" versions for // more information. // - // Note: |StartSerializeImplNoLock()| is actually called with |lock_| NOT + // Note: |StartSerializeImplNoLock()| is actually called with |mutex_| NOT // held, since the dispatcher should only be accessible to the calling thread. - // On Debug builds, |EndSerializeAndCloseImplNoLock()| is called with |lock_| - // held, to satisfy any |lock_.AssertAcquired()| (e.g., in |CloseImplNoLock()| - // -- and anything it calls); disentangling those assertions is + // On Debug builds, |EndSerializeAndCloseImplNoLock()| is called with |mutex_| + // held, to satisfy any |mutex_.AssertHeld()| (e.g., in |CloseImplNoLock()| -- + // and anything it calls); disentangling those assertions is // difficult/fragile, and would weaken our general checking of invariants. // // TODO(vtl): Consider making these pure virtual once most things support // being passed over a message pipe. virtual void StartSerializeImplNoLock(Channel* channel, size_t* max_size, - size_t* max_platform_handles); + size_t* max_platform_handles) + MOJO_NOT_THREAD_SAFE; virtual bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles); + embedder::PlatformHandleVector* platform_handles) MOJO_NOT_THREAD_SAFE; // This should be overridden to return true if/when there's an ongoing // operation (e.g., two-phase read/writes on data pipes) that should prevent a // handle from being sent over a message pipe (with status "busy"). - virtual bool IsBusyNoLock() const; + virtual bool IsBusyNoLock() const MOJO_SHARED_LOCKS_REQUIRED(mutex_); - // Available to subclasses. (Note: Returns a non-const reference, just like - // |base::AutoLock|'s constructor takes a non-const reference.) - base::Lock& lock() const { return lock_; } + Mutex& mutex() const MOJO_LOCK_RETURNED(mutex_) { return mutex_; } private: friend class DispatcherTransport; @@ -314,20 +323,29 @@ class MOJO_SYSTEM_IMPL_EXPORT Dispatcher // the dispatcher must not be closed already. (This is the "equivalent" of // |CreateEquivalentDispatcherAndCloseNoLock()|, for situations where the // dispatcher must be disposed of instead of "transferred".) - void CloseNoLock(); + void CloseNoLock() MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); // Creates an equivalent dispatcher -- representing the same resource as this // dispatcher -- and close (i.e., disable) this dispatcher. I.e., this // dispatcher will look as though it was closed, but the resource it // represents will be assigned to the new dispatcher. This must be called // under the dispatcher's lock. - scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndCloseNoLock(); + scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndCloseNoLock() + MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); // API to serialize dispatchers to a |Channel|, exposed to only // |TransportData| (via |TransportData|). They may only be called on a // dispatcher attached to a |MessageInTransit| (and in particular not in // |CoreImpl|'s handle table). // + // TODO(vtl): The serialization API (and related implementation methods, + // including |DispatcherTransport|'s methods) is marked + // |MOJO_NOT_THREAD_SAFE|. This is because the threading requirements are + // somewhat complicated (e.g., |HandleTableAccess::TryStartTransport()| is + // really a try-lock function, amongst other things). We could/should do a + // more careful job annotating these methods. + // https://github.com/domokit/mojo/issues/322 + // // Starts the serialization. Returns (via the two "out" parameters) the // maximum amount of space that may be needed to serialize this dispatcher to // the given |Channel| (no more than @@ -340,7 +358,7 @@ class MOJO_SYSTEM_IMPL_EXPORT Dispatcher // dispatcher cannot be serialized to the given |Channel|). void StartSerialize(Channel* channel, size_t* max_size, - size_t* max_platform_handles); + size_t* max_platform_handles) MOJO_NOT_THREAD_SAFE; // Completes the serialization of this dispatcher to the given |Channel| and // closes it. (This call will always follow an earlier call to // |StartSerialize()|, with the same |Channel|.) This does so by writing to @@ -355,12 +373,13 @@ class MOJO_SYSTEM_IMPL_EXPORT Dispatcher bool EndSerializeAndClose(Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles); + embedder::PlatformHandleVector* platform_handles) + MOJO_NOT_THREAD_SAFE; // This protects the following members as well as any state added by // subclasses. - mutable base::Lock lock_; - bool is_closed_; + mutable Mutex mutex_; + bool is_closed_ MOJO_GUARDED_BY(mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(Dispatcher); }; @@ -375,12 +394,15 @@ class MOJO_SYSTEM_IMPL_EXPORT DispatcherTransport { public: DispatcherTransport() : dispatcher_(nullptr) {} - void End(); + void End() MOJO_NOT_THREAD_SAFE; Dispatcher::Type GetType() const { return dispatcher_->GetType(); } - bool IsBusy() const { return dispatcher_->IsBusyNoLock(); } - void Close() { dispatcher_->CloseNoLock(); } - scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndClose() { + bool IsBusy() const MOJO_NOT_THREAD_SAFE { + return dispatcher_->IsBusyNoLock(); + } + void Close() MOJO_NOT_THREAD_SAFE { dispatcher_->CloseNoLock(); } + scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndClose() + MOJO_NOT_THREAD_SAFE { return dispatcher_->CreateEquivalentDispatcherAndCloseNoLock(); } diff --git a/third_party/mojo/src/mojo/edk/system/dispatcher_unittest.cc b/third_party/mojo/src/mojo/edk/system/dispatcher_unittest.cc index 10c2b86..c6431ff 100644 --- a/third_party/mojo/src/mojo/edk/system/dispatcher_unittest.cc +++ b/third_party/mojo/src/mojo/edk/system/dispatcher_unittest.cc @@ -31,7 +31,7 @@ class TrivialDispatcher final : public Dispatcher { scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndCloseImplNoLock() override { - lock().AssertAcquired(); + mutex().AssertHeld(); return scoped_refptr<Dispatcher>(new TrivialDispatcher()); } diff --git a/third_party/mojo/src/mojo/edk/system/endpoint_relayer.cc b/third_party/mojo/src/mojo/edk/system/endpoint_relayer.cc index 9630216..9655f9c 100644 --- a/third_party/mojo/src/mojo/edk/system/endpoint_relayer.cc +++ b/third_party/mojo/src/mojo/edk/system/endpoint_relayer.cc @@ -31,14 +31,14 @@ void EndpointRelayer::Init(ChannelEndpoint* endpoint0, } void EndpointRelayer::SetFilter(scoped_ptr<Filter> filter) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); filter_ = filter.Pass(); } bool EndpointRelayer::OnReadMessage(unsigned port, MessageInTransit* message) { DCHECK(message); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); // If we're no longer the client, then reject the message. if (!endpoints_[port]) @@ -59,7 +59,7 @@ bool EndpointRelayer::OnReadMessage(unsigned port, MessageInTransit* message) { } void EndpointRelayer::OnDetachFromChannel(unsigned port) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (endpoints_[port]) { endpoints_[port]->DetachFromClient(); diff --git a/third_party/mojo/src/mojo/edk/system/endpoint_relayer.h b/third_party/mojo/src/mojo/edk/system/endpoint_relayer.h index 8b17a50..cfeda24 100644 --- a/third_party/mojo/src/mojo/edk/system/endpoint_relayer.h +++ b/third_party/mojo/src/mojo/edk/system/endpoint_relayer.h @@ -7,8 +7,8 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/synchronization/lock.h" #include "mojo/edk/system/channel_endpoint_client.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/system_impl_export.h" #include "mojo/public/cpp/system/macros.h" @@ -68,7 +68,8 @@ class MOJO_SYSTEM_IMPL_EXPORT EndpointRelayer final static unsigned GetPeerPort(unsigned port); // Initialize this object. This must be called before any other method. - void Init(ChannelEndpoint* endpoint0, ChannelEndpoint* endpoint1); + void Init(ChannelEndpoint* endpoint0, + ChannelEndpoint* endpoint1) MOJO_NOT_THREAD_SAFE; // Sets (or resets) the filter, which can (optionally) handle/filter // |Type::ENDPOINT_CLIENT| messages (see |Filter| above). @@ -81,11 +82,9 @@ class MOJO_SYSTEM_IMPL_EXPORT EndpointRelayer final private: ~EndpointRelayer() override; - // TODO(vtl): We could probably get away without the lock if we had a - // thread-safe |scoped_refptr|. - base::Lock lock_; // Protects the following members. - scoped_refptr<ChannelEndpoint> endpoints_[2]; - scoped_ptr<Filter> filter_; + Mutex mutex_; + scoped_refptr<ChannelEndpoint> endpoints_[2] MOJO_GUARDED_BY(mutex_); + scoped_ptr<Filter> filter_ MOJO_GUARDED_BY(mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(EndpointRelayer); }; diff --git a/third_party/mojo/src/mojo/edk/system/incoming_endpoint.cc b/third_party/mojo/src/mojo/edk/system/incoming_endpoint.cc index cd091b1..eaa1825 100644 --- a/third_party/mojo/src/mojo/edk/system/incoming_endpoint.cc +++ b/third_party/mojo/src/mojo/edk/system/incoming_endpoint.cc @@ -23,7 +23,7 @@ scoped_refptr<ChannelEndpoint> IncomingEndpoint::Init() { } scoped_refptr<MessagePipe> IncomingEndpoint::ConvertToMessagePipe() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); scoped_refptr<MessagePipe> message_pipe( MessagePipe::CreateLocalProxyFromExisting(&message_queue_, endpoint_.get())); @@ -35,7 +35,7 @@ scoped_refptr<MessagePipe> IncomingEndpoint::ConvertToMessagePipe() { scoped_refptr<DataPipe> IncomingEndpoint::ConvertToDataPipeProducer( const MojoCreateDataPipeOptions& validated_options, size_t consumer_num_bytes) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); scoped_refptr<DataPipe> data_pipe(DataPipe::CreateRemoteConsumerFromExisting( validated_options, consumer_num_bytes, &message_queue_, endpoint_.get())); DCHECK(message_queue_.IsEmpty()); @@ -45,7 +45,7 @@ scoped_refptr<DataPipe> IncomingEndpoint::ConvertToDataPipeProducer( scoped_refptr<DataPipe> IncomingEndpoint::ConvertToDataPipeConsumer( const MojoCreateDataPipeOptions& validated_options) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); scoped_refptr<DataPipe> data_pipe(DataPipe::CreateRemoteProducerFromExisting( validated_options, &message_queue_, endpoint_.get())); DCHECK(message_queue_.IsEmpty()); @@ -54,7 +54,7 @@ scoped_refptr<DataPipe> IncomingEndpoint::ConvertToDataPipeConsumer( } void IncomingEndpoint::Close() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (endpoint_) { endpoint_->DetachFromClient(); endpoint_ = nullptr; @@ -63,7 +63,7 @@ void IncomingEndpoint::Close() { bool IncomingEndpoint::OnReadMessage(unsigned /*port*/, MessageInTransit* message) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (!endpoint_) return false; diff --git a/third_party/mojo/src/mojo/edk/system/incoming_endpoint.h b/third_party/mojo/src/mojo/edk/system/incoming_endpoint.h index 22bf0b9..bf0acf3 100644 --- a/third_party/mojo/src/mojo/edk/system/incoming_endpoint.h +++ b/third_party/mojo/src/mojo/edk/system/incoming_endpoint.h @@ -8,9 +8,9 @@ #include <stddef.h> #include "base/memory/ref_counted.h" -#include "base/synchronization/lock.h" #include "mojo/edk/system/channel_endpoint_client.h" #include "mojo/edk/system/message_in_transit_queue.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/system_impl_export.h" #include "mojo/public/cpp/system/macros.h" @@ -32,7 +32,7 @@ class MOJO_SYSTEM_IMPL_EXPORT IncomingEndpoint final IncomingEndpoint(); // Must be called before any other method. - scoped_refptr<ChannelEndpoint> Init(); + scoped_refptr<ChannelEndpoint> Init() MOJO_NOT_THREAD_SAFE; scoped_refptr<MessagePipe> ConvertToMessagePipe(); scoped_refptr<DataPipe> ConvertToDataPipeProducer( @@ -52,9 +52,9 @@ class MOJO_SYSTEM_IMPL_EXPORT IncomingEndpoint final private: ~IncomingEndpoint() override; - base::Lock lock_; // Protects the following members. - scoped_refptr<ChannelEndpoint> endpoint_; - MessageInTransitQueue message_queue_; + Mutex mutex_; + scoped_refptr<ChannelEndpoint> endpoint_ MOJO_GUARDED_BY(mutex_); + MessageInTransitQueue message_queue_ MOJO_GUARDED_BY(mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(IncomingEndpoint); }; diff --git a/third_party/mojo/src/mojo/edk/system/ipc_support.cc b/third_party/mojo/src/mojo/edk/system/ipc_support.cc index 424a130..7c4f3d4 100644 --- a/third_party/mojo/src/mojo/edk/system/ipc_support.cc +++ b/third_party/mojo/src/mojo/edk/system/ipc_support.cc @@ -141,8 +141,9 @@ embedder::ScopedPlatformHandle IPCSupport::ConnectToSlaveInternal( system::ProcessIdentifier peer_id = system::kInvalidProcessIdentifier; embedder::ScopedPlatformHandle platform_connection_handle; - CHECK(connection_manager()->Connect(connection_id, &peer_id, - &platform_connection_handle)); + CHECK_EQ(connection_manager()->Connect(connection_id, &peer_id, + &platform_connection_handle), + ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION); DCHECK_EQ(peer_id, *slave_process_identifier); DCHECK(platform_connection_handle.is_valid()); return platform_connection_handle; @@ -154,8 +155,9 @@ embedder::ScopedPlatformHandle IPCSupport::ConnectToMasterInternal( system::ProcessIdentifier peer_id; embedder::ScopedPlatformHandle platform_connection_handle; - CHECK(connection_manager()->Connect(connection_id, &peer_id, - &platform_connection_handle)); + CHECK_EQ(connection_manager()->Connect(connection_id, &peer_id, + &platform_connection_handle), + ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION); DCHECK_EQ(peer_id, system::kMasterProcessIdentifier); DCHECK(platform_connection_handle.is_valid()); return platform_connection_handle; diff --git a/third_party/mojo/src/mojo/edk/system/master_connection_manager.cc b/third_party/mojo/src/mojo/edk/system/master_connection_manager.cc index d756f73..7e24a4a8 100644 --- a/third_party/mojo/src/mojo/edk/system/master_connection_manager.cc +++ b/third_party/mojo/src/mojo/edk/system/master_connection_manager.cc @@ -21,6 +21,8 @@ namespace mojo { namespace system { +namespace { + const ProcessIdentifier kFirstSlaveProcessIdentifier = 2; static_assert(kMasterProcessIdentifier != kInvalidProcessIdentifier, @@ -30,6 +32,29 @@ static_assert(kFirstSlaveProcessIdentifier != kInvalidProcessIdentifier, static_assert(kMasterProcessIdentifier != kFirstSlaveProcessIdentifier, "Master and first slave process identifiers are the same"); +MessageInTransit::Subtype ConnectionManagerResultToMessageInTransitSubtype( + ConnectionManager::Result result) { + switch (result) { + case ConnectionManager::Result::FAILURE: + return MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_FAILURE; + case ConnectionManager::Result::SUCCESS: + return MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_SUCCESS; + case ConnectionManager::Result::SUCCESS_CONNECT_SAME_PROCESS: + return MessageInTransit::Subtype:: + CONNECTION_MANAGER_ACK_SUCCESS_CONNECT_SAME_PROCESS; + case ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION: + return MessageInTransit::Subtype:: + CONNECTION_MANAGER_ACK_SUCCESS_CONNECT_NEW_CONNECTION; + case ConnectionManager::Result::SUCCESS_CONNECT_REUSE_CONNECTION: + return MessageInTransit::Subtype:: + CONNECTION_MANAGER_ACK_SUCCESS_CONNECT_REUSE_CONNECTION; + } + NOTREACHED(); + return MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_FAILURE; +} + +} // namespace + // MasterConnectionManager::Helper --------------------------------------------- // |MasterConnectionManager::Helper| is not thread-safe, and must only be used @@ -115,28 +140,38 @@ void MasterConnectionManager::Helper::OnReadMessage( const ConnectionIdentifier* connection_id = reinterpret_cast<const ConnectionIdentifier*>(message_view.bytes()); - bool result; + Result result = Result::FAILURE; ProcessIdentifier peer_process_identifier = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle platform_handle; uint32_t num_bytes = 0; const void* bytes = nullptr; switch (message_view.subtype()) { case MessageInTransit::Subtype::CONNECTION_MANAGER_ALLOW_CONNECT: - result = owner_->AllowConnectImpl(process_identifier_, *connection_id); + result = owner_->AllowConnectImpl(process_identifier_, *connection_id) + ? Result::SUCCESS + : Result::FAILURE; break; case MessageInTransit::Subtype::CONNECTION_MANAGER_CANCEL_CONNECT: - result = owner_->CancelConnectImpl(process_identifier_, *connection_id); + result = owner_->CancelConnectImpl(process_identifier_, *connection_id) + ? Result::SUCCESS + : Result::FAILURE; break; - case MessageInTransit::Subtype::CONNECTION_MANAGER_CONNECT: + case MessageInTransit::Subtype::CONNECTION_MANAGER_CONNECT: { result = owner_->ConnectImpl(process_identifier_, *connection_id, &peer_process_identifier, &platform_handle); + DCHECK_NE(result, Result::SUCCESS); + // TODO(vtl): FIXME -- currently, nothing should generate + // SUCCESS_CONNECT_REUSE_CONNECTION. + CHECK_NE(result, Result::SUCCESS_CONNECT_REUSE_CONNECTION); // Success acks for "connect" have the peer process identifier as data - // (and maybe also a platform handle). - if (result) { + // (and also a platform handle in the case of "new connection" -- handled + // further below). + if (result != Result::FAILURE) { num_bytes = static_cast<uint32_t>(sizeof(peer_process_identifier)); bytes = &peer_process_identifier; } break; + } default: LOG(ERROR) << "Invalid message subtype " << message_view.subtype(); FatalError(); // WARNING: This destroys us. @@ -145,22 +180,21 @@ void MasterConnectionManager::Helper::OnReadMessage( scoped_ptr<MessageInTransit> response(new MessageInTransit( MessageInTransit::Type::CONNECTION_MANAGER_ACK, - result ? MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_SUCCESS - : MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_FAILURE, - num_bytes, bytes)); + ConnectionManagerResultToMessageInTransitSubtype(result), num_bytes, + bytes)); - if (platform_handle.is_valid()) { - // Only success acks for "connect" *may* have a platform handle attached. - DCHECK(result); + if (result == Result::SUCCESS_CONNECT_NEW_CONNECTION) { DCHECK_EQ(message_view.subtype(), MessageInTransit::Subtype::CONNECTION_MANAGER_CONNECT); - + DCHECK(platform_handle.is_valid()); embedder::ScopedPlatformHandleVectorPtr platform_handles( new embedder::PlatformHandleVector()); platform_handles->push_back(platform_handle.release()); response->SetTransportData(make_scoped_ptr( new TransportData(platform_handles.Pass(), raw_channel_->GetSerializedPlatformHandleSize()))); + } else { + DCHECK(!platform_handle.is_valid()); } if (!raw_channel_->WriteMessage(response.Pass())) { @@ -261,7 +295,7 @@ ProcessIdentifier MasterConnectionManager::AddSlave( ProcessIdentifier slave_process_identifier; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); CHECK_NE(next_process_identifier_, kMasterProcessIdentifier); slave_process_identifier = next_process_identifier_; next_process_identifier_++; @@ -288,7 +322,7 @@ ProcessIdentifier MasterConnectionManager::AddSlaveAndBootstrap( ProcessIdentifier slave_process_identifier = AddSlave(slave_info, platform_handle.Pass()); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); DCHECK(pending_connections_.find(connection_id) == pending_connections_.end()); PendingConnectionInfo* info = @@ -328,11 +362,10 @@ bool MasterConnectionManager::CancelConnect( return CancelConnectImpl(kMasterProcessIdentifier, connection_id); } -bool MasterConnectionManager::Connect( +ConnectionManager::Result MasterConnectionManager::Connect( const ConnectionIdentifier& connection_id, ProcessIdentifier* peer_process_identifier, embedder::ScopedPlatformHandle* platform_handle) { - AssertNotOnPrivateThread(); return ConnectImpl(kMasterProcessIdentifier, connection_id, peer_process_identifier, platform_handle); } @@ -342,7 +375,7 @@ bool MasterConnectionManager::AllowConnectImpl( const ConnectionIdentifier& connection_id) { DCHECK_NE(process_identifier, kInvalidProcessIdentifier); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); auto it = pending_connections_.find(connection_id); if (it == pending_connections_.end()) { @@ -381,7 +414,7 @@ bool MasterConnectionManager::CancelConnectImpl( const ConnectionIdentifier& connection_id) { DCHECK_NE(process_identifier, kInvalidProcessIdentifier); - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); auto it = pending_connections_.find(connection_id); if (it == pending_connections_.end()) { @@ -408,7 +441,7 @@ bool MasterConnectionManager::CancelConnectImpl( return true; } -bool MasterConnectionManager::ConnectImpl( +ConnectionManager::Result MasterConnectionManager::ConnectImpl( ProcessIdentifier process_identifier, const ConnectionIdentifier& connection_id, ProcessIdentifier* peer_process_identifier, @@ -418,7 +451,7 @@ bool MasterConnectionManager::ConnectImpl( DCHECK(platform_handle); DCHECK(!platform_handle->is_valid()); // Not technically wrong, but unlikely. - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); auto it = pending_connections_.find(connection_id); if (it == pending_connections_.end()) { @@ -426,7 +459,7 @@ bool MasterConnectionManager::ConnectImpl( LOG(ERROR) << "Connect() from process " << process_identifier << " for connection ID " << connection_id.ToString() << " which is not pending"; - return false; + return Result::FAILURE; } PendingConnectionInfo* info = it->second; @@ -443,23 +476,27 @@ bool MasterConnectionManager::ConnectImpl( LOG(ERROR) << "Connect() from process " << process_identifier << " for connection ID " << connection_id.ToString() << " which is neither connectee"; - return false; + return Result::FAILURE; } + // TODO(vtl): FIXME -- add stuff for SUCCESS_CONNECT_REUSE_CONNECTION here. + Result result = Result::FAILURE; if (info->first == info->second) { platform_handle->reset(); DCHECK(!info->remaining_handle.is_valid()); + result = Result::SUCCESS_CONNECT_SAME_PROCESS; } else { embedder::PlatformChannelPair platform_channel_pair; *platform_handle = platform_channel_pair.PassServerHandle(); DCHECK(platform_handle->is_valid()); info->remaining_handle = platform_channel_pair.PassClientHandle(); DCHECK(info->remaining_handle.is_valid()); + result = Result::SUCCESS_CONNECT_NEW_CONNECTION; } DVLOG(1) << "Connection ID " << connection_id.ToString() << ": first Connect() from process identifier " << process_identifier; - return true; + return result; } ProcessIdentifier remaining_connectee; @@ -479,7 +516,7 @@ bool MasterConnectionManager::ConnectImpl( << " in state " << info->state; pending_connections_.erase(it); delete info; - return false; + return Result::FAILURE; } if (process_identifier != remaining_connectee) { @@ -488,18 +525,28 @@ bool MasterConnectionManager::ConnectImpl( << " which is not the remaining connectee"; pending_connections_.erase(it); delete info; - return false; + return Result::FAILURE; } *peer_process_identifier = peer; - *platform_handle = info->remaining_handle.Pass(); - DCHECK((info->first == info->second) ^ platform_handle->is_valid()); + + // TODO(vtl): FIXME -- add stuff for SUCCESS_CONNECT_REUSE_CONNECTION here. + Result result = Result::FAILURE; + if (info->first == info->second) { + platform_handle->reset(); + DCHECK(!info->remaining_handle.is_valid()); + result = Result::SUCCESS_CONNECT_SAME_PROCESS; + } else { + *platform_handle = info->remaining_handle.Pass(); + DCHECK(platform_handle->is_valid()); + result = Result::SUCCESS_CONNECT_NEW_CONNECTION; + } pending_connections_.erase(it); delete info; DVLOG(1) << "Connection ID " << connection_id.ToString() << ": second Connect() from process identifier " << process_identifier; - return true; + return result; } void MasterConnectionManager::ShutdownOnPrivateThread() { @@ -555,7 +602,7 @@ void MasterConnectionManager::OnError(ProcessIdentifier process_identifier) { delete helper; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); // TODO(vtl): This isn't very efficient. for (auto it = pending_connections_.begin(); diff --git a/third_party/mojo/src/mojo/edk/system/master_connection_manager.h b/third_party/mojo/src/mojo/edk/system/master_connection_manager.h index b7992c8..497f449 100644 --- a/third_party/mojo/src/mojo/edk/system/master_connection_manager.h +++ b/third_party/mojo/src/mojo/edk/system/master_connection_manager.h @@ -9,10 +9,10 @@ #include "base/containers/hash_tables.h" #include "base/memory/ref_counted.h" -#include "base/synchronization/lock.h" #include "base/threading/thread.h" #include "mojo/edk/embedder/scoped_platform_handle.h" #include "mojo/edk/system/connection_manager.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/system_impl_export.h" #include "mojo/public/cpp/system/macros.h" @@ -51,7 +51,8 @@ class MOJO_SYSTEM_IMPL_EXPORT MasterConnectionManager final // thread", on which |master_process_delegate|'s methods will be called. Both // must stay alive at least until after |Shutdown()| has been called. void Init(scoped_refptr<base::TaskRunner> delegate_thread_task_runner, - embedder::MasterProcessDelegate* master_process_delegate); + embedder::MasterProcessDelegate* master_process_delegate) + MOJO_NOT_THREAD_SAFE; // Adds a slave process and sets up/tracks a connection to that slave (using // |platform_handle|). |slave_info| is used by the caller/implementation of @@ -75,12 +76,12 @@ class MOJO_SYSTEM_IMPL_EXPORT MasterConnectionManager final const ConnectionIdentifier& connection_id); // |ConnectionManager| methods: - void Shutdown() override; + void Shutdown() override MOJO_NOT_THREAD_SAFE; bool AllowConnect(const ConnectionIdentifier& connection_id) override; bool CancelConnect(const ConnectionIdentifier& connection_id) override; - bool Connect(const ConnectionIdentifier& connection_id, - ProcessIdentifier* peer_process_identifier, - embedder::ScopedPlatformHandle* platform_handle) override; + Result Connect(const ConnectionIdentifier& connection_id, + ProcessIdentifier* peer_process_identifier, + embedder::ScopedPlatformHandle* platform_handle) override; private: class Helper; @@ -91,13 +92,13 @@ class MOJO_SYSTEM_IMPL_EXPORT MasterConnectionManager final const ConnectionIdentifier& connection_id); bool CancelConnectImpl(ProcessIdentifier process_identifier, const ConnectionIdentifier& connection_id); - bool ConnectImpl(ProcessIdentifier process_identifier, - const ConnectionIdentifier& connection_id, - ProcessIdentifier* peer_process_identifier, - embedder::ScopedPlatformHandle* platform_handle); + Result ConnectImpl(ProcessIdentifier process_identifier, + const ConnectionIdentifier& connection_id, + ProcessIdentifier* peer_process_identifier, + embedder::ScopedPlatformHandle* platform_handle); // These should only be called on |private_thread_|: - void ShutdownOnPrivateThread(); + void ShutdownOnPrivateThread() MOJO_NOT_THREAD_SAFE; // Signals |*event| on completion. void AddSlaveOnPrivateThread(embedder::SlaveInfo slave_info, embedder::ScopedPlatformHandle platform_handle, @@ -132,15 +133,15 @@ class MOJO_SYSTEM_IMPL_EXPORT MasterConnectionManager final // The following members are only accessed on |private_thread_|: base::hash_map<ProcessIdentifier, Helper*> helpers_; // Owns its values. - // Protects the members below (except in the constructor, |Init()|, - // |Shutdown()|/|ShutdownOnPrivateThread()|, and the destructor). - base::Lock lock_; + // Note: |mutex_| is not needed in the constructor, |Init()|, + // |Shutdown()|/|ShutdownOnPrivateThread()|, or the destructor + Mutex mutex_; - ProcessIdentifier next_process_identifier_; + ProcessIdentifier next_process_identifier_ MOJO_GUARDED_BY(mutex_); struct PendingConnectionInfo; base::hash_map<ConnectionIdentifier, PendingConnectionInfo*> - pending_connections_; // Owns its values. + pending_connections_ MOJO_GUARDED_BY(mutex_); // Owns its values. MOJO_DISALLOW_COPY_AND_ASSIGN(MasterConnectionManager); }; diff --git a/third_party/mojo/src/mojo/edk/system/message_in_transit.h b/third_party/mojo/src/mojo/edk/system/message_in_transit.h index 063c8d6..c61cfd6 100644 --- a/third_party/mojo/src/mojo/edk/system/message_in_transit.h +++ b/third_party/mojo/src/mojo/edk/system/message_in_transit.h @@ -80,11 +80,18 @@ class MOJO_SYSTEM_IMPL_EXPORT MessageInTransit { CONNECTION_MANAGER_ALLOW_CONNECT = 0, CONNECTION_MANAGER_CANCEL_CONNECT = 1, CONNECTION_MANAGER_CONNECT = 2, - // Subtypes for type |Type::CONNECTION_MANAGER_ACK| (failure acks never have - // any message contents; success acks for "connect" always have a - // |ProcessIdentifier| as data and *may* have a platform handle attached): + // Subtypes for type |Type::CONNECTION_MANAGER_ACK|, corresponding to + // |ConnectionManager::Result| values (failure and non-"connect" acks never + // have any message contents; success acks for "connect" always have a + // |ProcessIdentifier| as data and also a platform handle attached for "new + // connection"): + // TODO(vtl): FIXME -- probably, in the "connect, reuse connection" case, + // we'll have to send more information. CONNECTION_MANAGER_ACK_FAILURE = 0, CONNECTION_MANAGER_ACK_SUCCESS = 1, + CONNECTION_MANAGER_ACK_SUCCESS_CONNECT_SAME_PROCESS = 2, + CONNECTION_MANAGER_ACK_SUCCESS_CONNECT_NEW_CONNECTION = 3, + CONNECTION_MANAGER_ACK_SUCCESS_CONNECT_REUSE_CONNECTION = 4, }; // Messages (the header and data) must always be aligned to a multiple of this diff --git a/third_party/mojo/src/mojo/edk/system/message_pipe_dispatcher.cc b/third_party/mojo/src/mojo/edk/system/message_pipe_dispatcher.cc index ddbe863..c329e87 100644 --- a/third_party/mojo/src/mojo/edk/system/message_pipe_dispatcher.cc +++ b/third_party/mojo/src/mojo/edk/system/message_pipe_dispatcher.cc @@ -105,22 +105,22 @@ MessagePipeDispatcher::~MessagePipeDispatcher() { } MessagePipe* MessagePipeDispatcher::GetMessagePipeNoLock() const { - lock().AssertAcquired(); + mutex().AssertHeld(); return message_pipe_.get(); } unsigned MessagePipeDispatcher::GetPortNoLock() const { - lock().AssertAcquired(); + mutex().AssertHeld(); return port_; } void MessagePipeDispatcher::CancelAllAwakablesNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); message_pipe_->CancelAllAwakables(port_); } void MessagePipeDispatcher::CloseImplNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); message_pipe_->Close(port_); message_pipe_ = nullptr; port_ = kInvalidPort; @@ -128,7 +128,7 @@ void MessagePipeDispatcher::CloseImplNoLock() { scoped_refptr<Dispatcher> MessagePipeDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); // TODO(vtl): Currently, there are no options, so we just use // |kDefaultCreateOptions|. Eventually, we'll have to duplicate the options @@ -149,7 +149,7 @@ MojoResult MessagePipeDispatcher::WriteMessageImplNoLock( (transports->size() > 0 && transports->size() <= GetConfiguration().max_message_num_handles)); - lock().AssertAcquired(); + mutex().AssertHeld(); if (num_bytes > GetConfiguration().max_message_num_bytes) return MOJO_RESULT_RESOURCE_EXHAUSTED; @@ -164,14 +164,14 @@ MojoResult MessagePipeDispatcher::ReadMessageImplNoLock( DispatcherVector* dispatchers, uint32_t* num_dispatchers, MojoReadMessageFlags flags) { - lock().AssertAcquired(); + mutex().AssertHeld(); return message_pipe_->ReadMessage(port_, bytes, num_bytes, dispatchers, num_dispatchers, flags); } HandleSignalsState MessagePipeDispatcher::GetHandleSignalsStateImplNoLock() const { - lock().AssertAcquired(); + mutex().AssertHeld(); return message_pipe_->GetHandleSignalsState(port_); } @@ -180,7 +180,7 @@ MojoResult MessagePipeDispatcher::AddAwakableImplNoLock( MojoHandleSignals signals, uint32_t context, HandleSignalsState* signals_state) { - lock().AssertAcquired(); + mutex().AssertHeld(); return message_pipe_->AddAwakable(port_, awakable, signals, context, signals_state); } @@ -188,7 +188,7 @@ MojoResult MessagePipeDispatcher::AddAwakableImplNoLock( void MessagePipeDispatcher::RemoveAwakableImplNoLock( Awakable* awakable, HandleSignalsState* signals_state) { - lock().AssertAcquired(); + mutex().AssertHeld(); message_pipe_->RemoveAwakable(port_, awakable, signals_state); } diff --git a/third_party/mojo/src/mojo/edk/system/message_pipe_dispatcher.h b/third_party/mojo/src/mojo/edk/system/message_pipe_dispatcher.h index 9df5fed..23bb1ed 100644 --- a/third_party/mojo/src/mojo/edk/system/message_pipe_dispatcher.h +++ b/third_party/mojo/src/mojo/edk/system/message_pipe_dispatcher.h @@ -42,7 +42,8 @@ class MOJO_SYSTEM_IMPL_EXPORT MessagePipeDispatcher final : public Dispatcher { MojoCreateMessagePipeOptions* out_options); // Must be called before any other methods. (This method is not thread-safe.) - void Init(scoped_refptr<MessagePipe> message_pipe, unsigned port); + void Init(scoped_refptr<MessagePipe> message_pipe, + unsigned port) MOJO_NOT_THREAD_SAFE; // |Dispatcher| public methods: Type GetType() const override; @@ -99,16 +100,18 @@ class MOJO_SYSTEM_IMPL_EXPORT MessagePipeDispatcher final : public Dispatcher { HandleSignalsState* signals_state) override; void StartSerializeImplNoLock(Channel* channel, size_t* max_size, - size_t* max_platform_handles) override; + size_t* max_platform_handles) override + MOJO_NOT_THREAD_SAFE; bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) override; + embedder::PlatformHandleVector* platform_handles) override + MOJO_NOT_THREAD_SAFE; - // Protected by |lock()|: - scoped_refptr<MessagePipe> message_pipe_; // This will be null if closed. - unsigned port_; + // This will be null if closed. + scoped_refptr<MessagePipe> message_pipe_ MOJO_GUARDED_BY(mutex()); + unsigned port_ MOJO_GUARDED_BY(mutex()); MOJO_DISALLOW_COPY_AND_ASSIGN(MessagePipeDispatcher); }; diff --git a/third_party/mojo/src/mojo/edk/system/mutex.cc b/third_party/mojo/src/mojo/edk/system/mutex.cc new file mode 100644 index 0000000..216b35a --- /dev/null +++ b/third_party/mojo/src/mojo/edk/system/mutex.cc @@ -0,0 +1,38 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "mojo/edk/system/mutex.h" + +#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON) + +#include "base/logging.h" + +namespace mojo { +namespace system { + +Mutex::Mutex() : lock_() { +} + +Mutex::~Mutex() { + DCHECK(owning_thread_ref_.is_null()); +} + +void Mutex::AssertHeld() const { + DCHECK(owning_thread_ref_ == base::PlatformThread::CurrentRef()); +} + +void Mutex::CheckHeldAndUnmark() { + DCHECK(owning_thread_ref_ == base::PlatformThread::CurrentRef()); + owning_thread_ref_ = base::PlatformThreadRef(); +} + +void Mutex::CheckUnheldAndMark() { + DCHECK(owning_thread_ref_.is_null()); + owning_thread_ref_ = base::PlatformThread::CurrentRef(); +} + +} // namespace system +} // namespace mojo + +#endif // !NDEBUG || DCHECK_ALWAYS_ON diff --git a/third_party/mojo/src/mojo/edk/system/mutex.h b/third_party/mojo/src/mojo/edk/system/mutex.h new file mode 100644 index 0000000..acb410a --- /dev/null +++ b/third_party/mojo/src/mojo/edk/system/mutex.h @@ -0,0 +1,93 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// A mutex class, with support for thread annotations. +// +// TODO(vtl): Currently, this is a fork of Chromium's +// base/synchronization/lock.h (with names changed and minor modifications; it +// still cheats and uses Chromium's lock_impl.*), but eventually we'll want our +// own and, e.g., add support for non-exclusive (reader) locks. + +#ifndef MOJO_EDK_SYSTEM_MUTEX_H_ +#define MOJO_EDK_SYSTEM_MUTEX_H_ + +#include "base/synchronization/lock_impl.h" +#include "base/threading/platform_thread.h" +#include "mojo/edk/system/system_impl_export.h" +#include "mojo/edk/system/thread_annotations.h" +#include "mojo/public/cpp/system/macros.h" + +namespace mojo { +namespace system { + +// Mutex ----------------------------------------------------------------------- + +class MOJO_SYSTEM_IMPL_EXPORT MOJO_LOCKABLE Mutex { + public: +#if defined(NDEBUG) && !defined(DCHECK_ALWAYS_ON) + Mutex() : lock_() {} + ~Mutex() {} + + void Lock() MOJO_EXCLUSIVE_LOCK_FUNCTION() { lock_.Lock(); } + void Unlock() MOJO_UNLOCK_FUNCTION() { lock_.Unlock(); } + + bool TryLock() MOJO_EXCLUSIVE_TRYLOCK_FUNCTION(true) { return lock_.Try(); } + + void AssertHeld() const MOJO_ASSERT_EXCLUSIVE_LOCK() {} +#else + Mutex(); + ~Mutex(); + + void Lock() MOJO_EXCLUSIVE_LOCK_FUNCTION() { + lock_.Lock(); + CheckUnheldAndMark(); + } + void Unlock() MOJO_UNLOCK_FUNCTION() { + CheckHeldAndUnmark(); + lock_.Unlock(); + } + + bool TryLock() MOJO_EXCLUSIVE_TRYLOCK_FUNCTION(true) { + bool rv = lock_.Try(); + if (rv) + CheckUnheldAndMark(); + return rv; + } + + void AssertHeld() const MOJO_ASSERT_EXCLUSIVE_LOCK(); +#endif // NDEBUG && !DCHECK_ALWAYS_ON + + private: +#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON) + void CheckHeldAndUnmark(); + void CheckUnheldAndMark(); + + base::PlatformThreadRef owning_thread_ref_; +#endif // !NDEBUG || DCHECK_ALWAYS_ON + + base::internal::LockImpl lock_; + + MOJO_DISALLOW_COPY_AND_ASSIGN(Mutex); +}; + +// MutexLocker ----------------------------------------------------------------- + +class MOJO_SYSTEM_IMPL_EXPORT MOJO_SCOPED_LOCKABLE MutexLocker { + public: + explicit MutexLocker(Mutex* mutex) MOJO_EXCLUSIVE_LOCK_FUNCTION(mutex) + : mutex_(mutex) { + this->mutex_->Lock(); + } + ~MutexLocker() MOJO_UNLOCK_FUNCTION() { this->mutex_->Unlock(); } + + private: + Mutex* const mutex_; + + MOJO_DISALLOW_COPY_AND_ASSIGN(MutexLocker); +}; + +} // namespace system +} // namespace mojo + +#endif // MOJO_EDK_SYSTEM_MUTEX_H_ diff --git a/third_party/mojo/src/mojo/edk/system/mutex_unittest.cc b/third_party/mojo/src/mojo/edk/system/mutex_unittest.cc new file mode 100644 index 0000000..9b51653 --- /dev/null +++ b/third_party/mojo/src/mojo/edk/system/mutex_unittest.cc @@ -0,0 +1,250 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "mojo/edk/system/mutex.h" + +#include <stdlib.h> + +#include "base/threading/platform_thread.h" +#include "mojo/edk/system/test_utils.h" +#include "mojo/public/cpp/system/macros.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace mojo { +namespace system { +namespace { + +// Sleeps for a "very small" amount of time. +void EpsilonRandomSleep() { + test::Sleep(test::DeadlineFromMilliseconds(rand() % 20)); +} + +// Basic test to make sure that Lock()/Unlock()/TryLock() don't crash ---------- + +class BasicMutexTestThread : public base::PlatformThread::Delegate { + public: + explicit BasicMutexTestThread(Mutex* mutex) : mutex_(mutex), acquired_(0) {} + + void ThreadMain() override { + for (int i = 0; i < 10; i++) { + mutex_->Lock(); + mutex_->AssertHeld(); + acquired_++; + mutex_->Unlock(); + } + for (int i = 0; i < 10; i++) { + mutex_->Lock(); + mutex_->AssertHeld(); + acquired_++; + EpsilonRandomSleep(); + mutex_->Unlock(); + } + for (int i = 0; i < 10; i++) { + if (mutex_->TryLock()) { + mutex_->AssertHeld(); + acquired_++; + EpsilonRandomSleep(); + mutex_->Unlock(); + } + } + } + + int acquired() const { return acquired_; } + + private: + Mutex* mutex_; + int acquired_; + + MOJO_DISALLOW_COPY_AND_ASSIGN(BasicMutexTestThread); +}; + +TEST(MutexTest, Basic) { + Mutex mutex; + BasicMutexTestThread thread(&mutex); + base::PlatformThreadHandle handle; + + ASSERT_TRUE(base::PlatformThread::Create(0, &thread, &handle)); + + int acquired = 0; + for (int i = 0; i < 5; i++) { + mutex.Lock(); + mutex.AssertHeld(); + acquired++; + mutex.Unlock(); + } + for (int i = 0; i < 10; i++) { + mutex.Lock(); + mutex.AssertHeld(); + acquired++; + EpsilonRandomSleep(); + mutex.Unlock(); + } + for (int i = 0; i < 10; i++) { + if (mutex.TryLock()) { + mutex.AssertHeld(); + acquired++; + EpsilonRandomSleep(); + mutex.Unlock(); + } + } + for (int i = 0; i < 5; i++) { + mutex.Lock(); + mutex.AssertHeld(); + acquired++; + EpsilonRandomSleep(); + mutex.Unlock(); + } + + base::PlatformThread::Join(handle); + + EXPECT_GE(acquired, 20); + EXPECT_GE(thread.acquired(), 20); +} + +// Test that TryLock() works as expected --------------------------------------- + +class TryLockTestThread : public base::PlatformThread::Delegate { + public: + explicit TryLockTestThread(Mutex* mutex) : mutex_(mutex), got_lock_(false) {} + + void ThreadMain() override MOJO_NO_THREAD_SAFETY_ANALYSIS { + got_lock_ = mutex_->TryLock(); + if (got_lock_) { + mutex_->AssertHeld(); + mutex_->Unlock(); + } + } + + bool got_lock() const { return got_lock_; } + + private: + Mutex* mutex_; + bool got_lock_; + + MOJO_DISALLOW_COPY_AND_ASSIGN(TryLockTestThread); +}; + +TEST(MutexTest, TryLock) MOJO_NO_THREAD_SAFETY_ANALYSIS { + Mutex mutex; + + ASSERT_TRUE(mutex.TryLock()); + // We now have the mutex.... + + // This thread will not be able to get the mutex. + { + TryLockTestThread thread(&mutex); + base::PlatformThreadHandle handle; + + ASSERT_TRUE(base::PlatformThread::Create(0, &thread, &handle)); + + base::PlatformThread::Join(handle); + + ASSERT_FALSE(thread.got_lock()); + } + + mutex.Unlock(); + + // This thread will.... + { + TryLockTestThread thread(&mutex); + base::PlatformThreadHandle handle; + + ASSERT_TRUE(base::PlatformThread::Create(0, &thread, &handle)); + + base::PlatformThread::Join(handle); + + ASSERT_TRUE(thread.got_lock()); + // But it released it.... + ASSERT_TRUE(mutex.TryLock()); + } + + mutex.Unlock(); +} + +// Tests that mutexes actually exclude ----------------------------------------- + +class MutexLockTestThread : public base::PlatformThread::Delegate { + public: + MutexLockTestThread(Mutex* mutex, int* value) + : mutex_(mutex), value_(value) {} + + // Static helper which can also be called from the main thread. + static void DoStuff(Mutex* mutex, int* value) { + for (int i = 0; i < 40; i++) { + mutex->Lock(); + int v = *value; + EpsilonRandomSleep(); + *value = v + 1; + mutex->Unlock(); + } + } + + void ThreadMain() override { DoStuff(mutex_, value_); } + + private: + Mutex* mutex_; + int* value_; + + MOJO_DISALLOW_COPY_AND_ASSIGN(MutexLockTestThread); +}; + +TEST(MutexTest, MutexTwoThreads) { + Mutex mutex; + int value = 0; + + MutexLockTestThread thread(&mutex, &value); + base::PlatformThreadHandle handle; + + ASSERT_TRUE(base::PlatformThread::Create(0, &thread, &handle)); + + MutexLockTestThread::DoStuff(&mutex, &value); + + base::PlatformThread::Join(handle); + + EXPECT_EQ(2 * 40, value); +} + +TEST(MutexTest, MutexFourThreads) { + Mutex mutex; + int value = 0; + + MutexLockTestThread thread1(&mutex, &value); + MutexLockTestThread thread2(&mutex, &value); + MutexLockTestThread thread3(&mutex, &value); + base::PlatformThreadHandle handle1; + base::PlatformThreadHandle handle2; + base::PlatformThreadHandle handle3; + + ASSERT_TRUE(base::PlatformThread::Create(0, &thread1, &handle1)); + ASSERT_TRUE(base::PlatformThread::Create(0, &thread2, &handle2)); + ASSERT_TRUE(base::PlatformThread::Create(0, &thread3, &handle3)); + + MutexLockTestThread::DoStuff(&mutex, &value); + + base::PlatformThread::Join(handle1); + base::PlatformThread::Join(handle2); + base::PlatformThread::Join(handle3); + + EXPECT_EQ(4 * 40, value); +} + +// MutexLocker ----------------------------------------------------------------- + +TEST(MutexTest, MutexLocker) { + Mutex mutex; + + { + MutexLocker locker(&mutex); + mutex.AssertHeld(); + } + + // The destruction of |locker| should unlock |mutex|. + ASSERT_TRUE(mutex.TryLock()); + mutex.AssertHeld(); + mutex.Unlock(); +} + +} // namespace +} // namespace system +} // namespace mojo diff --git a/third_party/mojo/src/mojo/edk/system/platform_handle_dispatcher.cc b/third_party/mojo/src/mojo/edk/system/platform_handle_dispatcher.cc index d33c960..681b406 100644 --- a/third_party/mojo/src/mojo/edk/system/platform_handle_dispatcher.cc +++ b/third_party/mojo/src/mojo/edk/system/platform_handle_dispatcher.cc @@ -22,7 +22,7 @@ struct SerializedPlatformHandleDispatcher { } // namespace embedder::ScopedPlatformHandle PlatformHandleDispatcher::PassPlatformHandle() { - base::AutoLock locker(lock()); + MutexLocker locker(&mutex()); return platform_handle_.Pass(); } @@ -73,13 +73,13 @@ PlatformHandleDispatcher::~PlatformHandleDispatcher() { } void PlatformHandleDispatcher::CloseImplNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); platform_handle_.reset(); } scoped_refptr<Dispatcher> PlatformHandleDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); return Create(platform_handle_.Pass()); } diff --git a/third_party/mojo/src/mojo/edk/system/platform_handle_dispatcher.h b/third_party/mojo/src/mojo/edk/system/platform_handle_dispatcher.h index 9e2e37d..8e8cf69 100644 --- a/third_party/mojo/src/mojo/edk/system/platform_handle_dispatcher.h +++ b/third_party/mojo/src/mojo/edk/system/platform_handle_dispatcher.h @@ -48,14 +48,16 @@ class MOJO_SYSTEM_IMPL_EXPORT PlatformHandleDispatcher final override; void StartSerializeImplNoLock(Channel* channel, size_t* max_size, - size_t* max_platform_handles) override; + size_t* max_platform_handles) override + MOJO_NOT_THREAD_SAFE; bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) override; + embedder::PlatformHandleVector* platform_handles) override + MOJO_NOT_THREAD_SAFE; - embedder::ScopedPlatformHandle platform_handle_; + embedder::ScopedPlatformHandle platform_handle_ MOJO_GUARDED_BY(mutex()); MOJO_DISALLOW_COPY_AND_ASSIGN(PlatformHandleDispatcher); }; diff --git a/third_party/mojo/src/mojo/edk/system/raw_channel_unittest.cc b/third_party/mojo/src/mojo/edk/system/raw_channel_unittest.cc index bc27889..a901791 100644 --- a/third_party/mojo/src/mojo/edk/system/raw_channel_unittest.cc +++ b/third_party/mojo/src/mojo/edk/system/raw_channel_unittest.cc @@ -19,7 +19,6 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_vector.h" #include "base/rand_util.h" -#include "base/synchronization/lock.h" #include "base/synchronization/waitable_event.h" #include "base/test/test_io_thread.h" #include "base/threading/simple_thread.h" @@ -28,6 +27,7 @@ #include "mojo/edk/embedder/platform_handle.h" #include "mojo/edk/embedder/scoped_platform_handle.h" #include "mojo/edk/system/message_in_transit.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/test_utils.h" #include "mojo/edk/system/transport_data.h" #include "mojo/edk/test/test_utils.h" @@ -233,7 +233,7 @@ class ReadCheckerRawChannelDelegate : public RawChannel::Delegate { size_t expected_size; bool should_signal = false; { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); CHECK_LT(position_, expected_sizes_.size()); position = position_; expected_size = expected_sizes_[position]; @@ -261,7 +261,7 @@ class ReadCheckerRawChannelDelegate : public RawChannel::Delegate { void Wait() { done_event_.Wait(); } void SetExpectedSizes(const std::vector<uint32_t>& expected_sizes) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); CHECK_EQ(position_, expected_sizes_.size()); expected_sizes_ = expected_sizes; position_ = 0; @@ -270,9 +270,9 @@ class ReadCheckerRawChannelDelegate : public RawChannel::Delegate { private: base::WaitableEvent done_event_; - base::Lock lock_; // Protects the following members. - std::vector<uint32_t> expected_sizes_; - size_t position_; + Mutex mutex_; + std::vector<uint32_t> expected_sizes_ MOJO_GUARDED_BY(mutex_); + size_t position_ MOJO_GUARDED_BY(mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(ReadCheckerRawChannelDelegate); }; diff --git a/third_party/mojo/src/mojo/edk/system/shared_buffer_dispatcher.cc b/third_party/mojo/src/mojo/edk/system/shared_buffer_dispatcher.cc index 8ab8f10..caf8e0b 100644 --- a/third_party/mojo/src/mojo/edk/system/shared_buffer_dispatcher.cc +++ b/third_party/mojo/src/mojo/edk/system/shared_buffer_dispatcher.cc @@ -177,14 +177,14 @@ MojoResult SharedBufferDispatcher::ValidateDuplicateOptions( } void SharedBufferDispatcher::CloseImplNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); DCHECK(shared_buffer_); shared_buffer_ = nullptr; } scoped_refptr<Dispatcher> SharedBufferDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); DCHECK(shared_buffer_); return CreateInternal(shared_buffer_.Pass()); } @@ -192,7 +192,7 @@ SharedBufferDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { MojoResult SharedBufferDispatcher::DuplicateBufferHandleImplNoLock( UserPointer<const MojoDuplicateBufferHandleOptions> options, scoped_refptr<Dispatcher>* new_dispatcher) { - lock().AssertAcquired(); + mutex().AssertHeld(); MojoDuplicateBufferHandleOptions validated_options; MojoResult result = ValidateDuplicateOptions(options, &validated_options); @@ -209,7 +209,7 @@ MojoResult SharedBufferDispatcher::MapBufferImplNoLock( uint64_t num_bytes, MojoMapBufferFlags flags, scoped_ptr<embedder::PlatformSharedBufferMapping>* mapping) { - lock().AssertAcquired(); + mutex().AssertHeld(); DCHECK(shared_buffer_); if (offset > static_cast<uint64_t>(std::numeric_limits<size_t>::max())) diff --git a/third_party/mojo/src/mojo/edk/system/shared_buffer_dispatcher.h b/third_party/mojo/src/mojo/edk/system/shared_buffer_dispatcher.h index deef5b4..1222e03 100644 --- a/third_party/mojo/src/mojo/edk/system/shared_buffer_dispatcher.h +++ b/third_party/mojo/src/mojo/edk/system/shared_buffer_dispatcher.h @@ -93,14 +93,17 @@ class MOJO_SYSTEM_IMPL_EXPORT SharedBufferDispatcher final scoped_ptr<embedder::PlatformSharedBufferMapping>* mapping) override; void StartSerializeImplNoLock(Channel* channel, size_t* max_size, - size_t* max_platform_handles) override; + size_t* max_platform_handles) override + MOJO_NOT_THREAD_SAFE; bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) override; + embedder::PlatformHandleVector* platform_handles) override + MOJO_NOT_THREAD_SAFE; - scoped_refptr<embedder::PlatformSharedBuffer> shared_buffer_; + scoped_refptr<embedder::PlatformSharedBuffer> shared_buffer_ + MOJO_GUARDED_BY(mutex()); MOJO_DISALLOW_COPY_AND_ASSIGN(SharedBufferDispatcher); }; diff --git a/third_party/mojo/src/mojo/edk/system/simple_dispatcher.cc b/third_party/mojo/src/mojo/edk/system/simple_dispatcher.cc index f7db875..7264a12 100644 --- a/third_party/mojo/src/mojo/edk/system/simple_dispatcher.cc +++ b/third_party/mojo/src/mojo/edk/system/simple_dispatcher.cc @@ -16,12 +16,12 @@ SimpleDispatcher::~SimpleDispatcher() { } void SimpleDispatcher::HandleSignalsStateChangedNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); awakable_list_.AwakeForStateChange(GetHandleSignalsStateImplNoLock()); } void SimpleDispatcher::CancelAllAwakablesNoLock() { - lock().AssertAcquired(); + mutex().AssertHeld(); awakable_list_.CancelAll(); } @@ -30,7 +30,7 @@ MojoResult SimpleDispatcher::AddAwakableImplNoLock( MojoHandleSignals signals, uint32_t context, HandleSignalsState* signals_state) { - lock().AssertAcquired(); + mutex().AssertHeld(); HandleSignalsState state(GetHandleSignalsStateImplNoLock()); if (state.satisfies(signals)) { @@ -51,7 +51,7 @@ MojoResult SimpleDispatcher::AddAwakableImplNoLock( void SimpleDispatcher::RemoveAwakableImplNoLock( Awakable* awakable, HandleSignalsState* signals_state) { - lock().AssertAcquired(); + mutex().AssertHeld(); awakable_list_.Remove(awakable); if (signals_state) *signals_state = GetHandleSignalsStateImplNoLock(); diff --git a/third_party/mojo/src/mojo/edk/system/simple_dispatcher.h b/third_party/mojo/src/mojo/edk/system/simple_dispatcher.h index 1ec7ba7..a828435 100644 --- a/third_party/mojo/src/mojo/edk/system/simple_dispatcher.h +++ b/third_party/mojo/src/mojo/edk/system/simple_dispatcher.h @@ -25,9 +25,8 @@ class MOJO_SYSTEM_IMPL_EXPORT SimpleDispatcher : public Dispatcher { ~SimpleDispatcher() override; // To be called by subclasses when the state changes (so - // |GetHandleSignalsStateImplNoLock()| should be checked again). Must be - // called under lock. - void HandleSignalsStateChangedNoLock(); + // |GetHandleSignalsStateImplNoLock()| should be checked again). + void HandleSignalsStateChangedNoLock() MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex()); // |Dispatcher| protected methods: void CancelAllAwakablesNoLock() override; @@ -39,8 +38,7 @@ class MOJO_SYSTEM_IMPL_EXPORT SimpleDispatcher : public Dispatcher { HandleSignalsState* signals_state) override; private: - // Protected by |lock()|: - AwakableList awakable_list_; + AwakableList awakable_list_ MOJO_GUARDED_BY(mutex()); MOJO_DISALLOW_COPY_AND_ASSIGN(SimpleDispatcher); }; diff --git a/third_party/mojo/src/mojo/edk/system/simple_dispatcher_unittest.cc b/third_party/mojo/src/mojo/edk/system/simple_dispatcher_unittest.cc index c9a1510..051e108 100644 --- a/third_party/mojo/src/mojo/edk/system/simple_dispatcher_unittest.cc +++ b/third_party/mojo/src/mojo/edk/system/simple_dispatcher_unittest.cc @@ -28,9 +28,11 @@ class MockSimpleDispatcher final : public SimpleDispatcher { MockSimpleDispatcher() : state_(MOJO_HANDLE_SIGNAL_NONE, MOJO_HANDLE_SIGNAL_READABLE | MOJO_HANDLE_SIGNAL_WRITABLE) {} + explicit MockSimpleDispatcher(const HandleSignalsState& state) + : state_(state) {} void SetSatisfiedSignals(MojoHandleSignals new_satisfied_signals) { - base::AutoLock locker(lock()); + MutexLocker locker(&mutex()); // Any new signals that are set should be satisfiable. CHECK_EQ(new_satisfied_signals & ~state_.satisfied_signals, @@ -45,7 +47,7 @@ class MockSimpleDispatcher final : public SimpleDispatcher { } void SetSatisfiableSignals(MojoHandleSignals new_satisfiable_signals) { - base::AutoLock locker(lock()); + MutexLocker locker(&mutex()); // Satisfied implies satisfiable. CHECK_EQ(new_satisfiable_signals & state_.satisfied_signals, @@ -66,19 +68,17 @@ class MockSimpleDispatcher final : public SimpleDispatcher { scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndCloseImplNoLock() override { - scoped_refptr<MockSimpleDispatcher> rv(new MockSimpleDispatcher()); - rv->state_ = state_; + scoped_refptr<MockSimpleDispatcher> rv(new MockSimpleDispatcher(state_)); return scoped_refptr<Dispatcher>(rv.get()); } // |Dispatcher| override: HandleSignalsState GetHandleSignalsStateImplNoLock() const override { - lock().AssertAcquired(); + mutex().AssertHeld(); return state_; } - // Protected by |lock()|: - HandleSignalsState state_; + HandleSignalsState state_ MOJO_GUARDED_BY(mutex()); MOJO_DISALLOW_COPY_AND_ASSIGN(MockSimpleDispatcher); }; diff --git a/third_party/mojo/src/mojo/edk/system/slave_connection_manager.cc b/third_party/mojo/src/mojo/edk/system/slave_connection_manager.cc index d59bd82..7d0a998 100644 --- a/third_party/mojo/src/mojo/edk/system/slave_connection_manager.cc +++ b/third_party/mojo/src/mojo/edk/system/slave_connection_manager.cc @@ -22,9 +22,9 @@ SlaveConnectionManager::SlaveConnectionManager( slave_process_delegate_(), private_thread_("SlaveConnectionManagerPrivateThread"), awaiting_ack_type_(NOT_AWAITING_ACK), - ack_result_(), - ack_peer_process_identifier_(), - ack_platform_handle_(), + ack_result_(nullptr), + ack_peer_process_identifier_(nullptr), + ack_platform_handle_(nullptr), event_(false, false) { // Auto-reset, not initially signalled. } @@ -78,38 +78,40 @@ bool SlaveConnectionManager::AllowConnect( const ConnectionIdentifier& connection_id) { AssertNotOnPrivateThread(); - base::AutoLock locker(lock_); - bool result = false; + MutexLocker locker(&mutex_); + Result result = Result::FAILURE; private_thread_.message_loop()->PostTask( FROM_HERE, base::Bind(&SlaveConnectionManager::AllowConnectOnPrivateThread, base::Unretained(this), connection_id, &result)); event_.Wait(); - return result; + DCHECK(result == Result::FAILURE || result == Result::SUCCESS); + return result == Result::SUCCESS; } bool SlaveConnectionManager::CancelConnect( const ConnectionIdentifier& connection_id) { AssertNotOnPrivateThread(); - base::AutoLock locker(lock_); - bool result = false; + MutexLocker locker(&mutex_); + Result result = Result::FAILURE; private_thread_.message_loop()->PostTask( FROM_HERE, base::Bind(&SlaveConnectionManager::CancelConnectOnPrivateThread, base::Unretained(this), connection_id, &result)); event_.Wait(); - return result; + DCHECK(result == Result::FAILURE || result == Result::SUCCESS); + return result == Result::SUCCESS; } -bool SlaveConnectionManager::Connect( +ConnectionManager::Result SlaveConnectionManager::Connect( const ConnectionIdentifier& connection_id, ProcessIdentifier* peer_process_identifier, embedder::ScopedPlatformHandle* platform_handle) { AssertNotOnPrivateThread(); - base::AutoLock locker(lock_); - bool result = false; + MutexLocker locker(&mutex_); + Result result = Result::FAILURE; private_thread_.message_loop()->PostTask( FROM_HERE, base::Bind(&SlaveConnectionManager::ConnectOnPrivateThread, base::Unretained(this), connection_id, &result, @@ -139,12 +141,12 @@ void SlaveConnectionManager::ShutdownOnPrivateThread() { void SlaveConnectionManager::AllowConnectOnPrivateThread( const ConnectionIdentifier& connection_id, - bool* result) { + Result* result) { DCHECK(result); AssertOnPrivateThread(); // This should only posted (from another thread, to |private_thread_|) with // the lock held (until this thread triggers |event_|). - DCHECK(!lock_.Try()); + DCHECK(!mutex_.TryLock()); DCHECK_EQ(awaiting_ack_type_, NOT_AWAITING_ACK); DVLOG(1) << "Sending AllowConnect: connection ID " @@ -154,7 +156,7 @@ void SlaveConnectionManager::AllowConnectOnPrivateThread( MessageInTransit::Subtype::CONNECTION_MANAGER_ALLOW_CONNECT, sizeof(connection_id), &connection_id)))) { // Don't tear things down; possibly we'll still read some messages. - *result = false; + *result = Result::FAILURE; event_.Signal(); return; } @@ -164,12 +166,12 @@ void SlaveConnectionManager::AllowConnectOnPrivateThread( void SlaveConnectionManager::CancelConnectOnPrivateThread( const ConnectionIdentifier& connection_id, - bool* result) { + Result* result) { DCHECK(result); AssertOnPrivateThread(); // This should only posted (from another thread, to |private_thread_|) with // the lock held (until this thread triggers |event_|). - DCHECK(!lock_.Try()); + DCHECK(!mutex_.TryLock()); DCHECK_EQ(awaiting_ack_type_, NOT_AWAITING_ACK); DVLOG(1) << "Sending CancelConnect: connection ID " @@ -179,7 +181,7 @@ void SlaveConnectionManager::CancelConnectOnPrivateThread( MessageInTransit::Subtype::CONNECTION_MANAGER_CANCEL_CONNECT, sizeof(connection_id), &connection_id)))) { // Don't tear things down; possibly we'll still read some messages. - *result = false; + *result = Result::FAILURE; event_.Signal(); return; } @@ -189,7 +191,7 @@ void SlaveConnectionManager::CancelConnectOnPrivateThread( void SlaveConnectionManager::ConnectOnPrivateThread( const ConnectionIdentifier& connection_id, - bool* result, + Result* result, ProcessIdentifier* peer_process_identifier, embedder::ScopedPlatformHandle* platform_handle) { DCHECK(result); @@ -198,7 +200,7 @@ void SlaveConnectionManager::ConnectOnPrivateThread( AssertOnPrivateThread(); // This should only posted (from another thread, to |private_thread_|) with // the lock held (until this thread triggers |event_|). - DCHECK(!lock_.Try()); + DCHECK(!mutex_.TryLock()); DCHECK_EQ(awaiting_ack_type_, NOT_AWAITING_ACK); DVLOG(1) << "Sending Connect: connection ID " << connection_id.ToString(); @@ -207,7 +209,7 @@ void SlaveConnectionManager::ConnectOnPrivateThread( MessageInTransit::Subtype::CONNECTION_MANAGER_CONNECT, sizeof(connection_id), &connection_id)))) { // Don't tear things down; possibly we'll still read some messages. - *result = false; + *result = Result::FAILURE; platform_handle->reset(); event_.Signal(); return; @@ -223,8 +225,8 @@ void SlaveConnectionManager::OnReadMessage( embedder::ScopedPlatformHandleVectorPtr platform_handles) { AssertOnPrivateThread(); - // Set |*ack_result_| to false by default. - *ack_result_ = false; + // Set |*ack_result_| to failure by default. + *ack_result_ = Result::FAILURE; // Note: Since we should be able to trust the master, simply crash (i.e., // |CHECK()|-fail) if it sends us something invalid. @@ -240,38 +242,53 @@ void SlaveConnectionManager::OnReadMessage( if (message_view.subtype() == MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_FAILURE) { // Failure acks never have any contents. - CHECK_EQ(num_bytes, 0u); - CHECK_EQ(num_platform_handles, 0u); - // Leave |*ack_result_| false. - } else if (message_view.subtype() == - MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_SUCCESS) { - if (awaiting_ack_type_ == AWAITING_ACCEPT_CONNECT_ACK || - awaiting_ack_type_ == AWAITING_CANCEL_CONNECT_ACK) { - // Success acks for "accept/cancel connect" have no contents. - CHECK_EQ(num_bytes, 0u); - CHECK_EQ(num_platform_handles, 0u); - *ack_result_ = true; + DCHECK_EQ(num_bytes, 0u); + DCHECK_EQ(num_platform_handles, 0u); + // Leave |*ack_result_| as failure. + } else { + if (awaiting_ack_type_ != AWAITING_CONNECT_ACK) { + // In the non-"connect" case, there's only one type of success ack, which + // never has any contents. + CHECK_EQ(message_view.subtype(), + MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_SUCCESS); + DCHECK_EQ(num_bytes, 0u); + DCHECK_EQ(num_platform_handles, 0u); + *ack_result_ = Result::SUCCESS; DCHECK(!ack_peer_process_identifier_); DCHECK(!ack_platform_handle_); } else { - DCHECK_EQ(awaiting_ack_type_, AWAITING_CONNECT_ACK); - // Success acks for "connect" always have a |ProcessIdentifier| as data, - // and *maybe* one platform handle. + // Success acks for "connect" always have a |ProcessIdentifier| as data. CHECK_EQ(num_bytes, sizeof(ProcessIdentifier)); - CHECK_LE(num_platform_handles, 1u); - *ack_result_ = true; *ack_peer_process_identifier_ = *reinterpret_cast<const ProcessIdentifier*>(message_view.bytes()); - if (num_platform_handles > 0) { - ack_platform_handle_->reset(platform_handles->at(0)); - platform_handles->at(0) = embedder::PlatformHandle(); - } else { - ack_platform_handle_->reset(); + + switch (message_view.subtype()) { + case MessageInTransit::Subtype:: + CONNECTION_MANAGER_ACK_SUCCESS_CONNECT_SAME_PROCESS: + DCHECK_EQ(num_platform_handles, 0u); + *ack_result_ = Result::SUCCESS_CONNECT_SAME_PROCESS; + ack_platform_handle_->reset(); + break; + case MessageInTransit::Subtype:: + CONNECTION_MANAGER_ACK_SUCCESS_CONNECT_NEW_CONNECTION: + CHECK_EQ(num_platform_handles, 1u); + *ack_result_ = Result::SUCCESS_CONNECT_NEW_CONNECTION; + ack_platform_handle_->reset(platform_handles->at(0)); + platform_handles->at(0) = embedder::PlatformHandle(); + break; + case MessageInTransit::Subtype:: + CONNECTION_MANAGER_ACK_SUCCESS_CONNECT_REUSE_CONNECTION: + DCHECK_EQ(num_platform_handles, 0u); + *ack_result_ = Result::SUCCESS_CONNECT_REUSE_CONNECTION; + ack_platform_handle_->reset(); + // TODO(vtl): FIXME -- currently, nothing should generate + // SUCCESS_CONNECT_REUSE_CONNECTION. + CHECK(false); + break; + default: + CHECK(false); } } - } else { - // Bad message subtype. - CHECK(false); } awaiting_ack_type_ = NOT_AWAITING_ACK; diff --git a/third_party/mojo/src/mojo/edk/system/slave_connection_manager.h b/third_party/mojo/src/mojo/edk/system/slave_connection_manager.h index 40fc79b..b2cd92c 100644 --- a/third_party/mojo/src/mojo/edk/system/slave_connection_manager.h +++ b/third_party/mojo/src/mojo/edk/system/slave_connection_manager.h @@ -7,12 +7,12 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/synchronization/lock.h" #include "base/synchronization/waitable_event.h" #include "base/threading/thread.h" #include "mojo/edk/embedder/scoped_platform_handle.h" #include "mojo/edk/embedder/slave_process_delegate.h" #include "mojo/edk/system/connection_manager.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/raw_channel.h" #include "mojo/edk/system/system_impl_export.h" #include "mojo/public/cpp/system/macros.h" @@ -58,20 +58,20 @@ class MOJO_SYSTEM_IMPL_EXPORT SlaveConnectionManager final void Shutdown() override; bool AllowConnect(const ConnectionIdentifier& connection_id) override; bool CancelConnect(const ConnectionIdentifier& connection_id) override; - bool Connect(const ConnectionIdentifier& connection_id, - ProcessIdentifier* peer_process_identifier, - embedder::ScopedPlatformHandle* platform_handle) override; + Result Connect(const ConnectionIdentifier& connection_id, + ProcessIdentifier* peer_process_identifier, + embedder::ScopedPlatformHandle* platform_handle) override; private: // These should only be called on |private_thread_|: void InitOnPrivateThread(embedder::ScopedPlatformHandle platform_handle); void ShutdownOnPrivateThread(); void AllowConnectOnPrivateThread(const ConnectionIdentifier& connection_id, - bool* result); + Result* result); void CancelConnectOnPrivateThread(const ConnectionIdentifier& connection_id, - bool* result); + Result* result); void ConnectOnPrivateThread(const ConnectionIdentifier& connection_id, - bool* result, + Result* result, ProcessIdentifier* peer_process_identifier, embedder::ScopedPlatformHandle* platform_handle); @@ -112,7 +112,7 @@ class MOJO_SYSTEM_IMPL_EXPORT SlaveConnectionManager final AWAITING_CONNECT_ACK }; AwaitingAckType awaiting_ack_type_; - bool* ack_result_; + Result* ack_result_; // Used only when waiting for the ack to "connect": ProcessIdentifier* ack_peer_process_identifier_; embedder::ScopedPlatformHandle* ack_platform_handle_; @@ -143,7 +143,7 @@ class MOJO_SYSTEM_IMPL_EXPORT SlaveConnectionManager final // // TODO(vtl): This is all a hack. It'd really suffice to have a version of // |RawChannel| with fully synchronous reading and writing. - base::Lock lock_; + Mutex mutex_; base::WaitableEvent event_; MOJO_DISALLOW_COPY_AND_ASSIGN(SlaveConnectionManager); diff --git a/third_party/mojo/src/mojo/edk/system/test_channel_endpoint_client.cc b/third_party/mojo/src/mojo/edk/system/test_channel_endpoint_client.cc index 51d2a58..b3e69b1 100644 --- a/third_party/mojo/src/mojo/edk/system/test_channel_endpoint_client.cc +++ b/third_party/mojo/src/mojo/edk/system/test_channel_endpoint_client.cc @@ -17,7 +17,7 @@ TestChannelEndpointClient::TestChannelEndpointClient() } void TestChannelEndpointClient::Init(unsigned port, ChannelEndpoint* endpoint) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); ASSERT_EQ(0u, port_); ASSERT_FALSE(endpoint_); port_ = port; @@ -25,30 +25,30 @@ void TestChannelEndpointClient::Init(unsigned port, ChannelEndpoint* endpoint) { } bool TestChannelEndpointClient::IsDetached() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return !endpoint_; } size_t TestChannelEndpointClient::NumMessages() const { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); return messages_.Size(); } scoped_ptr<MessageInTransit> TestChannelEndpointClient::PopMessage() { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (messages_.IsEmpty()) return nullptr; return messages_.GetMessage(); } void TestChannelEndpointClient::SetReadEvent(base::WaitableEvent* read_event) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); read_event_ = read_event; } bool TestChannelEndpointClient::OnReadMessage(unsigned port, MessageInTransit* message) { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); EXPECT_EQ(port_, port); EXPECT_TRUE(endpoint_); @@ -61,9 +61,9 @@ bool TestChannelEndpointClient::OnReadMessage(unsigned port, } void TestChannelEndpointClient::OnDetachFromChannel(unsigned port) { - EXPECT_EQ(port_, port); + MutexLocker locker(&mutex_); - base::AutoLock locker(lock_); + EXPECT_EQ(port_, port); ASSERT_TRUE(endpoint_); endpoint_->DetachFromClient(); endpoint_ = nullptr; diff --git a/third_party/mojo/src/mojo/edk/system/test_channel_endpoint_client.h b/third_party/mojo/src/mojo/edk/system/test_channel_endpoint_client.h index c2347d5..bb812f0 100644 --- a/third_party/mojo/src/mojo/edk/system/test_channel_endpoint_client.h +++ b/third_party/mojo/src/mojo/edk/system/test_channel_endpoint_client.h @@ -7,10 +7,10 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/synchronization/lock.h" #include "mojo/edk/system/channel_endpoint.h" #include "mojo/edk/system/channel_endpoint_client.h" #include "mojo/edk/system/message_in_transit_queue.h" +#include "mojo/edk/system/mutex.h" #include "mojo/public/cpp/system/macros.h" namespace base { @@ -49,15 +49,15 @@ class TestChannelEndpointClient final : public ChannelEndpointClient { private: ~TestChannelEndpointClient() override; - mutable base::Lock lock_; // Protects the members below. + mutable Mutex mutex_; - unsigned port_; - scoped_refptr<ChannelEndpoint> endpoint_; + unsigned port_ MOJO_GUARDED_BY(mutex_); + scoped_refptr<ChannelEndpoint> endpoint_ MOJO_GUARDED_BY(mutex_); - MessageInTransitQueue messages_; + MessageInTransitQueue messages_ MOJO_GUARDED_BY(mutex_); // Event to trigger if we read a message (may be null). - base::WaitableEvent* read_event_; + base::WaitableEvent* read_event_ MOJO_GUARDED_BY(mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(TestChannelEndpointClient); }; diff --git a/third_party/mojo/src/mojo/edk/system/test_utils.h b/third_party/mojo/src/mojo/edk/system/test_utils.h index 6f8a96d..144e07b 100644 --- a/third_party/mojo/src/mojo/edk/system/test_utils.h +++ b/third_party/mojo/src/mojo/edk/system/test_utils.h @@ -25,7 +25,7 @@ MojoDeadline EpsilonDeadline(); // the order of 100 ms.) MojoDeadline TinyDeadline(); -// |TestTimeouts::action_timeout()|, as a |MojoDeadline|.(Expect this to be on +// |TestTimeouts::action_timeout()|, as a |MojoDeadline|. (Expect this to be on // the order of 10 s.) MojoDeadline ActionDeadline(); diff --git a/third_party/mojo/src/mojo/edk/system/thread_annotations.h b/third_party/mojo/src/mojo/edk/system/thread_annotations.h new file mode 100644 index 0000000..ffac558 --- /dev/null +++ b/third_party/mojo/src/mojo/edk/system/thread_annotations.h @@ -0,0 +1,85 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Macros for static thread-safety analysis. +// +// These are from http://clang.llvm.org/docs/ThreadSafetyAnalysis.html (and thus +// really derive from google3's thread_annotations.h). +// +// TODO(vtl): We're still using the old-fashioned, deprecated annotations +// ("locks" instead of "capabilities"), since the new ones don't work yet (in +// particular, |TRY_ACQUIRE()| doesn't work: b/19264527). +// https://github.com/domokit/mojo/issues/314 + +#ifndef MOJO_EDK_SYSTEM_THREAD_ANNOTATIONS_H_ +#define MOJO_EDK_SYSTEM_THREAD_ANNOTATIONS_H_ + +// Enable thread-safety attributes only with clang. +// The attributes can be safely erased when compiling with other compilers. +#if defined(__clang__) +#define MOJO_THREAD_ANNOTATION_ATTRIBUTE__(x) __attribute__((x)) +#else +#define MOJO_THREAD_ANNOTATION_ATTRIBUTE__(x) +#endif + +#define MOJO_GUARDED_BY(x) MOJO_THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x)) + +#define MOJO_PT_GUARDED_BY(x) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(pt_guarded_by(x)) + +#define MOJO_ACQUIRED_AFTER(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(acquired_after(__VA_ARGS__)) + +#define MOJO_ACQUIRED_BEFORE(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(acquired_before(__VA_ARGS__)) + +#define MOJO_EXCLUSIVE_LOCKS_REQUIRED(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(exclusive_locks_required(__VA_ARGS__)) + +#define MOJO_SHARED_LOCKS_REQUIRED(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(shared_locks_required(__VA_ARGS__)) + +#define MOJO_LOCKS_EXCLUDED(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(locks_excluded(__VA_ARGS__)) + +#define MOJO_LOCK_RETURNED(x) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(lock_returned(x)) + +#define MOJO_LOCKABLE MOJO_THREAD_ANNOTATION_ATTRIBUTE__(lockable) + +#define MOJO_SCOPED_LOCKABLE MOJO_THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable) + +#define MOJO_EXCLUSIVE_LOCK_FUNCTION(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(exclusive_lock_function(__VA_ARGS__)) + +#define MOJO_SHARED_LOCK_FUNCTION(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(shared_lock_function(__VA_ARGS__)) + +#define MOJO_ASSERT_EXCLUSIVE_LOCK(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(assert_exclusive_lock(__VA_ARGS__)) + +#define MOJO_ASSERT_SHARED_LOCK(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(assert_shared_lock(__VA_ARGS__)) + +#define MOJO_EXCLUSIVE_TRYLOCK_FUNCTION(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(exclusive_trylock_function(__VA_ARGS__)) + +#define MOJO_SHARED_TRYLOCK_FUNCTION(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(shared_trylock_function(__VA_ARGS__)) + +#define MOJO_UNLOCK_FUNCTION(...) \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(unlock_function(__VA_ARGS__)) + +#define MOJO_NO_THREAD_SAFETY_ANALYSIS \ + MOJO_THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis) + +// Use this in the header to annotate a function/method as not being +// thread-safe. This is equivalent to |MOJO_NO_THREAD_SAFETY_ANALYSIS|, but +// semantically different: it declares that the caller must abide by additional +// restrictions. Limitation: Unfortunately, you can't apply this to a method in +// an interface (i.e., pure virtual method) and have it applied automatically to +// implementations. +#define MOJO_NOT_THREAD_SAFE MOJO_NO_THREAD_SAFETY_ANALYSIS + +#endif // MOJO_EDK_SYSTEM_THREAD_ANNOTATIONS_H_ diff --git a/third_party/mojo/src/mojo/edk/system/thread_annotations_unittest.cc b/third_party/mojo/src/mojo/edk/system/thread_annotations_unittest.cc new file mode 100644 index 0000000..d56814f --- /dev/null +++ b/third_party/mojo/src/mojo/edk/system/thread_annotations_unittest.cc @@ -0,0 +1,124 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Tests of the static thread annotation macros. These fall into two categories, +// positive tests (testing that correct code compiles and works) and negative +// tests (testing that incorrect code does not compile). +// +// Unfortunately, we don't have systematic/automated negative compilation tests. +// So instead we have some cheesy macros that you can define to enable +// individual compilation failures. + +#include "mojo/edk/system/thread_annotations.h" + +#include "mojo/edk/system/mutex.h" +#include "mojo/public/cpp/system/macros.h" +#include "testing/gtest/include/gtest/gtest.h" + +// Uncomment these to enable particular compilation failure tests. +// #define NC_GUARDED_BY +// TODO(vtl): |ACQUIRED_{BEFORE,AFTER}()| are currently unimplemented in clang +// as of 2015-07-06 ("To be fixed in a future update."). So this actually +// compiles! +// #define NC_ACQUIRED_BEFORE + +namespace mojo { +namespace system { +namespace { + +// Test MOJO_GUARDED_BY -------------------------------------------------------- + +class GuardedByClass { + public: + GuardedByClass() : x_() {} + ~GuardedByClass() {} + + void GoodSet(int x) { + mu_.Lock(); + x_ = x; + mu_.Unlock(); + } + +#ifdef NC_GUARDED_BY + void BadSet(int x) { x_ = x; } +#endif + + private: + Mutex mu_; + int x_ MOJO_GUARDED_BY(mu_); + + MOJO_DISALLOW_COPY_AND_ASSIGN(GuardedByClass); +}; + +TEST(ThreadAnnotationsTest, GuardedBy) { + GuardedByClass c; + c.GoodSet(123); +} + +// Test MOJO_ACQUIRED_BEFORE --------------------------------------------------- + +class AcquiredBeforeClass2; + +class AcquiredBeforeClass1 { + public: + AcquiredBeforeClass1() {} + ~AcquiredBeforeClass1() {} + + void NoOp() { + mu_.Lock(); + mu_.Unlock(); + } + +#ifdef NC_ACQUIRED_BEFORE + void BadMethod(AcquiredBeforeClass2* c2) { + mu_.Lock(); + c2->NoOp(); + mu_.Unlock(); + } +#endif + + private: + friend class AcquiredBeforeClass2; + + Mutex mu_; + + MOJO_DISALLOW_COPY_AND_ASSIGN(AcquiredBeforeClass1); +}; + +class AcquiredBeforeClass2 { + public: + AcquiredBeforeClass2() {} + ~AcquiredBeforeClass2() {} + + void NoOp() { + mu_.Lock(); + mu_.Unlock(); + } + + void GoodMethod(AcquiredBeforeClass1* c1) { + mu_.Lock(); + c1->NoOp(); + mu_.Unlock(); + } + + private: + Mutex mu_ MOJO_ACQUIRED_BEFORE(AcquiredBeforeClass1::mu_); + + MOJO_DISALLOW_COPY_AND_ASSIGN(AcquiredBeforeClass2); +}; + +TEST(ThreadAnnotationsTest, AcquiredBefore) { + AcquiredBeforeClass1 c1; + AcquiredBeforeClass2 c2; + c2.GoodMethod(&c1); +#ifdef NC_ACQUIRED_BEFORE + c1.BadMethod(&c2); +#endif +} + +// TODO(vtl): Test more things. + +} // namespace +} // namespace system +} // namespace mojo diff --git a/third_party/mojo/src/mojo/edk/system/waiter_unittest.cc b/third_party/mojo/src/mojo/edk/system/waiter_unittest.cc index 68f8106..29bd5bc 100644 --- a/third_party/mojo/src/mojo/edk/system/waiter_unittest.cc +++ b/third_party/mojo/src/mojo/edk/system/waiter_unittest.cc @@ -11,8 +11,8 @@ #include <stdint.h> -#include "base/synchronization/lock.h" #include "base/threading/simple_thread.h" +#include "mojo/edk/system/mutex.h" #include "mojo/edk/system/test_utils.h" #include "mojo/public/cpp/system/macros.h" #include "testing/gtest/include/gtest/gtest.h" @@ -41,7 +41,7 @@ class WaitingThread : public base::SimpleThread { MojoDeadline* elapsed) { for (;;) { { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); if (done_) { *result = result_; *context = context_; @@ -68,7 +68,7 @@ class WaitingThread : public base::SimpleThread { elapsed = stopwatch.Elapsed(); { - base::AutoLock locker(lock_); + MutexLocker locker(&mutex_); done_ = true; result_ = result; context_ = context; @@ -79,11 +79,11 @@ class WaitingThread : public base::SimpleThread { const MojoDeadline deadline_; Waiter waiter_; // Thread-safe. - base::Lock lock_; // Protects the following members. - bool done_; - MojoResult result_; - uint32_t context_; - MojoDeadline elapsed_; + Mutex mutex_; + bool done_ MOJO_GUARDED_BY(mutex_); + MojoResult result_ MOJO_GUARDED_BY(mutex_); + uint32_t context_ MOJO_GUARDED_BY(mutex_); + MojoDeadline elapsed_ MOJO_GUARDED_BY(mutex_); MOJO_DISALLOW_COPY_AND_ASSIGN(WaitingThread); }; diff --git a/third_party/mojo/src/mojo/edk/test/BUILD.gn b/third_party/mojo/src/mojo/edk/test/BUILD.gn index fa6f836..834d837 100644 --- a/third_party/mojo/src/mojo/edk/test/BUILD.gn +++ b/third_party/mojo/src/mojo/edk/test/BUILD.gn @@ -89,6 +89,7 @@ mojo_edk_source_set("test_support_impl") { group("public_tests") { testonly = true deps = [ + ":mojo_public_bindings_perftests", ":mojo_public_bindings_unittests", ":mojo_public_environment_unittests", ":mojo_public_system_perftests", @@ -118,6 +119,13 @@ test("mojo_public_bindings_unittests") { ] } +test("mojo_public_bindings_perftests") { + deps = [ + ":run_all_perftests", + "../../public/cpp/bindings/tests:perftests", + ] +} + test("mojo_public_environment_unittests") { deps = [ ":run_all_unittests", diff --git a/third_party/mojo/src/mojo/public/VERSION b/third_party/mojo/src/mojo/public/VERSION index 0841b63..cb44eda 100644 --- a/third_party/mojo/src/mojo/public/VERSION +++ b/third_party/mojo/src/mojo/public/VERSION @@ -1 +1 @@ -a05bfef8096006056b2fff78092faf14d1319782
\ No newline at end of file +c02a28868825edfa57ab77947b8cb15e741c5598
\ No newline at end of file diff --git a/third_party/mojo/src/mojo/public/c/gpu/DEPS b/third_party/mojo/src/mojo/public/c/gpu/DEPS new file mode 100644 index 0000000..9be0bc0 --- /dev/null +++ b/third_party/mojo/src/mojo/public/c/gpu/DEPS @@ -0,0 +1,3 @@ +include_rules = [ + "+gpu", +] diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/BUILD.gn b/third_party/mojo/src/mojo/public/cpp/bindings/BUILD.gn index 05e92be..e345313 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/BUILD.gn +++ b/third_party/mojo/src/mojo/public/cpp/bindings/BUILD.gn @@ -8,7 +8,6 @@ mojo_sdk_source_set("bindings") { sources = [ "array.h", "binding.h", - "error_handler.h", "interface_ptr.h", "interface_ptr_info.h", "interface_request.h", @@ -42,8 +41,6 @@ mojo_sdk_source_set("bindings") { "lib/message_header_validator.cc", "lib/message_header_validator.h", "lib/message_internal.h", - "lib/message_queue.cc", - "lib/message_queue.h", "lib/no_interface.cc", "lib/router.cc", "lib/router.h", diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/binding.h b/third_party/mojo/src/mojo/public/cpp/bindings/binding.h index 4609521..250945a 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/binding.h +++ b/third_party/mojo/src/mojo/public/cpp/bindings/binding.h @@ -7,7 +7,6 @@ #include "mojo/public/c/environment/async_waiter.h" #include "mojo/public/cpp/bindings/callback.h" -#include "mojo/public/cpp/bindings/error_handler.h" #include "mojo/public/cpp/bindings/interface_ptr.h" #include "mojo/public/cpp/bindings/interface_ptr_info.h" #include "mojo/public/cpp/bindings/interface_request.h" @@ -102,9 +101,8 @@ class Binding { // Tears down the binding, closing the message pipe and leaving the interface // implementation unbound. ~Binding() { - if (internal_router_) { + if (internal_router_) Close(); - } } // Completes a binding that was constructed with only an interface @@ -187,19 +185,6 @@ class Binding { connection_error_handler_ = error_handler; } - // Similar to the method above but uses the ErrorHandler interface instead of - // a callback. - // NOTE: Deprecated. Please use the method above. - // TODO(yzshen): Remove this method once all callsites are converted. - void set_error_handler(ErrorHandler* error_handler) { - if (error_handler) { - set_connection_error_handler( - [error_handler]() { error_handler->OnConnectionError(); }); - } else { - set_connection_error_handler(Closure()); - } - } - // Returns the interface implementation that was previously specified. Caller // does not take ownership. Interface* impl() { return impl_; } diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/callback.h b/third_party/mojo/src/mojo/public/cpp/bindings/callback.h index 17d1caf..beec1a1 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/callback.h +++ b/third_party/mojo/src/mojo/public/cpp/bindings/callback.h @@ -37,7 +37,7 @@ class Callback<void(Args...)> { explicit Callback(Runnable* runnable) : sink_(runnable) {} // As above, but can take an object that isn't derived from Runnable, so long - // as it has a compatible operator() or Run() method. operator() will be + // as it has a compatible operator() or Run() method. operator() will be // preferred if the type has both. template <typename Sink> Callback(const Sink& sink) { @@ -47,6 +47,11 @@ class Callback<void(Args...)> { sink_ = internal::SharedPtr<Runnable>(new sink_type(sink)); } + // As above, but can take a compatible function pointer. + Callback(void (*function_ptr)( + typename internal::Callback_ParamTraits<Args>::ForwardType...)) + : sink_(new FunctionPtrAdapter(function_ptr)) {} + // Executes the callback function, invoking Pass() on move-only types. void Run(typename internal::Callback_ParamTraits<Args>::ForwardType... args) const { @@ -73,7 +78,7 @@ class Callback<void(Args...)> { Sink sink; }; - // Adapts a class that has a compatible operator() callable by Callback. + // Adapts a class that has a compatible operator() to be callable by Callback. template <typename Sink> struct FunctorAdapter : public Runnable { explicit FunctorAdapter(const Sink& sink) : sink(sink) {} @@ -85,6 +90,20 @@ class Callback<void(Args...)> { Sink sink; }; + // Adapts a function pointer. + struct FunctionPtrAdapter : public Runnable { + explicit FunctionPtrAdapter(void (*function_ptr)( + typename internal::Callback_ParamTraits<Args>::ForwardType...)) + : function_ptr(function_ptr) {} + virtual void Run( + typename internal::Callback_ParamTraits<Args>::ForwardType... args) + const override { + (*function_ptr)(internal::Forward(args)...); + } + void (*function_ptr)( + typename internal::Callback_ParamTraits<Args>::ForwardType...); + }; + internal::SharedPtr<Runnable> sink_; }; diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/error_handler.h b/third_party/mojo/src/mojo/public/cpp/bindings/error_handler.h deleted file mode 100644 index 8ce1af2..0000000 --- a/third_party/mojo/src/mojo/public/cpp/bindings/error_handler.h +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef MOJO_PUBLIC_CPP_BINDINGS_ERROR_HANDLER_H_ -#define MOJO_PUBLIC_CPP_BINDINGS_ERROR_HANDLER_H_ - -namespace mojo { - -// This interface is used to report connection errors. -class ErrorHandler { - public: - virtual ~ErrorHandler() {} - virtual void OnConnectionError() = 0; -}; - -} // namespace mojo - -#endif // MOJO_PUBLIC_CPP_BINDINGS_ERROR_HANDLER_H_ diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/interface_impl.h b/third_party/mojo/src/mojo/public/cpp/bindings/interface_impl.h deleted file mode 100644 index c65f6e3..0000000 --- a/third_party/mojo/src/mojo/public/cpp/bindings/interface_impl.h +++ /dev/null @@ -1,178 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef MOJO_PUBLIC_CPP_BINDINGS_INTERFACE_IMPL_H_ -#define MOJO_PUBLIC_CPP_BINDINGS_INTERFACE_IMPL_H_ - -#include "mojo/public/cpp/bindings/binding.h" -#include "mojo/public/cpp/bindings/interface_ptr_info.h" -#include "mojo/public/cpp/bindings/interface_request.h" -#include "mojo/public/cpp/environment/environment.h" -#include "mojo/public/cpp/system/macros.h" - -namespace mojo { - -// DEPRECATED! Please use mojo::Binding instead of InterfaceImpl<> in new code. -// -// InterfaceImpl<..> is designed to be the base class of an interface -// implementation. It may be bound to a pipe or a proxy, see BindToPipe and -// BindToProxy. -template <typename Interface> -class InterfaceImpl : public Interface, public ErrorHandler { - public: - using ImplementedInterface = Interface; - - InterfaceImpl() : binding_(this), error_handler_impl_(this) { - binding_.set_error_handler(&error_handler_impl_); - } - virtual ~InterfaceImpl() {} - - void BindToHandle( - ScopedMessagePipeHandle handle, - const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) { - binding_.Bind(handle.Pass(), waiter); - } - - bool WaitForIncomingMethodCall() { - return binding_.WaitForIncomingMethodCall(); - } - - internal::Router* internal_router() { return binding_.internal_router(); } - - // Implements ErrorHandler. - // - // Called when the underlying pipe is closed. After this point no more - // calls will be made on Interface and all responses will be silently ignored. - void OnConnectionError() override {} - - void set_delete_on_error(bool delete_on_error) { - error_handler_impl_.set_delete_on_error(delete_on_error); - } - - private: - class ErrorHandlerImpl : public ErrorHandler { - public: - explicit ErrorHandlerImpl(InterfaceImpl* impl) : impl_(impl) {} - ~ErrorHandlerImpl() override {} - - // ErrorHandler implementation: - void OnConnectionError() override { - // If the the instance is not bound to the pipe, the instance might choose - // to delete the binding in the OnConnectionError handler, which would in - // turn delete |this|. Save the error behavior before invoking the error - // handler so we can correctly decide what to do. - bool delete_on_error = delete_on_error_; - impl_->OnConnectionError(); - if (delete_on_error) - delete impl_; - } - - void set_delete_on_error(bool delete_on_error) { - delete_on_error_ = delete_on_error; - } - - private: - InterfaceImpl* impl_; - bool delete_on_error_ = false; - - MOJO_DISALLOW_COPY_AND_ASSIGN(ErrorHandlerImpl); - }; - - Binding<Interface> binding_; - ErrorHandlerImpl error_handler_impl_; - - MOJO_DISALLOW_COPY_AND_ASSIGN(InterfaceImpl); -}; - -// Takes an instance of an InterfaceImpl<..> subclass and binds it to the given -// MessagePipe. The instance is returned for convenience in member initializer -// lists, etc. -// -// If the pipe is closed, the instance's OnConnectionError method will be called -// and then the instance will be deleted. -// -// The instance is also bound to the current thread. Its methods will only be -// called on the current thread, and if the current thread exits, then the end -// point of the pipe will be closed and the error handler's OnConnectionError -// method will be called. -template <typename Impl> -Impl* BindToPipe( - Impl* instance, - ScopedMessagePipeHandle handle, - const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) { - instance->set_delete_on_error(true); - instance->BindToHandle(handle.Pass(), waiter); - return instance; -} - -// Like BindToPipe but does not delete the instance after a channel error. -template <typename Impl> -Impl* WeakBindToPipe( - Impl* instance, - ScopedMessagePipeHandle handle, - const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) { - instance->BindToHandle(handle.Pass(), waiter); - return instance; -} - -// Takes an instance of an InterfaceImpl<..> subclass and binds it to the given -// InterfacePtr<..>. The instance is returned for convenience in member -// initializer lists, etc. If the pipe is closed, the instance's -// OnConnectionError method will be called and then the instance will be -// deleted. -// -// The instance is also bound to the current thread. Its methods will only be -// called on the current thread, and if the current thread exits, then it will -// also be deleted, and along with it, its end point of the pipe will be closed. -template <typename Impl, typename Interface> -Impl* BindToProxy( - Impl* instance, - InterfacePtr<Interface>* ptr, - const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) { - instance->set_delete_on_error(true); - WeakBindToProxy(instance, ptr, waiter); - return instance; -} - -// Like BindToProxy but does not delete the instance after a channel error. -template <typename Impl, typename Interface> -Impl* WeakBindToProxy( - Impl* instance, - InterfacePtr<Interface>* ptr, - const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) { - MessagePipe pipe; - ptr->Bind(InterfacePtrInfo<Interface>(pipe.handle0.Pass(), 0u), waiter); - instance->BindToHandle(pipe.handle1.Pass(), waiter); - return instance; -} - -// Takes an instance of an InterfaceImpl<..> subclass and binds it to the given -// InterfaceRequest<..>. The instance is returned for convenience in member -// initializer lists, etc. If the pipe is closed, the instance's -// OnConnectionError method will be called and then the instance will be -// deleted. -// -// The instance is also bound to the current thread. Its methods will only be -// called on the current thread, and if the current thread exits, then it will -// also be deleted, and along with it, its end point of the pipe will be closed. -template <typename Impl, typename Interface> -Impl* BindToRequest( - Impl* instance, - InterfaceRequest<Interface>* request, - const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) { - return BindToPipe(instance, request->PassMessagePipe(), waiter); -} - -// Like BindToRequest but does not delete the instance after a channel error. -template <typename Impl, typename Interface> -Impl* WeakBindToRequest( - Impl* instance, - InterfaceRequest<Interface>* request, - const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) { - return WeakBindToPipe(instance, request->PassMessagePipe(), waiter); -} - -} // namespace mojo - -#endif // MOJO_PUBLIC_CPP_BINDINGS_INTERFACE_IMPL_H_ diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/interface_ptr.h b/third_party/mojo/src/mojo/public/cpp/bindings/interface_ptr.h index 04f4902..00514dd 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/interface_ptr.h +++ b/third_party/mojo/src/mojo/public/cpp/bindings/interface_ptr.h @@ -8,7 +8,6 @@ #include <algorithm> #include "mojo/public/cpp/bindings/callback.h" -#include "mojo/public/cpp/bindings/error_handler.h" #include "mojo/public/cpp/bindings/interface_ptr_info.h" #include "mojo/public/cpp/bindings/lib/interface_ptr_internal.h" #include "mojo/public/cpp/environment/environment.h" @@ -74,6 +73,9 @@ class InterfacePtr { internal_state_.Bind(info.Pass(), waiter); } + // Returns whether or not this InterfacePtr is bound to a message pipe. + bool is_bound() const { return internal_state_.is_bound(); } + // Returns a raw pointer to the local proxy. Caller does not take ownership. // Note that the local proxy is thread hostile, as stated above. Interface* get() const { return internal_state_.instance(); } @@ -134,19 +136,6 @@ class InterfacePtr { internal_state_.set_connection_error_handler(error_handler); } - // Similar to the method above but uses the ErrorHandler interface instead of - // a callback. - // NOTE: Deprecated. Please use the method above. - // TODO(yzshen): Remove this method once all callsites are converted. - void set_error_handler(ErrorHandler* error_handler) { - if (error_handler) { - set_connection_error_handler( - [error_handler]() { error_handler->OnConnectionError(); }); - } else { - set_connection_error_handler(Closure()); - } - } - // Unbinds the InterfacePtr and returns the information which could be used // to setup an InterfacePtr again. This method may be used to move the proxy // to a different thread (see class comments for details). diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/lib/connector.cc b/third_party/mojo/src/mojo/public/cpp/bindings/lib/connector.cc index 24d993d1..597bf8d 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/lib/connector.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/lib/connector.cc @@ -4,7 +4,6 @@ #include "mojo/public/cpp/bindings/lib/connector.h" -#include "mojo/public/cpp/bindings/error_handler.h" #include "mojo/public/cpp/environment/logging.h" namespace mojo { diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/lib/connector.h b/third_party/mojo/src/mojo/public/cpp/bindings/lib/connector.h index a6c0d1b..f36761c 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/lib/connector.h +++ b/third_party/mojo/src/mojo/public/cpp/bindings/lib/connector.h @@ -7,7 +7,6 @@ #include "mojo/public/c/environment/async_waiter.h" #include "mojo/public/cpp/bindings/callback.h" -#include "mojo/public/cpp/bindings/lib/message_queue.h" #include "mojo/public/cpp/bindings/message.h" #include "mojo/public/cpp/environment/environment.h" #include "mojo/public/cpp/system/core.h" diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/lib/map_data_internal.h b/third_party/mojo/src/mojo/public/cpp/bindings/lib/map_data_internal.h index 8315dbc..6d629b4 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/lib/map_data_internal.h +++ b/third_party/mojo/src/mojo/public/cpp/bindings/lib/map_data_internal.h @@ -13,22 +13,25 @@ namespace mojo { namespace internal { -inline const ArrayValidateParams* GetMapKeyValidateParamsDefault() { - // The memory allocated here never gets released to not cause an exit time - // destructor. +namespace { +const ArrayValidateParams* GetMapKeyValidateParamsDefault() { + // The memory allocated here never gets released because calling a + // destructor at exit time makes clang unhappy. static const ArrayValidateParams* validate_params = new ArrayValidateParams(0, false, nullptr); return validate_params; } -inline const ArrayValidateParams* GetMapKeyValidateParamsForStrings() { - // The memory allocated here never gets released to not cause an exit time - // destructor. +const ArrayValidateParams* GetMapKeyValidateParamsForStrings() { + // The memory allocated here never gets released because calling a + // destructor at exit time makes clang unhappy. static const ArrayValidateParams* validate_params = new ArrayValidateParams( 0, false, new ArrayValidateParams(0, false, nullptr)); return validate_params; } +} // namespace + template <typename MapKey> struct MapKeyValidateParamsFactory { static const ArrayValidateParams* Get() { diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/strong_binding.h b/third_party/mojo/src/mojo/public/cpp/bindings/strong_binding.h index 48c74c6..860260e 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/strong_binding.h +++ b/third_party/mojo/src/mojo/public/cpp/bindings/strong_binding.h @@ -10,7 +10,6 @@ #include "mojo/public/c/environment/async_waiter.h" #include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/callback.h" -#include "mojo/public/cpp/bindings/error_handler.h" #include "mojo/public/cpp/bindings/interface_ptr.h" #include "mojo/public/cpp/bindings/interface_request.h" #include "mojo/public/cpp/bindings/lib/filter_chain.h" @@ -104,21 +103,11 @@ class StrongBinding { return binding_.WaitForIncomingMethodCall(); } + // Note: The error handler must not delete the interface implementation. void set_connection_error_handler(const Closure& error_handler) { connection_error_handler_ = error_handler; } - // NOTE: Deprecated. Please use the method above. - // TODO(yzshen): Remove this method once all callsites are converted. - void set_error_handler(ErrorHandler* error_handler) { - if (error_handler) { - set_connection_error_handler( - [error_handler]() { error_handler->OnConnectionError(); }); - } else { - set_connection_error_handler(Closure()); - } - } - Interface* impl() { return binding_.impl(); } // Exposed for testing, should not generally be used. internal::Router* internal_router() { return binding_.internal_router(); } diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/BUILD.gn b/third_party/mojo/src/mojo/public/cpp/bindings/tests/BUILD.gn index bb94bc2..d087cbf 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/BUILD.gn +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/BUILD.gn @@ -23,6 +23,8 @@ mojo_sdk_source_set("tests") { "handle_passing_unittest.cc", "interface_ptr_unittest.cc", "map_unittest.cc", + "message_queue.cc", + "message_queue.h", "request_response_unittest.cc", "router_unittest.cc", "sample_service_unittest.cc", @@ -51,6 +53,28 @@ mojo_sdk_source_set("tests") { ] } +mojo_sdk_source_set("perftests") { + testonly = true + + sources = [ + "bindings_perftest.cc", + ] + + deps = [ + "//testing/gtest", + ] + + mojo_sdk_deps = [ + "mojo/public/cpp/bindings", + "mojo/public/cpp/bindings:callback", + "mojo/public/cpp/environment:standalone", + "mojo/public/cpp/system", + "mojo/public/cpp/test_support:test_utils", + "mojo/public/cpp/utility", + "mojo/public/interfaces/bindings/tests:test_interfaces", + ] +} + mojo_sdk_source_set("mojo_public_bindings_test_utils") { sources = [ "validation_test_input_parser.cc", diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/binding_callback_unittest.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/binding_callback_unittest.cc index 8ec51bf..3bc018c 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/binding_callback_unittest.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/binding_callback_unittest.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "build/build_config.h" +#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/interface_ptr.h" #include "mojo/public/cpp/bindings/string.h" #include "mojo/public/cpp/environment/environment.h" diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/binding_unittest.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/binding_unittest.cc index ef23279..59d2501 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/binding_unittest.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/binding_unittest.cc @@ -2,8 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +// Note: This file tests both binding.h (mojo::Binding) and strong_binding.h +// (mojo::StrongBinding). + #include "mojo/public/cpp/bindings/binding.h" +#include "mojo/public/cpp/bindings/strong_binding.h" #include "mojo/public/cpp/environment/environment.h" +#include "mojo/public/cpp/system/macros.h" #include "mojo/public/cpp/utility/run_loop.h" #include "mojo/public/interfaces/bindings/tests/sample_interfaces.mojom.h" #include "mojo/public/interfaces/bindings/tests/sample_service.mojom.h" @@ -12,10 +17,28 @@ namespace mojo { namespace { +class BindingTestBase : public testing::Test { + public: + BindingTestBase() {} + ~BindingTestBase() override {} + + RunLoop& loop() { return loop_; } + + private: + Environment env_; + RunLoop loop_; + + MOJO_DISALLOW_COPY_AND_ASSIGN(BindingTestBase); +}; + class ServiceImpl : public sample::Service { public: - ServiceImpl() {} - ~ServiceImpl() override {} + explicit ServiceImpl(bool* was_deleted = nullptr) + : was_deleted_(was_deleted) {} + ~ServiceImpl() override { + if (was_deleted_) + *was_deleted_ = true; + } private: // sample::Service implementation @@ -26,30 +49,29 @@ class ServiceImpl : public sample::Service { callback.Run(1); } void GetPort(InterfaceRequest<sample::Port> port) override {} -}; -class IntegerAccessorImpl : public sample::IntegerAccessor { - public: - IntegerAccessorImpl() {} - ~IntegerAccessorImpl() override {} + bool* const was_deleted_; - private: - // sample::IntegerAccessor implementation. - void GetInteger(const GetIntegerCallback& callback) override { - callback.Run(1, sample::ENUM_VALUE); - } - void SetInteger(int64_t data, sample::Enum type) override {} + MOJO_DISALLOW_COPY_AND_ASSIGN(ServiceImpl); }; -class BindingTest : public testing::Test { - public: - BindingTest() {} - ~BindingTest() override {} +// BindingTest ----------------------------------------------------------------- - protected: - Environment env_; - RunLoop loop_; -}; +using BindingTest = BindingTestBase; + +TEST_F(BindingTest, Close) { + bool called = false; + sample::ServicePtr ptr; + auto request = GetProxy(&ptr); + ptr.set_connection_error_handler([&called]() { called = true; }); + ServiceImpl impl; + Binding<sample::Service> binding(&impl, request.Pass()); + + binding.Close(); + EXPECT_FALSE(called); + loop().RunUntilIdle(); + EXPECT_TRUE(called); +} // Tests that destroying a mojo::Binding closes the bound message pipe handle. TEST_F(BindingTest, DestroyClosesMessagePipe) { @@ -65,23 +87,87 @@ TEST_F(BindingTest, DestroyClosesMessagePipe) { Binding<sample::Service> binding(&impl, request.Pass()); ptr->Frobinate(nullptr, sample::Service::BAZ_OPTIONS_REGULAR, nullptr, called_cb); - loop_.RunUntilIdle(); + loop().RunUntilIdle(); EXPECT_TRUE(called); EXPECT_FALSE(encountered_error); } // Now that the Binding is out of scope we should detect an error on the other // end of the pipe. - loop_.RunUntilIdle(); + loop().RunUntilIdle(); EXPECT_TRUE(encountered_error); // And calls should fail. called = false; ptr->Frobinate(nullptr, sample::Service::BAZ_OPTIONS_REGULAR, nullptr, called_cb); - loop_.RunUntilIdle(); + loop().RunUntilIdle(); + EXPECT_FALSE(called); +} + +// Tests that the binding's connection error handler gets called when the other +// end is closed. +TEST_F(BindingTest, ConnectionError) { + bool called = false; + { + ServiceImpl impl; + sample::ServicePtr ptr; + Binding<sample::Service> binding(&impl, GetProxy(&ptr)); + binding.set_connection_error_handler([&called]() { called = true; }); + ptr.reset(); + EXPECT_FALSE(called); + loop().RunUntilIdle(); + EXPECT_TRUE(called); + // We want to make sure that it isn't called again during destruction. + called = false; + } + EXPECT_FALSE(called); +} + +// Tests that calling Close doesn't result in the connection error handler being +// called. +TEST_F(BindingTest, CloseDoesntCallConnectionErrorHandler) { + ServiceImpl impl; + sample::ServicePtr ptr; + Binding<sample::Service> binding(&impl, GetProxy(&ptr)); + bool called = false; + binding.set_connection_error_handler([&called]() { called = true; }); + binding.Close(); + loop().RunUntilIdle(); + EXPECT_FALSE(called); + + // We can also close the other end, and the error handler still won't be + // called. + ptr.reset(); + loop().RunUntilIdle(); EXPECT_FALSE(called); } +class ServiceImplWithBinding : public ServiceImpl { + public: + ServiceImplWithBinding(bool* was_deleted, + InterfaceRequest<sample::Service> request) + : ServiceImpl(was_deleted), binding_(this, request.Pass()) { + binding_.set_connection_error_handler([this]() { delete this; }); + } + + private: + Binding<sample::Service> binding_; + + MOJO_DISALLOW_COPY_AND_ASSIGN(ServiceImplWithBinding); +}; + +// Tests that the binding may be deleted in the connection error handler. +TEST_F(BindingTest, SelfDeleteOnConnectionError) { + bool was_deleted = false; + sample::ServicePtr ptr; + // This should delete itself on connection error. + new ServiceImplWithBinding(&was_deleted, GetProxy(&ptr)); + ptr.reset(); + EXPECT_FALSE(was_deleted); + loop().RunUntilIdle(); + EXPECT_TRUE(was_deleted); +} + // Tests that explicitly calling Unbind followed by rebinding works. TEST_F(BindingTest, Unbind) { ServiceImpl impl; @@ -92,7 +178,7 @@ TEST_F(BindingTest, Unbind) { auto called_cb = [&called](int32_t result) { called = true; }; ptr->Frobinate(nullptr, sample::Service::BAZ_OPTIONS_REGULAR, nullptr, called_cb); - loop_.RunUntilIdle(); + loop().RunUntilIdle(); EXPECT_TRUE(called); called = false; @@ -101,7 +187,7 @@ TEST_F(BindingTest, Unbind) { // All calls should fail when not bound... ptr->Frobinate(nullptr, sample::Service::BAZ_OPTIONS_REGULAR, nullptr, called_cb); - loop_.RunUntilIdle(); + loop().RunUntilIdle(); EXPECT_FALSE(called); called = false; @@ -110,10 +196,25 @@ TEST_F(BindingTest, Unbind) { // ...and should succeed again when the rebound. ptr->Frobinate(nullptr, sample::Service::BAZ_OPTIONS_REGULAR, nullptr, called_cb); - loop_.RunUntilIdle(); + loop().RunUntilIdle(); EXPECT_TRUE(called); } +class IntegerAccessorImpl : public sample::IntegerAccessor { + public: + IntegerAccessorImpl() {} + ~IntegerAccessorImpl() override {} + + private: + // sample::IntegerAccessor implementation. + void GetInteger(const GetIntegerCallback& callback) override { + callback.Run(1, sample::ENUM_VALUE); + } + void SetInteger(int64_t data, sample::Enum type) override {} + + MOJO_DISALLOW_COPY_AND_ASSIGN(IntegerAccessorImpl); +}; + TEST_F(BindingTest, SetInterfacePtrVersion) { IntegerAccessorImpl impl; sample::IntegerAccessorPtr ptr; @@ -121,5 +222,102 @@ TEST_F(BindingTest, SetInterfacePtrVersion) { EXPECT_EQ(3u, ptr.version()); } +// StrongBindingTest ----------------------------------------------------------- + +using StrongBindingTest = BindingTestBase; + +// Tests that destroying a mojo::StrongBinding closes the bound message pipe +// handle but does *not* destroy the implementation object. +TEST_F(StrongBindingTest, DestroyClosesMessagePipe) { + bool encountered_error = false; + bool was_deleted = false; + ServiceImpl impl(&was_deleted); + sample::ServicePtr ptr; + auto request = GetProxy(&ptr); + ptr.set_connection_error_handler( + [&encountered_error]() { encountered_error = true; }); + bool called = false; + auto called_cb = [&called](int32_t result) { called = true; }; + { + StrongBinding<sample::Service> binding(&impl, request.Pass()); + ptr->Frobinate(nullptr, sample::Service::BAZ_OPTIONS_REGULAR, nullptr, + called_cb); + loop().RunUntilIdle(); + EXPECT_TRUE(called); + EXPECT_FALSE(encountered_error); + } + // Now that the StrongBinding is out of scope we should detect an error on the + // other end of the pipe. + loop().RunUntilIdle(); + EXPECT_TRUE(encountered_error); + // But destroying the StrongBinding doesn't destroy the object. + ASSERT_FALSE(was_deleted); +} + +class ServiceImplWithStrongBinding : public ServiceImpl { + public: + ServiceImplWithStrongBinding(bool* was_deleted, + InterfaceRequest<sample::Service> request) + : ServiceImpl(was_deleted), binding_(this, request.Pass()) {} + + StrongBinding<sample::Service>& binding() { return binding_; } + + private: + StrongBinding<sample::Service> binding_; + + MOJO_DISALLOW_COPY_AND_ASSIGN(ServiceImplWithStrongBinding); +}; + +// Tests the typical case, where the implementation object owns the +// StrongBinding (and should be destroyed on connection error). +TEST_F(StrongBindingTest, ConnectionErrorDestroysImpl) { + sample::ServicePtr ptr; + bool was_deleted = false; + // Will delete itself. + new ServiceImplWithBinding(&was_deleted, GetProxy(&ptr)); + + loop().RunUntilIdle(); + EXPECT_FALSE(was_deleted); + + ptr.reset(); + EXPECT_FALSE(was_deleted); + loop().RunUntilIdle(); + EXPECT_TRUE(was_deleted); +} + +// Tests that even when the implementation object owns the StrongBinding, that +// the implementation can still be deleted (which should result in the message +// pipe being closed). Also checks that the connection error handler doesn't get +// called. +TEST_F(StrongBindingTest, ExplicitDeleteImpl) { + bool ptr_error_handler_called = false; + sample::ServicePtr ptr; + auto request = GetProxy(&ptr); + ptr.set_connection_error_handler( + [&ptr_error_handler_called]() { ptr_error_handler_called = true; }); + bool was_deleted = false; + ServiceImplWithStrongBinding* impl = + new ServiceImplWithStrongBinding(&was_deleted, request.Pass()); + bool binding_error_handler_called = false; + impl->binding().set_connection_error_handler( + [&binding_error_handler_called]() { + binding_error_handler_called = true; + }); + + loop().RunUntilIdle(); + EXPECT_FALSE(ptr_error_handler_called); + EXPECT_FALSE(was_deleted); + + delete impl; + EXPECT_FALSE(ptr_error_handler_called); + EXPECT_TRUE(was_deleted); + was_deleted = false; // It shouldn't be double-deleted! + loop().RunUntilIdle(); + EXPECT_TRUE(ptr_error_handler_called); + EXPECT_FALSE(was_deleted); + + EXPECT_FALSE(binding_error_handler_called); +} + } // namespace } // mojo diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/bindings_perftest.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/bindings_perftest.cc new file mode 100644 index 0000000..7440acc --- /dev/null +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/bindings_perftest.cc @@ -0,0 +1,127 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "mojo/public/cpp/bindings/binding.h" +#include "mojo/public/cpp/test_support/test_support.h" +#include "mojo/public/cpp/test_support/test_utils.h" +#include "mojo/public/cpp/utility/run_loop.h" +#include "mojo/public/interfaces/bindings/tests/ping_service.mojom.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace mojo { +namespace { + +const double kMojoTicksPerSecond = 1000000.0; + +double MojoTicksToSeconds(MojoTimeTicks ticks) { + return ticks / kMojoTicksPerSecond; +} + +class PingServiceImpl : public test::PingService { + public: + explicit PingServiceImpl() {} + ~PingServiceImpl() override {} + + // |PingService| methods: + void Ping(const Callback<void()>& callback) override; + + private: + MOJO_DISALLOW_COPY_AND_ASSIGN(PingServiceImpl); +}; + +void PingServiceImpl::Ping(const Callback<void()>& callback) { + callback.Run(); +} + +class PingPongTest { + public: + explicit PingPongTest(test::PingServicePtr service); + + void Run(unsigned int iterations); + + private: + void OnPingDone(); + + test::PingServicePtr service_; + unsigned int iterations_to_run_; + unsigned int current_iterations_; + + MOJO_DISALLOW_COPY_AND_ASSIGN(PingPongTest); +}; + +PingPongTest::PingPongTest(test::PingServicePtr service) + : service_(service.Pass()) { +} + +void PingPongTest::Run(unsigned int iterations) { + iterations_to_run_ = iterations; + current_iterations_ = 0; + + service_->Ping([this]() { OnPingDone(); }); + RunLoop::current()->Run(); +} + +void PingPongTest::OnPingDone() { + current_iterations_++; + if (current_iterations_ >= iterations_to_run_) { + RunLoop::current()->Quit(); + return; + } + + service_->Ping([this]() { OnPingDone(); }); +} + +struct BoundPingService { + BoundPingService() : binding(&impl) { + binding.Bind(GetProxy(&service)); + } + + PingServiceImpl impl; + test::PingServicePtr service; + Binding<test::PingService> binding; +}; + +class MojoBindingsPerftest : public testing::Test { + protected: + Environment env_; + RunLoop run_loop_; +}; + +TEST_F(MojoBindingsPerftest, InProcessPingPong) { + test::PingServicePtr service; + PingServiceImpl impl; + Binding<test::PingService> binding(&impl, GetProxy(&service)); + PingPongTest test(service.Pass()); + + { + const unsigned int kIterations = 100000; + const MojoTimeTicks start_time = MojoGetTimeTicksNow(); + test.Run(kIterations); + const MojoTimeTicks end_time = MojoGetTimeTicksNow(); + test::LogPerfResult( + "InProcessPingPong", "0_Inactive", + kIterations / MojoTicksToSeconds(end_time - start_time), + "pings/second"); + } + + { + const size_t kNumInactiveServices = 1000; + BoundPingService* inactive_services = + new BoundPingService[kNumInactiveServices]; + + const unsigned int kIterations = 10000; + const MojoTimeTicks start_time = MojoGetTimeTicksNow(); + test.Run(kIterations); + const MojoTimeTicks end_time = MojoGetTimeTicksNow(); + test::LogPerfResult( + "InProcessPingPong", "1000_Inactive", + kIterations / MojoTicksToSeconds(end_time - start_time), + "pings/second"); + + delete[] inactive_services; + } +} + +} // namespace +} // namespace mojo diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/callback_unittest.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/callback_unittest.cc index 6b788fc..158b21e 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/callback_unittest.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/callback_unittest.cc @@ -44,6 +44,24 @@ struct RunnableMoveOnlyParam { int* calls; }; +int* g_calls = nullptr; + +void FunctionNoArgs() { + (*g_calls)++; +} + +void FunctionOneArg(int increment) { + (*g_calls) += increment; +} + +void FunctionStringArgByConstRef(const String& s) { + (*g_calls)++; +} + +void FunctionMoveOnlyType(ExampleMoveOnlyType m) { + (*g_calls)++; +} + static_assert(!internal::HasCompatibleCallOperator<RunnableNoArgs>::value, "HasCompatibleCallOperator<Runnable>"); static_assert(!internal::HasCompatibleCallOperator<RunnableOneArg, int>::value, @@ -76,8 +94,9 @@ static_assert(internal::HasCompatibleCallOperator<decltype(lambda_four), "ExampleMoveOnlyType>"); // Tests constructing and invoking a mojo::Callback from objects with a -// compatible Run() method (called 'runnables') and from lambdas. -TEST(CallbackFromLambda, Create) { +// compatible Run() method (called 'runnables'), from lambdas, and from function +// pointers. +TEST(Callback, Create) { int calls = 0; RunnableNoArgs f(&calls); @@ -121,6 +140,55 @@ TEST(CallbackFromLambda, Create) { cb_with_move_only_param = [&calls](ExampleMoveOnlyType m) { calls++; }; cb_with_move_only_param.Run(m.Clone()); EXPECT_EQ(8, calls); + + // Construct from a function pointer. + g_calls = &calls; + + cb = &FunctionNoArgs; + cb.Run(); + EXPECT_EQ(9, calls); + + cb_with_param = &FunctionOneArg; + cb_with_param.Run(1); + EXPECT_EQ(10, calls); + + cb_with_string_param = &FunctionStringArgByConstRef; + cb_with_string_param.Run(String("hello")); + EXPECT_EQ(11, calls); + + cb_with_move_only_param = &FunctionMoveOnlyType; + cb_with_move_only_param.Run(m.Clone()); + EXPECT_EQ(12, calls); + + g_calls = nullptr; +} + +bool g_overloaded_function_with_int_param_called = false; + +void OverloadedFunction(int param) { + g_overloaded_function_with_int_param_called = true; +} + +bool g_overloaded_function_with_double_param_called = false; + +void OverloadedFunction(double param) { + g_overloaded_function_with_double_param_called = true; +} + +// Tests constructing and invoking a mojo::Callback from pointers to overloaded +// functions. +TEST(Callback, CreateFromOverloadedFunctionPtr) { + g_overloaded_function_with_int_param_called = false; + mojo::Callback<void(int)> cb_with_int_param = &OverloadedFunction; + cb_with_int_param.Run(123); + EXPECT_TRUE(g_overloaded_function_with_int_param_called); + g_overloaded_function_with_int_param_called = false; + + g_overloaded_function_with_double_param_called = false; + mojo::Callback<void(double)> cb_with_double_param = &OverloadedFunction; + cb_with_double_param.Run(123); + EXPECT_TRUE(g_overloaded_function_with_double_param_called); + g_overloaded_function_with_double_param_called = false; } } // namespace diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/connector_unittest.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/connector_unittest.cc index b5e3405..aab5a39 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/connector_unittest.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/connector_unittest.cc @@ -7,7 +7,7 @@ #include "mojo/public/cpp/bindings/lib/connector.h" #include "mojo/public/cpp/bindings/lib/message_builder.h" -#include "mojo/public/cpp/bindings/lib/message_queue.h" +#include "mojo/public/cpp/bindings/tests/message_queue.h" #include "mojo/public/cpp/environment/environment.h" #include "mojo/public/cpp/system/macros.h" #include "mojo/public/cpp/utility/run_loop.h" @@ -31,7 +31,7 @@ class MessageAccumulator : public MessageReceiver { void Pop(Message* message) { queue_.Pop(message); } private: - internal::MessageQueue queue_; + MessageQueue queue_; }; class ConnectorDeletingMessageAccumulator : public MessageAccumulator { diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc index 6486192..681cc6d 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc @@ -192,6 +192,13 @@ class InterfacePtrTest : public testing::Test { RunLoop loop_; }; +TEST_F(InterfacePtrTest, IsBound) { + math::CalculatorPtr calc; + EXPECT_FALSE(calc.is_bound()); + MathCalculatorImpl calc_impl(GetProxy(&calc)); + EXPECT_TRUE(calc.is_bound()); +} + TEST_F(InterfacePtrTest, EndToEnd) { math::CalculatorPtr calc; MathCalculatorImpl calc_impl(GetProxy(&calc)); diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/lib/message_queue.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/message_queue.cc index fd701e97..65b9a5c 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/lib/message_queue.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/message_queue.cc @@ -2,13 +2,13 @@ // 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/lib/message_queue.h" +#include "mojo/public/cpp/bindings/tests/message_queue.h" #include "mojo/public/cpp/bindings/message.h" #include "mojo/public/cpp/environment/logging.h" namespace mojo { -namespace internal { +namespace test { MessageQueue::MessageQueue() { } @@ -22,11 +22,6 @@ bool MessageQueue::IsEmpty() const { return queue_.empty(); } -Message* MessageQueue::Peek() { - MOJO_DCHECK(!queue_.empty()); - return queue_.front(); -} - void MessageQueue::Push(Message* message) { queue_.push(new Message()); queue_.back()->Swap(message); @@ -44,5 +39,5 @@ void MessageQueue::Pop() { queue_.pop(); } -} // namespace internal +} // namespace test } // namespace mojo diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/lib/message_queue.h b/third_party/mojo/src/mojo/public/cpp/bindings/tests/message_queue.h index 4e46b54..c47ca99 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/lib/message_queue.h +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/message_queue.h @@ -12,7 +12,7 @@ namespace mojo { class Message; -namespace internal { +namespace test { // A queue for Message objects. class MessageQueue { @@ -21,27 +21,23 @@ class MessageQueue { ~MessageQueue(); bool IsEmpty() const; - Message* Peek(); - // This method transfers ownership of |message->data| and |message->handles| - // to the message queue, resetting |message| in the process. + // This method copies the message data and steals ownership of its handles. void Push(Message* message); - // Removes the next message from the queue, transferring ownership of its - // data and handles to the given |message|. + // Removes the next message from the queue, copying its data and transferring + // ownership of its handles to the given |message|. void Pop(Message* message); - // Removes the next message from the queue, discarding its data and handles. - // This is meant to be used in conjunction with |Peek|. + private: void Pop(); - private: std::queue<Message*> queue_; MOJO_DISALLOW_COPY_AND_ASSIGN(MessageQueue); }; -} // namespace internal +} // namespace test } // namespace mojo #endif // MOJO_PUBLIC_CPP_BINDINGS_LIB_MESSAGE_QUEUE_H_ diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/request_response_unittest.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/request_response_unittest.cc index f7c3cf6..8bfeb5d 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/request_response_unittest.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/request_response_unittest.cc @@ -2,6 +2,7 @@ // 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/binding.h" #include "mojo/public/cpp/environment/environment.h" #include "mojo/public/cpp/test_support/test_utils.h" #include "mojo/public/cpp/utility/run_loop.h" diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/router_unittest.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/router_unittest.cc index def9ceb..7aeb414 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/router_unittest.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/router_unittest.cc @@ -6,8 +6,8 @@ #include <string.h> #include "mojo/public/cpp/bindings/lib/message_builder.h" -#include "mojo/public/cpp/bindings/lib/message_queue.h" #include "mojo/public/cpp/bindings/lib/router.h" +#include "mojo/public/cpp/bindings/tests/message_queue.h" #include "mojo/public/cpp/environment/environment.h" #include "mojo/public/cpp/system/macros.h" #include "mojo/public/cpp/utility/run_loop.h" @@ -36,7 +36,7 @@ void AllocResponseMessage(uint32_t name, class MessageAccumulator : public MessageReceiver { public: - explicit MessageAccumulator(internal::MessageQueue* queue) : queue_(queue) {} + explicit MessageAccumulator(MessageQueue* queue) : queue_(queue) {} bool Accept(Message* message) override { queue_->Push(message); @@ -44,7 +44,7 @@ class MessageAccumulator : public MessageReceiver { } private: - internal::MessageQueue* queue_; + MessageQueue* queue_; }; class ResponseGenerator : public MessageReceiverWithResponderStatus { @@ -152,7 +152,7 @@ TEST_F(RouterTest, BasicRequestResponse) { Message request; AllocRequestMessage(1, "hello", &request); - internal::MessageQueue message_queue; + MessageQueue message_queue; router0.AcceptWithResponder(&request, new MessageAccumulator(&message_queue)); PumpMessages(); @@ -192,7 +192,7 @@ TEST_F(RouterTest, BasicRequestResponse_Synchronous) { Message request; AllocRequestMessage(1, "hello", &request); - internal::MessageQueue message_queue; + MessageQueue message_queue; router0.AcceptWithResponder(&request, new MessageAccumulator(&message_queue)); router1.WaitForIncomingMessage(MOJO_DEADLINE_INDEFINITE); @@ -234,7 +234,7 @@ TEST_F(RouterTest, RequestWithNoReceiver) { Message request; AllocRequestMessage(1, "hello", &request); - internal::MessageQueue message_queue; + MessageQueue message_queue; router0.AcceptWithResponder(&request, new MessageAccumulator(&message_queue)); PumpMessages(); @@ -256,7 +256,7 @@ TEST_F(RouterTest, LazyResponses) { Message request; AllocRequestMessage(1, "hello", &request); - internal::MessageQueue message_queue; + MessageQueue message_queue; router0.AcceptWithResponder(&request, new MessageAccumulator(&message_queue)); PumpMessages(); @@ -311,7 +311,7 @@ TEST_F(RouterTest, MissingResponses) { Message request; AllocRequestMessage(1, "hello", &request); - internal::MessageQueue message_queue; + MessageQueue message_queue; router0.AcceptWithResponder(&request, new MessageAccumulator(&message_queue)); PumpMessages(); @@ -354,7 +354,7 @@ TEST_F(RouterTest, LateResponse) { Message request; AllocRequestMessage(1, "hello", &request); - internal::MessageQueue message_queue; + MessageQueue message_queue; router0.AcceptWithResponder(&request, new MessageAccumulator(&message_queue)); diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/validation_unittest.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/validation_unittest.cc index 9b4e271..39befaf 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/validation_unittest.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/tests/validation_unittest.cc @@ -9,7 +9,7 @@ #include <vector> #include "mojo/public/c/system/macros.h" -#include "mojo/public/cpp/bindings/interface_impl.h" +#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/interface_ptr.h" #include "mojo/public/cpp/bindings/lib/connector.h" #include "mojo/public/cpp/bindings/lib/filter_chain.h" diff --git a/third_party/mojo/src/mojo/public/dart/.analysis_options b/third_party/mojo/src/mojo/public/dart/.analysis_options new file mode 100644 index 0000000..4886c56 --- /dev/null +++ b/third_party/mojo/src/mojo/public/dart/.analysis_options @@ -0,0 +1,3 @@ +analyzer: + exclude: + - 'sdk_ext/**' diff --git a/third_party/mojo/src/mojo/public/dart/.gitignore b/third_party/mojo/src/mojo/public/dart/.gitignore index 3b66ccd..d0a207a 100644 --- a/third_party/mojo/src/mojo/public/dart/.gitignore +++ b/third_party/mojo/src/mojo/public/dart/.gitignore @@ -1,2 +1,4 @@ +lib/_sdkext +.packages packages/ pubspec.lock diff --git a/third_party/mojo/src/mojo/public/dart/BUILD.gn b/third_party/mojo/src/mojo/public/dart/BUILD.gn index b820d96..fb14555 100644 --- a/third_party/mojo/src/mojo/public/dart/BUILD.gn +++ b/third_party/mojo/src/mojo/public/dart/BUILD.gn @@ -21,10 +21,6 @@ dart_mojo_sdk_sources = [ "lib/src/stub.dart", "lib/src/union.dart", "lib/src/types.dart", - "sdk_ext/internal.dart", - "sdk_ext/src/handle_watcher.dart", - "sdk_ext/src/natives.dart", - "sdk_ext/src/timer_queue.dart", ] dartzip_package("dart") { @@ -49,6 +45,9 @@ dart_pkg("mojo") { "README.md", ] + sdk_ext_directory = "sdk_ext" + sdk_ext_mappings = [ "dart:mojo.internal,internal.dart" ] + # List of mojom targets that the mojo pkg exports deps = [ "../interfaces", diff --git a/third_party/mojo/src/mojo/public/dart/CHANGELOG.md b/third_party/mojo/src/mojo/public/dart/CHANGELOG.md index dcb340c..03b981d 100644 --- a/third_party/mojo/src/mojo/public/dart/CHANGELOG.md +++ b/third_party/mojo/src/mojo/public/dart/CHANGELOG.md @@ -1,3 +1,23 @@ +## 0.0.18 + + - 89 changes: https://github.com/domokit/mojo/compare/0fd4d06...c3119f6 + +## 0.0.17 + + - 18 changes: https://github.com/domokit/mojo/compare/e7433cf...8879bfd + +## 0.0.16 + + - 27 changes: https://github.com/domokit/mojo/compare/e028733...e7433cf + +## 0.0.15 + + - 6 changes: https://github.com/domokit/mojo/compare/4df2d39...e028733 + +## 0.0.14 + + - 138 changes: https://github.com/domokit/mojo/compare/850ac24...cf84c48 + ## 0.0.13 - 70 changes: https://github.com/domokit/mojo/compare/889091e...136e0d4 diff --git a/third_party/mojo/src/mojo/public/dart/lib/io.dart b/third_party/mojo/src/mojo/public/dart/lib/io.dart deleted file mode 100644 index bb8e64dc..0000000 --- a/third_party/mojo/src/mojo/public/dart/lib/io.dart +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -library dart.io; - -import 'dart:async'; -import 'dart:collection' show HashMap, - HashSet, - Queue, - ListQueue, - LinkedList, - LinkedListEntry, - UnmodifiableMapView; -import 'dart:convert'; -import 'dart:isolate'; -import 'dart:math'; -import 'dart:typed_data'; - - -part '../../../../dart/sdk/lib/io/bytes_builder.dart'; -part '../../../../dart/sdk/lib/io/common.dart'; -part '../../../../dart/sdk/lib/io/crypto.dart'; -part '../../../../dart/sdk/lib/io/data_transformer.dart'; -part '../../../../dart/sdk/lib/io/directory.dart'; -part '../../../../dart/sdk/lib/io/directory_impl.dart'; -part '../../../../dart/sdk/lib/io/file.dart'; -part '../../../../dart/sdk/lib/io/file_impl.dart'; -part '../../../../dart/sdk/lib/io/file_system_entity.dart'; -part '../../../../dart/sdk/lib/io/http.dart'; -part '../../../../dart/sdk/lib/io/http_date.dart'; -part '../../../../dart/sdk/lib/io/http_headers.dart'; -part '../../../../dart/sdk/lib/io/http_impl.dart'; -part '../../../../dart/sdk/lib/io/http_parser.dart'; -part '../../../../dart/sdk/lib/io/http_session.dart'; -part '../../../../dart/sdk/lib/io/io_sink.dart'; -part '../../../../dart/sdk/lib/io/io_service.dart'; -part '../../../../dart/sdk/lib/io/link.dart'; -part '../../../../dart/sdk/lib/io/platform.dart'; -part '../../../../dart/sdk/lib/io/platform_impl.dart'; -part '../../../../dart/sdk/lib/io/process.dart'; -part '../../../../dart/sdk/lib/io/service_object.dart'; -part '../../../../dart/sdk/lib/io/socket.dart'; -part '../../../../dart/sdk/lib/io/stdio.dart'; -part '../../../../dart/sdk/lib/io/string_transformer.dart'; -part '../../../../dart/sdk/lib/io/secure_socket.dart'; -part '../../../../dart/sdk/lib/io/secure_server_socket.dart'; -part '../../../../dart/sdk/lib/io/websocket.dart'; -part '../../../../dart/sdk/lib/io/websocket_impl.dart'; diff --git a/third_party/mojo/src/mojo/public/dart/pubspec.yaml b/third_party/mojo/src/mojo/public/dart/pubspec.yaml index 3944b34..bdcad2e 100644 --- a/third_party/mojo/src/mojo/public/dart/pubspec.yaml +++ b/third_party/mojo/src/mojo/public/dart/pubspec.yaml @@ -4,4 +4,4 @@ dependencies: description: Dart files to support executing inside Mojo. homepage: https://github.com/domokit/mojo name: mojo -version: 0.0.13 +version: 0.0.18 diff --git a/third_party/mojo/src/mojo/public/dart/rules.gni b/third_party/mojo/src/mojo/public/dart/rules.gni index 36fc92d..d4adbb1 100644 --- a/third_party/mojo/src/mojo/public/dart/rules.gni +++ b/third_party/mojo/src/mojo/public/dart/rules.gni @@ -116,8 +116,6 @@ template("dartzip_package") { rebase_path(package_output), rebase_path("$target_gen_dir/${package_target_name}_analyze.stamp"), "--no-hints", - "--url-mapping=dart:io,/" + - rebase_path("mojo/public/dart/lib/io.dart", "/", mojo_sdk_root), ] deps = [ @@ -226,6 +224,12 @@ template("dartzip_packaged_application") { # sdk_ext_directory (optional) # Directory containing sdk-ext .dart sources. # +# sdk_ext_files (optional) +# List of sources to include in sdk-ext. +# +# sdk_ext_mappings (optional) +# Mappings for dart libraries that are part of of sdk_ext. +# template("dart_pkg") { if (defined(invoker.pkg_dir)) { pubspec_yaml_path = rebase_path("pubspec.yaml", "", invoker.pkg_dir) @@ -245,6 +249,7 @@ template("dart_pkg") { pkg_directory = rebase_path("$root_gen_dir/dart-pkg") package_root = rebase_path("$root_gen_dir/dart-pkg/packages") stamp_file = "$root_gen_dir/dart-pkg/${package_name}.stamp" + output_dir = "$root_gen_dir/dart-pkg/${package_name}" assert(defined(invoker.sources) || defined(invoker.pkg_dir)) @@ -278,8 +283,19 @@ template("dart_pkg") { sdk_ext_directory += [ invoker.sdk_ext_directory ] } + sdk_ext_files = [] + if (defined(invoker.sdk_ext_files)) { + sdk_ext_files += invoker.sdk_ext_files + } + + sdk_ext_mappings = [] + if (defined(invoker.sdk_ext_mappings)) { + sdk_ext_mappings += invoker.sdk_ext_mappings + } + script = rebase_path("mojo/public/tools/dart_pkg.py", ".", mojo_sdk_root) outputs = [ + output_dir, stamp_file, ] @@ -315,6 +331,8 @@ template("dart_pkg") { "--package-sources", ] + rebase_path(sources) + [ "--mojom-sources" ] + rebase_path(mojom_sources, "", mojo_sdk_root) + - [ "--sdk-ext-directories" ] + rebase_path(sdk_ext_directory) + [ "--sdk-ext-directories" ] + rebase_path(sdk_ext_directory) + + [ "--sdk-ext-files" ] + rebase_path(sdk_ext_files) + + [ "--sdk-ext-mappings" ] + sdk_ext_mappings } } diff --git a/third_party/mojo/src/mojo/public/interfaces/bindings/tests/BUILD.gn b/third_party/mojo/src/mojo/public/interfaces/bindings/tests/BUILD.gn index 9bf206c..e7efd3d 100644 --- a/third_party/mojo/src/mojo/public/interfaces/bindings/tests/BUILD.gn +++ b/third_party/mojo/src/mojo/public/interfaces/bindings/tests/BUILD.gn @@ -9,6 +9,7 @@ mojom("test_interfaces") { sources = [ "math_calculator.mojom", "no_module.mojom", + "ping_service.mojom", "rect.mojom", "regression_tests.mojom", "sample_factory.mojom", diff --git a/third_party/mojo/src/mojo/public/interfaces/bindings/tests/ping_service.mojom b/third_party/mojo/src/mojo/public/interfaces/bindings/tests/ping_service.mojom new file mode 100644 index 0000000..3e1f1c4 --- /dev/null +++ b/third_party/mojo/src/mojo/public/interfaces/bindings/tests/ping_service.mojom @@ -0,0 +1,10 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +[JavaPackage="org.chromium.mojo.bindings.test.mojom.ping"] +module mojo.test; + +interface PingService { + Ping() => (); +}; diff --git a/third_party/mojo/src/mojo/public/interfaces/bindings/tests/test_unions.mojom b/third_party/mojo/src/mojo/public/interfaces/bindings/tests/test_unions.mojom index 6e631b2..e998f47 100644 --- a/third_party/mojo/src/mojo/public/interfaces/bindings/tests/test_unions.mojom +++ b/third_party/mojo/src/mojo/public/interfaces/bindings/tests/test_unions.mojom @@ -69,6 +69,7 @@ struct SmallStructNonNullableUnion { struct SmallObjStruct { ObjectUnion obj_union; + int8 f_int8; }; interface SmallCache { @@ -84,3 +85,12 @@ struct TryNonNullStruct { DummyStruct? nullable; DummyStruct non_nullable; }; + +union OldUnion { + int8 f_int8; +}; + +union NewUnion { + int8 f_int8; + int16 f_int16; +}; diff --git a/third_party/mojo/src/mojo/public/mojo_application.gni b/third_party/mojo/src/mojo/public/mojo_application.gni index 7ab5a31..32bdb7a 100644 --- a/third_party/mojo/src/mojo/public/mojo_application.gni +++ b/third_party/mojo/src/mojo/public/mojo_application.gni @@ -20,6 +20,7 @@ template("mojo_native_application") { if (!is_nacl) { output = base_target_name + ".mojo" library_target_name = base_target_name + "_library" + copy_symbols_target = "__${final_target_name}_copy_symbols" if (is_linux || is_android) { library_name = "lib${library_target_name}.so" @@ -130,7 +131,27 @@ template("mojo_native_application") { testonly = invoker.testonly } + visibility = [ + ":${final_target_name}", + ":${copy_symbols_target}", + ] + } + + copy(copy_symbols_target) { + if (defined(invoker.testonly)) { + testonly = invoker.testonly + } visibility = [ ":${final_target_name}" ] + deps = [ + ":${library_target_name}", + ] + + sources = [ + "${root_out_dir}/${library_name}", + ] + outputs = [ + "${root_out_dir}/symbols/${library_name}", + ] } copy(final_target_name) { @@ -142,6 +163,7 @@ template("mojo_native_application") { } deps = [ ":${library_target_name}", + ":${copy_symbols_target}", ] sources = [ @@ -289,30 +311,111 @@ template("mojo_native_application") { } if (is_android) { + import("//build/config/android/rules.gni") + # Declares an Android Mojo application consisting of an .so file and a # corresponding .dex.jar file. # # Variables: - # input_so: the .so file to bundle - # input_dex_jar: the .dex.jar file to bundle - # deps / public_deps / data_deps (optional): - # Dependencies. The targets that generate the .so/jar inputs should be - # listed in either deps or public_deps. + # sources (optional): The c++ sources. + # deps (optional): The c++ dependencies. + # java_sources (optional): The java sources. + # java_deps (optional): The java dependencies. + # jni_package (optional): The c++ package for the generated jni headers. # output_name (optional): override for the output file name + # public_deps / data_deps (optional): Dependencies. template("mojo_android_application") { - assert(defined(invoker.input_so)) - assert(defined(invoker.input_dex_jar)) + shared_library_name = "__${target_name}_lib" + library_basename = "lib${shared_library_name}.so" + if (defined(invoker.jni_package)) { + assert(defined(invoker.java_sources)) + generate_jni_name = "__${target_name}_jni" + } + if (defined(invoker.java_sources)) { + java_library_name = "__${target_name}_java" + } + android_standalone_library_name = "__${target_name}_java_lib" + dex_output_path = "${target_gen_dir}/${target_name}.dex.jar" + zip_action_name = "__${target_name}_zip" + zip_action_output = "${target_gen_dir}/${target_name}.zip" + copy_symbols_target = "__${target_name}_copy_symbols" + final_target_name = target_name + + if (defined(invoker.jni_package)) { + generate_jni(generate_jni_name) { + visibility = [ ":${shared_library_name}" ] + + sources = invoker.java_sources + jni_package = invoker.jni_package + } + } + + shared_library(shared_library_name) { + visibility = [ + ":${copy_symbols_target}", + ":${zip_action_name}", + ] + + if (defined(invoker.sources)) { + sources = invoker.sources + } + + deps = [] + if (defined(invoker.jni_package)) { + deps += [ ":${generate_jni_name}" ] + } + if (defined(invoker.deps)) { + deps += invoker.deps + } + } + + copy(copy_symbols_target) { + visibility = [ ":${final_target_name}" ] + deps = [ + ":${shared_library_name}", + ] + + sources = [ + "${root_out_dir}/${library_basename}", + ] + outputs = [ + "${root_out_dir}/symbols/${library_basename}", + ] + } + + if (defined(invoker.java_sources)) { + android_library(java_library_name) { + visibility = [ ":*" ] + + java_files = invoker.java_sources + + if (defined(invoker.java_deps)) { + deps = invoker.java_deps + } + } + } + + android_standalone_library(android_standalone_library_name) { + deps = [] + + if (defined(invoker.java_sources)) { + deps += [ ":${java_library_name}" ] + } + + if (defined(invoker.java_deps)) { + deps += invoker.java_deps + } + + dex_path = dex_output_path + } - zip_action_name = "${target_name}_zip" - zip_action_output = "$target_gen_dir/${target_name}.zip" - prepend_action_name = target_name action(zip_action_name) { - visibility = [ ":$prepend_action_name" ] + visibility = [ ":${final_target_name}" ] script = "//build/android/gn/zip.py" inputs = [ - invoker.input_so, - invoker.input_dex_jar, + "${root_out_dir}/lib.stripped/${library_basename}", + dex_output_path, ] output = zip_action_output @@ -323,8 +426,8 @@ if (is_android) { rebase_inputs = rebase_path(inputs, root_build_dir) rebase_output = rebase_path(output, root_build_dir) args = [ - "--inputs=$rebase_inputs", - "--output=$rebase_output", + "--inputs=${rebase_inputs}", + "--output=${rebase_output}", ] if (defined(invoker.deps)) { @@ -339,12 +442,12 @@ if (is_android) { } if (defined(invoker.output_name)) { - mojo_output = "$root_out_dir/" + invoker.output_name + ".mojo" + mojo_output = "${root_out_dir}/" + invoker.output_name + ".mojo" } else { - mojo_output = "$root_out_dir/" + target_name + ".mojo" + mojo_output = "${root_out_dir}/" + target_name + ".mojo" } - action(target_name) { + action(final_target_name) { script = rebase_path("mojo/public/tools/prepend.py", ".", mojo_root) input = zip_action_output @@ -360,13 +463,17 @@ if (is_android) { rebase_input = rebase_path(input, root_build_dir) rebase_output = rebase_path(output, root_build_dir) args = [ - "--input=$rebase_input", - "--output=$rebase_output", + "--input=${rebase_input}", + "--output=${rebase_output}", "--line=#!mojo mojo:android_handler", ] + deps = [ + ":${copy_symbols_target}", + ] + public_deps = [ - ":$zip_action_name", + ":${zip_action_name}", ] } } diff --git a/third_party/mojo/src/mojo/public/python/mojo_bindings/descriptor.py b/third_party/mojo/src/mojo/public/python/mojo_bindings/descriptor.py index efba73d..c359e44 100644 --- a/third_party/mojo/src/mojo/public/python/mojo_bindings/descriptor.py +++ b/third_party/mojo/src/mojo/public/python/mojo_bindings/descriptor.py @@ -34,6 +34,13 @@ class Type(object): """ return self.Convert(value) + def IsUnion(self): + """ + Returns true if the type is a union. This is necessary to be able to + identify a union when descriptor.py cannot be imported. + """ + return False + class SerializableType(Type): """Describe a type that can be serialized by itself.""" @@ -153,6 +160,47 @@ class FloatType(NumericType): return float(value) +class UnionType(SerializableType): + """Base Type object for union.""" + + def __init__(self, union_type_getter, nullable=False): + SerializableType.__init__(self, 'IIQ') + self.nullable = nullable + self._union_type_getter = union_type_getter + self._union_type = None + + def IsUnion(self): + return True + + @property + def union_type(self): + if not self._union_type: + self._union_type = self._union_type_getter() + return self._union_type + + def Serialize(self, value, data_offset, data, handle_offset): + if not value: + if not self.nullable: + raise serialization.SerializationException( + 'Trying to serialize null for non nullable type.') + return ((0, 0, 0), []) + + ((size, tag, entry, new_data), new_handles) = ( + value.SerializeInline(handle_offset)) + if len(new_data) > 0: + data.extend(new_data) + entry = data_offset - 8 + + return ((size, tag, entry), new_handles) + + def Deserialize(self, value, context): + result = self.union_type.Deserialize(context) + if not result and not self.nullable: + raise serialization.DeserializationException( + 'Trying to deserialize null for non nullable type.') + return result + + class PointerType(SerializableType): """Base Type object for pointers.""" @@ -434,16 +482,19 @@ class GenericArrayType(BaseArrayType): to_pack.extend(serialization.Flatten(new_data)) returned_handles.extend(new_handles) position = position + self.sub_type.GetByteSize() + serialization.HEADER_STRUCT.pack_into(data, data_end, size, len(value)) - struct.pack_into('%d%s' % (len(value), self.sub_type.GetTypeCode()), + # TODO(azani): Refactor so we don't have to create big formatting strings. + struct.pack_into(('%s' % self.sub_type.GetTypeCode()) * len(value), data, data_end + serialization.HEADER_STRUCT.size, *to_pack) return (data_offset, returned_handles) def DeserializeArray(self, size, nb_elements, context): + # TODO(azani): Refactor so the format string isn't so big. values = struct.unpack_from( - '%d%s' % (nb_elements, self.sub_type.GetTypeCode()), + nb_elements * self.sub_type.GetTypeCode(), buffer(context.data, serialization.HEADER_STRUCT.size)) values_per_element = len(self.sub_type.GetTypeCode()) assert nb_elements * values_per_element == len(values) diff --git a/third_party/mojo/src/mojo/public/python/mojo_bindings/reflection.py b/third_party/mojo/src/mojo/public/python/mojo_bindings/reflection.py index 6c4767b..cfd7e64 100644 --- a/third_party/mojo/src/mojo/public/python/mojo_bindings/reflection.py +++ b/third_party/mojo/src/mojo/public/python/mojo_bindings/reflection.py @@ -135,6 +135,103 @@ class MojoStructType(type): raise AttributeError('can\'t delete attribute') +class MojoUnionType(type): + + def __new__(mcs, name, bases, dictionary): + dictionary['__slots__'] = ('_cur_field', '_data') + descriptor = dictionary.pop('DESCRIPTOR', {}) + + fields = descriptor.get('fields', []) + def _BuildUnionProperty(field): + + # pylint: disable=W0212 + def Get(self): + if self._cur_field != field: + raise AttributeError('%s is not currently set' % field.name, + field.name, self._cur_field.name) + return self._data + + # pylint: disable=W0212 + def Set(self, value): + self._cur_field = field + self._data = field.field_type.Convert(value) + + return property(Get, Set) + + for field in fields: + dictionary[field.name] = _BuildUnionProperty(field) + + def UnionInit(self, **kwargs): + self.SetInternals(None, None) + items = kwargs.items() + if len(items) == 0: + return + + if len(items) > 1: + raise TypeError('only 1 member may be set on a union.') + + setattr(self, items[0][0], items[0][1]) + dictionary['__init__'] = UnionInit + + serializer = serialization.UnionSerializer(fields) + def SerializeUnionInline(self, handle_offset=0): + return serializer.SerializeInline(self, handle_offset) + dictionary['SerializeInline'] = SerializeUnionInline + + def SerializeUnion(self, handle_offset=0): + return serializer.Serialize(self, handle_offset) + dictionary['Serialize'] = SerializeUnion + + def DeserializeUnion(cls, context): + return serializer.Deserialize(context, cls) + dictionary['Deserialize'] = classmethod(DeserializeUnion) + + class Tags(object): + __metaclass__ = MojoEnumType + VALUES = [(field.name, field.index) for field in fields] + dictionary['Tags'] = Tags + + def GetTag(self): + return self._cur_field.index + dictionary['tag'] = property(GetTag, None) + + def GetData(self): + return self._data + dictionary['data'] = property(GetData, None) + + def IsUnknown(self): + return not self._cur_field + dictionary['IsUnknown'] = IsUnknown + + def UnionEq(self, other): + return ( + (type(self) is type(other)) + and (self.tag == other.tag) + and (self.data == other.data)) + dictionary['__eq__'] = UnionEq + + def UnionNe(self, other): + return not self.__eq__(other) + dictionary['__ne__'] = UnionNe + + def UnionStr(self): + return '<%s.%s(%s): %s>' % ( + self.__class__.__name__, + self._cur_field.name, + self.tag, + self.data) + dictionary['__str__'] = UnionStr + dictionary['__repr__'] = UnionStr + + def SetInternals(self, field, data): + self._cur_field = field + self._data = data + dictionary['SetInternals'] = SetInternals + + + return type.__new__(mcs, name, bases, dictionary) + + class InterfaceRequest(object): """ An interface request allows to send a request for an interface to a remote diff --git a/third_party/mojo/src/mojo/public/python/mojo_bindings/serialization.py b/third_party/mojo/src/mojo/public/python/mojo_bindings/serialization.py index 32f60f0..b1a35ec 100644 --- a/third_party/mojo/src/mojo/public/python/mojo_bindings/serialization.py +++ b/third_party/mojo/src/mojo/public/python/mojo_bindings/serialization.py @@ -7,9 +7,12 @@ import struct -# Format of a header for a struct or an array. +# Format of a header for a struct, array or union. HEADER_STRUCT = struct.Struct("<II") +# Format for a pointer. +POINTER_STRUCT = struct.Struct("<Q") + def Flatten(value): """Flattens nested lists/tuples into an one-level list. If value is not a @@ -218,3 +221,89 @@ def _GetStruct(groups): if alignment_needed: codes.append('x' * alignment_needed) return struct.Struct(''.join(codes)) + + +class UnionSerializer(object): + """ + Helper class to serialize/deserialize a union. + """ + def __init__(self, fields): + self._fields = {field.index: field for field in fields} + + def SerializeInline(self, union, handle_offset): + data = bytearray() + field = self._fields[union.tag] + + # If the union value is a simple type or a nested union, it is returned as + # entry. + # Otherwise, the serialized value is appended to data and the value of entry + # is -1. The caller will need to set entry to the location where the + # caller will append data. + (entry, handles) = field.field_type.Serialize( + union.data, -1, data, handle_offset) + + # If the value contained in the union is itself a union, we append its + # serialized value to data and set entry to -1. The caller will need to set + # entry to the location where the caller will append data. + if field.field_type.IsUnion(): + nested_union = bytearray(16) + HEADER_STRUCT.pack_into(nested_union, 0, entry[0], entry[1]) + POINTER_STRUCT.pack_into(nested_union, 8, entry[2]) + + data = nested_union + data + + # Since we do not know where the caller will append the nested union, + # we set entry to an invalid value and let the caller figure out the right + # value. + entry = -1 + + return (16, union.tag, entry, data), handles + + def Serialize(self, union, handle_offset): + (size, tag, entry, extra_data), handles = self.SerializeInline( + union, handle_offset) + data = bytearray(16) + if extra_data: + entry = 8 + data.extend(extra_data) + + field = self._fields[union.tag] + + HEADER_STRUCT.pack_into(data, 0, size, tag) + typecode = field.GetTypeCode() + + # If the value is a nested union, we store a 64 bits pointer to it. + if field.field_type.IsUnion(): + typecode = 'Q' + + struct.pack_into('<%s' % typecode, data, 8, entry) + return data, handles + + def Deserialize(self, context, union_class): + if len(context.data) < HEADER_STRUCT.size: + raise DeserializationException( + 'Available data too short to contain header.') + (size, tag) = HEADER_STRUCT.unpack_from(context.data) + + if size == 0: + return None + + if size != 16: + raise DeserializationException('Invalid union size %s' % size) + + union = union_class.__new__(union_class) + if tag not in self._fields: + union.SetInternals(None, None) + return union + + field = self._fields[tag] + if field.field_type.IsUnion(): + ptr = POINTER_STRUCT.unpack_from(context.data, 8)[0] + value = field.field_type.Deserialize(ptr, context.GetSubContext(ptr+8)) + else: + raw_value = struct.unpack_from( + field.GetTypeCode(), context.data, 8)[0] + value = field.field_type.Deserialize(raw_value, context.GetSubContext(8)) + + union.SetInternals(field, value) + return union diff --git a/third_party/mojo/src/mojo/public/tools/NETWORK_SERVICE_VERSION b/third_party/mojo/src/mojo/public/tools/NETWORK_SERVICE_VERSION index 7b21d79..09436b5 100644 --- a/third_party/mojo/src/mojo/public/tools/NETWORK_SERVICE_VERSION +++ b/third_party/mojo/src/mojo/public/tools/NETWORK_SERVICE_VERSION @@ -1 +1 @@ -36d008970120c7bce0e7cb0a6543fd0b653dad88
\ No newline at end of file +f4116cee892c698374ee7ae9ad8e6cea69741122 diff --git a/third_party/mojo/src/mojo/public/tools/bindings/generators/cpp_templates/enum_declaration.tmpl b/third_party/mojo/src/mojo/public/tools/bindings/generators/cpp_templates/enum_declaration.tmpl index 86c85d7d..d212e8b 100644 --- a/third_party/mojo/src/mojo/public/tools/bindings/generators/cpp_templates/enum_declaration.tmpl +++ b/third_party/mojo/src/mojo/public/tools/bindings/generators/cpp_templates/enum_declaration.tmpl @@ -1,4 +1,4 @@ -enum {{enum.name}} { +enum {{enum.name}} : int32_t { {%- for field in enum.fields %} {%- if field.value %} {{enum.name|to_all_caps}}_{{field.name}} = {{field.value|expression_to_text}}, diff --git a/third_party/mojo/src/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl b/third_party/mojo/src/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl index d96fb60..56a7047 100644 --- a/third_party/mojo/src/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl +++ b/third_party/mojo/src/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl @@ -8,9 +8,10 @@ #ifndef {{header_guard}} #define {{header_guard}} +#include <stdint.h> + #include "mojo/public/cpp/bindings/array.h" #include "mojo/public/cpp/bindings/callback.h" -#include "mojo/public/cpp/bindings/interface_impl.h" #include "mojo/public/cpp/bindings/interface_ptr.h" #include "mojo/public/cpp/bindings/interface_request.h" #include "mojo/public/cpp/bindings/lib/control_message_handler.h" diff --git a/third_party/mojo/src/mojo/public/tools/bindings/generators/java_templates/interface_definition.tmpl b/third_party/mojo/src/mojo/public/tools/bindings/generators/java_templates/interface_definition.tmpl index 39c17cc..320abeb 100644 --- a/third_party/mojo/src/mojo/public/tools/bindings/generators/java_templates/interface_definition.tmpl +++ b/third_party/mojo/src/mojo/public/tools/bindings/generators/java_templates/interface_definition.tmpl @@ -36,14 +36,14 @@ interface {{method|interface_response_name}} extends org.chromium.mojo.bindings. {%- endif -%} {%- endmacro -%} -{%- macro flags_for_method(method, is_parameter) -%} -{{flags(method.response_parameters, is_parameter)}} +{%- macro flags_for_method(method, is_request) -%} +{{flags(method.response_parameters != None, is_request)}} {%- endmacro -%} -{%- macro flags(has_response_parameters, is_parameter) -%} -{%- if not has_response_parameters -%} +{%- macro flags(use_response_flag, is_request) -%} +{%- if not use_response_flag -%} org.chromium.mojo.bindings.MessageHeader.NO_FLAG -{%- elif is_parameter: -%} +{%- elif is_request: -%} org.chromium.mojo.bindings.MessageHeader.MESSAGE_EXPECTS_RESPONSE_FLAG {%- else -%} org.chromium.mojo.bindings.MessageHeader.MESSAGE_IS_RESPONSE_FLAG diff --git a/third_party/mojo/src/mojo/public/tools/bindings/generators/mojom_dart_generator.py b/third_party/mojo/src/mojo/public/tools/bindings/generators/mojom_dart_generator.py index 87c16ef..c324c9c 100644 --- a/third_party/mojo/src/mojo/public/tools/bindings/generators/mojom_dart_generator.py +++ b/third_party/mojo/src/mojo/public/tools/bindings/generators/mojom_dart_generator.py @@ -9,6 +9,7 @@ import re import shutil import sys +import mojom.fileutil as fileutil import mojom.generate.constant_resolver as resolver import mojom.generate.generator as generator import mojom.generate.module as mojom @@ -421,20 +422,25 @@ class Generator(generator.Generator): def GenerateFiles(self, args): elements = self.module.namespace.split('.') elements.append("%s.dart" % self.module.name) - path = os.path.join("dart-pkg", "mojom/lib", *elements) - self.Write(self.GenerateLibModule(args), path) - path = os.path.join("dart-gen", "mojom/lib", *elements) - self.Write(self.GenerateLibModule(args), path) + + lib_module = self.GenerateLibModule(args) + pkg_path = os.path.join("dart-pkg", "mojom/lib", *elements) + self.Write(lib_module, pkg_path) + + gen_path = os.path.join("dart-gen", "mojom/lib", *elements) + full_gen_path = os.path.join(self.output_dir, gen_path) + self.Write(lib_module, gen_path) + link = self.MatchMojomFilePath("%s.dart" % self.module.name) - if os.path.exists(os.path.join(self.output_dir, link)): - os.unlink(os.path.join(self.output_dir, link)) + full_link_path = os.path.join(self.output_dir, link) + if os.path.exists(full_link_path): + os.unlink(full_link_path) + fileutil.EnsureDirectoryExists(os.path.dirname(full_link_path)) try: if sys.platform == "win32": - shutil.copy(os.path.join(self.output_dir, path), - os.path.join(self.output_dir, link)) + shutil.copy(full_gen_path, full_link_path) else: - os.symlink(os.path.join(self.output_dir, path), - os.path.join(self.output_dir, link)) + os.symlink(full_gen_path, full_link_path) except OSError as e: # Errno 17 is file already exists. If the link fails because file already # exists assume another instance of this script tried to create the same diff --git a/third_party/mojo/src/mojo/public/tools/bindings/generators/mojom_python_generator.py b/third_party/mojo/src/mojo/public/tools/bindings/generators/mojom_python_generator.py index 1f726b6..a12f5e1 100644 --- a/third_party/mojo/src/mojo/public/tools/bindings/generators/mojom_python_generator.py +++ b/third_party/mojo/src/mojo/public/tools/bindings/generators/mojom_python_generator.py @@ -146,7 +146,13 @@ def GetFieldType(kind, field=None): arguments.append('nullable=True') return '_descriptor.MapType(%s)' % ', '.join(arguments) - if mojom.IsStructKind(kind) or mojom.IsUnionKind(kind): + if mojom.IsUnionKind(kind): + arguments = [ 'lambda: %s' % GetFullyQualifiedName(kind) ] + if mojom.IsNullableKind(kind): + arguments.append('nullable=True') + return '_descriptor.UnionType(%s)' % ', '.join(arguments) + + if mojom.IsStructKind(kind): arguments = [ 'lambda: %s' % GetFullyQualifiedName(kind) ] if mojom.IsNullableKind(kind): arguments.append('nullable=True') @@ -169,15 +175,14 @@ def GetFieldType(kind, field=None): return _kind_to_type[kind] -def GetFieldDescriptor(packed_field): - field = packed_field.field +def GetFieldDescriptor(field, index, min_version): class_name = 'SingleFieldGroup' if field.kind == mojom.BOOL: class_name = 'FieldDescriptor' arguments = [ '%r' % GetNameForElement(field) ] arguments.append(GetFieldType(field.kind, field)) - arguments.append(str(packed_field.index)) - arguments.append(str(packed_field.min_version)) + arguments.append(str(index)) + arguments.append(str(min_version)) if field.default: if mojom.IsStructKind(field.kind): arguments.append('default_value=True') @@ -185,12 +190,19 @@ def GetFieldDescriptor(packed_field): arguments.append('default_value=%s' % ExpressionToText(field.default)) return '_descriptor.%s(%s)' % (class_name, ', '.join(arguments)) +def GetStructFieldDescriptor(packed_field): + return GetFieldDescriptor( + packed_field.field, packed_field.index, packed_field.min_version) + +def GetUnionFieldDescriptor(field): + return GetFieldDescriptor(field, field.ordinal, 0) + def GetFieldGroup(byte): if byte.packed_fields[0].field.kind == mojom.BOOL: - descriptors = map(GetFieldDescriptor, byte.packed_fields) + descriptors = map(GetStructFieldDescriptor, byte.packed_fields) return '_descriptor.BooleanGroup([%s])' % ', '.join(descriptors) assert len(byte.packed_fields) == 1 - return GetFieldDescriptor(byte.packed_fields[0]) + return GetStructFieldDescriptor(byte.packed_fields[0]) def MojomToPythonImport(mojom): return mojom.replace('.mojom', '_mojom') @@ -200,6 +212,7 @@ class Generator(generator.Generator): python_filters = { 'expression_to_text': ExpressionToText, 'field_group': GetFieldGroup, + 'union_field_descriptor': GetUnionFieldDescriptor, 'fully_qualified_name': GetFullyQualifiedName, 'name': GetNameForElement, } @@ -213,6 +226,7 @@ class Generator(generator.Generator): 'module': resolver.ResolveConstants(self.module, ExpressionToText), 'namespace': self.module.namespace, 'structs': self.GetStructs(), + 'unions': self.GetUnions(), } def GenerateFiles(self, args): diff --git a/third_party/mojo/src/mojo/public/tools/bindings/generators/python_templates/module.py.tmpl b/third_party/mojo/src/mojo/public/tools/bindings/generators/python_templates/module.py.tmpl index 2a22932..9d57600 100644 --- a/third_party/mojo/src/mojo/public/tools/bindings/generators/python_templates/module.py.tmpl +++ b/third_party/mojo/src/mojo/public/tools/bindings/generators/python_templates/module.py.tmpl @@ -1,5 +1,6 @@ {% from "module_macros.tmpl" import enum_values %} {% from "module_macros.tmpl" import struct_descriptor %} +{% from "module_macros.tmpl" import union_descriptor %} # Copyright 2014 The Chromium Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. @@ -34,6 +35,13 @@ class {{struct|name}}(object): __metaclass__ = _reflection.MojoStructType DESCRIPTOR = {{struct_descriptor(struct)|indent(2)}} {% endfor %} +{% for union in unions %} + +class {{union|name}}(object): + __metaclass__ = _reflection.MojoUnionType + DESCRIPTOR = {{union_descriptor(union)|indent(2)}} +{% endfor %} + {% for interface in interfaces %} class {{interface|name}}(object): diff --git a/third_party/mojo/src/mojo/public/tools/bindings/generators/python_templates/module_macros.tmpl b/third_party/mojo/src/mojo/public/tools/bindings/generators/python_templates/module_macros.tmpl index 21c70db..c979f59 100644 --- a/third_party/mojo/src/mojo/public/tools/bindings/generators/python_templates/module_macros.tmpl +++ b/third_party/mojo/src/mojo/public/tools/bindings/generators/python_templates/module_macros.tmpl @@ -37,3 +37,13 @@ {% endif %} } {%- endmacro -%} + +{%- macro union_descriptor(union) -%} +{ + 'fields': [ +{% for field in union.fields %} + {{field|union_field_descriptor}}, +{% endfor %} + ], + } +{%- endmacro -%} diff --git a/third_party/mojo/src/mojo/public/tools/dart_pkg.py b/third_party/mojo/src/mojo/public/tools/dart_pkg.py index d0f0d6eb..86ae5d5 100755 --- a/third_party/mojo/src/mojo/public/tools/dart_pkg.py +++ b/third_party/mojo/src/mojo/public/tools/dart_pkg.py @@ -8,6 +8,7 @@ import argparse import errno +import json import os import shutil import sys @@ -47,8 +48,9 @@ def mojom_filter(path): def ensure_dir_exists(path): - if not os.path.exists(path): - os.makedirs(path) + abspath = os.path.abspath(path) + if not os.path.exists(abspath): + os.makedirs(abspath) def has_pubspec_yaml(paths): @@ -105,6 +107,13 @@ def copy_or_link(from_root, to_root, filter_func=None): copy(from_root, to_root, filter_func) +def remove_if_exists(path): + try: + os.remove(path) + except OSError as e: + if e.errno != errno.ENOENT: + raise + def list_files(from_root, filter_func=None): file_list = [] for root, dirs, files in os.walk(from_root): @@ -187,13 +196,39 @@ def main(): help='Directory containing .dart sources', nargs='*', default=[]) + parser.add_argument('--sdk-ext-files', + metavar='sdk_ext_files', + help='List of .dart files that are part of of sdk_ext.', + nargs='*', + default=[]) + parser.add_argument('--sdk-ext-mappings', + metavar='sdk_ext_mappings', + help='Mappings for SDK extension libraries.', + nargs='*', + default=[]) args = parser.parse_args() # We must have a pubspec.yaml. assert has_pubspec_yaml(args.package_sources) - # Copy or symlink package sources into pkg directory. target_dir = os.path.join(args.pkg_directory, args.package_name) + lib_path = os.path.join(target_dir, "lib") + + mappings = {} + for mapping in args.sdk_ext_mappings: + library, path = mapping.split(',', 1) + mappings[library] = '../sdk_ext/%s' % path + + sdkext_path = os.path.join(lib_path, '_sdkext') + if mappings: + ensure_dir_exists(lib_path) + with open(sdkext_path, 'w') as stream: + json.dump(mappings, stream, sort_keys=True, + indent=2, separators=(',', ': ')) + else: + remove_if_exists(sdkext_path) + + # Copy or symlink package sources into pkg directory. common_source_prefix = os.path.commonprefix(args.package_sources) for source in args.package_sources: relative_source = os.path.relpath(source, common_source_prefix) @@ -209,8 +244,12 @@ def main(): relative_source = os.path.relpath(source, common_prefix) target = os.path.join(sdk_ext_dir, relative_source) copy_or_link(source, target) + for source in args.sdk_ext_files: + common_prefix = os.path.commonprefix(args.sdk_ext_files) + relative_source = os.path.relpath(source, common_prefix) + target = os.path.join(sdk_ext_dir, relative_source) + copy_or_link(source, target) - lib_path = os.path.join(target_dir, "lib") lib_mojom_path = os.path.join(lib_path, "mojom") # Copy generated mojom.dart files. |