When launching the GMP process, or querying its state, via PGMPService,
the design was already asynchronous. As such we can remove the sync
requirement quite easily and allow more flexibility on the parent
process side.
Differential Revision: https://phabricator.services.mozilla.com/D185335
In the Storage Access API's latest draft, a few items were added to the user-agent state. Relevant here,
the source snapshot params gained two fields that are initialized from the sourceDocument during
snapshotting source params while navigating: "has storage access" and "environment id".
https://privacycg.github.io/storage-access/#ua-state
These are used to identify self-initiated navigations that come from documents that have obtained storage access.
Combined with a same-origin check, this determines if the destination document of the navigation should start
with storage access.
This is stricter than the current behavior, where if the permission is available, all documents start with storage access.
Instead, now a document will only have storage access if it requests it explicitly or if a same-origin document that has
storage access navigates itself to that document. This is seen as a security win.
Security discussion of this change was here: https://github.com/privacycg/storage-access/issues/113
Artur at Google wrote up a great summary here: https://docs.google.com/document/d/1AsrETl-7XvnZNbG81Zy9BcZfKbqACQYBSrjM3VsIpjY/edit#
Differential Revision: https://phabricator.services.mozilla.com/D184821
In the Storage Access API's latest draft, a few items were added to the user-agent state. Relevant here,
the source snapshot params gained two fields that are initialized from the sourceDocument during
snapshotting source params while navigating: "has storage access" and "environment id".
https://privacycg.github.io/storage-access/#ua-state
These are used to identify self-initiated navigations that come from documents that have obtained storage access.
Combined with a same-origin check, this determines if the destination document of the navigation should start
with storage access.
This is stricter than the current behavior, where if the permission is available, all documents start with storage access.
Instead, now a document will only have storage access if it requests it explicitly or if a same-origin document that has
storage access navigates itself to that document. This is seen as a security win.
Security discussion of this change was here: https://github.com/privacycg/storage-access/issues/113
Artur at Google wrote up a great summary here: https://docs.google.com/document/d/1AsrETl-7XvnZNbG81Zy9BcZfKbqACQYBSrjM3VsIpjY/edit#
Differential Revision: https://phabricator.services.mozilla.com/D184821
When launching the GMP process, or querying its state, via PGMPService,
the design was already asynchronous. As such we can remove the sync
requirement quite easily and allow more flexibility on the parent
process side.
Differential Revision: https://phabricator.services.mozilla.com/D185335
This extends the previous support for `-greomni` and `-appomni` to the
fork server, so that it can pre-open the jar files and thus continue
using the correct versions in forked child processes even if the files on
disk are replaced by an update or deleted.
Differential Revision: https://phabricator.services.mozilla.com/D185331
Currently, each content process re-derives the path(s) of the omnijar
file(s). We used to pass it down as a command-line argument, but
those args were also accepted by the parent process and there were
issues with that (CVE-2020-6799) such that they were completely removed
(bug 1531475). However, content processes can generally trust their
arguments; note that they currently accept `-appDir`.
We were already using `-gredir` for this on Android (it has a unified
omnijar, so there's no `-appdir` in that case); this patch subsumes the
content-process case of that, but not the parent process (which consumes
basically a fake argv constructed in Java code).
Note that only the parent process and content processes use the
omnijars; every other process type uses either minimal XPCOM, which
doesn't include them, or no XPCOM at all (e.g., GMP before bug 1845946).
The end goal of this patch series is to use those flags with the fork
server (so that it can preload the files without needing any XPCOM), but
this patch changes only the case of content processes.
Differential Revision: https://phabricator.services.mozilla.com/D182510
The `-appomni` flag isn't currently used, but the next patch will bring
it back and change how both flags are processed in child processes.
Differential Revision: https://phabricator.services.mozilla.com/D185330
When running GTests on Windows 7, the TestManyHandles test crashes due
to the lpNumberOfBytesWritten argument to WriteFile not being passed.
This argument is required on earlier versions of Windows like Windows 7.
While we no longer support Windows 7 on Nightly, we still support it on
ESR, and are trying to run GTests on Windows 7 there. It also doesn't
hurt to add extra assertions that the full amount of data is
read/written, so checking this could be valuable on m-c as well.
Differential Revision: https://phabricator.services.mozilla.com/D184393
This also does minimal refactoring of cases where the directives were
protecting a simple expression that could be refactored back to the
callers.
Differential Revision: https://phabricator.services.mozilla.com/D184399
This also does minimal refactoring of cases where the directives were
protecting a simple expression that could be refactored back to the
callers.
Differential Revision: https://phabricator.services.mozilla.com/D184399
The special handling for android is in chromium's SharedMemory type,
meaning we can use SharedMemoryBasic_chromium now on Android.
This patch removes the android special casing to use the chromium
backend on that platform as well.
Differential Revision: https://phabricator.services.mozilla.com/D182808
This improves the quality of the logging from IPC::Channel, and also
adds extra assertions to ensure that it's aligned with the values
sent/received in the HELLO message.
This patch also makes the other_pid type more consistent, using
base::ProcessId instead of int32_t in IPC::Channel.
Differential Revision: https://phabricator.services.mozilla.com/D183410
There were a few types of errors which could spuriously happen on Windows, and
are being fixed by this patch. The `ERROR_NO_DATA` error is now being silenced
similar to `ERROR_BROKEN_PIPE`. This appears to happen in some specific
edge-cases around the pipe's lifecycle, such as when the pipe has been created
and then destroyed, but no data has been sent yet.
This patch also silences errors when transferring or accepting handles if
`DuplicateHandle` fails due to the target process terminating. In this
situation, `DuplicateHandle` returns `ERROR_ACCESS_DENIED`, which is
unfortunately ambiguous. To avoid that ambiguity, this patch takes the same
approach as Chromium, and uses the `RtlGetLastNtStatus` function from ntdll to
get the true error status, and compare it to `STATUS_PROCESS_IS_TERMINATING`.
This matches the behaviour in Chromium to silence the same error.
Differential Revision: https://phabricator.services.mozilla.com/D183409
Previously these logs would produce misleading errors due to the HANDLE being
duplicated having been clobbered by the failed DuplicateHandle call. This patch
changes them to include more information in the error log, and makes the error
logs more accurate.
Differential Revision: https://phabricator.services.mozilla.com/D183408
This bug uses the existing process hang messaging infrastructure to raise priorities on main threads. To ensure reduced latency, we also increase the priority of ProcessHangMon threads. We also address an edge-case bug where flipping the QoS prefs during use might lead to a tab getting stuck at the wrong priority.
Due to increasing the priority of the hang monitor thread, we may see some increase in its utilization on MacOS during high CPU load. I'm not sure the extent of how this may impact the browser, but as it makes the thread "faster" it may be more responsive than some other threads during this case.
I tested thread responsiveness by using the stress tool and dispatching various numbers of worker threads, up to 250. During these tests, even when other parts of firefox were less responsive under stress, tab switching appeared to remain snappy and responsive.
I captured some updated power profiles using the change. Profile where pref flipped off during use: https://share.firefox.dev/46BksO3 (Note that we start with the prefs on, then we flip off the prefs half way and repeat the same behavior to observe the fix to the previous bug that left tabs trapped in the background)
Profile with the pref fully enabled: https://share.firefox.dev/46EBIC7
In regards to the edge case, to avoid spurious tab wakeups, we won't reinstate normal thread priority when pref is disabled until the tab is interacted with again.
Differential Revision: https://phabricator.services.mozilla.com/D182787
This bug uses the existing process hang messaging infrastructure to raise priorities on main threads. To ensure reduced latency, we also increase the priority of ProcessHangMon threads. We also address an edge-case bug where flipping the QoS prefs during use might lead to a tab getting stuck at the wrong priority.
Due to increasing the priority of the hang monitor thread, we may see some increase in its utilization on MacOS during high CPU load. I'm not sure the extent of how this may impact the browser, but as it makes the thread "faster" it may be more responsive than some other threads during this case.
I tested thread responsiveness by using the stress tool and dispatching various numbers of worker threads, up to 250. During these tests, even when other parts of firefox were less responsive under stress, tab switching appeared to remain snappy and responsive.
I captured some updated power profiles using the change. Profile where pref flipped off during use: https://share.firefox.dev/46BksO3 (Note that we start with the prefs on, then we flip off the prefs half way and repeat the same behavior to observe the fix to the previous bug that left tabs trapped in the background)
Profile with the pref fully enabled: https://share.firefox.dev/46EBIC7
In regards to the edge case, to avoid spurious tab wakeups, we won't reinstate normal thread priority when pref is disabled until the tab is interacted with again.
Differential Revision: https://phabricator.services.mozilla.com/D182787
This call is no longer required after the other changes in part 1 and 2,
and adds substantial complexity to the type. This just removes all of
the ConnectNamedPipe handling logic from ipc_channel_win now that it is
unnecessary.
Differential Revision: https://phabricator.services.mozilla.com/D181803
Now that we've simplified the startup process somewhat, it is easier to clean
up IPC channel creation for NodeChannel connections. This stops having
GeckoChildProcessHost inherit from IPC::Channel::Listener, as it would never
receive most of the relevant callbacks, and instead implements the one callback
it would receive directly as a method on that type.
Differential Revision: https://phabricator.services.mozilla.com/D181282
This allows simplifying how IPC::Channel is created and passed between
processes, as all platforms now inherit the initial handle/fd into the content
process in a similar way. To keep things simple for now, I've continued to use
the base::CommandLine class to pass the HANDLE's identity on Windows, however
we may want to change this to make it a bit easier to follow, perhaps treating
it more like how we handle the IPC fd on Android.
Differential Revision: https://phabricator.services.mozilla.com/D181281
There were cases that a lot of cmds messages were sent by WebGLChild::FlushPendingCmds() without calling ClientWebGLContext::GetFrontBuffer(). And the IPC messages were waiting to be handled by WebGLParent, since OpenGL handling could take longer. Then there were cases that pending IPC messages were accumulated and it caused out of file descriptor on Linux. The sync IPC could reduce the pending IPC messages if there are a lot of cmds flashes.
async Ping is used for checking if IPC message is congested at WebGLParent. If a count of flushes since last congestion check reaches maybeIPCMessageCongestion, we think that IPC message is congested at WebGLParent. Then send sync SyncPing message to flush pending IPC messages.
Differential Revision: https://phabricator.services.mozilla.com/D181484
Andrew Osmond
Benjamin VanderSloot
Cosmin Sabou
Cristian Tuns
Manuel Bucher
Gregory Pappas
Jed Davis
Natalia Csoregi
Christian Holler (:decoder)
Paul Adenot
Iulian Moraru
Nika Layzell
Brad Werth
Tom Schuster
Masatoshi Kimura
Alexandre Lissy
Stanca Serban
kriswright
Narcis Beleuzu
Jan Varga
Kagami Sascha Rosylight
serge-sans-paille
sotaro