HTML MessagePort as mojo::MessagePipeHandle

Desktop / Chromium - darin [chromium.org] - 16 February 2017 10:22 EST

This is mostly a reduction in code and complexity. The content::MessagePort class is added as an abstraction around a mojo::MessagePipeHandle. This provides some type safety but also encapsulates some of the details of reading and writing to a HTML MessagePort.

The MessagePort class also eases the transition from Chrome IPC to Mojo by being a ref-counted wrapper for the mojo::MessagePipeHandle. This makes it possible to pass a MessagePort around by value or to store it as a member of a Chrome IPC structure that is passed around by value. Eventually, this class will be made move-only.

A good bit of complexity in this CL has to do with bridging HTML message ports to Java as exposed through the Android WebView and CCT APIs. This necessitated having MessagePort be more than just a ref-counted wrapper for mojo::MessagePipeHandle. The methods for reading and writing to a message port need to live in content and be usable from the browser process. Otherwise, this code could just be part of Blink. The AppWebMessagePort Java class becomes a Java wrapper for the content::MessagePort. The content::AppWebMessagePort C++ class is machinery to accomplish this. The result is much less complexity in the Java code.

Further, to support using MessagePort directly from Java, it was necessary to cope with the serialization format of a message sent from Blink. This format is only known to Blink and V8, and the corresponding code is only designed to be used from a renderer process. Rather than IPC over to the renderer process to execute this code, this CL duplicates some of the serialization logic (just for simple string messages) in app_web_message_port.cc. This is a trade-off between overall complexity and code duplication. Fortunately, this serialization format is static as it is a format we currently persist to disk via IndexedDB.

There is also some complexity related to shared workers. Previously, the shared worker infrastructure relied on the message port ID being a static identifier that it could use for other purposes. But now with this CL, the message port ID is gone and we just have transferable MessagePipeHandles. As a result, during setup of a shared worker another ID is needed to represent a connection that is being established. This is what the connection_request_id is for.

Finally, Blob registration (RegisterBlob) and coining a Blob URL (RegisterPublicURL) are made into synchronous IPCs. This is to avoid a race condition between Blob(URL) registration and passing a Blob(URL) over a Mojo channel, which is now a different FIFO. The previous code relied on the Chrome IPC channel between the renderer and browser being a single FIFO over which Blob(URL)s are registered and MessagePort IPCs are transmitted. Using a Mojo message pipe has the consequence of creating a new FIFO, which is good for performance (the FIFO is also direct between end-points rather than routing through the browser IO thread), but it comes at the cost of introducing potential race conditions. A number of alternative designs were considered that come with much greater complexity (see https://goo.gl/bfdE64). Instead, this CL adds UMA metrics to measure the cost of these synchronous IPCs. The cost should be acceptably low as these IPCs terminate on the browser IO thread.

BUG=361001 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2422793002 Cr-Commit-Position: refs/heads/master@{#450970}

2d145fe HTML MessagePort as mojo::MessagePipeHandle
.../chromium/android_webview/AwBrowserContext.java | 9 -
.../org/chromium/android_webview/AwContents.java | 56 +---
.../android_webview/test/PostMessageTest.java | 85 -----
android_webview/native/aw_contents.cc | 27 +-
android_webview/native/aw_contents.h | 4 -
.../browser/customtabs/PostMessageHandler.java | 55 +---
chrome/browser/printing/print_dialog_cloud_win.cc | 3 +-
content/browser/BUILD.gn | 12 +-
content/browser/android/DEPS | 3 +
content/browser/android/app_web_message_port.cc | 167 ++++++++++
content/browser/android/app_web_message_port.h | 59 ++++
.../android/app_web_message_port_message_filter.cc | 97 ------
.../android/app_web_message_port_message_filter.h | 57 ----
.../android/app_web_message_port_service_impl.cc | 251 ---------------
.../android/app_web_message_port_service_impl.h | 93 ------
content/browser/android/browser_jni_registrar.cc | 4 +-
content/browser/android/string_message_codec.cc | 153 +++++++++
content/browser/android/string_message_codec.h | 30 ++
.../android/string_message_codec_unittest.cc | 146 +++++++++
.../browser/frame_host/render_frame_host_impl.cc | 13 -
.../browser/frame_host/render_frame_host_impl.h | 12 -
.../browser/frame_host/render_frame_proxy_host.cc | 23 +-
content/browser/message_port_message_filter.cc | 113 -------
content/browser/message_port_message_filter.h | 79 -----
content/browser/message_port_provider.cc | 67 ++--
.../browser/message_port_provider_browsertest.cc | 46 +--
content/browser/message_port_service.cc | 347 ---------------------
content/browser/message_port_service.h | 106 -------
.../renderer_host/render_process_host_impl.cc | 17 +-
.../renderer_host/render_process_host_impl.h | 8 -
.../service_worker/embedded_worker_instance.cc | 5 -
.../service_worker/embedded_worker_instance.h | 2 -
.../service_worker/embedded_worker_registry.cc | 14 +-
.../service_worker/embedded_worker_registry.h | 11 +-
.../service_worker/embedded_worker_test_helper.cc | 37 +--
.../service_worker/embedded_worker_test_helper.h | 6 -
.../service_worker_dispatcher_host.cc | 30 +-
.../service_worker_dispatcher_host.h | 27 +-
.../service_worker_dispatcher_host_unittest.cc | 30 +-
.../service_worker_handle_unittest.cc | 2 +-
.../service_worker/service_worker_provider_host.cc | 9 +-
.../service_worker/service_worker_provider_host.h | 3 +-
.../service_worker/service_worker_version.cc | 4 +-
.../service_worker/service_worker_version.h | 8 +-
.../browser/shared_worker/shared_worker_host.cc | 57 ++--
content/browser/shared_worker/shared_worker_host.h | 27 +-
.../shared_worker/shared_worker_message_filter.cc | 8 +-
.../shared_worker/shared_worker_message_filter.h | 11 +-
.../shared_worker/shared_worker_service_impl.cc | 11 +-
.../shared_worker/shared_worker_service_impl.h | 7 +-
.../shared_worker_service_impl_unittest.cc | 301 +++++-------------
.../browser/web_contents/web_contents_android.cc | 20 +-
.../browser/web_contents/web_contents_android.h | 7 +-
content/browser/web_contents/web_contents_impl.cc | 1 -
.../blob_storage/blob_transport_controller.cc | 4 +
.../blob_transport_controller_unittest.cc | 3 +-
content/child/blob_storage/webblobregistry_impl.cc | 4 +
.../service_worker/service_worker_dispatcher.cc | 6 +-
.../service_worker_dispatcher_unittest.cc | 2 +-
.../service_worker/web_service_worker_impl.cc | 40 +--
.../child/service_worker/web_service_worker_impl.h | 2 +-
content/child/webmessageportchannel_impl.cc | 326 ++++---------------
content/child/webmessageportchannel_impl.h | 115 ++-----
content/common/BUILD.gn | 4 +-
content/common/app_web_message_port_messages.h | 86 -----
content/common/content_message_generator.h | 2 -
content/common/content_param_traits.cc | 27 ++
content/common/content_param_traits.h | 14 +
content/common/fileapi/webblob_messages.h | 27 +-
content/common/frame_messages.h | 4 +-
content/common/message_port.cc | 186 +++++++++++
content/common/message_port.h | 102 ++++++
content/common/message_port_messages.h | 92 ------
.../service_worker_event_dispatcher.mojom | 3 +-
.../service_worker/service_worker_messages.h | 8 +-
content/common/view_messages.h | 5 +-
content/common/worker_messages.h | 7 +-
content/public/android/BUILD.gn | 5 +-
.../content/browser/AppWebMessagePort.java | 187 +++++------
.../content/browser/AppWebMessagePortService.java | 167 ----------
.../content/browser/PostMessageSender.java | 163 ----------
.../browser/webcontents/WebContentsImpl.java | 29 +-
.../content_public/browser/MessagePortService.java | 17 -
.../content_public/browser/WebContents.java | 4 +-
content/public/browser/BUILD.gn | 2 -
.../browser/android/app_web_message_port_service.h | 32 --
content/public/browser/message_port_delegate.h | 42 ---
content/public/browser/message_port_provider.h | 20 +-
content/renderer/BUILD.gn | 2 -
.../android/app_web_message_port_client.cc | 114 -------
.../renderer/android/app_web_message_port_client.h | 41 ---
content/renderer/render_frame_impl.cc | 13 +-
content/renderer/render_frame_proxy.cc | 2 +-
content/renderer/renderer_blink_platform_impl.cc | 3 +-
.../service_worker_context_client.cc | 37 +--
.../service_worker/service_worker_context_client.h | 7 +-
.../shared_worker/embedded_shared_worker_stub.cc | 33 +-
.../shared_worker/embedded_shared_worker_stub.h | 12 +-
.../shared_worker/websharedworker_proxy.cc | 22 +-
.../renderer/shared_worker/websharedworker_proxy.h | 3 +-
content/test/BUILD.gn | 1 +
.../serialization/SerializedScriptValueFuzzer.cpp | 3 +-
.../serialization/V8ScriptValueSerializerTest.cpp | 5 +-
third_party/WebKit/Source/core/dom/MessagePort.cpp | 71 ++---
third_party/WebKit/Source/core/dom/MessagePort.h | 26 +-
.../WebKit/Source/core/events/MessageEvent.cpp | 2 +-
.../WebKit/Source/core/events/MessageEvent.h | 13 +-
third_party/WebKit/Source/core/frame/DOMWindow.cpp | 5 +-
.../core/workers/DedicatedWorkerGlobalScope.cpp | 2 +-
.../Source/core/workers/DedicatedWorkerTest.cpp | 2 +-
.../Source/core/workers/InProcessWorkerBase.cpp | 2 +-
.../core/workers/InProcessWorkerMessagingProxy.cpp | 6 +-
.../core/workers/InProcessWorkerMessagingProxy.h | 8 +-
.../core/workers/InProcessWorkerObjectProxy.cpp | 4 +-
.../core/workers/InProcessWorkerObjectProxy.h | 9 +-
.../CompositorWorkerGlobalScope.cpp | 2 +-
.../modules/serviceworkers/ServiceWorker.cpp | 6 +-
.../modules/serviceworkers/ServiceWorkerClient.cpp | 4 +-
.../serviceworkers/ServiceWorkerContainer.cpp | 6 +-
.../serviceworkers/ServiceWorkerContainer.h | 2 +-
.../ServiceWorkerGlobalScopeClient.h | 7 +-
.../WebKit/Source/platform/CrossThreadCopier.h | 10 +
.../web/ServiceWorkerGlobalScopeClientImpl.cpp | 4 +-
.../web/ServiceWorkerGlobalScopeClientImpl.h | 7 +-
.../Source/web/ServiceWorkerGlobalScopeProxy.cpp | 12 +-
.../Source/web/ServiceWorkerGlobalScopeProxy.h | 4 +-
.../WebKit/Source/web/WebDOMMessageEvent.cpp | 14 +-
.../Source/web/WebEmbeddedWorkerImplTest.cpp | 2 +-
.../WebKit/public/platform/WebMessagePortChannel.h | 32 +-
.../modules/serviceworker/WebServiceWorker.h | 3 +-
.../serviceworker/WebServiceWorkerProviderClient.h | 8 +-
third_party/WebKit/public/web/WebDOMMessageEvent.h | 3 +-
.../serviceworker/WebServiceWorkerContextClient.h | 2 +-
.../serviceworker/WebServiceWorkerContextProxy.h | 4 +-
tools/metrics/histograms/histograms.xml | 16 +
135 files changed, 1578 insertions(+), 3535 deletions(-)

Upstream: git.chromium.org


  • Share