We don't want to show the skeleton UI if there is already an instance of
Firefox running for that install. Accordingly, we implement something
similar to the profile lock, acquiring exclusive access to
~/AppData/Local/Mozilla/Firefox/SkeletonUILock-<installHash>. If we do not do
this, then when a user clicks firefox.exe while an existing instance is
running, under default conditions we will open the skeleton UI, then
almost immediately terminate and send a message to the existing instance to
open a new window.
Differential Revision: https://phabricator.services.mozilla.com/D98525
clang 12 (specifically https://reviews.llvm.org/D91379) made some refactorings to libc++ that exposed a problem in the MinGW headers. That has now been fixed upstream.
In the meantime the headers also gained definitions for ProcessPayloadRestrictionPolicy, so we can remove our workaround for that.
Differential Revision: https://phabricator.services.mozilla.com/D98945
Currently, printf_stderr doesn't show up when running with ./mach run.
This is because we run with -attach-console and that redirects stderr
to a different file descriptor using freopen in UseParentConsole.
The change from just using stderr directly happened in bug 340443 and was done
to avoid some linking issues. That problem doesn't seem to apply anymore so we
should be able to go back to the straightforward implemention that works even
if stderr has been redirected. The mozglue implementation was cargo culted from
xpcom, and there wasn't a reason other than that for the fdopen(dup()) there.
Differential Revision: https://phabricator.services.mozilla.com/D98550
I'm hoping that the comments in the code are sufficient documentation of this,
but here is a quick overview. The mockup we got from design has rounded rects
in it, and bordered rounded rects in it, so we need to support drawing those.
Initially we tried using the RoundRect function in GDI. However, 1) GDI doesn't
support anti-aliasing, which is unfortunately noticeable, 2) we were
observing strange issues with only some of the corners being rounded with
RoundRect at higher radii / stroke widths. 3) drawing on top of things drawn
with RoundRect would be complicated and inefficient unless we switched
everything over to GDI calls.
As it stands this drawing code is platform agnostic, assuming we have a way of
blitting a raw bitmap to the screen on a given platform, which is a nice trait
to have and makes me think twice about switching all of the drawing over to
direct GDI calls.
Differential Revision: https://phabricator.services.mozilla.com/D95317
I'm hoping that the comments in the code are sufficient documentation of this,
but here is a quick overview. The mockup we got from design has rounded rects
in it, and bordered rounded rects in it, so we need to support drawing those.
Initially we tried using the RoundRect function in GDI. However, 1) GDI doesn't
support anti-aliasing, which is unfortunately noticeable, 2) we were
observing strange issues with only some of the corners being rounded with
RoundRect at higher radii / stroke widths. 3) drawing on top of things drawn
with RoundRect would be complicated and inefficient unless we switched
everything over to GDI calls.
As it stands this drawing code is platform agnostic, assuming we have a way of
blitting a raw bitmap to the screen on a given platform, which is a nice trait
to have and makes me think twice about switching all of the drawing over to
direct GDI calls.
Differential Revision: https://phabricator.services.mozilla.com/D95317
This patch supports a skeleton UI for default, light, and dark themes.
It is not enabled for apenglow or any custom themes.
This also takes into account the system theme. If the user has the default
theme selected and is in dark mode, we override the theme and present the
dark theme skeleton UI.
Differential Revision: https://phabricator.services.mozilla.com/D96230
This patch supports a skeleton UI for default, light, and dark themes.
It is not enabled for apenglow or any custom themes.
This also takes into account the system theme. If the user has the default
theme selected and is in dark mode, we override the theme and present the
dark theme skeleton UI.
Differential Revision: https://phabricator.services.mozilla.com/D96230
This patch adds a list of the executable's dependent module's path to SharedSection
as an array of the offset to a string and a string buffer. A following patch will
use this data from the browser process and the sandboxed processes.
Depends on D96283
Differential Revision: https://phabricator.services.mozilla.com/D96284
We transfer several ntdll's function addresses to a child process directly via
`WriteProcessMemory`. This patch changes the way to transfer data to using
a section object as Chromium sandbox does, so that we can transfer more data
with the same cost as transferring a single handle value.
Depends on D96282
Differential Revision: https://phabricator.services.mozilla.com/D96283
If a searchbar is present, draw that similar to a urlbar.
This also changes how we store the urlbar, as we know will use our custom struct
to cleanly store the width and height.
Differential Revision: https://phabricator.services.mozilla.com/D95029
Most of this patch is a dance to avoid size flickering of the skeleton UI
window. We change all Resize/Move/SetSizeMode calls from before the first
nsWindow::Show call. Normally those have no effect, since the window isn't
shown yet, and if the window is not maximized, they typically match the
sizes we've gotten out of the registry anyway. However, if we are maximized,
then they produce a lot of visual noise. We can however achieve the desired
effect by just calling SetWindowPlacement.
Similarly, we switch the window styles of the skeleton UI window to match those
of the toplevel Windows window, and adjust the client rect from our window proc
in a way that matches the adjustments in nsWindow in the WM_NCCALCSIZE handler.
We do this because otherwise we get a flicker as soon as we change the styles
and nonclient margins as the fake chrome pops up and then back down.
Lastly we also change the extended window styles so that they match. We
historically added WS_EX_TOOLWINDOW here to hide the toolbar entry, because it
would otherwise switch out to a new toolbar entry when we changed the window
styles. However since our new styles match, we no longer need to do this. It
was also causing the maximized window to paint over the Windows taskbar.
Differential Revision: https://phabricator.services.mozilla.com/D93534
For a better user experience during slow startups (and to match the design
by Markus Jaritz), we want to animate the placeholder elements (the grey
rectangles which hold the place of text and icons) with a light moving
gradient.
A question may arise regarding whether the performance cost of running this
animation is worth the improved user experience. My claim is yes, hinging
mostly on the observation that the performance cost is small.
On my machine, one frame of the animation takes at most 0.15ms, and runs
every 16.15ms. This means we eat less than 1% CPU time on one core of the
system. It should also be noted that this animation runs primarily during
the window wherein we are prefetching dlls, AKA while we are blocked on IO
and the CPU is more likely to be idle.
On slower systems, one frame may take longer - however, on slower systems
we should also be blocked *more* by IO, making the trade favorable.
For further anecdotal evidence of this being okay, when I run this on slow
reference hardware shortly after OS startup, I do not see any dropped frames,
indicating that this has very little trouble completing, and is thus quite
cheap.
Regarding testing, I will invoke the same logic as for the rest of the
skeleton UI patches - it would involve quite a bit of work to test this in
automation given our existing frameworks. It may be worth it at some point,
but certainly not while this is a feature that we are just experimenting
with and which is not enabled by default.
Differential Revision: https://phabricator.services.mozilla.com/D91453
Most of this patch is a dance to avoid size flickering of the skeleton UI
window. We change all Resize/Move/SetSizeMode calls from before the first
nsWindow::Show call. Normally those have no effect, since the window isn't
shown yet, and if the window is not maximized, they typically match the
sizes we've gotten out of the registry anyway. However, if we are maximized,
then they produce a lot of visual noise. We can however achieve the desired
effect by just calling SetWindowPlacement.
Similarly, we switch the window styles of the skeleton UI window to match those
of the toplevel Windows window, and adjust the client rect from our window proc
in a way that matches the adjustments in nsWindow in the WM_NCCALCSIZE handler.
We do this because otherwise we get a flicker as soon as we change the styles
and nonclient margins as the fake chrome pops up and then back down.
Lastly we also change the extended window styles so that they match. We
historically added WS_EX_TOOLWINDOW here to hide the toolbar entry, because it
would otherwise switch out to a new toolbar entry when we changed the window
styles. However since our new styles match, we no longer need to do this. It
was also causing the maximized window to paint over the Windows taskbar.
Differential Revision: https://phabricator.services.mozilla.com/D93534
For a better user experience during slow startups (and to match the design
by Markus Jaritz), we want to animate the placeholder elements (the grey
rectangles which hold the place of text and icons) with a light moving
gradient.
A question may arise regarding whether the performance cost of running this
animation is worth the improved user experience. My claim is yes, hinging
mostly on the observation that the performance cost is small.
On my machine, one frame of the animation takes at most 0.15ms, and runs
every 16.15ms. This means we eat less than 1% CPU time on one core of the
system. It should also be noted that this animation runs primarily during
the window wherein we are prefetching dlls, AKA while we are blocked on IO
and the CPU is more likely to be idle.
On slower systems, one frame may take longer - however, on slower systems
we should also be blocked *more* by IO, making the trade favorable.
For further anecdotal evidence of this being okay, when I run this on slow
reference hardware shortly after OS startup, I do not see any dropped frames,
indicating that this has very little trouble completing, and is thus quite
cheap.
Regarding testing, I will invoke the same logic as for the rest of the
skeleton UI patches - it would involve quite a bit of work to test this in
automation given our existing frameworks. It may be worth it at some point,
but certainly not while this is a feature that we are just experimenting
with and which is not enabled by default.
Differential Revision: https://phabricator.services.mozilla.com/D91453
If the urlbar is zoomed (defined by attribute breakout-extend), then we
scale it down to the unfocused coordinate values.
Differential Revision: https://phabricator.services.mozilla.com/D92303
The latest launcher process showed one of the top failures was `WriteProcessMemory` in
`CopyStubToChildProcess` failed with `ERROR_INVALID_ADDRESS` or `ERROR_NOACCESS`, that
is to store a trampoline address to the global variable of firefox.exe failed. Its root
cause should be the same as bug 1662560, the executable was loaded onto a different
address from the browser process.
The fix is to to expand the usage of `CrossExecTransferManager` to `FuncHookCrossProcess`
and `Kernel32ExportsSolver`.
Depends on D94652
Differential Revision: https://phabricator.services.mozilla.com/D94653
This patch introduces a class `CrossExecTransferManager` to manage the data
transfer from the current process to a remote process via `WriteProcessMemory`.
The class also encapsulates a logic to bridge the gap between two executable's
imagebase.
Differential Revision: https://phabricator.services.mozilla.com/D94652
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Make some ad-hoc manual updates to `testing/marionette/client/setup.py`, `testing/marionette/harness/setup.py`, and `testing/firefox-ui/harness/setup.py`, which have hard-coded regexes that break after the reformat.
5. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Make some ad-hoc manual updates to `testing/marionette/client/setup.py`, `testing/marionette/harness/setup.py`, and `testing/firefox-ui/harness/setup.py`, which have hard-coded regexes that break after the reformat.
5. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
The latest launcher process ping showed one of the reasons why we failed to
detour `NtMapViewOfSection` is that `MMPolicyBase::FindRegion` failed to find
a free region. Inspecting the function carefully, there were three problems.
Firstly, `FindRegion` did not fully scan the given range. To randomize
the address of a free region we use, we start scanning from a random address
within the given range. The problem is we scan only addresses bigger than
that random address, without scanning smaller addresses. Probably this is
the reason why `FindRegion` fails.
Secondly, `FindRegion` may return an address not aligned with the allocation
granularity because `VirtualQueryEx` returns such an address. If that happens,
the subsequent mapping API fails with the alignment error.
Lastly, when we randomize an address to start scanning from, we divide a random
number by `maxOffset`, but with that, we never start scanning from the last
region. It does not affect the product's behavior, but to have fair randomization,
a divisor should be `maxOffset + 1`.
This patch fixes all of these three problems along with a new test program.
Differential Revision: https://phabricator.services.mozilla.com/D94110
This is the third attempt to investigate the launcher failure of our detour.
The previous commits d8315e4ed18d and 1b81ea85c43d added the assembly bytes
of a detour target and a special error code `DetourResultCode` to the launcher
failure ping.
In the latest telemetry data, however, the most common value of `hresult`
is still `ERROR_UNIDENTIFIED_ERROR`, meaning the previous commit missed to
set an error code in the common fallible codepath we wanted to know.
Besides `ERROR_UNIDENTIFIED_ERROR`, we're seeing `DETOUR_PATCHER_DO_RESERVE_ERROR`
in the telemetry, but having that code is not enough to pinpoint a falling
operation.
For further investigation, this patch adds ten more values to `DetourResultCode`.
`FUNCHOOKCROSSPROCESS_COPYSTUB_ERROR` is the last codepath we forgot to cover
in the previous commit. The values of `MMPOLICY_RESERVE_*` are to investigate
`DETOUR_PATCHER_DO_RESERVE_ERROR` in the MMPolicy level. In both cases, we add
the last Windows error code to `DetourError::mOrigBytes`.
Differential Revision: https://phabricator.services.mozilla.com/D92974
This is the third attempt to investigate the launcher failure of our detour.
The previous commits d8315e4ed18d and 1b81ea85c43d added the assembly bytes
of a detour target and a special error code `DetourResultCode` to the launcher
failure ping.
In the latest telemetry data, however, the most common value of `hresult`
is still `ERROR_UNIDENTIFIED_ERROR`, meaning the previous commit missed to
set an error code in the common fallible codepath we wanted to know.
Besides `ERROR_UNIDENTIFIED_ERROR`, we're seeing `DETOUR_PATCHER_DO_RESERVE_ERROR`
in the telemetry, but having that code is not enough to pinpoint a falling
operation.
For further investigation, this patch adds ten more values to `DetourResultCode`.
`FUNCHOOKCROSSPROCESS_COPYSTUB_ERROR` is the last codepath we forgot to cover
in the previous commit. The values of `MMPOLICY_RESERVE_*` are to investigate
`DETOUR_PATCHER_DO_RESERVE_ERROR` in the MMPolicy level. In both cases, we add
the last Windows error code to `DetourError::mOrigBytes`.
Differential Revision: https://phabricator.services.mozilla.com/D92974
The latest Windows Insider Preview (version 20226.1000) changes the machine code for BaseThreadInitThunk to have a preamble like the following:
00007FFDBF244C40 48 83 EC 28 sub rsp,28h
00007FFDBF244C44 85 C9 test ecx,ecx
00007FFDBF244C46 75 25 jne 00007FFDBF244C6D
00007FFDBF244C48 49 BA 70 A2 DC 12 6A 97 99 B0 mov r10,0B099976A12DCA270h
This patch adds "MOV r64, imm64" capability to the DLL interceptor so that we can hook this.
Differential Revision: https://phabricator.services.mozilla.com/D92146
The previous commit d8315e4ed18d introduced a new telemetry field
in the launcher process ping to collect the assembly pattern of
a target function on detour failure, but most of the crash instances
do not have a value in the field. This means the failure happens
before or after `CreateTrampoline`.
To narrow down the root cause, this patch puts a fine-grained error value
in the "hresult" field instead of the hardcoded ERROR_UNIDENTIFIED_ERROR.
This patch also adds `IsPageAccessible` check before fetching data from
a different process because fetching data from an invalid address hits
`MOZ_RELEASE_ASSERT` in `EnsureLimit`, resulting in crash without sending
the launcher process failure.
Differential Revision: https://phabricator.services.mozilla.com/D91881
The last Avast Antivirus's hook function contains `CALL [disp32]` instruction.
Our detour needs to be able to handle that pattern.
Differential Revision: https://phabricator.services.mozilla.com/D91155
Doug Thayer
David Major
Razvan Maries
Jeff Muizelaar
Simon Giesecke
Sylvestre Ledru
Mihai Alexandru Michis
Emma Malysz
Bogdan Tara
Toshihito Kikuchi
Narcis Beleuzu
Florian Quèze
Gabriele Svelto
Butkovits Atila
Sebastian Hengst
Ricky Stewart
sanketh
Dorel Luca
Andreea Pavel
David Parks