We can move this information into ProtocolState and save having two
virtual functions for every protocol. Moving some bits out of the
codegen'd IPC code is a nice bonus, though we keep the strange setup
where toplevel protocols have two mChannel member variables.
ProtocolName() is only used for producing error messages and annotating
crash reports. But examining actual crash reports that would have used
the result of ProtocolName() indicates that we can always tell what the
erroring protocol is due to the stack backtrace. So having this virtual
function around just provides duplicate information, and it takes up too
much space in the vtable besides. Let's get rid of it.
lower.py generates repetitious:
SetManager(...);
Register(...); // Or RegisterID.
SetIPCChannel(...);
calls, which are moderately sized, given that the above call sequence
requires virtual calls in several places. Instead of codegenning this
sequence, let's consolidate the sequence into IProtocol and change the
code generator to call into the consolidated function instead.
This function is only overriden in two places, both of which go away
after early beta is done. We shouldn't be paying for its vtable entry
after that point.
The reasoning here is the same as for the protocol register/lookup
functions: these functions are all basic functionality that should not
be overriden by subclasses.
This functionality is base functionality for top-level and non-toplevel
protocols; nobody overrides this stuff, so it's safe to move into
ProtocolState.
IProtocol, which is inherited by every generated IPDL protocol and every
concrete protocol implementation in-tree, has a number of virtual
methods that are only relevant when distinguishing between top-level
protocols (IToplevelProtocol) and managed protocols (everything else).
These virtual methods require pointers in every protocol's vtable, which
is wasteful, and it's also somewhat confusing that many methods exist
but don't really need to be overridable in any useful way.
Let's clean this up, by creating a ProtocolState class to hold methods
that solely differ between top-level protocols and everything else.
This commit does that work and moves Shmem-related methods into this
class as a proof that this can be done in a reasonable way.
This function is just pure bloat when it gets inlined, and it will
disappear on non-Nightly builds anyway. Make it MOZ_NEVER_INLINE so our
size statistics on Nightly are somewhat more reflective of our size
statistics on Release.
This replaces using file_util to open and unlink temporary files
(/dev/shm on Linux, $TMPDIR or /tmp otherwise) with the POSIX shm_open
API, or ashmem on Android (which doesn't implement shm_open).
glibc maps shm_open/shm_unlink to open and unlink in /dev/shm (as does
musl libc), so the Linux situation is mostly unchanged except we aren't
duplicating code from system libraries. Other OSes may (and some do)
use more efficient implementations than temporary files.
FreeBSD's SHM_ANON extension is used if available. Sadly, it's not
standard; it would make this patch much simpler if it were.
This patch changes the shm file names; they now start with "org.mozilla"
instead of "org.chromium" because the original Chromium code is mostly
gone at this point. When running as a Snap package, the required
filename prefix is added; other container/sandbox environments using
AppArmor to restrict the allowed filenames may need to be adjusted.
The shm names now include the creating process's pid, to allow
using sandboxing to prevent interfering with shm belonging to other
applications or other processes within the same browser instance.
MozReview-Commit-ID: 7PirIlcblh4
--HG--
extra : rebase_source : 550a0ab013429c29a57bde5c0e4593d9b426da8e
I've also manually verified that no other references to HANDLE_EINTR are
wrapping a close() in any less syntactically obvious way.
MozReview-Commit-ID: 3KkBwFIhEIq
--HG--
extra : rebase_source : 4e79a70b3be22a7721b6f85b19ee5a31c98df456
This is based on the current security/sandbox/chromium version of eintr_wrapper.h,
taken from upstream commit 937db09514e061d7983e90e0c448cfa61680f605.
I've edited it to remove some things that aren't relevant to us: the
debug-mode loop limit in HANDLE_EINTR, because we don't seem to be
having the problem it's meant to fix and it risks regressions, and
references to Fuchsia, which we don't (yet) support. I also kept the
original include guards (the file path has changed upstream).
What this patch *does* do is add IGNORE_EINTR and modernize the C++
slightly (using decltype instead of nonstandard typeof).
MozReview-Commit-ID: BO4uQL9jUtf
--HG--
extra : rebase_source : ab3343c6d93e0ce753859217a55af131a0c4ea68
This patch was reviewed in parts, however the intermediate states would not build:
Bug 1443954 - Part 3A: Strip pointers from the argument to WriteParam and WriteIPDLParam before selecting the ParamTraits impl, r=froydnj
Bug 1443954 - Part 3B: Move nsIAlertNotification serialization to the refcounted system, r=bz
Bug 1443954 - Part 3C: Move geolocation serialization to the refcounted system, r=bz
Bug 1443954 - Part 3D: Move nsIInputStream serialization to the refcounted system, r=baku
Bug 1443954 - Part 3E: Move BlobImpl serialization to the refcounted system, r=baku
Bug 1443954 - Part 3F: Correctly implement ParamTraits for actors after the ParamTraits changes, r=froydnj
We want to spin for faster response, but we only want to spin for a very short time.
If we're waiting for a while, we don't want to be burning CPU for the entire time.
Therefore, only spin for 30 ms, then fall back to waiting on an event.
MozReview-Commit-ID: ErAIwpsIqYz
--HG--
extra : rebase_source : b6ac024adb7853456fd06c4385cf32184c8aeca2
I missed this one in bug 1431404.
This condition is rare and does indicate a problem which breaks accessibility.
However, we aren't getting any closer to diagnosing this as a result of this crash, so it causes user pain without any gain to us.
MozReview-Commit-ID: GncQGeZckrV
--HG--
extra : rebase_source : b6670005d53bc6d0009f0b8b142c2ab837b7d2e7
The current code is a bit of a mess. This patch does the following.
- Changes the processing from backwards to forwards. This avoids the need for
all the `found` booleans, because if a flag is present multiple times, the
last one will naturally override.
- Tightens up the checking. It now doesn't use assertions, but instead returns
false if any of the options are missing arguments, or have malformed
arguments, or any of the mandatory flags are missing. (It assumes that
-appdir and -profile are optional.)
- Renames the loop variable `idx` as `i`.
- Changes `!strcmp(...)` to `strcmp(...) == 0`, because I find that clearer.
- Avoids a redundant nsCString when handling -appdir.
The patch also tweaks GeckoChildProcessHost::mGroupId, which was buggy. It
holds the appModelUserId argument, which XRE_InitChildProcess() always expects
is present in the command. But it's only set to a non-empty value in
InitWindowsGroupID(), which is only called for plugin processes. So in lots of
cases the appModelUserId argument was missing, and a different argument would
be interpreted as the appModelUserId argument (seemingly without noticeable ill
effect).
The patch changes things to mGroupId defaults to "-", which means it's always
present in the command.
Note: all this explains why the old code for ContentProcess::Init() started
processing from argument aArgc, instead of aArgc-1 as you might expect -- it
had to read one extra arg in order to see the argument following -appdir,
because XRE_InitChildProcess() was decrementing aArgc for the appModelUserId
argument even when that argument wasn't present. The new code for
ContentProcess::Init() doesn't have to read past aArgc-1 because the mGroupId
fix ensures the appModelUserId argument is always present.
MozReview-Commit-ID: 8a8k6ABYMgo
--HG--
extra : rebase_source : 70695125ee26e67af3337119f4dfc293a0dab74c
Once we've figured out that some task needs to be reposted, there's no
reason to continue scanning the list to find other tasks that need to be
reposted, since the logic in this function just requires one task that
needs to be reposted.
This code isn't blocking anything, but it's dead and I don't think we
have any plans to use it.
MozReview-Commit-ID: KBoEfLceDns
--HG--
extra : rebase_source : 1eee3d961e249939f02d4cc40a707739eb2a596a
We're not using named shared memory, and supporting only anonymous
shared memory allows using other backends that are more compatible
with preventing a process from accessing any shared memory it wasn't
explicitly granted (i.e., sandboxing).
Specifically: SharedMemory::Open is removed; SharedMemory::Create no
longer takes a name, no longer has the open_existing option which doesn't
apply to anonymous memory, and no longer supports read-only memory
(anonymous memory which can never have been written isn't very useful).
This patch also fixes some comments in what remains of SharedMemory::Create.
MozReview-Commit-ID: 4kBrURtxq20
--HG--
extra : rebase_source : f6b1fb2fc79b6e9cdd251b3d9041036c0be503f9
This deletes some dead code and removes a dependency on the shared
memory object's name, which will be removed in the next patch (and is
always empty in our usage).
MozReview-Commit-ID: 1ub0nLCBucO
--HG--
extra : rebase_source : 6a29261e00b89773a2f2ace47303d9d9842c089b
This remotes the APZInputBridge interface over the PAPZInputBridge
protocol in the case of the GPU process, and makes the GPU process'
main thread act as the APZ controller thread in that process. If
there is no GPU process we continue as before and the APZInputBridge
interface implementation is the concrete APZCTreeManager instance
in the UI process.
The main changes in this patch are moving all the code associated with
these messages out of APZCTreeManager{Parent,Child} and into
APZInputBridge{Parent,Child}. APZCTreeManagerChild now returns an
APZInputBridgeChild instance via InputBridge(), instead of returning
itself. The SetControllerThread call in the GPU process is also updated.
MozReview-Commit-ID: M4AaIW1Q0h
--HG--
extra : rebase_source : e5a8f14e23be34229fe80a47f6789d19b19e0a9f
They are not yet fully async because ContentParent::InitInternal calls
OtherPid(), which will block until the process is spawned. Deferring the calls
to OtherPid() will be a subject of a follow up patch.
MozReview-Commit-ID: 4TFkMpdQtRw
--HG--
extra : rebase_source : 3e7567679ae04aa4c04ea6f6c146e70417e7ce05
This will let us manipulate it from multiple threads in a future patch.
MozReview-Commit-ID: 2AOgho8SEX9
--HG--
extra : rebase_source : 49e7d82b38e6f7b7eb9f6cb7c61a2b41d34f2bdc