1751948 - Add support for running IPDL gtests

Status: RESOLVED FIXED

(Core :: IPC, task)

Description

[Nika Layzell [:nika] (ni? for response)]

The tests in ipc/ipdl/tests/cxx haven't been running for quite a while, and have become broken and unmaintained. This bug tracks building a new system for running similar tests on top of gtest instead, which will allow us to ensure that IPDL unit tests continue to be run.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1751948 - Part 1: Fix IPDL unified build issues, r=#ipc-reviewers

The changes in this patch stack will change how IPDL generated files are built,
such that these issues are now surfaced, especially during non-unified builds.

Depends on D137164

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1751948 - Part 2: Add support for generated UNIFIED_SOURCES files, r=#firefox-build-system-reviewers

This patch adds support for generated files to be placed in
UNIFIED_SOURCES. This will be used in part 3 to build .cpp files
generated by IPDL_SOURCES by adding them to IPDL's UNIFIED_SOURCES under
the hood. Using a unified build for IPDL files is important, as a
significant build time regression was observed locally when using
non-unified sources to build ipdl sources, due to the large number of
generated files.

Depends on D137165

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1751948 - Part 3: Build IPDL sources within the directory they were declared, r=#firefox-build-system-reviewers

This change allows IPDL sources to respect FINAL_LIBRARY when building, which
is important for allowing us to build gtest-only IPDL_SOURCES files.

Depends on D137166

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1751948 - Part 4: Correctly initialize OtherPid() for in-process actors, r=#ipc-reviewers

Before this change, OtherPid() would only be initialized for one side of an
actor pair when opening an in-process actor using IToplevelProtocol::Open or
IToplevelProtocol::OpenOnSameThread. This changes the function to directly
accept the other protocol, and initializes the value correctly in all cases.

Depends on D137167

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1751948 - Part 5: Ensure we don't release ActorLifecycleProxy while holding MessageChannel's Monitor, r=#ipc-reviewers

Releasing ActorLifecycleProxy can lead to the IToplevelProtocol being
destroyed, due to the reference being the last reference. If this happens, we
can deadlock due to the MessageChannel embedded in IToplevelProtocol locking
mMonitor during its' destructor. This patch moves acquiring the proxy earlier
in the method, so that we do not deadlock in this case any longer.

Depends on D137168

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1751948 - Part 6: Add basic support for running IPDL unit tests during gtest, r=#ipc-reviewers

This patch introduces a new system for building IPDL unit tests, which is
roughly inspired by the old cxx unit test design, however designed to work with
gtest. It re-uses the existing IPDLUnitTest process type, using static
constructors only present in xul-gtest to register child actor constructors and
ProcessChild implementations to avoid bloating xul.

The IPDLUnitTest actor is used as a backchannel to communicate test failures
from the child process back to the parent process, and manage the test's async
lifecycle more reliably.

The IPDLUnitTest process also needed to have various properties about how it was
initialized adjusted:

• The IPDLUnitTest process should now always load xul-gtest when running
  gtests, by using the "Self" process type and adding a DYLD_LIBRARY_PATH override
  on macOS where the "Self" process type cannot be used.
• The IPDLUnitTest process now initializes minimal XPCOM, and runs a
  frankeneventloop to allow it to use XPCOM event loop structures such as
  SpinEventLoopUntil in tests.
• Support for creating IPDLUnitTest processes on Android was added, as these
  tests had never run on android before.

Depends on D137169

Comment 7

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1751948 - Part 7: Fix non-unified filename conflict between generated and non-generated ChannelInfo.cpp files, r=asuth

Due to the earlier changes in this patch series, we now build IPDL files
within the directories which they're declared in, meaning that the
generated ChannelInfo.cpp file made by ChannelInfo.ipdlh will try to
compile a ChannelInfo.o file on top of ChannelInfo.cpp's ChannelInfo.o
file when building in non-unified mode.

This works around the issue by renaming the ipdlh file to
IPCChannelInfo.ipdlh.

Depends on D137170

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63545319b2dd
Part 1: Fix IPDL unified build issues, r=ipc-reviewers,mccr8
https://hg.mozilla.org/integration/autoland/rev/ab9f87d2451e
Part 2: Add support for generated UNIFIED_SOURCES files, r=glandium
https://hg.mozilla.org/integration/autoland/rev/4ac102698b52
Part 3: Build IPDL sources within the directory they were declared, r=glandium
https://hg.mozilla.org/integration/autoland/rev/d9bd956b3935
Part 4: Correctly initialize OtherPid() for in-process actors, r=ipc-reviewers,mccr8
https://hg.mozilla.org/integration/autoland/rev/6198b0a5e72a
Part 5: Ensure we don't release ActorLifecycleProxy while holding MessageChannel's Monitor, r=jld
https://hg.mozilla.org/integration/autoland/rev/e68c91f0ee8c
Part 6: Add basic support for running IPDL unit tests during gtest, r=ipc-reviewers,andi,handyman
https://hg.mozilla.org/integration/autoland/rev/52f5eaeb4340
Part 7: Fix non-unified filename conflict between generated and non-generated ChannelInfo.cpp files, r=asuth

Comment 9

[Marian-Vasile Laza]

Backed out 7 changesets (Bug 1751948) for causing gtest failures.
Backout link
Push with failures
Failure Log

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd0d7f4ad7f1
Part 1: Fix IPDL unified build issues, r=ipc-reviewers,mccr8
https://hg.mozilla.org/integration/autoland/rev/ebb0cf46f290
Part 2: Add support for generated UNIFIED_SOURCES files, r=glandium
https://hg.mozilla.org/integration/autoland/rev/90b065b65547
Part 3: Build IPDL sources within the directory they were declared, r=glandium
https://hg.mozilla.org/integration/autoland/rev/dec37b799f2a
Part 4: Correctly initialize OtherPid() for in-process actors, r=ipc-reviewers,mccr8
https://hg.mozilla.org/integration/autoland/rev/296670d89ad6
Part 5: Ensure we don't release ActorLifecycleProxy while holding MessageChannel's Monitor, r=jld
https://hg.mozilla.org/integration/autoland/rev/e83184193f3a
Part 6: Add basic support for running IPDL unit tests during gtest, r=ipc-reviewers,andi,handyman
https://hg.mozilla.org/integration/autoland/rev/1d9f17244335
Part 7: Fix non-unified filename conflict between generated and non-generated ChannelInfo.cpp files, r=asuth

Comment 11

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/cd0d7f4ad7f1
https://hg.mozilla.org/mozilla-central/rev/ebb0cf46f290
https://hg.mozilla.org/mozilla-central/rev/90b065b65547
https://hg.mozilla.org/mozilla-central/rev/dec37b799f2a
https://hg.mozilla.org/mozilla-central/rev/296670d89ad6
https://hg.mozilla.org/mozilla-central/rev/e83184193f3a
https://hg.mozilla.org/mozilla-central/rev/1d9f17244335