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

Issue 753541 link

Starred by 1 user
Status: Fixed
Owner:
█ roc...@chromium.org
please use my google.com address
Closed: Jun 2018
Cc:
w...@chromium.org
█ roc...@chromium.org
erikc...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Migrate mojo::edk::ScopedPlatformHandle & PlatformHandle to move-only container

Project Member Reported by w...@chromium.org, Aug 8 2017 
Following handle lifetimes in the Mojo EDK is complicated by the myriad of handle classes (MojoHandle, MojoPlatformHandle, PlatformHandle, ScopedPlatformhandle), which pre-date C++ move-only semantics.

It would be awesome to update the EDK to use move-only handle containers, e.g. base::ScopedGeneric, if possible, to provide stronger handle-lifetime guarantees.

Comment 1 by roc...@chromium.org, Aug 8 2017 

To be clear, there is exactly *one* handle type here which has move
semantics and can be wrapped by ScopedGeneric, and that is PlatformHandle.
ScopedPlatformHandle is just a non-base::ScopedGeneric wrapper around that.

MojoHandle and MojoPlatformHandle are completely unrelated to
mojo::edk::PlatformHandle and are (and must remain) C-compatible types
which cannot be scoped. The public C++ system API provides convenient, more
strongly-typed scopers around MojoHandle. We could change those scopers to
use base::ScopedGeneric I guess, but I don't really know what the value
would be.

Comment 2 by roc...@chromium.org, Aug 8 2017 

Err... which lacks move semantics but which could be changed to implement
them.

Comment 3 by w...@chromium.org, Nov 2 2017

Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 21 2017 

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

commit 7df6feaddfff789abbef65a84c1a4296c5927a0a
Author: Wez <wez@chromium.org>
Date: Tue Nov 21 19:57:38 2017

Migrate call-sites away from un-managed PlatformHandle[Vector]s.

As the first step to merging ScopedPlatformHandle into PlatformHandle
to have a single, always-managed container for platform handles, we:

- Replace ScopedPlatformHandleVectorPtr and PlatformHandleVector
  with vector<ScopedPlatformHandle>.
- Replace PlatformHandle with ScopedPlatformHandle in some core APIs.
- Use const& to pass handles to APIs without passing ownership, for
  both individual platform handles, and vectors.

Bug:  753541 
Change-Id: Id965dcf3770ef11bf36f78277fa7199ea779599e
Reviewed-on: https://chromium-review.googlesource.com/742309
Reviewed-by: Ricky Liang <jcliang@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518343}
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/chrome/browser/apps/app_shim/unix_domain_socket_acceptor.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/components/arc/arc_session_impl.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/media/capture/video/chromeos/camera_hal_dispatcher_impl.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/embedder/BUILD.gn
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/embedder/platform_channel_pair_posix_unittest.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/embedder/platform_channel_utils_posix.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/embedder/platform_channel_utils_posix.h
[delete] https://crrev.com/dd94f65429e48697a50269b65e7b2e0972414bfa/mojo/edk/embedder/platform_handle_vector.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/embedder/scoped_platform_handle.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/broker_host.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/broker_posix.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/broker_win.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/channel.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/channel.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/channel_fuchsia.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/channel_posix.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/channel_unittest.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/channel_win.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/data_pipe_consumer_dispatcher.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/data_pipe_consumer_dispatcher.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/data_pipe_control_message.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/data_pipe_producer_dispatcher.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/data_pipe_producer_dispatcher.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/dispatcher.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/dispatcher.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/mach_port_relay.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/mach_port_relay.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/message_pipe_dispatcher.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/message_pipe_dispatcher.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/node_channel.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/node_channel.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/node_controller.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/node_controller.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/platform_handle_dispatcher.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/platform_handle_dispatcher.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/platform_handle_dispatcher_unittest.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/shared_buffer_dispatcher.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/shared_buffer_dispatcher.h
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/user_message_impl.cc
[modify] https://crrev.com/7df6feaddfff789abbef65a84c1a4296c5927a0a/mojo/edk/system/user_message_impl.h
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 27 2017 

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

commit 2c820264b8fbb36234c19b5e3a7e4849a4b30471
Author: Wez <wez@chromium.org>
Date: Mon Nov 27 21:09:11 2017

Revert "Migrate call-sites away from un-managed PlatformHandle[Vector]s."

This reverts commit 7df6feaddfff789abbef65a84c1a4296c5927a0a.

Reason for revert: Likely responsible for crbug.com/788192 and  crbug.com/788559  - crashes and aborts under Mac & Linux (so likely a bug in POSIX-specific changes).

Original change's description:
> Migrate call-sites away from un-managed PlatformHandle[Vector]s.
> 
> As the first step to merging ScopedPlatformHandle into PlatformHandle
> to have a single, always-managed container for platform handles, we:
> 
> - Replace ScopedPlatformHandleVectorPtr and PlatformHandleVector
>   with vector<ScopedPlatformHandle>.
> - Replace PlatformHandle with ScopedPlatformHandle in some core APIs.
> - Use const& to pass handles to APIs without passing ownership, for
>   both individual platform handles, and vectors.
> 
> Bug:  753541 
> Change-Id: Id965dcf3770ef11bf36f78277fa7199ea779599e
> Reviewed-on: https://chromium-review.googlesource.com/742309
> Reviewed-by: Ricky Liang <jcliang@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Dominick Ng <dominickn@chromium.org>
> Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
> Commit-Queue: Wez <wez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#518343}

TBR=posciak@chromium.org,wez@chromium.org,jcliang@chromium.org,rockot@chromium.org,lhchavez@chromium.org,elijahtaylor@chromium.org,dominickn@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  753541 
Change-Id: I890e833ae6d68ac4a346d230781ca0ac4798f135
Reviewed-on: https://chromium-review.googlesource.com/791390
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519398}
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/chrome/browser/apps/app_shim/unix_domain_socket_acceptor.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/components/arc/arc_session_impl.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/media/capture/video/chromeos/camera_hal_dispatcher_impl.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/embedder/BUILD.gn
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/embedder/platform_channel_pair_posix_unittest.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/embedder/platform_channel_utils_posix.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/embedder/platform_channel_utils_posix.h
[add] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/embedder/platform_handle_vector.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/embedder/scoped_platform_handle.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/broker_host.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/broker_posix.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/broker_win.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/channel.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/channel.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/channel_fuchsia.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/channel_posix.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/channel_unittest.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/channel_win.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/data_pipe_consumer_dispatcher.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/data_pipe_consumer_dispatcher.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/data_pipe_control_message.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/data_pipe_producer_dispatcher.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/data_pipe_producer_dispatcher.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/dispatcher.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/dispatcher.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/mach_port_relay.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/mach_port_relay.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/message_pipe_dispatcher.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/message_pipe_dispatcher.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/node_channel.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/node_channel.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/node_controller.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/node_controller.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/platform_handle_dispatcher.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/platform_handle_dispatcher.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/platform_handle_dispatcher_unittest.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/shared_buffer_dispatcher.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/shared_buffer_dispatcher.h
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/user_message_impl.cc
[modify] https://crrev.com/2c820264b8fbb36234c19b5e3a7e4849a4b30471/mojo/edk/system/user_message_impl.h
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 28 2017 

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

commit 6d83a0bea79407e74c8ba4a6be36c72053c43b7d
Author: Wez <wez@chromium.org>
Date: Tue Nov 28 01:17:15 2017

Reland "Migrate call-sites away from un-managed PlatformHandle[Vector]s."

This is a reland of 7df6feaddfff789abbef65a84c1a4296c5927a0a, which was
speculatively reverted as possible cause of crbug.com/788192, since the
CL originally landed only after that regression.

Original change's description:
> Migrate call-sites away from un-managed PlatformHandle[Vector]s.
>
> As the first step to merging ScopedPlatformHandle into PlatformHandle
> to have a single, always-managed container for platform handles, we:
>
> - Replace ScopedPlatformHandleVectorPtr and PlatformHandleVector
>   with vector<ScopedPlatformHandle>.
> - Replace PlatformHandle with ScopedPlatformHandle in some core APIs.
> - Use const& to pass handles to APIs without passing ownership, for
>   both individual platform handles, and vectors.
>
> Bug:  753541 
> Change-Id: Id965dcf3770ef11bf36f78277fa7199ea779599e
> Reviewed-on: https://chromium-review.googlesource.com/742309
> Reviewed-by: Ricky Liang <jcliang@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Dominick Ng <dominickn@chromium.org>
> Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
> Commit-Queue: Wez <wez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#518343}

Bug:  753541 
TBR: lhchavez, jcliang, dominickn, rockot
Change-Id: I9d50f06052b2118aec8872ca149504fbed245f1a
Reviewed-on: https://chromium-review.googlesource.com/792255
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519505}
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/chrome/browser/apps/app_shim/unix_domain_socket_acceptor.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/components/arc/arc_session_impl.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/media/capture/video/chromeos/camera_hal_dispatcher_impl.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/embedder/BUILD.gn
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/embedder/platform_channel_pair_posix_unittest.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/embedder/platform_channel_utils_posix.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/embedder/platform_channel_utils_posix.h
[delete] https://crrev.com/6d1a552b302672d63b3c7e8d2490334cafba7d1a/mojo/edk/embedder/platform_handle_vector.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/embedder/scoped_platform_handle.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/broker_host.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/broker_posix.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/broker_win.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/channel.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/channel.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/channel_fuchsia.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/channel_posix.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/channel_unittest.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/channel_win.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/data_pipe_consumer_dispatcher.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/data_pipe_consumer_dispatcher.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/data_pipe_control_message.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/data_pipe_producer_dispatcher.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/data_pipe_producer_dispatcher.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/dispatcher.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/dispatcher.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/mach_port_relay.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/mach_port_relay.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/message_pipe_dispatcher.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/message_pipe_dispatcher.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/node_channel.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/node_channel.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/node_controller.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/node_controller.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/platform_handle_dispatcher.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/platform_handle_dispatcher.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/platform_handle_dispatcher_unittest.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/shared_buffer_dispatcher.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/shared_buffer_dispatcher.h
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/user_message_impl.cc
[modify] https://crrev.com/6d83a0bea79407e74c8ba4a6be36c72053c43b7d/mojo/edk/system/user_message_impl.h

Comment 7 by w...@chromium.org, May 23 2018

Owner: roc...@chromium.org
Status: Assigned (was: Started)
-> rockot@ who is reworking the PlatformHandle to have the EDK provide a public API.

Comment 8 by w...@chromium.org, May 30 2018

Cc: w...@chromium.org erikc...@chromium.org
 Issue 791212  has been merged into this issue.
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 21 2018 

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

commit a22cf312c60d6145f792e23113ef7bbc659d30c1
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Jun 21 19:56:54 2018

Begin migration of EDK away from InternalPlatformHandle

Migrates a few bits of the EDK from ScopedInternalPlatformHandle
and InternalPlatformHandle over to the public PlatformHandle type.

Adds an explicit Type enumeration to PlatformHandle since there
can be subtle semantic differences between an abstract invalid
handle, vs (e.g.) an invalid FD, vs (e.g.) a null Mach port.

Also moves some stuff from mojo/edk/embedder to mojo/edk/system
where it belongs.

Bug:  753541 
Change-Id: I362757a27c3fba3794097b1d32341d1e703b90d0
Reviewed-on: https://chromium-review.googlesource.com/1107392
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569361}
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/BUILD.gn
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/embedder/embedder.cc
[delete] https://crrev.com/3bf89d01f380a9b608c209125d903a7eee8a00a4/mojo/edk/embedder/platform_handle_utils.cc
[delete] https://crrev.com/3bf89d01f380a9b608c209125d903a7eee8a00a4/mojo/edk/embedder/platform_handle_utils.h
[delete] https://crrev.com/3bf89d01f380a9b608c209125d903a7eee8a00a4/mojo/edk/embedder/platform_handle_utils_fuchsia.cc
[delete] https://crrev.com/3bf89d01f380a9b608c209125d903a7eee8a00a4/mojo/edk/embedder/platform_handle_utils_posix.cc
[delete] https://crrev.com/3bf89d01f380a9b608c209125d903a7eee8a00a4/mojo/edk/embedder/platform_handle_utils_win.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/mojo_core.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/broker_posix.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/broker_win.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/core.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/core.h
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/data_pipe_consumer_dispatcher.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/data_pipe_producer_dispatcher.cc
[rename] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/entrypoints.cc
[rename] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/entrypoints.h
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/invitation_unittest.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/platform_handle_dispatcher.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/platform_handle_dispatcher.h
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/platform_handle_dispatcher_unittest.cc
[add] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/platform_handle_utils.cc
[add] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/platform_handle_utils.h
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/edk/system/shared_buffer_dispatcher.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/public/cpp/platform/BUILD.gn
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/public/cpp/platform/DEPS
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/public/cpp/platform/platform_handle.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/public/cpp/platform/platform_handle.h
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/public/cpp/platform/tests/platform_handle_unittest.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/public/cpp/system/invitation.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/public/cpp/system/platform_handle.cc
[modify] https://crrev.com/a22cf312c60d6145f792e23113ef7bbc659d30c1/mojo/public/cpp/system/platform_handle.h
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 22 2018 

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

commit 5732c6d477358e5e0c1c4c4fa5841a63b069aa2a
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Jun 22 17:43:38 2018

Move more stuff out of mojo/edk/embedder

Moves some POSIX socket utilities into mojo/public/cpp/platform, moves
ConnectionParams into mojo/edk/system, deletes TransportProtocol.

Bug:  753541 
Change-Id: I1575af268e89c0424e618f2a0974693f610f5969
Reviewed-on: https://chromium-review.googlesource.com/1109199
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569686}
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/BUILD.gn
[delete] https://crrev.com/a9cd34ec652eeeef52d3326234c93df499cf54e7/mojo/edk/embedder/platform_channel_utils_posix.cc
[delete] https://crrev.com/a9cd34ec652eeeef52d3326234c93df499cf54e7/mojo/edk/embedder/platform_channel_utils_posix.h
[delete] https://crrev.com/a9cd34ec652eeeef52d3326234c93df499cf54e7/mojo/edk/embedder/transport_protocol.h
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/broker_posix.cc
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/channel.h
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/channel_posix.cc
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/channel_unittest.cc
[rename] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/connection_params.cc
[rename] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/connection_params.h
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/core.cc
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/node_channel.h
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/node_controller.cc
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/edk/system/platform_channel_pair_posix_unittest.cc
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/public/cpp/platform/socket_utils_posix.cc
[modify] https://crrev.com/5732c6d477358e5e0c1c4c4fa5841a63b069aa2a/mojo/public/cpp/platform/socket_utils_posix.h
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 22 2018 

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

commit 6a6dca0fe5a86c60e7d3bc98f5b6153cbc37a47f
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Jun 22 18:41:07 2018

Mojo: Delete EDK internal channel endpoint types

Deletes PlatformChannelPair, NamedPlatformChannelPair, and
NamedPlatformHandle types from the EDK. These are superceded by
equivalent types in the public platform support API.

TBR=jcivelli@chromium.org

Bug:  753541 
Change-Id: Ic1982aac93f0b6a78730497fc04dc44bb128cef0
Reviewed-on: https://chromium-review.googlesource.com/1109259
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569707}
[modify] https://crrev.com/6a6dca0fe5a86c60e7d3bc98f5b6153cbc37a47f/mojo/edk/BUILD.gn
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/named_platform_channel_pair.cc
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/named_platform_channel_pair.h
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/named_platform_handle.h
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/named_platform_handle_posix.h
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/named_platform_handle_utils.h
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/named_platform_handle_utils_fuchsia.cc
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/named_platform_handle_utils_posix.cc
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/named_platform_handle_utils_win.cc
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/named_platform_handle_win.h
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/platform_channel_pair.cc
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/platform_channel_pair.h
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/platform_channel_pair_fuchsia.cc
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/platform_channel_pair_posix.cc
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/embedder/platform_channel_pair_win.cc
[modify] https://crrev.com/6a6dca0fe5a86c60e7d3bc98f5b6153cbc37a47f/mojo/edk/system/BUILD.gn
[modify] https://crrev.com/6a6dca0fe5a86c60e7d3bc98f5b6153cbc37a47f/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/6a6dca0fe5a86c60e7d3bc98f5b6153cbc37a47f/mojo/edk/system/broker_win.cc
[modify] https://crrev.com/6a6dca0fe5a86c60e7d3bc98f5b6153cbc37a47f/mojo/edk/system/channel_unittest.cc
[modify] https://crrev.com/6a6dca0fe5a86c60e7d3bc98f5b6153cbc37a47f/mojo/edk/system/data_pipe_unittest.cc
[modify] https://crrev.com/6a6dca0fe5a86c60e7d3bc98f5b6153cbc37a47f/mojo/edk/system/multiprocess_message_pipe_unittest.cc
[modify] https://crrev.com/6a6dca0fe5a86c60e7d3bc98f5b6153cbc37a47f/mojo/edk/system/node_controller.cc
[delete] https://crrev.com/3c7f51e9cfef79acfc9f337a4f1568b7bcca4b48/mojo/edk/system/platform_channel_pair_posix_unittest.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 22 2018 

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

commit a86f4b9aed52053c0e672f41209fc94ba3148ab6
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Jun 22 18:54:33 2018

Mojo EDK: Move internal platform handle types

mojo/edk/embedder -> mojo/edk/system

TBR=jcivelli@chromium.org

Bug:  753541 
Change-Id: I4b73748986c4b728e42c5a19fe8b7a6a685238b5
Reviewed-on: https://chromium-review.googlesource.com/1109034
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569715}
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/BUILD.gn
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/broker.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/broker_host.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/broker_posix.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/broker_win.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/channel.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/channel.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/channel_posix.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/channel_win.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/connection_params.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/core.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/data_pipe_consumer_dispatcher.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/data_pipe_control_message.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/data_pipe_control_message.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/data_pipe_producer_dispatcher.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/dispatcher.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/invitation_dispatcher.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/mach_port_relay.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/mach_port_relay.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/message_pipe_perftest.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/multiprocess_message_pipe_unittest.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/node_channel.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/node_controller.h
[rename] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/platform_handle.cc
[rename] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/platform_handle.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/platform_handle_dispatcher_unittest.cc
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/platform_handle_utils.h
[rename] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/scoped_platform_handle.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/shared_buffer_dispatcher.h
[modify] https://crrev.com/a86f4b9aed52053c0e672f41209fc94ba3148ab6/mojo/edk/system/user_message_impl.h
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 22 2018 

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

commit 0afff4bd889e93353cf41e26e050e35c70b1e018
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Jun 22 23:39:37 2018

Fix FD transmission on OS X

Was botched a bit by r569686. This CL fixes the two problems:

- FDs will once again be properly retained by a sender until
  the receiver acknowledges their transmission
- If a message fails to send and is queued for retransmision,
  it will actually retain its FDs instead of replacing them
  with invalid ones.

TBR=jcivelli@chromium.org

Bug:  753541 
Change-Id: I0c045f25eb1c9f1a89925f6cdc29ffcffea3d7b3
Reviewed-on: https://chromium-review.googlesource.com/1112651
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569842}
[modify] https://crrev.com/0afff4bd889e93353cf41e26e050e35c70b1e018/mojo/edk/system/channel_posix.cc
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 27 2018 

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

commit f88821e74821d9040fe8eb7afdccc3b7d20fd122
Author: Ken Rockot <rockot@chromium.org>
Date: Wed Jun 27 20:48:24 2018

Mojo: Refactor internal Windows handle rewriting logic

Gets rid of Channel::Message::RewriteHandles in favor of inlining
DuplicateHandle calls where needed; this is because it was only called
in a few places, and each place had slightly different preconditions.

Makes Channel aware of the remote process handle when possible, allowing
handle rewriting to take place implicitly on send and receive with no
action on the part of the Channel consumer.

This all ultimately makes the handle rewriting logic a bit easier to
follow, and is done in preparation for introducing a new
PlatformHandleInTransit type to capture PlatformHandle +
ScopedProcessHandle and get rid of ScopedInternalPlatformHandles usage
for handle passing.

Bug:  753541 
Change-Id: Ib7914121b448b23209e1eef5286a808fb4f835f2
Reviewed-on: https://chromium-review.googlesource.com/1112951
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570894}
[modify] https://crrev.com/f88821e74821d9040fe8eb7afdccc3b7d20fd122/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/f88821e74821d9040fe8eb7afdccc3b7d20fd122/mojo/edk/system/channel.cc
[modify] https://crrev.com/f88821e74821d9040fe8eb7afdccc3b7d20fd122/mojo/edk/system/channel.h
[modify] https://crrev.com/f88821e74821d9040fe8eb7afdccc3b7d20fd122/mojo/edk/system/channel_win.cc
[modify] https://crrev.com/f88821e74821d9040fe8eb7afdccc3b7d20fd122/mojo/edk/system/node_channel.cc
[modify] https://crrev.com/f88821e74821d9040fe8eb7afdccc3b7d20fd122/mojo/edk/system/node_controller.cc
Project Member

Comment 15 by bugdroid1@chromium.org, Jun 27 2018 

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

commit 0e45f8023860a6e7f0146988d13f583e9da5b9b1
Author: Ken Rockot <rockot@chromium.org>
Date: Wed Jun 27 23:56:01 2018

Mojo: Consolidate Mach port brokering logic

Moves all Mach port brokering stuff into the Channel implementation,
making it a bit easier to understand. Previously relevant logic was
spread over Channel, NodeChannel, and NodeController.

This is in preparation for introducing a new PlatformHandleInTransit
type to encapsulate a PlatformHandle + ScopedProcessHandle, as a
replacement for the weird and sort of unsafe
ScopedInternalPlatformHandle usage we have today.

Bug:  753541 
Change-Id: I7c17fb75d5930d8a8816a2927c8529d75c8b7675
Reviewed-on: https://chromium-review.googlesource.com/1112926
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570947}
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/channel.cc
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/channel.h
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/channel_fuchsia.cc
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/channel_posix.cc
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/channel_unittest.cc
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/channel_win.cc
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/core.cc
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/core.h
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/mach_port_relay.cc
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/mach_port_relay.h
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/node_channel.cc
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/node_channel.h
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/node_controller.cc
[modify] https://crrev.com/0e45f8023860a6e7f0146988d13f583e9da5b9b1/mojo/edk/system/node_controller.h
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 29 2018 

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

commit 043152da69d28f95a0d036c598010ecabcb3b404
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Jun 29 03:22:16 2018

Mojo: Use PlatformHandleInTransit for message attachments

Introduces an internal PlatformHandleInTransit type to safely
scope the lifetime of platform handles which may belong to a remote
process.

Uses this everywhere for handles stored in messages, eliminating
all remaining use of some InternalPlatformHandle properties which
aren't covered by PlatformHandle (namely |owning_process| and the
MACH_NAME "handle" type).

This is a precursor to removing InternalPlatformHandle and
ScopedInternalPlatformHandle altogether.

Bug:  753541 
Change-Id: Ibc223669c935bdde72c2af3b6b3d7cd1474e2bdf
Reviewed-on: https://chromium-review.googlesource.com/1114462
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571373}
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/BUILD.gn
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/broker_host.h
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/broker_win.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/channel.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/channel.h
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/channel_fuchsia.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/channel_posix.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/channel_unittest.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/channel_win.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/mach_port_relay.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/mach_port_relay.h
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/node_channel.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/node_channel.h
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/node_controller.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/platform_handle.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/platform_handle.h
[add] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/platform_handle_in_transit.cc
[add] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/platform_handle_in_transit.h
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/scoped_process_handle.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/scoped_process_handle.h
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/edk/system/user_message_impl.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/public/cpp/platform/platform_handle.cc
[modify] https://crrev.com/043152da69d28f95a0d036c598010ecabcb3b404/mojo/public/cpp/platform/platform_handle.h
Project Member

Comment 17 by bugdroid1@chromium.org, Jun 29 2018 

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

commit 29b0a328b80d43f89273311a4198771522b78fa4
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Jun 29 17:38:07 2018

Mojo: Delete [Scoped]InternalPlatformHandle

Hurray! Replaces all usage of ScopedInternalPlatformHandle or
InternalPlatformHandle with the public PlatformHandle type,
a thing which is inherently scoped and also not weirdly overloaded
a bunch of random non-handle junk. Hurray!

Bug:  753541 
Change-Id: Ie828e5545e24d4b57ea6fb9f21e635a02a52f9ff
Reviewed-on: https://chromium-review.googlesource.com/1115804
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571514}
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/BUILD.gn
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/broker.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/broker_host.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/broker_host.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/broker_posix.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/broker_win.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/channel.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/channel.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/channel_fuchsia.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/channel_posix.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/channel_unittest.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/channel_win.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/connection_params.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/connection_params.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/core.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/core.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/data_pipe_consumer_dispatcher.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/data_pipe_consumer_dispatcher.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/data_pipe_control_message.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/data_pipe_control_message.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/data_pipe_producer_dispatcher.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/data_pipe_producer_dispatcher.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/dispatcher.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/dispatcher.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/invitation_dispatcher.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/mach_port_relay.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/mach_port_relay.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/message_pipe_dispatcher.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/message_pipe_dispatcher.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/message_pipe_perftest.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/multiprocess_message_pipe_unittest.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/node_channel.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/node_channel.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/node_controller.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/node_controller.h
[delete] https://crrev.com/845945b6b26062ff8e5e87227244d966adb4872a/mojo/edk/system/platform_handle.cc
[delete] https://crrev.com/845945b6b26062ff8e5e87227244d966adb4872a/mojo/edk/system/platform_handle.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/platform_handle_dispatcher.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/platform_handle_dispatcher.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/platform_handle_dispatcher_unittest.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/platform_handle_utils.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/platform_handle_utils.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/platform_wrapper_unittest.cc
[delete] https://crrev.com/845945b6b26062ff8e5e87227244d966adb4872a/mojo/edk/system/scoped_platform_handle.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/shared_buffer_dispatcher.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/shared_buffer_dispatcher.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/user_message_impl.cc
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/system/user_message_impl.h
[modify] https://crrev.com/29b0a328b80d43f89273311a4198771522b78fa4/mojo/edk/test/test_utils.h

Comment 18 by roc...@chromium.org, Jun 29 2018

Status: Fixed (was: Assigned)
Project Member

Comment 19 by bugdroid1@chromium.org, Jul 12 

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

commit 1e5b3af2899e6c4888da7ad662ca1d2928e01ce8
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Jul 12 00:01:39 2018

Mojo: Fix Mac Mach port brokering race

https://crrev.com/0e45f8023 somehow lost this logic before
landing, probably during extensive debugging churn. :{

Without this, it's possible for Mach port transfer to fail
when sending to a new process. Seems to have gone undetected
in production so far, but https://crrev.com/3e126191 managed
to trip over it consistently in ipc_tests.

Bug:  753541 
Change-Id: I835dc5f26326718af678acd6199c285d709b9040
Reviewed-on: https://chromium-review.googlesource.com/1134079
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574418}
[modify] https://crrev.com/1e5b3af2899e6c4888da7ad662ca1d2928e01ce8/mojo/core/channel_posix.cc
►

Sign in to add a comment