New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 826213 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
please use my google.com address
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 795291



Sign in to add a comment

Move Mojo shared buffers to the new shared memory API

Project Member Reported by alexilin@chromium.org, Mar 27 2018

Issue description

Mojo should eventually use the new more secure API for shared memory regions for the implementation of Mojo shared buffers.
 

Comment 1 by roc...@chromium.org, Mar 27 2018

Labels: -Pri-3 Pri-1
Status: Started (was: Assigned)
This is a high priority for Mojo.
Blocking: 795291
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/062470b4cb18c829095024eade51f177734c666f

commit 062470b4cb18c829095024eade51f177734c666f
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Mar 30 03:51:57 2018

Add mojo base types for new shared memory APIs

Adds mojom equivalents of {ReadOnly,Writable,Unsafe}SharedMemoryRegion.
These are typemapped to the new base types.

The typemap implementation is currently written by wrapping and
unwrapping generic platform handles rather than relying on Mojo's
shared buffer implementation.

Existing mojom shared buffer handle consumers will be converted to use
these types in place of raw handle<shared_buffer>. The (mojom) types
here will eventually wrap handle<shared_buffer> instead of raw platform
handles.

This is to ease the transition to new shared memory semantics while
also getting us something we want, i.e., stronger shared memory type
safety for common mojom uses.

Bug:  826213 
Change-Id: Iba36758b70953492997e744eeecaf7b7403627fc
Reviewed-on: https://chromium-review.googlesource.com/982577
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547104}
[modify] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/base/memory/read_only_shared_memory_region.cc
[modify] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/base/memory/read_only_shared_memory_region.h
[modify] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/base/memory/unsafe_shared_memory_region.cc
[modify] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/base/memory/unsafe_shared_memory_region.h
[modify] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/base/memory/writable_shared_memory_region.cc
[modify] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/base/memory/writable_shared_memory_region.h
[modify] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/mojo/public/cpp/base/BUILD.gn
[add] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/mojo/public/cpp/base/shared_memory.typemap
[add] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/mojo/public/cpp/base/shared_memory_mojom_traits.cc
[add] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/mojo/public/cpp/base/shared_memory_mojom_traits.h
[add] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/mojo/public/cpp/base/shared_memory_unittest.cc
[modify] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/mojo/public/cpp/base/typemaps.gni
[modify] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/mojo/public/mojom/base/BUILD.gn
[add] https://crrev.com/062470b4cb18c829095024eade51f177734c666f/mojo/public/mojom/base/shared_memory.mojom

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5

commit aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Mar 30 16:40:09 2018

Convert visitedlink component to new shared memory API

This includes moving mojom shared_buffer usage to the more
strongly-typed base memory region types.

Bug:  826213 
Change-Id: Ifb304267dc0c5a18a7a9df204b2b70020337b4e9
Reviewed-on: https://chromium-review.googlesource.com/985420
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547186}
[modify] https://crrev.com/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5/components/visitedlink/browser/visitedlink_event_listener.cc
[modify] https://crrev.com/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5/components/visitedlink/browser/visitedlink_event_listener.h
[modify] https://crrev.com/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5/components/visitedlink/browser/visitedlink_master.cc
[modify] https://crrev.com/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5/components/visitedlink/browser/visitedlink_master.h
[modify] https://crrev.com/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5/components/visitedlink/common/BUILD.gn
[modify] https://crrev.com/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5/components/visitedlink/common/visitedlink.mojom
[modify] https://crrev.com/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5/components/visitedlink/renderer/visitedlink_slave.cc
[modify] https://crrev.com/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5/components/visitedlink/renderer/visitedlink_slave.h
[modify] https://crrev.com/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5/components/visitedlink/test/visitedlink_perftest.cc
[modify] https://crrev.com/aeb7eca0a312cb0d0eefd3a8eee0931a9fad80b5/components/visitedlink/test/visitedlink_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d120f859cc097def09457bac31976ac69320b376

commit d120f859cc097def09457bac31976ac69320b376
Author: Ken Rockot <rockot@chromium.org>
Date: Mon Apr 02 22:28:42 2018

Make new shared memory duplication and mapping APIs const

Duplicate, Map, and MapAt on these various types do not and should not
need to modify the calling object.

Bug:  826213 
Change-Id: Ice07621c23f36da00e20a13076a783cf1e41d7f2
Reviewed-on: https://chromium-review.googlesource.com/990673
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547539}
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/platform_shared_memory_region.h
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/platform_shared_memory_region_android.cc
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/platform_shared_memory_region_fuchsia.cc
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/platform_shared_memory_region_mac.cc
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/platform_shared_memory_region_posix.cc
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/platform_shared_memory_region_win.cc
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/read_only_shared_memory_region.cc
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/read_only_shared_memory_region.h
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/unsafe_shared_memory_region.cc
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/unsafe_shared_memory_region.h
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/writable_shared_memory_region.cc
[modify] https://crrev.com/d120f859cc097def09457bac31976ac69320b376/base/memory/writable_shared_memory_region.h

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39bd21e42f727b991022337f9168cb13e2207d5e

commit 39bd21e42f727b991022337f9168cb13e2207d5e
Author: Ken Rockot <rockot@chromium.org>
Date: Tue Apr 03 16:22:26 2018

Mojo: More shared buffer API prep work

Cherry-picks a couple of things:

  - Adds support for wrapping and unwrapping new base shm regions
  - Ports one use (PDF compositor test code) to use the above
  - Updates base::SharedMemory wrapping to force read-only regions
    when wrapping a handle as read-only

Bug:  826213 
Change-Id: I11d923b23688454ffdbf26ba5efa93ba1605b855
Reviewed-on: https://chromium-review.googlesource.com/992360
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547728}
[modify] https://crrev.com/39bd21e42f727b991022337f9168cb13e2207d5e/components/printing/service/pdf_compositor_service_unittest.cc
[modify] https://crrev.com/39bd21e42f727b991022337f9168cb13e2207d5e/mojo/public/cpp/system/platform_handle.cc
[modify] https://crrev.com/39bd21e42f727b991022337f9168cb13e2207d5e/mojo/public/cpp/system/platform_handle.h

Comment 7 by klausw@chromium.org, Apr 16 2018

Cc: klausw@chromium.org
If I'm understanding it right, I think the AHardwareBuffer GpuMemoryBufferHandle will also need updating. It's currently implemented as a SharedMemoryHandle subtype since it's also transported via file descriptor, but it needs custom transport/mapping methods.

(I had initially implemented this as a custom subtype, but we had decided during review to move to the SharedMemoryHandle API for consistency and lifecycle handling.)

See for example:
https://cs.chromium.org/chromium/src/base/memory/shared_memory_handle_android.cc?sq=package:chromium&dr&l=50
https://cs.chromium.org/chromium/src/ui/gfx/gpu_memory_buffer.h?type=cs&q=gpumemorybuffer&sq=package:chromium&l=37

Please let me know what the best way is to proceed here. Unfortunately this doesn't have good end-to-end test coverage since AHardwareBuffer only works on Android O, and at the time of implementation that wasn't available on any of the bots.

Comment 8 by roc...@chromium.org, Apr 16 2018

Ah. Probably. I didn't realize this wasn't used outside of Android O and
therefore has no test coverage. That seems like a really bad situation.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5eb3b9dd62243224c39b8928856839978b75ded6

commit 5eb3b9dd62243224c39b8928856839978b75ded6
Author: Ken Rockot <rockot@chromium.org>
Date: Tue Apr 17 03:11:38 2018

Don't use SharedMemory[Handle] to hold a DXGI resource handle

Impending changes to Mojo IPC will require that handles serialized
as shared memory handles are actually shared memory handles. DXGI
resources are not, but are being transported using
GpuMemoryBufferHandle's shared_memory field.

This introduces a Windows-only GpuMemoryBufferHandle field for a
generic HANDLE to be used for DXGI resources, as well as a corresponding
mojom field for the same purpose.

There should be no net effect on the behavior of the system wrt handle
lifetime or usage.


Bug:  826213 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I5d510e0ebdb60d4872fc428a95ce681dd18369dc
Reviewed-on: https://chromium-review.googlesource.com/1011600
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Bill Orr <billorr@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551231}
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/gpu/ipc/common/gpu_memory_buffer_impl_dxgi.cc
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/gpu/ipc/common/gpu_memory_buffer_impl_dxgi.h
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/third_party/blink/renderer/platform/graphics/gpu/xr_frame_transport.cc
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/ui/gfx/BUILD.gn
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/ui/gfx/gpu_memory_buffer.cc
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/ui/gfx/gpu_memory_buffer.h
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/ui/gfx/ipc/gfx_param_traits_macros.h
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/ui/gfx/mojo/buffer_types.mojom
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/ui/gfx/mojo/buffer_types_struct_traits.cc
[modify] https://crrev.com/5eb3b9dd62243224c39b8928856839978b75ded6/ui/gfx/mojo/buffer_types_struct_traits.h

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/097248f038f5c5545e17974359ce45095875bc0b

commit 097248f038f5c5545e17974359ce45095875bc0b
Author: Ken Rockot <rockot@chromium.org>
Date: Mon Apr 23 16:23:34 2018

Base [android]: Introduce ScopedHardwareBufferHandle

This is a scoping wrapper around an owned AHardwareBuffer reference.

Removes code in SharedMemory/SharedMemoryHandle to serve the same
purpose, such that shared memory objects on Android are now always
actually ashmem objects.

Updates GpuMemoryBufferHandle and
GpuMemoryBufferImplAndroidHardwareBuffer (the only existing use of the
special not-really-shared-memory type) to use a raw AHarwdwareBuffer*
(unowned) or ScopedHardwareBufferHandle (owned) instead.

Also fixes a bug in MailboxToSurfaceBridge which was stealing an
a AHardwareBuffer ref from its caller.

Bug:  826213 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I9e8403ad9e120cc9c68012dd537a0bc4af9a513c
Reviewed-on: https://chromium-review.googlesource.com/1014541
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552723}
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/base/BUILD.gn
[add] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/base/android/scoped_hardware_buffer_handle.cc
[add] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/base/android/scoped_hardware_buffer_handle.h
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/base/memory/shared_memory_handle.h
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/base/memory/shared_memory_handle_android.cc
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/chrome/browser/android/vr/mailbox_to_surface_bridge.cc
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/chrome/browser/android/vr/mailbox_to_surface_bridge.h
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/chrome/browser/android/vr/vr_shell_gl.cc
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/gpu/ipc/common/gpu_memory_buffer_impl_android_hardware_buffer.cc
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/gpu/ipc/common/gpu_memory_buffer_impl_android_hardware_buffer.h
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.cc
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/ipc/ipc_message_utils.cc
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/ipc/ipc_message_utils.h
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/mojo/public/cpp/system/BUILD.gn
[add] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/mojo/public/cpp/system/scope_to_message_pipe.cc
[add] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/mojo/public/cpp/system/scope_to_message_pipe.h
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/mojo/public/cpp/system/tests/BUILD.gn
[add] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/mojo/public/cpp/system/tests/scope_to_message_pipe_unittest.cc
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/ui/gfx/gpu_memory_buffer.cc
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/ui/gfx/gpu_memory_buffer.h
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/ui/gfx/ipc/gfx_param_traits_macros.h
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/ui/gfx/mojo/buffer_types.mojom
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/ui/gfx/mojo/buffer_types_struct_traits.cc
[modify] https://crrev.com/097248f038f5c5545e17974359ce45095875bc0b/ui/gfx/mojo/buffer_types_struct_traits.h

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/062ff1a58f51a17787de42e29d176f13658ef63a

commit 062ff1a58f51a17787de42e29d176f13658ef63a
Author: Ken Rockot <rockot@chromium.org>
Date: Mon Apr 23 20:42:38 2018

Move Mojo internals to new base shared memory API

Replaces the internal ref-counted PlatformSharedBuffer type with
base::subtle::PlatformSharedMemoryRegion throughout the EDK.

Takes egregious liberties abusing the new base shared memory API's
serialization and deserialization support interfaces to support Mojo API
behavior within the boundaries of the base API's constraints.

This *should* just work, though there is some risk of one or more Mojo
shared buffer consumers engaging in the following no-longer-supported
sequence of operations:

  1. Allocating a shared buffer
  2. Duplicating it non-read-only at least once
  3. Duplicating it read-only

Step 3 will always fail now.


This is a precursor to adding a creation flag to the Mojo API to support
distinction between "safe" writable-by-one-owner buffers and "unsafe"
writable-by-anyone buffers and deprecating the READ_ONLY duplication
flag in favor of this more explicit choice.

Bug:  826213 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I0ef125fdb7f512c045463dd54c7a43f3a19c60a3
Reviewed-on: https://chromium-review.googlesource.com/987318
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552818}
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/BUILD.gn
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/embedder/BUILD.gn
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/embedder/embedder.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/embedder/embedder.h
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/embedder/embedder_unittest.cc
[add] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/embedder/platform_handle_utils.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/embedder/platform_handle_utils.h
[delete] https://crrev.com/130805d201be7b494970de1677aebf37425d5c17/mojo/edk/embedder/platform_shared_buffer.cc
[delete] https://crrev.com/130805d201be7b494970de1677aebf37425d5c17/mojo/edk/embedder/platform_shared_buffer.h
[delete] https://crrev.com/130805d201be7b494970de1677aebf37425d5c17/mojo/edk/embedder/platform_shared_buffer_unittest.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/broker.h
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/broker_posix.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/broker_win.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/core.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/core.h
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/data_pipe_consumer_dispatcher.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/data_pipe_consumer_dispatcher.h
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/data_pipe_producer_dispatcher.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/data_pipe_producer_dispatcher.h
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/dispatcher.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/dispatcher.h
[delete] https://crrev.com/130805d201be7b494970de1677aebf37425d5c17/mojo/edk/system/mapping_table.cc
[delete] https://crrev.com/130805d201be7b494970de1677aebf37425d5c17/mojo/edk/system/mapping_table.h
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/multiprocess_message_pipe_unittest.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/node_controller.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/node_controller.h
[add] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/platform_shared_memory_mapping.cc
[add] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/platform_shared_memory_mapping.h
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/platform_wrapper_unittest.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/shared_buffer_dispatcher.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/shared_buffer_dispatcher.h
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/shared_buffer_dispatcher_unittest.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/edk/system/shared_buffer_unittest.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/public/c/system/tests/core_api_unittest.cc
[modify] https://crrev.com/062ff1a58f51a17787de42e29d176f13658ef63a/mojo/public/cpp/system/tests/core_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4569648522afab19100ef6ae8c28a87a804d6852

commit 4569648522afab19100ef6ae8c28a87a804d6852
Author: Lei Zhang <thestig@chromium.org>
Date: Tue Apr 24 20:35:57 2018

Implement base::RefCountedSharedMemoryMapping.

This base::RefCountedMemory subclass owns a
base::ReadOnlySharedMemoryMapping instance.

To demonstrate the usefulness of this new class, convert PdfCompositor
from mojo::ScopedSharedBufferHandle to base::ReadOnlySharedMemoryRegion.
Use base::RefCountedSharedMemoryMapping to wrap memory regions and avoid
data copies.

While changing PdfCompositor, turn some DCHECKs into actual checks,
since there is no guarantee shared memory options will always succeed.

BUG= 826213 

Change-Id: I3135235ea350f58c83606e530b33ed6aa7be3cb5
Reviewed-on: https://chromium-review.googlesource.com/989429
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Wei Li <weili@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553255}
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/base/memory/ref_counted_memory.cc
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/base/memory/ref_counted_memory.h
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/base/memory/ref_counted_memory_unittest.cc
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/chrome/browser/printing/print_preview_message_handler.cc
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/chrome/browser/printing/print_preview_message_handler.h
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/chrome/browser/printing/print_view_manager_base.cc
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/chrome/browser/printing/print_view_manager_base.h
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/components/printing/browser/print_composite_client.cc
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/components/printing/browser/print_composite_client.h
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/components/printing/service/pdf_compositor_impl.cc
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/components/printing/service/pdf_compositor_impl.h
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/components/printing/service/pdf_compositor_impl_unittest.cc
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/components/printing/service/pdf_compositor_service_unittest.cc
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/components/printing/service/public/cpp/pdf_service_mojo_utils.cc
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/components/printing/service/public/cpp/pdf_service_mojo_utils.h
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/components/printing/service/public/interfaces/BUILD.gn
[modify] https://crrev.com/4569648522afab19100ef6ae8c28a87a804d6852/components/printing/service/public/interfaces/pdf_compositor.mojom

Project Member

Comment 13 by bugdroid1@chromium.org, Apr 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae5e45c0b984a5e6ae27c4acd82a26145970629f

commit ae5e45c0b984a5e6ae27c4acd82a26145970629f
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Wed Apr 25 06:52:41 2018

Convert discardable shared memory to the new shared memory API

Bug:  826213 
Change-Id: Ifa1d7e3fd3625ef0161073d594f3f48aada7b7c8
Reviewed-on: https://chromium-review.googlesource.com/1019448
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553472}
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/base/memory/discardable_shared_memory.cc
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/base/memory/discardable_shared_memory.h
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/base/memory/discardable_shared_memory_unittest.cc
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/base/memory/shared_memory_tracker.cc
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/base/memory/shared_memory_tracker.h
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/base/memory/unsafe_shared_memory_region.h
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/components/discardable_memory/client/client_discardable_shared_memory_manager.cc
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/components/discardable_memory/client/client_discardable_shared_memory_manager.h
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/components/discardable_memory/public/interfaces/BUILD.gn
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/components/discardable_memory/public/interfaces/discardable_shared_memory_manager.mojom
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/components/discardable_memory/service/discardable_shared_memory_manager.cc
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/components/discardable_memory/service/discardable_shared_memory_manager.h
[modify] https://crrev.com/ae5e45c0b984a5e6ae27c4acd82a26145970629f/components/discardable_memory/service/discardable_shared_memory_manager_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, May 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/88be045ef0409b129a50955fb89d69c616e31ca2

commit 88be045ef0409b129a50955fb89d69c616e31ca2
Author: Ken Rockot <rockot@chromium.org>
Date: Wed May 16 17:31:04 2018

Update Mojo platform handle wrapping APIs

Updates platform handle wrapping/unwrapping API signatures to be more
extensible for a stable ABI, and adds support to shared memory region
wrapping/unwrapping for writable regions, which may be represented by
multiple platform handles on some platforms.

TBR=bajones@chromium.org

Bug:  826213 ,842037
Change-Id: Ia0e5da7b7e0f4c41853eb3a8d4a12da47071dd8c
Reviewed-on: https://chromium-review.googlesource.com/1058440
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559165}
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/device/vr/oculus/oculus_render_loop.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/device/vr/openvr/openvr_render_loop.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/ipc/ipc_message_attachment.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/embedder/entrypoints.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/system/core.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/system/core.h
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/system/message_pipe_unittest.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/system/platform_wrapper_unittest.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/system/shared_buffer_dispatcher.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/c/system/README.md
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/c/system/platform_handle.h
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/c/system/thunks.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/c/system/thunks.h
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/cpp/base/shared_memory_mojom_traits.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/cpp/system/platform_handle.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/cpp/system/platform_handle.h

Project Member

Comment 15 by bugdroid1@chromium.org, May 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca7ae9dd61c89912b01ae313d8486e3ebc96a0fb

commit ca7ae9dd61c89912b01ae313d8486e3ebc96a0fb
Author: Ken Rockot <rockot@chromium.org>
Date: Wed May 16 18:51:56 2018

Simplify mojo_base shared memory region support

Changes the SharedMemoryRegion types in mojo_base mojom to use a
handle<shared_buffer> as their internal representation. This simplifies
the traits and makes these region types usable in Java and JS.

Bug:  826213 
Change-Id: Id515f61f34b85c655b70d45d53302e873547a83a
Reviewed-on: https://chromium-review.googlesource.com/1059008
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559207}
[modify] https://crrev.com/ca7ae9dd61c89912b01ae313d8486e3ebc96a0fb/mojo/public/cpp/base/shared_memory_mojom_traits.cc
[modify] https://crrev.com/ca7ae9dd61c89912b01ae313d8486e3ebc96a0fb/mojo/public/cpp/base/shared_memory_mojom_traits.h
[modify] https://crrev.com/ca7ae9dd61c89912b01ae313d8486e3ebc96a0fb/mojo/public/cpp/base/shared_memory_unittest.cc
[modify] https://crrev.com/ca7ae9dd61c89912b01ae313d8486e3ebc96a0fb/mojo/public/mojom/base/shared_memory.mojom

Status: Fixed (was: Started)
Calling this done. Mojo implementation is now entirely moved away from SharedMemoryHandle, and there are ample facilities in Mojo public APIs for wrapping and unwrapping the new region types.

There are still quite a few uses of mojo::WrapSharedMemoryHandle and mojo::UnwrapSharedMemoryHandle, but I view these as separate concerns in the overall effort to port code away from SharedMemoryHandle. i.e. it's trivial to convert an UnwrapSharedMemoryHandle invocation to an (e.g.) UnwrapReadOnlySharedMemoryRegion invocation, provided the surrounded code is also ported to use ReadOnlySharedMemoryRegion instead of SharedMemoryHandle.
Thanks tons for your help with this, Ken!

Sign in to add a comment