diff options
author | rockot <rockot@chromium.org> | 2015-08-22 18:42:04 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-08-23 01:42:53 +0000 |
commit | bd9bf5a43f904f59ebac4f0982a3e3ddddf61e4b (patch) | |
tree | 3c161b06e3a5e95ad142a7f4ea28b0816ba0fb9e | |
parent | 59c45a7ebc21338ca0bf3b5e10b815a18e9ac7b9 (diff) | |
download | chromium_src-bd9bf5a43f904f59ebac4f0982a3e3ddddf61e4b.zip chromium_src-bd9bf5a43f904f59ebac4f0982a3e3ddddf61e4b.tar.gz chromium_src-bd9bf5a43f904f59ebac4f0982a3e3ddddf61e4b.tar.bz2 |
Revert of Update mojo sdk to rev c02a28868825edfa57ab77947b8cb15e741c5598 (patchset #1 id:1 of https://codereview.chromium.org/1310103002/ )
Reason for revert:
Looks like this breaks mojo_system_unittests on Android...
http://build.chromium.org/p/chromium.linux/buildstatus?builder=Android%20GN&number=29595
Original issue's description:
> 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
>
> Committed: https://crrev.com/0bfe154baad9ddfb1f0c6ecc7cace36e8f430e75
> Cr-Commit-Position: refs/heads/master@{#344994}
TBR=ben@chromium.org,jam@chromium.org,mek@chromium.org,mfoltz@chromium.org,shess@chromium.org,xhwang@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None
Review URL: https://codereview.chromium.org/1309103002
Cr-Commit-Position: refs/heads/master@{#344996}
138 files changed, 981 insertions, 2418 deletions
diff --git a/build/module_args/dart.gni b/build/module_args/dart.gni deleted file mode 100644 index be5e83b..0000000 --- a/build/module_args/dart.gni +++ /dev/null @@ -1,6 +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. - -# 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 5c28a5a..c9053af 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.h +++ b/chrome/browser/media/router/media_router_mojo_impl.h @@ -23,7 +23,6 @@ #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 69db798..0a2e933 100644 --- a/chrome/browser/media/router/media_router_mojo_test.h +++ b/chrome/browser/media/router/media_router_mojo_test.h @@ -15,7 +15,6 @@ #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 6906db6..a4f4eed 100644 --- a/components/devtools_service/devtools_http_server.h +++ b/components/devtools_service/devtools_http_server.h @@ -12,7 +12,6 @@ #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 43ffccf..5388a9e 100644 --- a/components/filesystem/files_test_base.h +++ b/components/filesystem/files_test_base.h @@ -8,7 +8,6 @@ #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 a2312e4..c0a2f62e 100644 --- a/components/html_viewer/document_resource_waiter.h +++ b/components/html_viewer/document_resource_waiter.h @@ -8,7 +8,6 @@ #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 7699a33..5cb3223 100644 --- a/components/html_viewer/html_frame.cc +++ b/components/html_viewer/html_frame.cc @@ -73,6 +73,7 @@ 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 8a82e15..7b47bdc 100644 --- a/components/html_viewer/html_frame.h +++ b/components/html_viewer/html_frame.h @@ -20,7 +20,6 @@ #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 ef08b04..fa103d4 100644 --- a/components/html_viewer/web_socket_handle_impl.cc +++ b/components/html_viewer/web_socket_handle_impl.cc @@ -17,7 +17,6 @@ #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 80e2aea..97b5ddd 100644 --- a/content/browser/presentation/presentation_service_impl.cc +++ b/content/browser/presentation/presentation_service_impl.cc @@ -169,10 +169,12 @@ void PresentationServiceImpl::Bind( mojo::InterfaceRequest<presentation::PresentationService> request) { binding_.reset(new mojo::Binding<presentation::PresentationService>( this, request.Pass())); - binding_->set_connection_error_handler([this]() { - DVLOG(1) << "Connection error"; - delete this; - }); + binding_->set_error_handler(this); +} + +void PresentationServiceImpl::OnConnectionError() { + DVLOG(1) << "OnConnectionError"; + delete this; } void PresentationServiceImpl::SetClient( diff --git a/content/browser/presentation/presentation_service_impl.h b/content/browser/presentation/presentation_service_impl.h index 0a3fe6f..2e991fa 100644 --- a/content/browser/presentation/presentation_service_impl.h +++ b/content/browser/presentation/presentation_service_impl.h @@ -26,7 +26,6 @@ #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 { @@ -49,6 +48,7 @@ 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,6 +198,10 @@ 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 e99c9be..7764be0 100644 --- a/content/child/navigator_connect/service_port_provider.h +++ b/content/child/navigator_connect/service_port_provider.h @@ -11,7 +11,6 @@ #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 71bbd0e..e332a46 100644 --- a/content/renderer/presentation/presentation_dispatcher.h +++ b/content/renderer/presentation/presentation_dispatcher.h @@ -5,8 +5,6 @@ #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" @@ -16,7 +14,6 @@ #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 ac4f681..84062d9 100644 --- a/content/renderer/usb/web_usb_client_impl.cc +++ b/content/renderer/usb/web_usb_client_impl.cc @@ -22,6 +22,7 @@ #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 d9b1dd8..e59d5bf 100644 --- a/content/renderer/usb/web_usb_device_impl.cc +++ b/content/renderer/usb/web_usb_device_impl.cc @@ -106,4 +106,8 @@ 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 3f6eab5..7f01d8d 100644 --- a/content/renderer/usb/web_usb_device_impl.h +++ b/content/renderer/usb/web_usb_device_impl.h @@ -14,6 +14,7 @@ #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; @@ -21,7 +22,7 @@ class Shell; namespace content { -class WebUSBDeviceImpl : public blink::WebUSBDevice { +class WebUSBDeviceImpl : public blink::WebUSBDevice, public mojo::ErrorHandler { public: WebUSBDeviceImpl(device::usb::DeviceManagerPtr device_manager, const blink::WebUSBDeviceInfo& device_info); @@ -61,6 +62,9 @@ class WebUSBDeviceImpl : public blink::WebUSBDevice { 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 0eba0a2..774d395 100644 --- a/device/serial/data_receiver.h +++ b/device/serial/data_receiver.h @@ -13,7 +13,6 @@ #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 733b0a1..8bb306e 100644 --- a/device/serial/data_sink_receiver.h +++ b/device/serial/data_sink_receiver.h @@ -13,7 +13,6 @@ #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 160dec9..2de7ce6 100644 --- a/device/serial/data_source_sender.h +++ b/device/serial/data_source_sender.h @@ -12,7 +12,6 @@ #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 be76bac..ae14208 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 { +class StashServiceTest : public testing::Test, public mojo::ErrorHandler { public: enum Event { EVENT_NONE, @@ -49,11 +49,11 @@ class StashServiceTest : public testing::Test { stash_backend_.reset(new StashBackend(base::Bind( &StashServiceTest::OnHandleReadyToRead, base::Unretained(this)))); stash_backend_->BindToRequest(mojo::GetProxy(&stash_service_)); - stash_service_.set_connection_error_handler(base::Bind(&OnConnectionError)); + stash_service_.set_error_handler(this); handles_ready_ = 0; } - static void OnConnectionError() { FAIL() << "Unexpected connection error"; } + void OnConnectionError() override { 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_connection_error_handler(base::Bind(&OnConnectionError)); + stash_service_.set_error_handler(this); 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 ff20719..08c3458 100644 --- a/ipc/mojo/ipc_channel_mojo.cc +++ b/ipc/mojo/ipc_channel_mojo.cc @@ -16,7 +16,6 @@ #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 5c20cca..0307112 100644 --- a/mandoline/ui/aura/surface_binding.cc +++ b/mandoline/ui/aura/surface_binding.cc @@ -24,7 +24,6 @@ #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 96b4532..ea4e8a4 100644 --- a/media/mojo/services/mojo_cdm.h +++ b/media/mojo/services/mojo_cdm.h @@ -14,7 +14,6 @@ #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 e574b15..f25fb4f 100644 --- a/media/mojo/services/mojo_renderer_impl.h +++ b/media/mojo/services/mojo_renderer_impl.h @@ -9,7 +9,6 @@ #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 01088f8..9b587b2 100644 --- a/mojo/application/public/cpp/application_impl.h +++ b/mojo/application/public/cpp/application_impl.h @@ -15,7 +15,6 @@ #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 8cb1d1b..7e4d604 100644 --- a/mojo/application/public/cpp/lib/service_registry.h +++ b/mojo/application/public/cpp/lib/service_registry.h @@ -11,7 +11,6 @@ #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 1ec0f1f..a01c12c 100644 --- a/mojo/gles2/command_buffer_client_impl.h +++ b/mojo/gles2/command_buffer_client_impl.h @@ -13,7 +13,6 @@ #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 8c62fdb..85ce4b3 100644 --- a/mojo/runner/child_process.cc +++ b/mojo/runner/child_process.cc @@ -26,7 +26,6 @@ #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 fa222d8..d3d14c7 100755 --- a/mojo/tools/rev_sdk.py +++ b/mojo/tools/rev_sdk.py @@ -33,7 +33,6 @@ 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 d39198d..7a7ccca 100644 --- a/sql/mojo/sql_test_base.h +++ b/sql/mojo/sql_test_base.h @@ -9,7 +9,6 @@ #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 0b5b475..8f60ae6 100644 --- a/third_party/mojo/mojo_edk.gyp +++ b/third_party/mojo/mojo_edk.gyp @@ -24,6 +24,7 @@ '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', @@ -140,6 +141,7 @@ '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 fc455ed..5bac8f3 100644 --- a/third_party/mojo/mojo_edk_system_impl.gypi +++ b/third_party/mojo/mojo_edk_system_impl.gypi @@ -60,7 +60,6 @@ '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', @@ -102,8 +101,6 @@ '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', @@ -125,7 +122,6 @@ '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 2e47591..db1fc36 100644 --- a/third_party/mojo/mojo_edk_tests.gyp +++ b/third_party/mojo/mojo_edk_tests.gyp @@ -17,7 +17,6 @@ # 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', @@ -53,8 +52,6 @@ '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', @@ -66,24 +63,6 @@ ], }, { - # 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', @@ -196,7 +175,6 @@ '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', @@ -209,7 +187,6 @@ '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 f5b8265..3f16424 100644 --- a/third_party/mojo/mojo_public.gyp +++ b/third_party/mojo/mojo_public.gyp @@ -104,6 +104,7 @@ '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', @@ -144,6 +145,8 @@ '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', @@ -330,7 +333,6 @@ '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 b3144f0..c3e0d53 100644 --- a/third_party/mojo/src/mojo/edk/embedder/BUILD.gn +++ b/third_party/mojo/src/mojo/edk/embedder/BUILD.gn @@ -87,6 +87,7 @@ 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 e9735b1..0d1f3d9 100644 --- a/third_party/mojo/src/mojo/edk/embedder/embedder_unittest.cc +++ b/third_party/mojo/src/mojo/edk/embedder/embedder_unittest.cc @@ -16,7 +16,6 @@ #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" @@ -181,7 +180,7 @@ class TestAsyncWaiter { TestAsyncWaiter() : event_(true, false), wait_result_(MOJO_RESULT_UNKNOWN) {} void Awake(MojoResult result) { - system::MutexLocker l(&wait_result_mutex_); + base::AutoLock l(wait_result_lock_); wait_result_ = result; event_.Signal(); } @@ -189,15 +188,15 @@ class TestAsyncWaiter { bool TryWait() { return event_.TimedWait(TestTimeouts::action_timeout()); } MojoResult wait_result() const { - system::MutexLocker l(&wait_result_mutex_); + base::AutoLock l(wait_result_lock_); return wait_result_; } private: base::WaitableEvent event_; - mutable system::Mutex wait_result_mutex_; - MojoResult wait_result_ MOJO_GUARDED_BY(wait_result_mutex_); + mutable base::Lock wait_result_lock_; + MojoResult wait_result_; 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 c7b49c2..897929f 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,7 +15,10 @@ #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" @@ -125,6 +128,9 @@ 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; @@ -144,7 +150,9 @@ TEST_F(PlatformChannelPairPosixTest, SendReceiveFDs) { const char c = '0' + (i % 10); ScopedPlatformHandleVectorPtr platform_handles(new PlatformHandleVector); for (size_t j = 1; j <= i; j++) { - base::ScopedFILE fp(tmpfile()); + base::FilePath unused; + base::ScopedFILE fp( + base::CreateAndOpenTemporaryFileInDir(temp_dir.path(), &unused)); ASSERT_TRUE(fp); ASSERT_EQ(j, fwrite(std::string(j, c).data(), 1, j, fp.get())); platform_handles->push_back( @@ -186,6 +194,9 @@ 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; @@ -195,7 +206,9 @@ TEST_F(PlatformChannelPairPosixTest, AppendReceivedFDs) { const std::string file_contents("hello world"); { - base::ScopedFILE fp(tmpfile()); + base::FilePath unused; + base::ScopedFILE fp( + base::CreateAndOpenTemporaryFileInDir(temp_dir.path(), &unused)); 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 4207120..5024087 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 "base/rand_util.h" +#include "crypto/random.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) { - base::RandBytes(bytes, num_bytes); + crypto::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 c012daf..3675f97 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,7 +14,6 @@ #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 21ad315..b90462d 100644 --- a/third_party/mojo/src/mojo/edk/mojo_edk.gni +++ b/third_party/mojo/src/mojo/edk/mojo_edk.gni @@ -95,9 +95,3 @@ 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 5dd2682..95ee992 100644 --- a/third_party/mojo/src/mojo/edk/system/BUILD.gn +++ b/third_party/mojo/src/mojo/edk/system/BUILD.gn @@ -82,8 +82,6 @@ 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", @@ -105,7 +103,6 @@ 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", @@ -192,7 +189,6 @@ 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", @@ -203,7 +199,6 @@ 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 601ddb0..470956f 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 |mutex_|, since this must be called before this object + // No need to take |lock_|, 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); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); DCHECK(!is_shutting_down_); DCHECK(!channel_manager_); channel_manager_ = channel_manager; @@ -61,7 +61,7 @@ void Channel::Shutdown() { IdToEndpointMap to_destroy; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (!is_running_) return; @@ -91,7 +91,7 @@ void Channel::Shutdown() { } void Channel::WillShutdownSoon() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); is_shutting_down_ = true; channel_manager_ = nullptr; } @@ -109,7 +109,7 @@ void Channel::SetBootstrapEndpointWithIds( DCHECK(endpoint); { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); DLOG_IF(WARNING, is_shutting_down_) << "SetBootstrapEndpoint() while shutting down"; @@ -125,7 +125,7 @@ void Channel::SetBootstrapEndpointWithIds( } bool Channel::WriteMessage(scoped_ptr<MessageInTransit> message) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (!is_running_) return true; return raw_channel_->IsWriteBufferEmpty(); @@ -154,7 +154,7 @@ void Channel::DetachEndpoint(ChannelEndpoint* endpoint, return; // Nothing to do. { - MutexLocker locker_(&mutex_); + base::AutoLock locker_(lock_); 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; - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); auto it = incoming_endpoints_.find(local_id); if (it == incoming_endpoints_.end()) { @@ -264,9 +264,6 @@ 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(); } @@ -306,7 +303,7 @@ void Channel::OnError(Error error) { DVLOG(1) << "RawChannel read error (shutdown)"; break; case ERROR_READ_BROKEN: { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); LOG_IF(ERROR, !is_shutting_down_) << "RawChannel read error (connection broken)"; break; @@ -343,7 +340,7 @@ void Channel::OnReadMessageForEndpoint( scoped_refptr<ChannelEndpoint> endpoint; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); // Since we own |raw_channel_|, and this method and |Shutdown()| should only // be called from the creation thread, |raw_channel_| should never be null @@ -462,7 +459,7 @@ bool Channel::OnAttachAndRunEndpoint(ChannelEndpointId local_id, bool success = true; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (local_id_to_endpoint_map_.find(local_id) == local_id_to_endpoint_map_.end()) { @@ -493,7 +490,7 @@ bool Channel::OnRemoveEndpoint(ChannelEndpointId local_id, scoped_refptr<ChannelEndpoint> endpoint; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); IdToEndpointMap::iterator it = local_id_to_endpoint_map_.find(local_id); if (it == local_id_to_endpoint_map_.end()) { @@ -529,7 +526,7 @@ bool Channel::OnRemoveEndpoint(ChannelEndpointId local_id, bool Channel::OnRemoveEndpointAck(ChannelEndpointId local_id) { DCHECK(creation_thread_checker_.CalledOnValidThread()); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); IdToEndpointMap::iterator it = local_id_to_endpoint_map_.find(local_id); if (it == local_id_to_endpoint_map_.end()) { @@ -572,7 +569,7 @@ ChannelEndpointId Channel::AttachAndRunEndpoint( ChannelEndpointId local_id; ChannelEndpointId remote_id; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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 9b7c3ae..015fa73 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) MOJO_NOT_THREAD_SAFE; + void Init(scoped_ptr<RawChannel> raw_channel); // Sets the channel manager associated with this channel. This should be set // at most once and only called before |WillShutdownSoon()| (and @@ -220,51 +220,47 @@ class MOJO_SYSTEM_IMPL_EXPORT Channel final ChannelEndpointId AttachAndRunEndpoint( scoped_refptr<ChannelEndpoint> endpoint); - // Helper to send channel control messages. Returns true on success. Callable - // from any thread. + // Helper to send channel control messages. Returns true on success. Should be + // called *without* |lock_| held. Callable from any thread. bool SendControlMessage(MessageInTransit::Subtype subtype, ChannelEndpointId source_id, - ChannelEndpointId destination_id) - MOJO_LOCKS_EXCLUDED(mutex_); + ChannelEndpointId destination_id); base::ThreadChecker creation_thread_checker_; embedder::PlatformSupport* const platform_support_; // Note: |ChannelEndpointClient|s (in particular, |MessagePipe|s) MUST NOT be - // used under |mutex_|. E.g., |mutex_| can only be acquired after + // used under |lock_|. E.g., |lock_| 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 |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_); + // |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_; // Set when |WillShutdownSoon()| is called. - bool is_shutting_down_ MOJO_GUARDED_BY(mutex_); + bool is_shutting_down_; // Has a reference to us. - ChannelManager* channel_manager_ MOJO_GUARDED_BY(mutex_); + ChannelManager* channel_manager_; 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_ MOJO_GUARDED_BY(mutex_); + IdToEndpointMap local_id_to_endpoint_map_; // Note: The IDs generated by this should be checked for existence before use. - LocalChannelEndpointIdGenerator local_id_generator_ MOJO_GUARDED_BY(mutex_); + LocalChannelEndpointIdGenerator local_id_generator_; 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_ MOJO_GUARDED_BY(mutex_); + IdToIncomingEndpointMap incoming_endpoints_; // TODO(vtl): We need to keep track of remote IDs (so that we don't collide // if/when we wrap). - RemoteChannelEndpointIdGenerator remote_id_generator_ MOJO_GUARDED_BY(mutex_); + RemoteChannelEndpointIdGenerator remote_id_generator_; 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 038de8d..113bbd58 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); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); switch (channel_state_) { case ChannelState::NOT_YET_ATTACHED: @@ -53,7 +53,7 @@ bool ChannelEndpoint::ReplaceClient(ChannelEndpointClient* client, unsigned client_port) { DCHECK(client); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); DCHECK(client_); DCHECK(client != client_.get() || client_port != client_port_); client_ = client; @@ -62,7 +62,7 @@ bool ChannelEndpoint::ReplaceClient(ChannelEndpointClient* client, } void ChannelEndpoint::DetachFromClient() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); DCHECK(client_); client_ = nullptr; @@ -79,7 +79,7 @@ void ChannelEndpoint::AttachAndRun(Channel* channel, DCHECK(local_id.is_valid()); DCHECK(remote_id.is_valid()); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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); - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(channel_); DCHECK(local_id_.is_valid()); @@ -187,7 +187,7 @@ void ChannelEndpoint::OnReadMessageForClient( // -- impose significant cost in the common case.) for (;;) { { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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 c70e66e..807b1a8b 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 |mutex_|.) [TODO(vtl): When I add remotely-allocated IDs, +// under |lock_|.) [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,18 +162,19 @@ class MOJO_SYSTEM_IMPL_EXPORT ChannelEndpoint final friend class base::RefCountedThreadSafe<ChannelEndpoint>; ~ChannelEndpoint(); - bool WriteMessageNoLock(scoped_ptr<MessageInTransit> message) - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + // Must be called with |lock_| held. + bool WriteMessageNoLock(scoped_ptr<MessageInTransit> message); // 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. - void ResetChannelNoLock() MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + // non-null. Must be called with |lock_| held. + void ResetChannelNoLock(); - Mutex mutex_; + // Protects the members below. + base::Lock lock_; // |client_| must be valid whenever it is non-null. Before |*client_| gives up // its reference to this object, it must call |DetachFromClient()|. @@ -181,17 +182,14 @@ 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 |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: |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: 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_ MOJO_GUARDED_BY(mutex_); - unsigned client_port_ MOJO_GUARDED_BY(mutex_); + scoped_refptr<ChannelEndpointClient> client_; + unsigned client_port_; // State with respect to interaction with the |Channel|. enum class ChannelState { @@ -203,17 +201,17 @@ class MOJO_SYSTEM_IMPL_EXPORT ChannelEndpoint final // |DetachFromChannel()| has been called (|channel_| is null). DETACHED }; - ChannelState channel_state_ MOJO_GUARDED_BY(mutex_); + ChannelState channel_state_; // |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_ MOJO_GUARDED_BY(mutex_); - ChannelEndpointId local_id_ MOJO_GUARDED_BY(mutex_); - ChannelEndpointId remote_id_ MOJO_GUARDED_BY(mutex_); + Channel* channel_; + ChannelEndpointId local_id_; + ChannelEndpointId remote_id_; // This queue is used before we're running on a channel and ready to send // messages to the channel. - MessageInTransitQueue channel_message_queue_ MOJO_GUARDED_BY(mutex_); + MessageInTransitQueue channel_message_queue_; 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 a2e4bf9..711b4da 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; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); channels.swap(channels_); } @@ -110,7 +110,7 @@ scoped_refptr<MessagePipeDispatcher> ChannelManager::CreateChannel( } scoped_refptr<Channel> ChannelManager::GetChannel(ChannelId channel_id) const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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); { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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 014c1b3..9a9c0e8 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,15 +135,12 @@ 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 |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_; + // Note: |Channel| methods should not be called under |lock_|. + mutable base::Lock lock_; // Protects the members below. using ChannelIdToChannelMap = base::hash_map<ChannelId, scoped_refptr<Channel>>; - ChannelIdToChannelMap channels_ MOJO_GUARDED_BY(mutex_); + ChannelIdToChannelMap channels_; 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 5923c75..c92a121 100644 --- a/third_party/mojo/src/mojo/edk/system/connection_manager.h +++ b/third_party/mojo/src/mojo/edk/system/connection_manager.h @@ -5,12 +5,9 @@ #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 { @@ -62,23 +59,13 @@ 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() MOJO_NOT_THREAD_SAFE = 0; + virtual void Shutdown() = 0; // TODO(vtl): Add a "get my own process identifier" method? @@ -98,14 +85,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, |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; + // 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; protected: // |platform_support| must be valid and remain alive until after |Shutdown()| @@ -119,13 +106,6 @@ 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 20fa73e..cb22ef3 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,14 +226,12 @@ TEST_F(ConnectionManagerTest, BasicConnectSlaves) { ProcessIdentifier peer1 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h1; - EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, - slave1.Connect(connection_id, &peer1, &h1)); + EXPECT_TRUE(slave1.Connect(connection_id, &peer1, &h1)); EXPECT_EQ(slave2_id, peer1); EXPECT_TRUE(h1.is_valid()); ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, - slave2.Connect(connection_id, &peer2, &h2)); + EXPECT_TRUE(slave2.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(slave1_id, peer2); EXPECT_TRUE(h2.is_valid()); @@ -321,8 +319,7 @@ TEST_F(ConnectionManagerTest, SlaveCancelConnect) { EXPECT_TRUE(slave1.CancelConnect(connection_id)); ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_EQ(ConnectionManager::Result::FAILURE, - slave2.Connect(connection_id, &peer2, &h2)); + EXPECT_FALSE(slave2.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(kInvalidProcessIdentifier, peer2); EXPECT_FALSE(h2.is_valid()); @@ -364,8 +361,7 @@ TEST_F(ConnectionManagerTest, ErrorRemovePending) { ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_EQ(ConnectionManager::Result::FAILURE, - slave2.Connect(connection_id, &peer2, &h2)); + EXPECT_FALSE(slave2.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(kInvalidProcessIdentifier, peer2); EXPECT_FALSE(h2.is_valid()); @@ -388,16 +384,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_EQ(ConnectionManager::Result::SUCCESS_CONNECT_SAME_PROCESS, - slave.Connect(connection_id, &peer1, &h1)); + EXPECT_TRUE(slave.Connect(connection_id, &peer1, &h1)); EXPECT_EQ(slave_id, peer1); EXPECT_FALSE(h1.is_valid()); ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_SAME_PROCESS, - slave.Connect(connection_id, &peer2, &h2)); + EXPECT_TRUE(slave.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(slave_id, peer2); EXPECT_FALSE(h2.is_valid()); @@ -429,36 +425,30 @@ TEST_F(ConnectionManagerTest, ConnectSlavesTwice) { ProcessIdentifier peer1 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h1; - EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, - slave1.Connect(connection_id, &peer1, &h1)); + EXPECT_TRUE(slave1.Connect(connection_id, &peer1, &h1)); EXPECT_EQ(slave2_id, peer1); ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, - slave2.Connect(connection_id, &peer2, &h2)); + EXPECT_TRUE(slave2.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(slave1_id, peer2); EXPECT_TRUE(ArePlatformHandlesConnected(h1.get(), h2.get())); - // 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). + // 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_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, - slave2.Connect(connection_id, &second_peer2, &h2)); + EXPECT_TRUE(slave2.Connect(connection_id, &second_peer2, &h2)); ProcessIdentifier second_peer1 = kInvalidProcessIdentifier; - EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, - slave1.Connect(connection_id, &second_peer1, &h1)); + EXPECT_TRUE(slave1.Connect(connection_id, &second_peer1, &h1)); EXPECT_EQ(peer1, second_peer1); EXPECT_EQ(peer2, second_peer2); @@ -486,14 +476,12 @@ TEST_F(ConnectionManagerTest, ConnectMasterToSlave) { ProcessIdentifier master_peer = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle master_h; - EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, - master.Connect(connection_id, &master_peer, &master_h)); + EXPECT_TRUE(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_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, - slave.Connect(connection_id, &slave_peer, &slave_h)); + EXPECT_TRUE(slave.Connect(connection_id, &slave_peer, &slave_h)); EXPECT_EQ(kMasterProcessIdentifier, slave_peer); EXPECT_TRUE(slave_h.is_valid()); @@ -512,16 +500,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_EQ(ConnectionManager::Result::SUCCESS_CONNECT_SAME_PROCESS, - master.Connect(connection_id, &peer1, &h1)); + EXPECT_TRUE(master.Connect(connection_id, &peer1, &h1)); EXPECT_EQ(kMasterProcessIdentifier, peer1); EXPECT_FALSE(h1.is_valid()); ProcessIdentifier peer2 = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_SAME_PROCESS, - master.Connect(connection_id, &peer2, &h2)); + EXPECT_TRUE(master.Connect(connection_id, &peer2, &h2)); EXPECT_EQ(kMasterProcessIdentifier, peer2); EXPECT_FALSE(h2.is_valid()); @@ -548,8 +536,7 @@ TEST_F(ConnectionManagerTest, MasterCancelConnect) { EXPECT_TRUE(master.CancelConnect(connection_id)); ProcessIdentifier peer = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h; - EXPECT_EQ(ConnectionManager::Result::FAILURE, - slave.Connect(connection_id, &peer, &h)); + EXPECT_FALSE(slave.Connect(connection_id, &peer, &h)); EXPECT_EQ(kInvalidProcessIdentifier, peer); EXPECT_FALSE(h.is_valid()); @@ -586,8 +573,7 @@ TEST_F(ConnectionManagerTest, AddSlaveAndBootstrap) { embedder::ScopedPlatformHandle h1; ProcessIdentifier master_peer = kInvalidProcessIdentifier; - EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, - master.Connect(connection_id, &master_peer, &h1)); + EXPECT_TRUE(master.Connect(connection_id, &master_peer, &h1)); EXPECT_EQ(slave_id, master_peer); EXPECT_TRUE(h1.is_valid()); @@ -599,8 +585,7 @@ TEST_F(ConnectionManagerTest, AddSlaveAndBootstrap) { ProcessIdentifier slave_peer = kInvalidProcessIdentifier; embedder::ScopedPlatformHandle h2; - EXPECT_EQ(ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION, - slave.Connect(connection_id, &slave_peer, &h2)); + EXPECT_TRUE(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 fbb6cf1..35d6614 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) { - MutexLocker locker(&handle_table_mutex_); + base::AutoLock locker(handle_table_lock_); return handle_table_.AddDispatcher(dispatcher); } @@ -94,7 +94,7 @@ scoped_refptr<Dispatcher> Core::GetDispatcher(MojoHandle handle) { if (handle == MOJO_HANDLE_INVALID) return nullptr; - MutexLocker locker(&handle_table_mutex_); + base::AutoLock locker(handle_table_lock_); return handle_table_.GetDispatcher(handle); } @@ -103,7 +103,7 @@ MojoResult Core::GetAndRemoveDispatcher(MojoHandle handle, if (handle == MOJO_HANDLE_INVALID) return MOJO_RESULT_INVALID_ARGUMENT; - MutexLocker locker(&handle_table_mutex_); + base::AutoLock locker(handle_table_lock_); return handle_table_.GetAndRemoveDispatcher(handle, dispatcher); } @@ -130,7 +130,7 @@ MojoResult Core::Close(MojoHandle handle) { scoped_refptr<Dispatcher> dispatcher; { - MutexLocker locker(&handle_table_mutex_); + base::AutoLock locker(handle_table_lock_); 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_mutex_|. As a result, there's a + // Note: This is done outside of |handle_table_lock_|. 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; { - MutexLocker locker(&handle_table_mutex_); + base::AutoLock locker(handle_table_lock_); 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. { - MutexLocker locker(&handle_table_mutex_); + base::AutoLock locker(handle_table_lock_); 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(); { - MutexLocker locker(&handle_table_mutex_); + base::AutoLock locker(handle_table_lock_); 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()); { - MutexLocker locker(&handle_table_mutex_); + base::AutoLock locker(handle_table_lock_); success = handle_table_.AddDispatcherVector( dispatchers, handles_writer.GetPointer()); } @@ -377,7 +377,7 @@ MojoResult Core::CreateDataPipe( std::pair<MojoHandle, MojoHandle> handle_pair; { - MutexLocker locker(&handle_table_mutex_); + base::AutoLock locker(handle_table_lock_); 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(); { - MutexLocker locker(&mapping_table_mutex_); + base::AutoLock locker(mapping_table_lock_); 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) { - MutexLocker locker(&mapping_table_mutex_); + base::AutoLock locker(mapping_table_lock_); 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 4ed5ede..407a064 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_mutex_| should be a reader-writer lock (if only we + // TODO(vtl): |handle_table_lock_| should be a reader-writer lock (if only we // had them). - Mutex handle_table_mutex_; - HandleTable handle_table_ MOJO_GUARDED_BY(handle_table_mutex_); + base::Lock handle_table_lock_; // Protects |handle_table_|. + HandleTable handle_table_; - Mutex mapping_table_mutex_; - MappingTable mapping_table_ MOJO_GUARDED_BY(mapping_table_mutex_); + base::Lock mapping_table_lock_; // Protects |mapping_table_|. + MappingTable mapping_table_; 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 2552fd2..bfdcb1b 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(); - mutex().AssertHeld(); + lock().AssertAcquired(); } MojoResult WriteMessageImplNoLock( @@ -52,7 +52,7 @@ class MockDispatcher : public Dispatcher { std::vector<DispatcherTransport>* transports, MojoWriteMessageFlags /*flags*/) override { info_->IncrementWriteMessageCallCount(); - mutex().AssertHeld(); + lock().AssertAcquired(); 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(); - mutex().AssertHeld(); + lock().AssertAcquired(); if (num_dispatchers) { *num_dispatchers = 1; @@ -86,7 +86,7 @@ class MockDispatcher : public Dispatcher { UserPointer<uint32_t> /*num_bytes*/, MojoWriteDataFlags /*flags*/) override { info_->IncrementWriteDataCallCount(); - mutex().AssertHeld(); + lock().AssertAcquired(); return MOJO_RESULT_UNIMPLEMENTED; } @@ -95,13 +95,13 @@ class MockDispatcher : public Dispatcher { UserPointer<uint32_t> /*buffer_num_bytes*/, MojoWriteDataFlags /*flags*/) override { info_->IncrementBeginWriteDataCallCount(); - mutex().AssertHeld(); + lock().AssertAcquired(); return MOJO_RESULT_UNIMPLEMENTED; } MojoResult EndWriteDataImplNoLock(uint32_t /*num_bytes_written*/) override { info_->IncrementEndWriteDataCallCount(); - mutex().AssertHeld(); + lock().AssertAcquired(); return MOJO_RESULT_UNIMPLEMENTED; } @@ -109,7 +109,7 @@ class MockDispatcher : public Dispatcher { UserPointer<uint32_t> /*num_bytes*/, MojoReadDataFlags /*flags*/) override { info_->IncrementReadDataCallCount(); - mutex().AssertHeld(); + lock().AssertAcquired(); return MOJO_RESULT_UNIMPLEMENTED; } @@ -117,13 +117,13 @@ class MockDispatcher : public Dispatcher { UserPointer<uint32_t> /*buffer_num_bytes*/, MojoReadDataFlags /*flags*/) override { info_->IncrementBeginReadDataCallCount(); - mutex().AssertHeld(); + lock().AssertAcquired(); return MOJO_RESULT_UNIMPLEMENTED; } MojoResult EndReadDataImplNoLock(uint32_t /*num_bytes_read*/) override { info_->IncrementEndReadDataCallCount(); - mutex().AssertHeld(); + lock().AssertAcquired(); return MOJO_RESULT_UNIMPLEMENTED; } @@ -132,7 +132,7 @@ class MockDispatcher : public Dispatcher { uint32_t /*context*/, HandleSignalsState* signals_state) override { info_->IncrementAddAwakableCallCount(); - mutex().AssertHeld(); + lock().AssertAcquired(); 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(); - mutex().AssertHeld(); + lock().AssertAcquired(); if (signals_state) *signals_state = HandleSignalsState(); } void CancelAllAwakablesNoLock() override { info_->IncrementCancelAllAwakablesCallCount(); - mutex().AssertHeld(); + lock().AssertAcquired(); } scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndCloseImplNoLock() @@ -215,167 +215,167 @@ CoreTestBase_MockHandleInfo::~CoreTestBase_MockHandleInfo() { } unsigned CoreTestBase_MockHandleInfo::GetCtorCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return ctor_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetDtorCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return dtor_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetCloseCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return close_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetWriteMessageCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return write_message_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetReadMessageCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return read_message_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetWriteDataCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return write_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetBeginWriteDataCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return begin_write_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetEndWriteDataCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return end_write_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetReadDataCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return read_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetBeginReadDataCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return begin_read_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetEndReadDataCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return end_read_data_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetAddAwakableCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return add_awakable_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetRemoveAwakableCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return remove_awakable_call_count_; } unsigned CoreTestBase_MockHandleInfo::GetCancelAllAwakablesCallCount() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return cancel_all_awakables_call_count_; } size_t CoreTestBase_MockHandleInfo::GetAddedAwakableSize() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return added_awakables_.size(); } Awakable* CoreTestBase_MockHandleInfo::GetAddedAwakableAt(unsigned i) const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return added_awakables_[i]; } void CoreTestBase_MockHandleInfo::IncrementCtorCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); ctor_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementDtorCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); dtor_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementCloseCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); close_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementWriteMessageCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); write_message_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementReadMessageCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); read_message_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementWriteDataCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); write_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementBeginWriteDataCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); begin_write_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementEndWriteDataCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); end_write_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementReadDataCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); read_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementBeginReadDataCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); begin_read_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementEndReadDataCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); end_read_data_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementAddAwakableCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); add_awakable_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementRemoveAwakableCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); remove_awakable_call_count_++; } void CoreTestBase_MockHandleInfo::IncrementCancelAllAwakablesCallCount() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); cancel_all_awakables_call_count_++; } void CoreTestBase_MockHandleInfo::AllowAddAwakable(bool alllow) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); add_awakable_allowed_ = alllow; } bool CoreTestBase_MockHandleInfo::IsAddAwakableAllowed() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return add_awakable_allowed_; } void CoreTestBase_MockHandleInfo::AwakableWasAdded(Awakable* awakable) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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 e2375cd..ce32918 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 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_); + 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_; 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 5a1bafb..ac917d9 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,11 +35,6 @@ DataPipeConsumerDispatcher::Deserialize(Channel* channel, return dispatcher; } -DataPipe* DataPipeConsumerDispatcher::GetDataPipeForTest() { - MutexLocker locker(&mutex()); - return data_pipe_.get(); -} - DataPipeConsumerDispatcher::DataPipeConsumerDispatcher() { } @@ -49,19 +44,19 @@ DataPipeConsumerDispatcher::~DataPipeConsumerDispatcher() { } void DataPipeConsumerDispatcher::CancelAllAwakablesNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); data_pipe_->ConsumerCancelAllAwakables(); } void DataPipeConsumerDispatcher::CloseImplNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); data_pipe_->ConsumerClose(); data_pipe_ = nullptr; } scoped_refptr<Dispatcher> DataPipeConsumerDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); scoped_refptr<DataPipeConsumerDispatcher> rv = Create(); rv->Init(data_pipe_); @@ -73,7 +68,7 @@ MojoResult DataPipeConsumerDispatcher::ReadDataImplNoLock( UserPointer<void> elements, UserPointer<uint32_t> num_bytes, MojoReadDataFlags flags) { - mutex().AssertHeld(); + lock().AssertAcquired(); if ((flags & MOJO_READ_DATA_FLAG_DISCARD)) { // These flags are mutally exclusive. @@ -104,7 +99,7 @@ MojoResult DataPipeConsumerDispatcher::BeginReadDataImplNoLock( UserPointer<const void*> buffer, UserPointer<uint32_t> buffer_num_bytes, MojoReadDataFlags flags) { - mutex().AssertHeld(); + lock().AssertAcquired(); // These flags may not be used in two-phase mode. if ((flags & MOJO_READ_DATA_FLAG_DISCARD) || @@ -117,14 +112,14 @@ MojoResult DataPipeConsumerDispatcher::BeginReadDataImplNoLock( MojoResult DataPipeConsumerDispatcher::EndReadDataImplNoLock( uint32_t num_bytes_read) { - mutex().AssertHeld(); + lock().AssertAcquired(); return data_pipe_->ConsumerEndReadData(num_bytes_read); } HandleSignalsState DataPipeConsumerDispatcher::GetHandleSignalsStateImplNoLock() const { - mutex().AssertHeld(); + lock().AssertAcquired(); return data_pipe_->ConsumerGetHandleSignalsState(); } @@ -133,7 +128,7 @@ MojoResult DataPipeConsumerDispatcher::AddAwakableImplNoLock( MojoHandleSignals signals, uint32_t context, HandleSignalsState* signals_state) { - mutex().AssertHeld(); + lock().AssertAcquired(); return data_pipe_->ConsumerAddAwakable(awakable, signals, context, signals_state); } @@ -141,7 +136,7 @@ MojoResult DataPipeConsumerDispatcher::AddAwakableImplNoLock( void DataPipeConsumerDispatcher::RemoveAwakableImplNoLock( Awakable* awakable, HandleSignalsState* signals_state) { - mutex().AssertHeld(); + lock().AssertAcquired(); data_pipe_->ConsumerRemoveAwakable(awakable, signals_state); } @@ -167,7 +162,7 @@ bool DataPipeConsumerDispatcher::EndSerializeAndCloseImplNoLock( } bool DataPipeConsumerDispatcher::IsBusyNoLock() const { - mutex().AssertHeld(); + lock().AssertAcquired(); 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 29df228e..7d8b5ee 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) MOJO_NOT_THREAD_SAFE; + void Init(scoped_refptr<DataPipe> data_pipe); // |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(); + DataPipe* GetDataPipeForTest() { return data_pipe_.get(); } private: DataPipeConsumerDispatcher(); @@ -64,18 +64,16 @@ 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 - MOJO_NOT_THREAD_SAFE; + size_t* max_platform_handles) override; bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) override - MOJO_NOT_THREAD_SAFE; + embedder::PlatformHandleVector* platform_handles) override; bool IsBusyNoLock() const override; - // This will be null if closed. - scoped_refptr<DataPipe> data_pipe_ MOJO_GUARDED_BY(mutex()); + // Protected by |lock()|: + scoped_refptr<DataPipe> data_pipe_; // This will be null if closed. 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 3b0470b..2846f8d 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, TwoPhaseAllOrNone) { +TYPED_TEST(DataPipeImplTest, DISABLED_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 ddb8b9f..88190cf 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,11 +35,6 @@ DataPipeProducerDispatcher::Deserialize(Channel* channel, return dispatcher; } -DataPipe* DataPipeProducerDispatcher::GetDataPipeForTest() { - MutexLocker locker(&mutex()); - return data_pipe_.get(); -} - DataPipeProducerDispatcher::DataPipeProducerDispatcher() { } @@ -49,19 +44,19 @@ DataPipeProducerDispatcher::~DataPipeProducerDispatcher() { } void DataPipeProducerDispatcher::CancelAllAwakablesNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); data_pipe_->ProducerCancelAllAwakables(); } void DataPipeProducerDispatcher::CloseImplNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); data_pipe_->ProducerClose(); data_pipe_ = nullptr; } scoped_refptr<Dispatcher> DataPipeProducerDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); scoped_refptr<DataPipeProducerDispatcher> rv = Create(); rv->Init(data_pipe_); @@ -73,7 +68,7 @@ MojoResult DataPipeProducerDispatcher::WriteDataImplNoLock( UserPointer<const void> elements, UserPointer<uint32_t> num_bytes, MojoWriteDataFlags flags) { - mutex().AssertHeld(); + lock().AssertAcquired(); return data_pipe_->ProducerWriteData( elements, num_bytes, (flags & MOJO_WRITE_DATA_FLAG_ALL_OR_NONE)); } @@ -82,7 +77,7 @@ MojoResult DataPipeProducerDispatcher::BeginWriteDataImplNoLock( UserPointer<void*> buffer, UserPointer<uint32_t> buffer_num_bytes, MojoWriteDataFlags flags) { - mutex().AssertHeld(); + lock().AssertAcquired(); return data_pipe_->ProducerBeginWriteData( buffer, buffer_num_bytes, (flags & MOJO_WRITE_DATA_FLAG_ALL_OR_NONE)); @@ -90,14 +85,14 @@ MojoResult DataPipeProducerDispatcher::BeginWriteDataImplNoLock( MojoResult DataPipeProducerDispatcher::EndWriteDataImplNoLock( uint32_t num_bytes_written) { - mutex().AssertHeld(); + lock().AssertAcquired(); return data_pipe_->ProducerEndWriteData(num_bytes_written); } HandleSignalsState DataPipeProducerDispatcher::GetHandleSignalsStateImplNoLock() const { - mutex().AssertHeld(); + lock().AssertAcquired(); return data_pipe_->ProducerGetHandleSignalsState(); } @@ -106,7 +101,7 @@ MojoResult DataPipeProducerDispatcher::AddAwakableImplNoLock( MojoHandleSignals signals, uint32_t context, HandleSignalsState* signals_state) { - mutex().AssertHeld(); + lock().AssertAcquired(); return data_pipe_->ProducerAddAwakable(awakable, signals, context, signals_state); } @@ -114,7 +109,7 @@ MojoResult DataPipeProducerDispatcher::AddAwakableImplNoLock( void DataPipeProducerDispatcher::RemoveAwakableImplNoLock( Awakable* awakable, HandleSignalsState* signals_state) { - mutex().AssertHeld(); + lock().AssertAcquired(); data_pipe_->ProducerRemoveAwakable(awakable, signals_state); } @@ -140,7 +135,7 @@ bool DataPipeProducerDispatcher::EndSerializeAndCloseImplNoLock( } bool DataPipeProducerDispatcher::IsBusyNoLock() const { - mutex().AssertHeld(); + lock().AssertAcquired(); 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 7ce5156..86697fe 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) MOJO_NOT_THREAD_SAFE; + void Init(scoped_refptr<DataPipe> data_pipe); // |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(); + DataPipe* GetDataPipeForTest() { return data_pipe_.get(); } private: DataPipeProducerDispatcher(); @@ -64,18 +64,16 @@ 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 - MOJO_NOT_THREAD_SAFE; + size_t* max_platform_handles) override; bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) override - MOJO_NOT_THREAD_SAFE; + embedder::PlatformHandleVector* platform_handles) override; bool IsBusyNoLock() const override; - // This will be null if closed. - scoped_refptr<DataPipe> data_pipe_ MOJO_GUARDED_BY(mutex()); + // Protected by |lock()|: + scoped_refptr<DataPipe> data_pipe_; // This will be null if closed. 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 ad362fe..24f2c26 100644 --- a/third_party/mojo/src/mojo/edk/system/dispatcher.cc +++ b/third_party/mojo/src/mojo/edk/system/dispatcher.cc @@ -26,18 +26,16 @@ 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) MOJO_NO_THREAD_SAFETY_ANALYSIS { + Dispatcher* dispatcher) { DCHECK(dispatcher); - if (!dispatcher->mutex_.TryLock()) + if (!dispatcher->lock_.Try()) return DispatcherTransport(); // We shouldn't race with things that close dispatchers, since closing can - // only take place either under |handle_table_mutex_| or when the handle is + // only take place either under |handle_table_lock_| or when the handle is // marked as busy. DCHECK(!dispatcher->is_closed_); @@ -98,7 +96,7 @@ scoped_refptr<Dispatcher> Dispatcher::TransportDataAccess::Deserialize( } MojoResult Dispatcher::Close() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -115,7 +113,7 @@ MojoResult Dispatcher::WriteMessage( (transports->size() > 0 && transports->size() < GetConfiguration().max_message_num_handles)); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -130,7 +128,7 @@ MojoResult Dispatcher::ReadMessage(UserPointer<void> bytes, DCHECK(!num_dispatchers || *num_dispatchers == 0 || (dispatchers && dispatchers->empty())); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -141,7 +139,7 @@ MojoResult Dispatcher::ReadMessage(UserPointer<void> bytes, MojoResult Dispatcher::WriteData(UserPointer<const void> elements, UserPointer<uint32_t> num_bytes, MojoWriteDataFlags flags) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -151,7 +149,7 @@ MojoResult Dispatcher::WriteData(UserPointer<const void> elements, MojoResult Dispatcher::BeginWriteData(UserPointer<void*> buffer, UserPointer<uint32_t> buffer_num_bytes, MojoWriteDataFlags flags) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -159,7 +157,7 @@ MojoResult Dispatcher::BeginWriteData(UserPointer<void*> buffer, } MojoResult Dispatcher::EndWriteData(uint32_t num_bytes_written) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -169,7 +167,7 @@ MojoResult Dispatcher::EndWriteData(uint32_t num_bytes_written) { MojoResult Dispatcher::ReadData(UserPointer<void> elements, UserPointer<uint32_t> num_bytes, MojoReadDataFlags flags) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -179,7 +177,7 @@ MojoResult Dispatcher::ReadData(UserPointer<void> elements, MojoResult Dispatcher::BeginReadData(UserPointer<const void*> buffer, UserPointer<uint32_t> buffer_num_bytes, MojoReadDataFlags flags) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -187,7 +185,7 @@ MojoResult Dispatcher::BeginReadData(UserPointer<const void*> buffer, } MojoResult Dispatcher::EndReadData(uint32_t num_bytes_read) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -197,7 +195,7 @@ MojoResult Dispatcher::EndReadData(uint32_t num_bytes_read) { MojoResult Dispatcher::DuplicateBufferHandle( UserPointer<const MojoDuplicateBufferHandleOptions> options, scoped_refptr<Dispatcher>* new_dispatcher) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -209,7 +207,7 @@ MojoResult Dispatcher::MapBuffer( uint64_t num_bytes, MojoMapBufferFlags flags, scoped_ptr<embedder::PlatformSharedBufferMapping>* mapping) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; @@ -217,7 +215,7 @@ MojoResult Dispatcher::MapBuffer( } HandleSignalsState Dispatcher::GetHandleSignalsState() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) return HandleSignalsState(); @@ -228,7 +226,7 @@ MojoResult Dispatcher::AddAwakable(Awakable* awakable, MojoHandleSignals signals, uint32_t context, HandleSignalsState* signals_state) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) { if (signals_state) *signals_state = HandleSignalsState(); @@ -240,7 +238,7 @@ MojoResult Dispatcher::AddAwakable(Awakable* awakable, void Dispatcher::RemoveAwakable(Awakable* awakable, HandleSignalsState* handle_signals_state) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (is_closed_) { if (handle_signals_state) *handle_signals_state = HandleSignalsState(); @@ -259,14 +257,14 @@ Dispatcher::~Dispatcher() { } void Dispatcher::CancelAllAwakablesNoLock() { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(is_closed_); // By default, waiting isn't supported. Only dispatchers that can be waited on // will do something nontrivial. } void Dispatcher::CloseImplNoLock() { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(is_closed_); // This may not need to do anything. Dispatchers should override this to do // any actual close-time cleanup necessary. @@ -277,7 +275,7 @@ MojoResult Dispatcher::WriteMessageImplNoLock( uint32_t /*num_bytes*/, std::vector<DispatcherTransport>* /*transports*/, MojoWriteMessageFlags /*flags*/) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, not supported. Only needed for message pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -289,7 +287,7 @@ MojoResult Dispatcher::ReadMessageImplNoLock( DispatcherVector* /*dispatchers*/, uint32_t* /*num_dispatchers*/, MojoReadMessageFlags /*flags*/) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, not supported. Only needed for message pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -298,7 +296,7 @@ MojoResult Dispatcher::ReadMessageImplNoLock( MojoResult Dispatcher::WriteDataImplNoLock(UserPointer<const void> /*elements*/, UserPointer<uint32_t> /*num_bytes*/, MojoWriteDataFlags /*flags*/) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, not supported. Only needed for data pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -308,14 +306,14 @@ MojoResult Dispatcher::BeginWriteDataImplNoLock( UserPointer<void*> /*buffer*/, UserPointer<uint32_t> /*buffer_num_bytes*/, MojoWriteDataFlags /*flags*/) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); 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*/) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, not supported. Only needed for data pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -324,7 +322,7 @@ MojoResult Dispatcher::EndWriteDataImplNoLock(uint32_t /*num_bytes_written*/) { MojoResult Dispatcher::ReadDataImplNoLock(UserPointer<void> /*elements*/, UserPointer<uint32_t> /*num_bytes*/, MojoReadDataFlags /*flags*/) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, not supported. Only needed for data pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -334,14 +332,14 @@ MojoResult Dispatcher::BeginReadDataImplNoLock( UserPointer<const void*> /*buffer*/, UserPointer<uint32_t> /*buffer_num_bytes*/, MojoReadDataFlags /*flags*/) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); 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*/) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, not supported. Only needed for data pipe dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -350,7 +348,7 @@ MojoResult Dispatcher::EndReadDataImplNoLock(uint32_t /*num_bytes_read*/) { MojoResult Dispatcher::DuplicateBufferHandleImplNoLock( UserPointer<const MojoDuplicateBufferHandleOptions> /*options*/, scoped_refptr<Dispatcher>* /*new_dispatcher*/) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, not supported. Only needed for buffer dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; @@ -361,14 +359,14 @@ MojoResult Dispatcher::MapBufferImplNoLock( uint64_t /*num_bytes*/, MojoMapBufferFlags /*flags*/, scoped_ptr<embedder::PlatformSharedBufferMapping>* /*mapping*/) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, not supported. Only needed for buffer dispatchers. return MOJO_RESULT_INVALID_ARGUMENT; } HandleSignalsState Dispatcher::GetHandleSignalsStateImplNoLock() const { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, waiting isn't supported. Only dispatchers that can be waited on // will do something nontrivial. @@ -380,7 +378,7 @@ MojoResult Dispatcher::AddAwakableImplNoLock( MojoHandleSignals /*signals*/, uint32_t /*context*/, HandleSignalsState* signals_state) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, waiting isn't supported. Only dispatchers that can be waited on // will do something nontrivial. @@ -391,7 +389,7 @@ MojoResult Dispatcher::AddAwakableImplNoLock( void Dispatcher::RemoveAwakableImplNoLock(Awakable* /*awakable*/, HandleSignalsState* signals_state) { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, waiting isn't supported. Only dispatchers that can be waited on // will do something nontrivial. @@ -421,7 +419,7 @@ bool Dispatcher::EndSerializeAndCloseImplNoLock( } bool Dispatcher::IsBusyNoLock() const { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); // Most dispatchers support only "atomic" operations, so they are never busy // (in this sense). @@ -429,7 +427,7 @@ bool Dispatcher::IsBusyNoLock() const { } void Dispatcher::CloseNoLock() { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); is_closed_ = true; @@ -439,7 +437,7 @@ void Dispatcher::CloseNoLock() { scoped_refptr<Dispatcher> Dispatcher::CreateEquivalentDispatcherAndCloseNoLock() { - mutex_.AssertHeld(); + lock_.AssertAcquired(); DCHECK(!is_closed_); is_closed_ = true; @@ -477,7 +475,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). - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); #endif return EndSerializeAndCloseImplNoLock(channel, destination, actual_size, @@ -488,7 +486,7 @@ bool Dispatcher::EndSerializeAndClose( void DispatcherTransport::End() { DCHECK(dispatcher_); - dispatcher_->mutex_.Unlock(); + dispatcher_->lock_.Release(); 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 98b39f4..ed6999d 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 mutex -// |mutex_|, which is also made available to implementation subclasses (via the -// |mutex()| method). +// 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). 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 |mutex_| and handle races with |Close()|. Then they call out to - // subclasses' |...ImplNoLock()| methods (still under |mutex_|), which - // actually implement the primitives. + // take |lock_| and handle races with |Close()|. Then they call out to + // subclasses' |...ImplNoLock()| methods (still under |lock_|), 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,100 +221,91 @@ 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. - virtual void CancelAllAwakablesNoLock() MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); - virtual void CloseImplNoLock() MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); - + // exactly once -- first |CancelAllAwakablesNoLock()|, then + // |CloseImplNoLock()|, + // when the dispatcher is being closed. They are called under |lock_|. + virtual void CancelAllAwakablesNoLock(); + virtual void CloseImplNoLock(); virtual scoped_refptr<Dispatcher> - CreateEquivalentDispatcherAndCloseImplNoLock() - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_) = 0; + CreateEquivalentDispatcherAndCloseImplNoLock() = 0; // These are to be overridden by subclasses (if necessary). They are never - // called after the dispatcher has been closed. See the descriptions of the - // methods without the "ImplNoLock" for more information. + // called after the dispatcher has been closed. They are called under |lock_|. + // 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) MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + MojoWriteMessageFlags flags); virtual MojoResult ReadMessageImplNoLock(UserPointer<void> bytes, UserPointer<uint32_t> num_bytes, DispatcherVector* dispatchers, uint32_t* num_dispatchers, - MojoReadMessageFlags flags) - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + MojoReadMessageFlags flags); virtual MojoResult WriteDataImplNoLock(UserPointer<const void> elements, UserPointer<uint32_t> num_bytes, - MojoWriteDataFlags flags) - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + MojoWriteDataFlags flags); virtual MojoResult BeginWriteDataImplNoLock( UserPointer<void*> buffer, UserPointer<uint32_t> buffer_num_bytes, - MojoWriteDataFlags flags) MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); - virtual MojoResult EndWriteDataImplNoLock(uint32_t num_bytes_written) - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + MojoWriteDataFlags flags); + virtual MojoResult EndWriteDataImplNoLock(uint32_t num_bytes_written); virtual MojoResult ReadDataImplNoLock(UserPointer<void> elements, UserPointer<uint32_t> num_bytes, - MojoReadDataFlags flags) - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + MojoReadDataFlags flags); virtual MojoResult BeginReadDataImplNoLock( UserPointer<const void*> buffer, UserPointer<uint32_t> buffer_num_bytes, - MojoReadDataFlags flags) MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); - virtual MojoResult EndReadDataImplNoLock(uint32_t num_bytes_read) - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + MojoReadDataFlags flags); + virtual MojoResult EndReadDataImplNoLock(uint32_t num_bytes_read); virtual MojoResult DuplicateBufferHandleImplNoLock( UserPointer<const MojoDuplicateBufferHandleOptions> options, - scoped_refptr<Dispatcher>* new_dispatcher) - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + scoped_refptr<Dispatcher>* new_dispatcher); virtual MojoResult MapBufferImplNoLock( uint64_t offset, uint64_t num_bytes, MojoMapBufferFlags flags, - scoped_ptr<embedder::PlatformSharedBufferMapping>* mapping) - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); - virtual HandleSignalsState GetHandleSignalsStateImplNoLock() const - MOJO_SHARED_LOCKS_REQUIRED(mutex_); + scoped_ptr<embedder::PlatformSharedBufferMapping>* mapping); + virtual HandleSignalsState GetHandleSignalsStateImplNoLock() const; virtual MojoResult AddAwakableImplNoLock(Awakable* awakable, MojoHandleSignals signals, uint32_t context, - HandleSignalsState* signals_state) - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + HandleSignalsState* signals_state); virtual void RemoveAwakableImplNoLock(Awakable* awakable, - HandleSignalsState* signals_state) - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + HandleSignalsState* signals_state); // 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 |mutex_| NOT + // Note: |StartSerializeImplNoLock()| is actually called with |lock_| NOT // held, since the dispatcher should only be accessible to the calling thread. - // 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 + // 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 // 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) - MOJO_NOT_THREAD_SAFE; + size_t* max_platform_handles); virtual bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) MOJO_NOT_THREAD_SAFE; + embedder::PlatformHandleVector* platform_handles); // 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 MOJO_SHARED_LOCKS_REQUIRED(mutex_); + virtual bool IsBusyNoLock() const; - Mutex& mutex() const MOJO_LOCK_RETURNED(mutex_) { return 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_; } private: friend class DispatcherTransport; @@ -323,29 +314,20 @@ 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() MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + void CloseNoLock(); // 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() - MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndCloseNoLock(); // 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 @@ -358,7 +340,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) MOJO_NOT_THREAD_SAFE; + size_t* max_platform_handles); // 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 @@ -373,13 +355,12 @@ class MOJO_SYSTEM_IMPL_EXPORT Dispatcher bool EndSerializeAndClose(Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) - MOJO_NOT_THREAD_SAFE; + embedder::PlatformHandleVector* platform_handles); // This protects the following members as well as any state added by // subclasses. - mutable Mutex mutex_; - bool is_closed_ MOJO_GUARDED_BY(mutex_); + mutable base::Lock lock_; + bool is_closed_; MOJO_DISALLOW_COPY_AND_ASSIGN(Dispatcher); }; @@ -394,15 +375,12 @@ class MOJO_SYSTEM_IMPL_EXPORT DispatcherTransport { public: DispatcherTransport() : dispatcher_(nullptr) {} - void End() MOJO_NOT_THREAD_SAFE; + void End(); Dispatcher::Type GetType() const { return dispatcher_->GetType(); } - 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 { + bool IsBusy() const { return dispatcher_->IsBusyNoLock(); } + void Close() { dispatcher_->CloseNoLock(); } + scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndClose() { 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 c6431ff..10c2b86 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 { - mutex().AssertHeld(); + lock().AssertAcquired(); 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 9655f9c..9630216 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) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); filter_ = filter.Pass(); } bool EndpointRelayer::OnReadMessage(unsigned port, MessageInTransit* message) { DCHECK(message); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); // 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) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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 cfeda24..8b17a50 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,8 +68,7 @@ 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) MOJO_NOT_THREAD_SAFE; + void Init(ChannelEndpoint* endpoint0, ChannelEndpoint* endpoint1); // Sets (or resets) the filter, which can (optionally) handle/filter // |Type::ENDPOINT_CLIENT| messages (see |Filter| above). @@ -82,9 +81,11 @@ class MOJO_SYSTEM_IMPL_EXPORT EndpointRelayer final private: ~EndpointRelayer() override; - Mutex mutex_; - scoped_refptr<ChannelEndpoint> endpoints_[2] MOJO_GUARDED_BY(mutex_); - scoped_ptr<Filter> filter_ MOJO_GUARDED_BY(mutex_); + // 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_; 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 eaa1825..cd091b1 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() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (endpoint_) { endpoint_->DetachFromClient(); endpoint_ = nullptr; @@ -63,7 +63,7 @@ void IncomingEndpoint::Close() { bool IncomingEndpoint::OnReadMessage(unsigned /*port*/, MessageInTransit* message) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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 bf0acf3..22bf0b9 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() MOJO_NOT_THREAD_SAFE; + scoped_refptr<ChannelEndpoint> Init(); scoped_refptr<MessagePipe> ConvertToMessagePipe(); scoped_refptr<DataPipe> ConvertToDataPipeProducer( @@ -52,9 +52,9 @@ class MOJO_SYSTEM_IMPL_EXPORT IncomingEndpoint final private: ~IncomingEndpoint() override; - Mutex mutex_; - scoped_refptr<ChannelEndpoint> endpoint_ MOJO_GUARDED_BY(mutex_); - MessageInTransitQueue message_queue_ MOJO_GUARDED_BY(mutex_); + base::Lock lock_; // Protects the following members. + scoped_refptr<ChannelEndpoint> endpoint_; + MessageInTransitQueue message_queue_; 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 7c4f3d4..424a130 100644 --- a/third_party/mojo/src/mojo/edk/system/ipc_support.cc +++ b/third_party/mojo/src/mojo/edk/system/ipc_support.cc @@ -141,9 +141,8 @@ embedder::ScopedPlatformHandle IPCSupport::ConnectToSlaveInternal( system::ProcessIdentifier peer_id = system::kInvalidProcessIdentifier; embedder::ScopedPlatformHandle platform_connection_handle; - CHECK_EQ(connection_manager()->Connect(connection_id, &peer_id, - &platform_connection_handle), - ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION); + CHECK(connection_manager()->Connect(connection_id, &peer_id, + &platform_connection_handle)); DCHECK_EQ(peer_id, *slave_process_identifier); DCHECK(platform_connection_handle.is_valid()); return platform_connection_handle; @@ -155,9 +154,8 @@ embedder::ScopedPlatformHandle IPCSupport::ConnectToMasterInternal( system::ProcessIdentifier peer_id; embedder::ScopedPlatformHandle platform_connection_handle; - CHECK_EQ(connection_manager()->Connect(connection_id, &peer_id, - &platform_connection_handle), - ConnectionManager::Result::SUCCESS_CONNECT_NEW_CONNECTION); + CHECK(connection_manager()->Connect(connection_id, &peer_id, + &platform_connection_handle)); 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 7e24a4a8..d756f73 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,8 +21,6 @@ namespace mojo { namespace system { -namespace { - const ProcessIdentifier kFirstSlaveProcessIdentifier = 2; static_assert(kMasterProcessIdentifier != kInvalidProcessIdentifier, @@ -32,29 +30,6 @@ 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 @@ -140,38 +115,28 @@ void MasterConnectionManager::Helper::OnReadMessage( const ConnectionIdentifier* connection_id = reinterpret_cast<const ConnectionIdentifier*>(message_view.bytes()); - Result result = Result::FAILURE; + bool result; 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::SUCCESS - : Result::FAILURE; + result = owner_->AllowConnectImpl(process_identifier_, *connection_id); break; case MessageInTransit::Subtype::CONNECTION_MANAGER_CANCEL_CONNECT: - result = owner_->CancelConnectImpl(process_identifier_, *connection_id) - ? Result::SUCCESS - : Result::FAILURE; + result = owner_->CancelConnectImpl(process_identifier_, *connection_id); 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 also a platform handle in the case of "new connection" -- handled - // further below). - if (result != Result::FAILURE) { + // (and maybe also a platform handle). + if (result) { 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. @@ -180,21 +145,22 @@ void MasterConnectionManager::Helper::OnReadMessage( scoped_ptr<MessageInTransit> response(new MessageInTransit( MessageInTransit::Type::CONNECTION_MANAGER_ACK, - ConnectionManagerResultToMessageInTransitSubtype(result), num_bytes, - bytes)); + result ? MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_SUCCESS + : MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_FAILURE, + num_bytes, bytes)); - if (result == Result::SUCCESS_CONNECT_NEW_CONNECTION) { + if (platform_handle.is_valid()) { + // Only success acks for "connect" *may* have a platform handle attached. + DCHECK(result); 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())) { @@ -295,7 +261,7 @@ ProcessIdentifier MasterConnectionManager::AddSlave( ProcessIdentifier slave_process_identifier; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); CHECK_NE(next_process_identifier_, kMasterProcessIdentifier); slave_process_identifier = next_process_identifier_; next_process_identifier_++; @@ -322,7 +288,7 @@ ProcessIdentifier MasterConnectionManager::AddSlaveAndBootstrap( ProcessIdentifier slave_process_identifier = AddSlave(slave_info, platform_handle.Pass()); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); DCHECK(pending_connections_.find(connection_id) == pending_connections_.end()); PendingConnectionInfo* info = @@ -362,10 +328,11 @@ bool MasterConnectionManager::CancelConnect( return CancelConnectImpl(kMasterProcessIdentifier, connection_id); } -ConnectionManager::Result MasterConnectionManager::Connect( +bool 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); } @@ -375,7 +342,7 @@ bool MasterConnectionManager::AllowConnectImpl( const ConnectionIdentifier& connection_id) { DCHECK_NE(process_identifier, kInvalidProcessIdentifier); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); auto it = pending_connections_.find(connection_id); if (it == pending_connections_.end()) { @@ -414,7 +381,7 @@ bool MasterConnectionManager::CancelConnectImpl( const ConnectionIdentifier& connection_id) { DCHECK_NE(process_identifier, kInvalidProcessIdentifier); - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); auto it = pending_connections_.find(connection_id); if (it == pending_connections_.end()) { @@ -441,7 +408,7 @@ bool MasterConnectionManager::CancelConnectImpl( return true; } -ConnectionManager::Result MasterConnectionManager::ConnectImpl( +bool MasterConnectionManager::ConnectImpl( ProcessIdentifier process_identifier, const ConnectionIdentifier& connection_id, ProcessIdentifier* peer_process_identifier, @@ -451,7 +418,7 @@ ConnectionManager::Result MasterConnectionManager::ConnectImpl( DCHECK(platform_handle); DCHECK(!platform_handle->is_valid()); // Not technically wrong, but unlikely. - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); auto it = pending_connections_.find(connection_id); if (it == pending_connections_.end()) { @@ -459,7 +426,7 @@ ConnectionManager::Result MasterConnectionManager::ConnectImpl( LOG(ERROR) << "Connect() from process " << process_identifier << " for connection ID " << connection_id.ToString() << " which is not pending"; - return Result::FAILURE; + return false; } PendingConnectionInfo* info = it->second; @@ -476,27 +443,23 @@ ConnectionManager::Result MasterConnectionManager::ConnectImpl( LOG(ERROR) << "Connect() from process " << process_identifier << " for connection ID " << connection_id.ToString() << " which is neither connectee"; - return Result::FAILURE; + return false; } - // 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 result; + return true; } ProcessIdentifier remaining_connectee; @@ -516,7 +479,7 @@ ConnectionManager::Result MasterConnectionManager::ConnectImpl( << " in state " << info->state; pending_connections_.erase(it); delete info; - return Result::FAILURE; + return false; } if (process_identifier != remaining_connectee) { @@ -525,28 +488,18 @@ ConnectionManager::Result MasterConnectionManager::ConnectImpl( << " which is not the remaining connectee"; pending_connections_.erase(it); delete info; - return Result::FAILURE; + return false; } *peer_process_identifier = peer; - - // 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; - } + *platform_handle = info->remaining_handle.Pass(); + DCHECK((info->first == info->second) ^ platform_handle->is_valid()); pending_connections_.erase(it); delete info; DVLOG(1) << "Connection ID " << connection_id.ToString() << ": second Connect() from process identifier " << process_identifier; - return result; + return true; } void MasterConnectionManager::ShutdownOnPrivateThread() { @@ -602,7 +555,7 @@ void MasterConnectionManager::OnError(ProcessIdentifier process_identifier) { delete helper; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); // 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 497f449..b7992c8 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,8 +51,7 @@ 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) - MOJO_NOT_THREAD_SAFE; + embedder::MasterProcessDelegate* master_process_delegate); // 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 @@ -76,12 +75,12 @@ class MOJO_SYSTEM_IMPL_EXPORT MasterConnectionManager final const ConnectionIdentifier& connection_id); // |ConnectionManager| methods: - void Shutdown() override MOJO_NOT_THREAD_SAFE; + void Shutdown() override; bool AllowConnect(const ConnectionIdentifier& connection_id) override; bool CancelConnect(const ConnectionIdentifier& connection_id) override; - Result Connect(const ConnectionIdentifier& connection_id, - ProcessIdentifier* peer_process_identifier, - embedder::ScopedPlatformHandle* platform_handle) override; + bool Connect(const ConnectionIdentifier& connection_id, + ProcessIdentifier* peer_process_identifier, + embedder::ScopedPlatformHandle* platform_handle) override; private: class Helper; @@ -92,13 +91,13 @@ class MOJO_SYSTEM_IMPL_EXPORT MasterConnectionManager final const ConnectionIdentifier& connection_id); bool CancelConnectImpl(ProcessIdentifier process_identifier, const ConnectionIdentifier& connection_id); - Result ConnectImpl(ProcessIdentifier process_identifier, - const ConnectionIdentifier& connection_id, - ProcessIdentifier* peer_process_identifier, - embedder::ScopedPlatformHandle* platform_handle); + bool 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() MOJO_NOT_THREAD_SAFE; + void ShutdownOnPrivateThread(); // Signals |*event| on completion. void AddSlaveOnPrivateThread(embedder::SlaveInfo slave_info, embedder::ScopedPlatformHandle platform_handle, @@ -133,15 +132,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. - // Note: |mutex_| is not needed in the constructor, |Init()|, - // |Shutdown()|/|ShutdownOnPrivateThread()|, or the destructor - Mutex mutex_; + // Protects the members below (except in the constructor, |Init()|, + // |Shutdown()|/|ShutdownOnPrivateThread()|, and the destructor). + base::Lock lock_; - ProcessIdentifier next_process_identifier_ MOJO_GUARDED_BY(mutex_); + ProcessIdentifier next_process_identifier_; struct PendingConnectionInfo; base::hash_map<ConnectionIdentifier, PendingConnectionInfo*> - pending_connections_ MOJO_GUARDED_BY(mutex_); // Owns its values. + pending_connections_; // 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 c61cfd6..063c8d6 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,18 +80,11 @@ 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|, 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. + // 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): 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 c329e87..ddbe863 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 { - mutex().AssertHeld(); + lock().AssertAcquired(); return message_pipe_.get(); } unsigned MessagePipeDispatcher::GetPortNoLock() const { - mutex().AssertHeld(); + lock().AssertAcquired(); return port_; } void MessagePipeDispatcher::CancelAllAwakablesNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); message_pipe_->CancelAllAwakables(port_); } void MessagePipeDispatcher::CloseImplNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); message_pipe_->Close(port_); message_pipe_ = nullptr; port_ = kInvalidPort; @@ -128,7 +128,7 @@ void MessagePipeDispatcher::CloseImplNoLock() { scoped_refptr<Dispatcher> MessagePipeDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); // 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)); - mutex().AssertHeld(); + lock().AssertAcquired(); 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) { - mutex().AssertHeld(); + lock().AssertAcquired(); return message_pipe_->ReadMessage(port_, bytes, num_bytes, dispatchers, num_dispatchers, flags); } HandleSignalsState MessagePipeDispatcher::GetHandleSignalsStateImplNoLock() const { - mutex().AssertHeld(); + lock().AssertAcquired(); return message_pipe_->GetHandleSignalsState(port_); } @@ -180,7 +180,7 @@ MojoResult MessagePipeDispatcher::AddAwakableImplNoLock( MojoHandleSignals signals, uint32_t context, HandleSignalsState* signals_state) { - mutex().AssertHeld(); + lock().AssertAcquired(); 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) { - mutex().AssertHeld(); + lock().AssertAcquired(); 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 23bb1ed..9df5fed 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,8 +42,7 @@ 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) MOJO_NOT_THREAD_SAFE; + void Init(scoped_refptr<MessagePipe> message_pipe, unsigned port); // |Dispatcher| public methods: Type GetType() const override; @@ -100,18 +99,16 @@ 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 - MOJO_NOT_THREAD_SAFE; + size_t* max_platform_handles) override; bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) override - MOJO_NOT_THREAD_SAFE; + embedder::PlatformHandleVector* platform_handles) override; - // This will be null if closed. - scoped_refptr<MessagePipe> message_pipe_ MOJO_GUARDED_BY(mutex()); - unsigned port_ MOJO_GUARDED_BY(mutex()); + // Protected by |lock()|: + scoped_refptr<MessagePipe> message_pipe_; // This will be null if closed. + unsigned port_; 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 deleted file mode 100644 index 216b35a..0000000 --- a/third_party/mojo/src/mojo/edk/system/mutex.cc +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "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 deleted file mode 100644 index acb410a..0000000 --- a/third_party/mojo/src/mojo/edk/system/mutex.h +++ /dev/null @@ -1,93 +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. - -// 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 deleted file mode 100644 index 9b51653..0000000 --- a/third_party/mojo/src/mojo/edk/system/mutex_unittest.cc +++ /dev/null @@ -1,250 +0,0 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "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 681b406..d33c960 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() { - MutexLocker locker(&mutex()); + base::AutoLock locker(lock()); return platform_handle_.Pass(); } @@ -73,13 +73,13 @@ PlatformHandleDispatcher::~PlatformHandleDispatcher() { } void PlatformHandleDispatcher::CloseImplNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); platform_handle_.reset(); } scoped_refptr<Dispatcher> PlatformHandleDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); 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 8e8cf69..9e2e37d 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,16 +48,14 @@ class MOJO_SYSTEM_IMPL_EXPORT PlatformHandleDispatcher final override; void StartSerializeImplNoLock(Channel* channel, size_t* max_size, - size_t* max_platform_handles) override - MOJO_NOT_THREAD_SAFE; + size_t* max_platform_handles) override; bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) override - MOJO_NOT_THREAD_SAFE; + embedder::PlatformHandleVector* platform_handles) override; - embedder::ScopedPlatformHandle platform_handle_ MOJO_GUARDED_BY(mutex()); + embedder::ScopedPlatformHandle platform_handle_; 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 a901791..bc27889 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,6 +19,7 @@ #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" @@ -27,7 +28,6 @@ #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; { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); 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_; - Mutex mutex_; - std::vector<uint32_t> expected_sizes_ MOJO_GUARDED_BY(mutex_); - size_t position_ MOJO_GUARDED_BY(mutex_); + base::Lock lock_; // Protects the following members. + std::vector<uint32_t> expected_sizes_; + size_t position_; 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 caf8e0b..8ab8f10 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() { - mutex().AssertHeld(); + lock().AssertAcquired(); DCHECK(shared_buffer_); shared_buffer_ = nullptr; } scoped_refptr<Dispatcher> SharedBufferDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); 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) { - mutex().AssertHeld(); + lock().AssertAcquired(); 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) { - mutex().AssertHeld(); + lock().AssertAcquired(); 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 1222e03..deef5b4 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,17 +93,14 @@ 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 - MOJO_NOT_THREAD_SAFE; + size_t* max_platform_handles) override; bool EndSerializeAndCloseImplNoLock( Channel* channel, void* destination, size_t* actual_size, - embedder::PlatformHandleVector* platform_handles) override - MOJO_NOT_THREAD_SAFE; + embedder::PlatformHandleVector* platform_handles) override; - scoped_refptr<embedder::PlatformSharedBuffer> shared_buffer_ - MOJO_GUARDED_BY(mutex()); + scoped_refptr<embedder::PlatformSharedBuffer> shared_buffer_; 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 7264a12..f7db875 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() { - mutex().AssertHeld(); + lock().AssertAcquired(); awakable_list_.AwakeForStateChange(GetHandleSignalsStateImplNoLock()); } void SimpleDispatcher::CancelAllAwakablesNoLock() { - mutex().AssertHeld(); + lock().AssertAcquired(); awakable_list_.CancelAll(); } @@ -30,7 +30,7 @@ MojoResult SimpleDispatcher::AddAwakableImplNoLock( MojoHandleSignals signals, uint32_t context, HandleSignalsState* signals_state) { - mutex().AssertHeld(); + lock().AssertAcquired(); HandleSignalsState state(GetHandleSignalsStateImplNoLock()); if (state.satisfies(signals)) { @@ -51,7 +51,7 @@ MojoResult SimpleDispatcher::AddAwakableImplNoLock( void SimpleDispatcher::RemoveAwakableImplNoLock( Awakable* awakable, HandleSignalsState* signals_state) { - mutex().AssertHeld(); + lock().AssertAcquired(); 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 a828435..1ec7ba7 100644 --- a/third_party/mojo/src/mojo/edk/system/simple_dispatcher.h +++ b/third_party/mojo/src/mojo/edk/system/simple_dispatcher.h @@ -25,8 +25,9 @@ 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). - void HandleSignalsStateChangedNoLock() MOJO_EXCLUSIVE_LOCKS_REQUIRED(mutex()); + // |GetHandleSignalsStateImplNoLock()| should be checked again). Must be + // called under lock. + void HandleSignalsStateChangedNoLock(); // |Dispatcher| protected methods: void CancelAllAwakablesNoLock() override; @@ -38,7 +39,8 @@ class MOJO_SYSTEM_IMPL_EXPORT SimpleDispatcher : public Dispatcher { HandleSignalsState* signals_state) override; private: - AwakableList awakable_list_ MOJO_GUARDED_BY(mutex()); + // Protected by |lock()|: + AwakableList awakable_list_; 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 051e108..c9a1510 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,11 +28,9 @@ 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) { - MutexLocker locker(&mutex()); + base::AutoLock locker(lock()); // Any new signals that are set should be satisfiable. CHECK_EQ(new_satisfied_signals & ~state_.satisfied_signals, @@ -47,7 +45,7 @@ class MockSimpleDispatcher final : public SimpleDispatcher { } void SetSatisfiableSignals(MojoHandleSignals new_satisfiable_signals) { - MutexLocker locker(&mutex()); + base::AutoLock locker(lock()); // Satisfied implies satisfiable. CHECK_EQ(new_satisfiable_signals & state_.satisfied_signals, @@ -68,17 +66,19 @@ class MockSimpleDispatcher final : public SimpleDispatcher { scoped_refptr<Dispatcher> CreateEquivalentDispatcherAndCloseImplNoLock() override { - scoped_refptr<MockSimpleDispatcher> rv(new MockSimpleDispatcher(state_)); + scoped_refptr<MockSimpleDispatcher> rv(new MockSimpleDispatcher()); + rv->state_ = state_; return scoped_refptr<Dispatcher>(rv.get()); } // |Dispatcher| override: HandleSignalsState GetHandleSignalsStateImplNoLock() const override { - mutex().AssertHeld(); + lock().AssertAcquired(); return state_; } - HandleSignalsState state_ MOJO_GUARDED_BY(mutex()); + // Protected by |lock()|: + HandleSignalsState state_; 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 7d0a998..d59bd82 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_(nullptr), - ack_peer_process_identifier_(nullptr), - ack_platform_handle_(nullptr), + ack_result_(), + ack_peer_process_identifier_(), + ack_platform_handle_(), event_(false, false) { // Auto-reset, not initially signalled. } @@ -78,40 +78,38 @@ bool SlaveConnectionManager::AllowConnect( const ConnectionIdentifier& connection_id) { AssertNotOnPrivateThread(); - MutexLocker locker(&mutex_); - Result result = Result::FAILURE; + base::AutoLock locker(lock_); + bool result = false; private_thread_.message_loop()->PostTask( FROM_HERE, base::Bind(&SlaveConnectionManager::AllowConnectOnPrivateThread, base::Unretained(this), connection_id, &result)); event_.Wait(); - DCHECK(result == Result::FAILURE || result == Result::SUCCESS); - return result == Result::SUCCESS; + return result; } bool SlaveConnectionManager::CancelConnect( const ConnectionIdentifier& connection_id) { AssertNotOnPrivateThread(); - MutexLocker locker(&mutex_); - Result result = Result::FAILURE; + base::AutoLock locker(lock_); + bool result = false; private_thread_.message_loop()->PostTask( FROM_HERE, base::Bind(&SlaveConnectionManager::CancelConnectOnPrivateThread, base::Unretained(this), connection_id, &result)); event_.Wait(); - DCHECK(result == Result::FAILURE || result == Result::SUCCESS); - return result == Result::SUCCESS; + return result; } -ConnectionManager::Result SlaveConnectionManager::Connect( +bool SlaveConnectionManager::Connect( const ConnectionIdentifier& connection_id, ProcessIdentifier* peer_process_identifier, embedder::ScopedPlatformHandle* platform_handle) { AssertNotOnPrivateThread(); - MutexLocker locker(&mutex_); - Result result = Result::FAILURE; + base::AutoLock locker(lock_); + bool result = false; private_thread_.message_loop()->PostTask( FROM_HERE, base::Bind(&SlaveConnectionManager::ConnectOnPrivateThread, base::Unretained(this), connection_id, &result, @@ -141,12 +139,12 @@ void SlaveConnectionManager::ShutdownOnPrivateThread() { void SlaveConnectionManager::AllowConnectOnPrivateThread( const ConnectionIdentifier& connection_id, - Result* result) { + bool* result) { DCHECK(result); AssertOnPrivateThread(); // This should only posted (from another thread, to |private_thread_|) with // the lock held (until this thread triggers |event_|). - DCHECK(!mutex_.TryLock()); + DCHECK(!lock_.Try()); DCHECK_EQ(awaiting_ack_type_, NOT_AWAITING_ACK); DVLOG(1) << "Sending AllowConnect: connection ID " @@ -156,7 +154,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 = Result::FAILURE; + *result = false; event_.Signal(); return; } @@ -166,12 +164,12 @@ void SlaveConnectionManager::AllowConnectOnPrivateThread( void SlaveConnectionManager::CancelConnectOnPrivateThread( const ConnectionIdentifier& connection_id, - Result* result) { + bool* result) { DCHECK(result); AssertOnPrivateThread(); // This should only posted (from another thread, to |private_thread_|) with // the lock held (until this thread triggers |event_|). - DCHECK(!mutex_.TryLock()); + DCHECK(!lock_.Try()); DCHECK_EQ(awaiting_ack_type_, NOT_AWAITING_ACK); DVLOG(1) << "Sending CancelConnect: connection ID " @@ -181,7 +179,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 = Result::FAILURE; + *result = false; event_.Signal(); return; } @@ -191,7 +189,7 @@ void SlaveConnectionManager::CancelConnectOnPrivateThread( void SlaveConnectionManager::ConnectOnPrivateThread( const ConnectionIdentifier& connection_id, - Result* result, + bool* result, ProcessIdentifier* peer_process_identifier, embedder::ScopedPlatformHandle* platform_handle) { DCHECK(result); @@ -200,7 +198,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(!mutex_.TryLock()); + DCHECK(!lock_.Try()); DCHECK_EQ(awaiting_ack_type_, NOT_AWAITING_ACK); DVLOG(1) << "Sending Connect: connection ID " << connection_id.ToString(); @@ -209,7 +207,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 = Result::FAILURE; + *result = false; platform_handle->reset(); event_.Signal(); return; @@ -225,8 +223,8 @@ void SlaveConnectionManager::OnReadMessage( embedder::ScopedPlatformHandleVectorPtr platform_handles) { AssertOnPrivateThread(); - // Set |*ack_result_| to failure by default. - *ack_result_ = Result::FAILURE; + // Set |*ack_result_| to false by default. + *ack_result_ = false; // Note: Since we should be able to trust the master, simply crash (i.e., // |CHECK()|-fail) if it sends us something invalid. @@ -242,53 +240,38 @@ void SlaveConnectionManager::OnReadMessage( if (message_view.subtype() == MessageInTransit::Subtype::CONNECTION_MANAGER_ACK_FAILURE) { // Failure acks never have any contents. - 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; + 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(!ack_peer_process_identifier_); DCHECK(!ack_platform_handle_); } else { - // Success acks for "connect" always have a |ProcessIdentifier| as data. + DCHECK_EQ(awaiting_ack_type_, AWAITING_CONNECT_ACK); + // Success acks for "connect" always have a |ProcessIdentifier| as data, + // and *maybe* one platform handle. 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()); - - 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); + if (num_platform_handles > 0) { + ack_platform_handle_->reset(platform_handles->at(0)); + platform_handles->at(0) = embedder::PlatformHandle(); + } else { + ack_platform_handle_->reset(); } } + } 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 b2cd92c..40fc79b 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; - Result Connect(const ConnectionIdentifier& connection_id, - ProcessIdentifier* peer_process_identifier, - embedder::ScopedPlatformHandle* platform_handle) override; + bool 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, - Result* result); + bool* result); void CancelConnectOnPrivateThread(const ConnectionIdentifier& connection_id, - Result* result); + bool* result); void ConnectOnPrivateThread(const ConnectionIdentifier& connection_id, - Result* result, + bool* 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_; - Result* ack_result_; + bool* 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. - Mutex mutex_; + base::Lock lock_; 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 b3e69b1..51d2a58 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) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); ASSERT_EQ(0u, port_); ASSERT_FALSE(endpoint_); port_ = port; @@ -25,30 +25,30 @@ void TestChannelEndpointClient::Init(unsigned port, ChannelEndpoint* endpoint) { } bool TestChannelEndpointClient::IsDetached() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return !endpoint_; } size_t TestChannelEndpointClient::NumMessages() const { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); return messages_.Size(); } scoped_ptr<MessageInTransit> TestChannelEndpointClient::PopMessage() { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (messages_.IsEmpty()) return nullptr; return messages_.GetMessage(); } void TestChannelEndpointClient::SetReadEvent(base::WaitableEvent* read_event) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); read_event_ = read_event; } bool TestChannelEndpointClient::OnReadMessage(unsigned port, MessageInTransit* message) { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); EXPECT_EQ(port_, port); EXPECT_TRUE(endpoint_); @@ -61,9 +61,9 @@ bool TestChannelEndpointClient::OnReadMessage(unsigned port, } void TestChannelEndpointClient::OnDetachFromChannel(unsigned port) { - MutexLocker locker(&mutex_); - EXPECT_EQ(port_, port); + + base::AutoLock locker(lock_); 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 bb812f0..c2347d5 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 Mutex mutex_; + mutable base::Lock lock_; // Protects the members below. - unsigned port_ MOJO_GUARDED_BY(mutex_); - scoped_refptr<ChannelEndpoint> endpoint_ MOJO_GUARDED_BY(mutex_); + unsigned port_; + scoped_refptr<ChannelEndpoint> endpoint_; - MessageInTransitQueue messages_ MOJO_GUARDED_BY(mutex_); + MessageInTransitQueue messages_; // Event to trigger if we read a message (may be null). - base::WaitableEvent* read_event_ MOJO_GUARDED_BY(mutex_); + base::WaitableEvent* read_event_; 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 144e07b..6f8a96d 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 deleted file mode 100644 index ffac558..0000000 --- a/third_party/mojo/src/mojo/edk/system/thread_annotations.h +++ /dev/null @@ -1,85 +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. - -// 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 deleted file mode 100644 index d56814f..0000000 --- a/third_party/mojo/src/mojo/edk/system/thread_annotations_unittest.cc +++ /dev/null @@ -1,124 +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. - -// 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 29bd5bc..68f8106 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 (;;) { { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); if (done_) { *result = result_; *context = context_; @@ -68,7 +68,7 @@ class WaitingThread : public base::SimpleThread { elapsed = stopwatch.Elapsed(); { - MutexLocker locker(&mutex_); + base::AutoLock locker(lock_); done_ = true; result_ = result; context_ = context; @@ -79,11 +79,11 @@ class WaitingThread : public base::SimpleThread { const MojoDeadline deadline_; Waiter waiter_; // Thread-safe. - 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_); + base::Lock lock_; // Protects the following members. + bool done_; + MojoResult result_; + uint32_t context_; + MojoDeadline elapsed_; 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 834d837..fa6f836 100644 --- a/third_party/mojo/src/mojo/edk/test/BUILD.gn +++ b/third_party/mojo/src/mojo/edk/test/BUILD.gn @@ -89,7 +89,6 @@ 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", @@ -119,13 +118,6 @@ 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 cb44eda..0841b63 100644 --- a/third_party/mojo/src/mojo/public/VERSION +++ b/third_party/mojo/src/mojo/public/VERSION @@ -1 +1 @@ -c02a28868825edfa57ab77947b8cb15e741c5598
\ No newline at end of file +a05bfef8096006056b2fff78092faf14d1319782
\ 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 deleted file mode 100644 index 9be0bc0..0000000 --- a/third_party/mojo/src/mojo/public/c/gpu/DEPS +++ /dev/null @@ -1,3 +0,0 @@ -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 e345313..05e92be 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/BUILD.gn +++ b/third_party/mojo/src/mojo/public/cpp/bindings/BUILD.gn @@ -8,6 +8,7 @@ mojo_sdk_source_set("bindings") { sources = [ "array.h", "binding.h", + "error_handler.h", "interface_ptr.h", "interface_ptr_info.h", "interface_request.h", @@ -41,6 +42,8 @@ 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 250945a..4609521 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/binding.h +++ b/third_party/mojo/src/mojo/public/cpp/bindings/binding.h @@ -7,6 +7,7 @@ #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" @@ -101,8 +102,9 @@ 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 @@ -185,6 +187,19 @@ 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 beec1a1..17d1caf 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,11 +47,6 @@ 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 { @@ -78,7 +73,7 @@ class Callback<void(Args...)> { Sink sink; }; - // Adapts a class that has a compatible operator() to be callable by Callback. + // Adapts a class that has a compatible operator() callable by Callback. template <typename Sink> struct FunctorAdapter : public Runnable { explicit FunctorAdapter(const Sink& sink) : sink(sink) {} @@ -90,20 +85,6 @@ 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 new file mode 100644 index 0000000..8ce1af2 --- /dev/null +++ b/third_party/mojo/src/mojo/public/cpp/bindings/error_handler.h @@ -0,0 +1,19 @@ +// 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 new file mode 100644 index 0000000..c65f6e3 --- /dev/null +++ b/third_party/mojo/src/mojo/public/cpp/bindings/interface_impl.h @@ -0,0 +1,178 @@ +// 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 00514dd..04f4902 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,6 +8,7 @@ #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" @@ -73,9 +74,6 @@ 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(); } @@ -136,6 +134,19 @@ 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 597bf8d..24d993d1 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,6 +4,7 @@ #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 f36761c..a6c0d1b 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,6 +7,7 @@ #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 6d629b4..8315dbc 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,25 +13,22 @@ namespace mojo { namespace internal { -namespace { -const ArrayValidateParams* GetMapKeyValidateParamsDefault() { - // The memory allocated here never gets released because calling a - // destructor at exit time makes clang unhappy. +inline const ArrayValidateParams* GetMapKeyValidateParamsDefault() { + // The memory allocated here never gets released to not cause an exit time + // destructor. static const ArrayValidateParams* validate_params = new ArrayValidateParams(0, false, nullptr); return validate_params; } -const ArrayValidateParams* GetMapKeyValidateParamsForStrings() { - // The memory allocated here never gets released because calling a - // destructor at exit time makes clang unhappy. +inline const ArrayValidateParams* GetMapKeyValidateParamsForStrings() { + // The memory allocated here never gets released to not cause an exit time + // destructor. 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/tests/message_queue.cc b/third_party/mojo/src/mojo/public/cpp/bindings/lib/message_queue.cc index 65b9a5c..fd701e97 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/message_queue.cc +++ b/third_party/mojo/src/mojo/public/cpp/bindings/lib/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/tests/message_queue.h" +#include "mojo/public/cpp/bindings/lib/message_queue.h" #include "mojo/public/cpp/bindings/message.h" #include "mojo/public/cpp/environment/logging.h" namespace mojo { -namespace test { +namespace internal { MessageQueue::MessageQueue() { } @@ -22,6 +22,11 @@ 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); @@ -39,5 +44,5 @@ void MessageQueue::Pop() { queue_.pop(); } -} // namespace test +} // namespace internal } // namespace mojo diff --git a/third_party/mojo/src/mojo/public/cpp/bindings/tests/message_queue.h b/third_party/mojo/src/mojo/public/cpp/bindings/lib/message_queue.h index c47ca99..4e46b54 100644 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/message_queue.h +++ b/third_party/mojo/src/mojo/public/cpp/bindings/lib/message_queue.h @@ -12,7 +12,7 @@ namespace mojo { class Message; -namespace test { +namespace internal { // A queue for Message objects. class MessageQueue { @@ -21,23 +21,27 @@ class MessageQueue { ~MessageQueue(); bool IsEmpty() const; + Message* Peek(); - // This method copies the message data and steals ownership of its handles. + // This method transfers ownership of |message->data| and |message->handles| + // to the message queue, resetting |message| in the process. void Push(Message* message); - // Removes the next message from the queue, copying its data and transferring - // ownership of its handles to the given |message|. + // Removes the next message from the queue, transferring ownership of its + // data and handles to the given |message|. void Pop(Message* message); - private: + // Removes the next message from the queue, discarding its data and handles. + // This is meant to be used in conjunction with |Peek|. void Pop(); + private: std::queue<Message*> queue_; MOJO_DISALLOW_COPY_AND_ASSIGN(MessageQueue); }; -} // namespace test +} // namespace internal } // namespace mojo #endif // MOJO_PUBLIC_CPP_BINDINGS_LIB_MESSAGE_QUEUE_H_ 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 860260e..48c74c6 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,6 +10,7 @@ #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" @@ -103,11 +104,21 @@ 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 d087cbf..bb94bc2 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,8 +23,6 @@ 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", @@ -53,28 +51,6 @@ 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 3bc018c..8ec51bf 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,7 +3,6 @@ // 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 59d2501..ef23279 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,13 +2,8 @@ // 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" @@ -17,28 +12,10 @@ 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: - explicit ServiceImpl(bool* was_deleted = nullptr) - : was_deleted_(was_deleted) {} - ~ServiceImpl() override { - if (was_deleted_) - *was_deleted_ = true; - } + ServiceImpl() {} + ~ServiceImpl() override {} private: // sample::Service implementation @@ -49,29 +26,30 @@ class ServiceImpl : public sample::Service { callback.Run(1); } void GetPort(InterfaceRequest<sample::Port> port) override {} - - bool* const was_deleted_; - - MOJO_DISALLOW_COPY_AND_ASSIGN(ServiceImpl); }; -// BindingTest ----------------------------------------------------------------- +class IntegerAccessorImpl : public sample::IntegerAccessor { + public: + IntegerAccessorImpl() {} + ~IntegerAccessorImpl() override {} -using BindingTest = BindingTestBase; + 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 {} +}; -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()); +class BindingTest : public testing::Test { + public: + BindingTest() {} + ~BindingTest() override {} - binding.Close(); - EXPECT_FALSE(called); - loop().RunUntilIdle(); - EXPECT_TRUE(called); -} + protected: + Environment env_; + RunLoop loop_; +}; // Tests that destroying a mojo::Binding closes the bound message pipe handle. TEST_F(BindingTest, DestroyClosesMessagePipe) { @@ -87,87 +65,23 @@ 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(); - 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(); + 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; @@ -178,7 +92,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; @@ -187,7 +101,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; @@ -196,25 +110,10 @@ 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; @@ -222,102 +121,5 @@ 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 deleted file mode 100644 index 7440acc..0000000 --- a/third_party/mojo/src/mojo/public/cpp/bindings/tests/bindings_perftest.cc +++ /dev/null @@ -1,127 +0,0 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "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 158b21e..6b788fc 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,24 +44,6 @@ 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, @@ -94,9 +76,8 @@ static_assert(internal::HasCompatibleCallOperator<decltype(lambda_four), "ExampleMoveOnlyType>"); // Tests constructing and invoking a mojo::Callback from objects with a -// compatible Run() method (called 'runnables'), from lambdas, and from function -// pointers. -TEST(Callback, Create) { +// compatible Run() method (called 'runnables') and from lambdas. +TEST(CallbackFromLambda, Create) { int calls = 0; RunnableNoArgs f(&calls); @@ -140,55 +121,6 @@ TEST(Callback, 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 aab5a39..b5e3405 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/tests/message_queue.h" +#include "mojo/public/cpp/bindings/lib/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: - MessageQueue queue_; + internal::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 681cc6d..6486192 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,13 +192,6 @@ 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/tests/request_response_unittest.cc b/third_party/mojo/src/mojo/public/cpp/bindings/tests/request_response_unittest.cc index 8bfeb5d..f7c3cf6 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,7 +2,6 @@ // 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 7aeb414..def9ceb 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(MessageQueue* queue) : queue_(queue) {} + explicit MessageAccumulator(internal::MessageQueue* queue) : queue_(queue) {} bool Accept(Message* message) override { queue_->Push(message); @@ -44,7 +44,7 @@ class MessageAccumulator : public MessageReceiver { } private: - MessageQueue* queue_; + internal::MessageQueue* queue_; }; class ResponseGenerator : public MessageReceiverWithResponderStatus { @@ -152,7 +152,7 @@ TEST_F(RouterTest, BasicRequestResponse) { Message request; AllocRequestMessage(1, "hello", &request); - MessageQueue message_queue; + internal::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); - MessageQueue message_queue; + internal::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); - MessageQueue message_queue; + internal::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); - MessageQueue message_queue; + internal::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); - MessageQueue message_queue; + internal::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); - MessageQueue message_queue; + internal::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 39befaf..9b4e271 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/binding.h" +#include "mojo/public/cpp/bindings/interface_impl.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 deleted file mode 100644 index 4886c56..0000000 --- a/third_party/mojo/src/mojo/public/dart/.analysis_options +++ /dev/null @@ -1,3 +0,0 @@ -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 d0a207a..3b66ccd 100644 --- a/third_party/mojo/src/mojo/public/dart/.gitignore +++ b/third_party/mojo/src/mojo/public/dart/.gitignore @@ -1,4 +1,2 @@ -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 fb14555..b820d96 100644 --- a/third_party/mojo/src/mojo/public/dart/BUILD.gn +++ b/third_party/mojo/src/mojo/public/dart/BUILD.gn @@ -21,6 +21,10 @@ 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") { @@ -45,9 +49,6 @@ 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 03b981d..dcb340c 100644 --- a/third_party/mojo/src/mojo/public/dart/CHANGELOG.md +++ b/third_party/mojo/src/mojo/public/dart/CHANGELOG.md @@ -1,23 +1,3 @@ -## 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 new file mode 100644 index 0000000..bb8e64dc --- /dev/null +++ b/third_party/mojo/src/mojo/public/dart/lib/io.dart @@ -0,0 +1,49 @@ +// 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 bdcad2e..3944b34 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.18 +version: 0.0.13 diff --git a/third_party/mojo/src/mojo/public/dart/rules.gni b/third_party/mojo/src/mojo/public/dart/rules.gni index d4adbb1..36fc92d 100644 --- a/third_party/mojo/src/mojo/public/dart/rules.gni +++ b/third_party/mojo/src/mojo/public/dart/rules.gni @@ -116,6 +116,8 @@ 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 = [ @@ -224,12 +226,6 @@ 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) @@ -249,7 +245,6 @@ 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)) @@ -283,19 +278,8 @@ 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, ] @@ -331,8 +315,6 @@ 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-files" ] + rebase_path(sdk_ext_files) + - [ "--sdk-ext-mappings" ] + sdk_ext_mappings + [ "--sdk-ext-directories" ] + rebase_path(sdk_ext_directory) } } 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 e7efd3d..9bf206c 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,7 +9,6 @@ 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 deleted file mode 100644 index 3e1f1c4..0000000 --- a/third_party/mojo/src/mojo/public/interfaces/bindings/tests/ping_service.mojom +++ /dev/null @@ -1,10 +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. - -[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 e998f47..6e631b2 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,7 +69,6 @@ struct SmallStructNonNullableUnion { struct SmallObjStruct { ObjectUnion obj_union; - int8 f_int8; }; interface SmallCache { @@ -85,12 +84,3 @@ 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 32bdb7a..7ab5a31 100644 --- a/third_party/mojo/src/mojo/public/mojo_application.gni +++ b/third_party/mojo/src/mojo/public/mojo_application.gni @@ -20,7 +20,6 @@ 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" @@ -131,27 +130,7 @@ 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) { @@ -163,7 +142,6 @@ template("mojo_native_application") { } deps = [ ":${library_target_name}", - ":${copy_symbols_target}", ] sources = [ @@ -311,111 +289,30 @@ 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: - # 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. + # 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. # output_name (optional): override for the output file name - # public_deps / data_deps (optional): Dependencies. template("mojo_android_application") { - 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 - } + assert(defined(invoker.input_so)) + assert(defined(invoker.input_dex_jar)) + 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 = [ ":${final_target_name}" ] + visibility = [ ":$prepend_action_name" ] script = "//build/android/gn/zip.py" inputs = [ - "${root_out_dir}/lib.stripped/${library_basename}", - dex_output_path, + invoker.input_so, + invoker.input_dex_jar, ] output = zip_action_output @@ -426,8 +323,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)) { @@ -442,12 +339,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(final_target_name) { + action(target_name) { script = rebase_path("mojo/public/tools/prepend.py", ".", mojo_root) input = zip_action_output @@ -463,17 +360,13 @@ 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 c359e44..efba73d 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,13 +34,6 @@ 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.""" @@ -160,47 +153,6 @@ 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.""" @@ -482,19 +434,16 @@ 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)) - # TODO(azani): Refactor so we don't have to create big formatting strings. - struct.pack_into(('%s' % self.sub_type.GetTypeCode()) * len(value), + struct.pack_into('%d%s' % (len(value), self.sub_type.GetTypeCode()), 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( - nb_elements * self.sub_type.GetTypeCode(), + '%d%s' % (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 cfd7e64..6c4767b 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,103 +135,6 @@ 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 b1a35ec..32f60f0 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,12 +7,9 @@ import struct -# Format of a header for a struct, array or union. +# Format of a header for a struct or an array. 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 @@ -221,89 +218,3 @@ 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 09436b5..7b21d79 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 @@ -f4116cee892c698374ee7ae9ad8e6cea69741122 +36d008970120c7bce0e7cb0a6543fd0b653dad88
\ No newline at end of file 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 d212e8b..86c85d7d 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}} : int32_t { +enum {{enum.name}} { {%- 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 56a7047..d96fb60 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,10 +8,9 @@ #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 320abeb..39c17cc 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_request) -%} -{{flags(method.response_parameters != None, is_request)}} +{%- macro flags_for_method(method, is_parameter) -%} +{{flags(method.response_parameters, is_parameter)}} {%- endmacro -%} -{%- macro flags(use_response_flag, is_request) -%} -{%- if not use_response_flag -%} +{%- macro flags(has_response_parameters, is_parameter) -%} +{%- if not has_response_parameters -%} org.chromium.mojo.bindings.MessageHeader.NO_FLAG -{%- elif is_request: -%} +{%- elif is_parameter: -%} 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 c324c9c..87c16ef 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,7 +9,6 @@ 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 @@ -422,25 +421,20 @@ class Generator(generator.Generator): def GenerateFiles(self, args): elements = self.module.namespace.split('.') elements.append("%s.dart" % self.module.name) - - 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) - + 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) link = self.MatchMojomFilePath("%s.dart" % self.module.name) - 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)) + if os.path.exists(os.path.join(self.output_dir, link)): + os.unlink(os.path.join(self.output_dir, link)) try: if sys.platform == "win32": - shutil.copy(full_gen_path, full_link_path) + shutil.copy(os.path.join(self.output_dir, path), + os.path.join(self.output_dir, link)) else: - os.symlink(full_gen_path, full_link_path) + os.symlink(os.path.join(self.output_dir, path), + os.path.join(self.output_dir, link)) 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 a12f5e1..1f726b6 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,13 +146,7 @@ def GetFieldType(kind, field=None): arguments.append('nullable=True') return '_descriptor.MapType(%s)' % ', '.join(arguments) - 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): + if mojom.IsStructKind(kind) or mojom.IsUnionKind(kind): arguments = [ 'lambda: %s' % GetFullyQualifiedName(kind) ] if mojom.IsNullableKind(kind): arguments.append('nullable=True') @@ -175,14 +169,15 @@ def GetFieldType(kind, field=None): return _kind_to_type[kind] -def GetFieldDescriptor(field, index, min_version): +def GetFieldDescriptor(packed_field): + field = packed_field.field class_name = 'SingleFieldGroup' if field.kind == mojom.BOOL: class_name = 'FieldDescriptor' arguments = [ '%r' % GetNameForElement(field) ] arguments.append(GetFieldType(field.kind, field)) - arguments.append(str(index)) - arguments.append(str(min_version)) + arguments.append(str(packed_field.index)) + arguments.append(str(packed_field.min_version)) if field.default: if mojom.IsStructKind(field.kind): arguments.append('default_value=True') @@ -190,19 +185,12 @@ def GetFieldDescriptor(field, index, min_version): 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(GetStructFieldDescriptor, byte.packed_fields) + descriptors = map(GetFieldDescriptor, byte.packed_fields) return '_descriptor.BooleanGroup([%s])' % ', '.join(descriptors) assert len(byte.packed_fields) == 1 - return GetStructFieldDescriptor(byte.packed_fields[0]) + return GetFieldDescriptor(byte.packed_fields[0]) def MojomToPythonImport(mojom): return mojom.replace('.mojom', '_mojom') @@ -212,7 +200,6 @@ class Generator(generator.Generator): python_filters = { 'expression_to_text': ExpressionToText, 'field_group': GetFieldGroup, - 'union_field_descriptor': GetUnionFieldDescriptor, 'fully_qualified_name': GetFullyQualifiedName, 'name': GetNameForElement, } @@ -226,7 +213,6 @@ 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 9d57600..2a22932 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,6 +1,5 @@ {% 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. @@ -35,13 +34,6 @@ 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 c979f59..21c70db 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,13 +37,3 @@ {% 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 86ae5d5..d0f0d6eb 100755 --- a/third_party/mojo/src/mojo/public/tools/dart_pkg.py +++ b/third_party/mojo/src/mojo/public/tools/dart_pkg.py @@ -8,7 +8,6 @@ import argparse import errno -import json import os import shutil import sys @@ -48,9 +47,8 @@ def mojom_filter(path): def ensure_dir_exists(path): - abspath = os.path.abspath(path) - if not os.path.exists(abspath): - os.makedirs(abspath) + if not os.path.exists(path): + os.makedirs(path) def has_pubspec_yaml(paths): @@ -107,13 +105,6 @@ 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): @@ -196,39 +187,13 @@ 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) - 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. + target_dir = os.path.join(args.pkg_directory, args.package_name) common_source_prefix = os.path.commonprefix(args.package_sources) for source in args.package_sources: relative_source = os.path.relpath(source, common_source_prefix) @@ -244,12 +209,8 @@ 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. |