Skip to content

Commit

Permalink
Bug 1779792 - Part 3: Use an endpoint to bind the initial actor in pa…
Browse files Browse the repository at this point in the history
…rent processes, r=ipc-reviewers,necko-reviewers,media-playback-reviewers,alwu,mccr8

This improves consistency with the child process case, and will make it easier
to attach additional state without needing to thread it through every child
process callsite manually.

Differential Revision: https://phabricator.services.mozilla.com/D153619
  • Loading branch information
mystor committed Aug 10, 2022
1 parent d45df27 commit 2ac29a4
Show file tree
Hide file tree
Showing 11 changed files with 24 additions and 27 deletions.
4 changes: 1 addition & 3 deletions dom/ipc/ContentParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2681,9 +2681,7 @@ bool ContentParent::LaunchSubprocessResolve(bool aIsSync,
sCreatedFirstContentProcess = true;
}

base::ProcessId procId =
base::GetProcId(mSubprocess->GetChildProcessHandle());
Open(mSubprocess->TakeInitialPort(), procId);
mSubprocess->TakeInitialEndpoint().Bind(this);

ContentProcessManager* cpm = ContentProcessManager::GetSingleton();
if (!cpm) {
Expand Down
3 changes: 1 addition & 2 deletions dom/media/gmp/GMPParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,7 @@ nsresult GMPParent::LoadProcess() {
mChildPid = base::GetProcId(mProcess->GetChildProcessHandle());
GMP_PARENT_LOG_DEBUG("%s: Launched new child process", __FUNCTION__);

bool opened = Open(mProcess->TakeInitialPort(),
base::GetProcId(mProcess->GetChildProcessHandle()));
bool opened = mProcess->TakeInitialEndpoint().Bind(this);
if (!opened) {
GMP_PARENT_LOG_DEBUG("%s: Failed to open channel to new child process",
__FUNCTION__);
Expand Down
3 changes: 1 addition & 2 deletions dom/media/ipc/RDDProcessHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,7 @@ void RDDProcessHost::InitAfterConnect(bool aSucceeded) {
}
mProcessToken = ++sRDDProcessTokenCounter;
mRDDChild = MakeUnique<RDDChild>(this);
DebugOnly<bool> rv = mRDDChild->Open(
TakeInitialPort(), base::GetProcId(GetChildProcessHandle()));
DebugOnly<bool> rv = TakeInitialEndpoint().Bind(mRDDChild.get());
MOZ_ASSERT(rv);

// Only clear mPrefSerializer in the success case to avoid a
Expand Down
3 changes: 1 addition & 2 deletions gfx/ipc/GPUProcessHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ void GPUProcessHost::InitAfterConnect(bool aSucceeded) {
if (aSucceeded) {
mProcessToken = ++sProcessTokenCounter;
mGPUChild = MakeUnique<GPUChild>(this);
DebugOnly<bool> rv = mGPUChild->Open(
TakeInitialPort(), base::GetProcId(GetChildProcessHandle()));
DebugOnly<bool> rv = TakeInitialEndpoint().Bind(mGPUChild.get());
MOZ_ASSERT(rv);

mGPUChild->Init();
Expand Down
3 changes: 1 addition & 2 deletions gfx/vr/ipc/VRProcessParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,7 @@ bool VRProcessParent::InitAfterConnect(bool aSucceeded) {

mVRChild = MakeUnique<VRChild>(this);

DebugOnly<bool> rv = mVRChild->Open(
TakeInitialPort(), base::GetProcId(GetChildProcessHandle()));
DebugOnly<bool> rv = TakeInitialEndpoint().Bind(mVRChild.get());
MOZ_ASSERT(rv);

mVRChild->Init();
Expand Down
16 changes: 9 additions & 7 deletions ipc/docs/processes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,8 @@ behavior is fairly clear:
return;
}

new DemoParent(std::move(host));
auto actor = MakeRefPtr<DemoParent>(std::move(host));
actor->Init();
});
}

Expand All @@ -579,20 +580,21 @@ the new host, if successful.

In this sample, the ``DemoParent`` is owned (in the reference-counting sense)
by IPDL, which is why it doesn't get assigned to anything. This simplifies the
design dramatically. IPDL takes ownership when the actor calls ``Open`` in its
constructor:
design dramatically. IPDL takes ownership when the actor calls ``Bind`` from
the ``Init`` method:

.. code-block:: c++

DemoParent::DemoParent(UniqueHost&& aHost)
: mHost(std::move(aHost)) {
Open(mHost->TakeInitialPort(),
base::GetProcId(mHost->GetChildProcessHandle()));
: mHost(std::move(aHost)) {}

DemoParent::Init() {
mHost->TakeInitialEndpoint().Bind(this);
// ...
mHost->MakeBridgeAndResolve();
}

After the ``Open`` call, the actor is live and communication with the new
After the ``Bind`` call, the actor is live and communication with the new
process can begin. The constructor concludes by initiating the process of
connecting the ``PDemoHelpline`` actors; ``Host::MakeBridgeAndResolve`` will be
covered in `Creating a New Top Level Actor`_. However, before we get into
Expand Down
7 changes: 6 additions & 1 deletion ipc/glue/GeckoChildProcessHost.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "chrome/common/ipc_message.h"
#include "mojo/core/ports/port_ref.h"

#include "mozilla/ipc/Endpoint.h"
#include "mozilla/ipc/FileDescriptor.h"
#include "mozilla/ipc/NodeChannel.h"
#include "mozilla/ipc/ScopedPort.h"
Expand Down Expand Up @@ -127,7 +128,11 @@ class GeckoChildProcessHost : public ChildProcessHost,
IPC::Channel* GetChannel() { return channelp(); }
ChannelId GetChannelId() { return channel_id(); }

ScopedPort TakeInitialPort() { return std::move(mInitialPort); }
UntypedEndpoint TakeInitialEndpoint() {
return UntypedEndpoint{PrivateIPDLInterface{}, std::move(mInitialPort),
base::GetCurrentProcId(),
base::GetProcId(mChildProcessHandle)};
}

// Returns a "borrowed" handle to the child process - the handle returned
// by this function must not be closed by the caller.
Expand Down
3 changes: 1 addition & 2 deletions ipc/glue/UtilityProcessHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ void UtilityProcessHost::InitAfterConnect(bool aSucceeded) {
}

mUtilityProcessParent = MakeRefPtr<UtilityProcessParent>(this);
DebugOnly<bool> rv = mUtilityProcessParent->Open(
TakeInitialPort(), base::GetProcId(GetChildProcessHandle()));
DebugOnly<bool> rv = TakeInitialEndpoint().Bind(mUtilityProcessParent.get());
MOZ_ASSERT(rv);

// Only clear mPrefSerializer in the success case to avoid a
Expand Down
4 changes: 1 addition & 3 deletions ipc/ipdl/test/gtest/IPDLUnitTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ already_AddRefed<IPDLUnitTestParent> IPDLUnitTestParent::CreateCrossProcess() {
return nullptr;
}

if (!parent->Open(
parent->mSubprocess->TakeInitialPort(),
base::GetProcId(parent->mSubprocess->GetChildProcessHandle()))) {
if (!parent->mSubprocess->TakeInitialEndpoint().Bind(parent.get())) {
ADD_FAILURE() << "Opening the parent actor failed";
return nullptr;
}
Expand Down
3 changes: 1 addition & 2 deletions netwerk/ipc/SocketProcessHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ void SocketProcessHost::InitAfterConnect(bool aSucceeded) {
}

mSocketProcessParent = MakeUnique<SocketProcessParent>(this);
DebugOnly<bool> rv = mSocketProcessParent->Open(
TakeInitialPort(), base::GetProcId(GetChildProcessHandle()));
DebugOnly<bool> rv = TakeInitialEndpoint().Bind(mSocketProcessParent.get());
MOZ_ASSERT(rv);

SocketPorcessInitAttributes attributes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ RefPtr<GenericPromise> RemoteSandboxBrokerParent::Launch(
// Note: we rely on the caller to keep this instance alive while we launch
// the process, so that these closures point to valid memory.
auto resolve = [this](base::ProcessHandle handle) {
mOpened = Open(mProcess->TakeInitialPort(), base::GetProcId(handle));
mOpened = mProcess->TakeInitialEndpoint().Bind(this);
if (!mOpened) {
mProcess->Destroy();
mProcess = nullptr;
Expand Down

0 comments on commit 2ac29a4

Please sign in to comment.