After the previous patch, all that is needed to pass eslint is some
whitespace fixes I generated using:
./mach eslint --fix ipc
The .eslintrc.js file makes eslint expect XPCShell global variables.
With those two changes, eslint can be enabled for the ipc/ directory.
MozReview-Commit-ID: BqSICp2iV6O
--HG--
extra : rebase_source : ccab8be9fc1ae1116ae55631d3978c39d35cad05
I don't fully understand what this test is trying to do, but as far as
I can see, test_ipcshell_child.js does nothing when loaded from
test_ipcshell.js besides define a few variables, so just define them
directly in test_ipcshell.js. I am doing this because eslint does not
like the way the test is currently written: the definition of runtime
in another file confuses it, and it does not like the assignment to
run_test.
MozReview-Commit-ID: Ek0HY9n49eZ
--HG--
extra : rebase_source : 5c99b20afe93c085a198e7824e82207be347f26b
Never store names in Message. One can get string names from
Message::name() or use IPC::StringFromIPCMessageType() when only
message id is available.
MozReview-Commit-ID: 15ksx6SE90c
--HG--
extra : rebase_source : 1a041dc365b7f42edd540d8c7a4dfd8912e48921
xpcshell can sometimes create and then almost immediately kill the GPU process. This can
cause the connection state of GPUParent's message channel to be in error or closed
when it attempts to send its build ID. In this case we should ignore the request
because the process will soon close.
MozReview-Commit-ID: EAupU844iFo
--HG--
extra : histedit_source : df05a01df5ad095b4e3c91e0e038d4e145da5f9d
The NS_LITERAL_CSTRING macro creates a temporary nsLiteralCString to encapsulate the string literal and its length, but AssignLiteral() can determine the string literal's length at compile-time without nsLiteralCString.
MozReview-Commit-ID: B5Y8KyExPQ8
--HG--
extra : rebase_source : e27b266c145daa5acd887e998c6d5b408101e1db
extra : source : 33f49977a33cbdb1c7127871b940eefccc018f65
Each protocol in IPDL has a bunch of autogenerated functions that
instantiate IPC::Message with various parameters. Each of these
functions, then:
1) Pays the cost of calling malloc()
2) Setting up various parameters
3) Calling IPC::Message()
There's no reason that we should be duplicating 1) across all of these
autogenerated functions. In step 2), several of the parameters we're
setting up are common across all or nearly all calls: the message
segment size is almost always zero, and we're always indicating that
IPDL-generated messages should be recorded in telemetry.
Instead of duplicating that code several thousand times, we can add a
small helper function that takes the only interesting parameters for an
IPDL message. This helper function can then deal with calling malloc in
a single place and setting up the common parameters. For messages that
require a custom segment size, we'll have to use the old scheme, but
such messages are uncommon.
The previous changes are not required for this scheme to work, but they
do help significantly, as the helper function (Message::IPDLMessage) can
now take four parameters, which ensures that its arguments are passed
solely in registers on Win64 and ARM. The wins from this change are
also larger than they would be without the previous parts: ~100K on
x86-64 Linux (!) and ~80K on ARM Android.
The current IPC::Message constructor takes a large number of arguments,
three of which--the nesting level, the priority, and the
compression--are almost always constant by virtue of the vast majority
of Message construction being done by auto-generated IPDL code. But
then we take these constant values into the Message constructor, we
check them for various values, and then based on those values, we
perform a bunch of bitfield operations to store flags based on those
values. This is wasted work.
Furthermore, for replies to IPDL messages, we'll construct a Message
object, and then call mutating setters on the Message object that will
perform even more bitfield manipulations. Again, these operations are
performing tasks at runtime that are the same every single time, and use
information we already have at compile time.
The impact of these extra operations is not large, maybe 15-30K of extra
code, depending on platform. Nonetheless, we can easily make them go
away, and make everything cleaner to boot.
This patch adds a HeaderFlags class that encapsulates all the knowledge
about the various kinds of flags Message needs to know about. We can
construct HeaderFlags objects with strongly-typed enum arguments for the
various kinds of flags, and the compiler can take care of folding all of
those flags together into a constant when possible (and it is possible
for all the IPDL-generated code that instantiates Messages). The upshot
is that we do no unnecessary work in the Message constructor itself. We
can also remove various mutating operations on Message, as those
operations were only there to support post-constructor flag twiddling,
which is no longer necessary.
There's no need to be repeating 'IPC::Message::' prefixes or spreading
around more ExprVar calls than we need here. Let's try to improve the
signal-to-noise ratio of this code by introducing a helper function to
inject some of the boilerplate for us.
_generateMessageConstructor takes a lot of `md.FOO`-style parameters,
which could be derived inside the function by simply passing `md`.
Especially with the upcoming changes to calculate things like reply-ness
of messages, sync-ness, etc, we'd be wanting to pass even more
parameters like `md.FOO`. So let's just pass `md` in, and then we can
make all the necessary future changes in a single place.
The GPU process doesn't have the directory service enabled, so it can't find
a tmp dir to put its .extra files for crash reports. Even if we do enable the
directory service, we still don't get the correct "content process tmp dir" in
the GPU process, because the UUID baked into that folder is passed via the
preferences service, and that isn't initialized in the GPU process either.
Rather than unneccessarily initialize all this stuff in the GPU process just
to get one folder name, we can pass that folder name directly in the argv list.
See comments 12-19 on the bug for further discussion of the various solutions
attempted/explored.
MozReview-Commit-ID: 1sFg27hIe7S
--HG--
extra : rebase_source : 78eb600a58fed45800b9df8303cc4d6898d96ae9
This function is very large and has many nested ifdefs. Without this cleanup
it was really hard to figure out where all I needed to add code.
MozReview-Commit-ID: IeA0AWW62Mn
--HG--
extra : rebase_source : a8447df42c60da79f1c982111c8f65bd0175f1de
The GPU process doesn't have the directory service enabled, so it can't find
a tmp dir to put its .extra files for crash reports. Even if we do enable the
directory service, we still don't get the correct "content process tmp dir" in
the GPU process, because the UUID baked into that folder is passed via the
preferences service, and that isn't initialized in the GPU process either.
Rather than unneccessarily initialize all this stuff in the GPU process just
to get one folder name, we can pass that folder name directly in the argv list.
See comments 12-19 on the bug for further discussion of the various solutions
attempted/explored.
MozReview-Commit-ID: 1sFg27hIe7S
--HG--
extra : rebase_source : 62ff819c5b03f642cd4b9af7c89c84b790397372