On Android, we must dispatch UiCompositorController::Destroy to run on
the UI thread synchronously. We were using NS_DISPATCH_SYNC to do so,
but that works by starting a nested event loop that continues to
execute tasks on the thread we have dispatched from. This means that
we can start to execute a task which calls
nsBaseWidget::CreateCompositor whilst we are midway through
nsBaseWidget::DestroyCompositor. As well as generally seeming like a
terrible idea, this also causes an assertion failure in some tests.
To avoid this use SynchronousTask rather than NS_DISPATCH_SYNC, as it
actually blocks synchronously. Additionally, do the same thing for
APZInputBridgeChild::Destroy, as it is called from the same location
and poses the same risk.
Ideally we wouldn't have to call UiCompositorControllerChild::Destroy
synchronously at all, but it was added in bug 1392705 to fix severe
crashes. It might be a good idea to re-evaluate whether it is still
required at some point in the future.
Differential Revision: https://phabricator.services.mozilla.com/D140084
On Android, we must dispatch UiCompositorController::Destroy to run on
the UI thread synchronously. We were using NS_DISPATCH_SYNC to do so,
but that works by starting a nested event loop that continues to
execute tasks on the thread we have dispatched from. This means that
we can start to execute a task which calls
nsBaseWidget::CreateCompositor whilst we are midway through
nsBaseWidget::DestroyCompositor. As well as generally seeming like a
terrible idea, this also causes an assertion failure in some tests.
To avoid this use SynchronousTask rather than NS_DISPATCH_SYNC, as it
actually blocks synchronously. Additionally, do the same thing for
APZInputBridgeChild::Destroy, as it is called from the same location
and poses the same risk.
Ideally we wouldn't have to call UiCompositorControllerChild::Destroy
synchronously at all, but it was added in bug 1392705 to fix severe
crashes. It might be a good idea to re-evaluate whether it is still
required at some point in the future.
Differential Revision: https://phabricator.services.mozilla.com/D140084
We noticed a cold_view_nav_start regression on Fenix from enabling the
GPU process, and profiles showed time spent synchronously waiting for
the GPU process to launch. This occured because the compositor was
being created in nsWindow::Create, and as it requires the GPU process
to be running it had to block until launch completed. The process is
launched when the gfxPlatform is first initialized, but that was only
occuring immediately prior to creating the compositor, which did not
give it enough time to complete asynchronously.
This patch makes it so that we initialize the gfxPlatform slightly
earlier, and importantly delay creating the compositor until it is
actually required. This gives the process enough time to launch
asynchronously meaning we do not have to block.
We started deliberately creating the compositor early on Android
because of bug 1453501, to avoid a race condition where the compositor
didn't exist when RemoteLayerTreeOwner::Initialize was called, causing
us to use a basic layer manager. However, since bug 1741156 landed we
now create the compositor on-demand, meaning this is no longer a
possibility.
Delaying compositor creation can, however, uncover another race
condition. If the UICompositorControllerChild is opened on the UI
thread before the main thread is able to set its pointer to the
widget, then the java GeckoSession will never be notified that the
compositor has been opened, and composition will never be
resumed. This patch fixes this issue by setting the
UiCompositorControllerChild's widget pointer in its constructor rather
than immediately afterwards.
Differential Revision: https://phabricator.services.mozilla.com/D139842
This cuts the time spent doing get_ct_font on macOS in half because of
various contention issues. It should also reduce overall memory usage
for large fonts because we have half as many threads loading the
fonts.
Differential Revision: https://phabricator.services.mozilla.com/D139946
We noticed a cold_view_nav_start regression on Fenix from enabling the
GPU process, and profiles showed time spent synchronously waiting for
the GPU process to launch. This occured because the compositor was
being created in nsWindow::Create, and as it requires the GPU process
to be running it had to block until launch completed. The process is
launched when the gfxPlatform is first initialized, but that was only
occuring immediately prior to creating the compositor, which did not
give it enough time to complete asynchronously.
This patch makes it so that we initialize the gfxPlatform slightly
earlier, and importantly delay creating the compositor until it is
actually required. This gives the process enough time to launch
asynchronously meaning we do not have to block.
We started deliberately creating the compositor early on Android
because of bug 1453501, to avoid a race condition where the compositor
didn't exist when RemoteLayerTreeOwner::Initialize was called, causing
us to use a basic layer manager. However, since bug 1741156 landed we
now create the compositor on-demand, meaning this is no longer a
possibility.
Delaying compositor creation can, however, uncover another race
condition. If the UICompositorControllerChild is opened on the UI
thread before the main thread is able to set its pointer to the
widget, then the java GeckoSession will never be notified that the
compositor has been opened, and composition will never be
resumed. This patch fixes this issue by setting the
UiCompositorControllerChild's widget pointer in its constructor rather
than immediately afterwards.
Differential Revision: https://phabricator.services.mozilla.com/D139842
Bug 1744851 removed the need for creating variations using a CGFont.
This removes some of the support code that was left behind and completes
the reverting of the initial fix for bug 1675185: c8875713fd6d193e6b017da0a88c264b7bdd9a78
Differential Revision: https://phabricator.services.mozilla.com/D139886
Bug 1744851 removed the need for creating variations using a CGFont.
This removes some of the support code that was left behind and completes
the reverting of the initial fix for bug 1675185: c8875713fd6d193e6b017da0a88c264b7bdd9a78
Differential Revision: https://phabricator.services.mozilla.com/D139886
The regular scroll position restoration code will restore it.
Specifically, in ScrollFrameHelper::SaveState
https://searchfox.org/mozilla-central/rev/44ae61c5eeb150cb77b6b8511ceee7ddd6892cb7/layout/generic/nsGfxScrollFrame.cpp#7376
we store the visual viewport offset. ScrollFrameHelper::ScrollToRestoredPosition will try to restore both the layout scroll offset and the visual viewport offset by calling ScrollToWithOrigin (which leads to ScrollToImpl) and PresShell::ScrollToVisual respectively.
I observed the following sequence of events when running gfx/layers/apz/test/mochitest/browser_test_background_tab_load_scroll.js in one of the cases that it fails for me locally.
-ScrollFrameHelper::NotifyApzTransaction is called after sending a transaction to webrender, this sets mAllowScrollOriginDowngrade = true.
-the scroll frame is destroyed and re-created (I'm not sure why this destroy and re-create happened, I didn't investigate), we call SaveState and RestoreState. So now mLastScrollOrigin == Other, and mAllowScrollOriginDowngrade == true.
-the test calls ScrollBy, so we get a call to ScrollToImpl with aOrigin == Relative
-aOrigin gets changed to Other here https://searchfox.org/mozilla-central/rev/9b0bdcc37419e6765223358a31a4a54d62e1cd97/layout/generic/nsGfxScrollFrame.cpp#2979
-the code just below that determines that it is not a downgrade, so we update mLastScrollOrigin to Other (not a change) and set mAllowScrollOriginDowngrade = false
-ScrollFrameHelper::NotifyApzTransaction is _not_ called (probably because we are in a background tab), so mAllowScrollOriginDowngrade remains false
-the scroll frame is now at position 20000 and the visual viewport offset (stored on the presshell) is also 20000.
-the scroll frame is destroyed and re-created (this one I investigated, it's because the font subsystem has finished loading and asks for a global reconstruct https://searchfox.org/mozilla-central/rev/9b0bdcc37419e6765223358a31a4a54d62e1cd97/layout/base/PresShell.cpp#9830), we call SaveState and RestoreState. So mLastScrollOrigin == Other, and mAllowScrollOriginDowngrade == true (the same values.
-we get a call to ScrollToRestoredPosition for the newly re-created scroll frame, we are trying to scroll from the current layout scroll position of 0 to the one we had before destruction and re-creation (20000). The visual viewport offset that is stored on the presshell has not been affected, it is still 20000.
-this gets to ScrollToImpl and with aOrigin == Restore
-we arrive at https://searchfox.org/mozilla-central/rev/9b0bdcc37419e6765223358a31a4a54d62e1cd97/layout/generic/nsGfxScrollFrame.cpp#3000 and Restore is a downgrade from Other, and mAllowScrollOriginDowngrade is false so we do not change mLastScrollOrigin (still Other) and mAllowScrollOriginDowngrade remains false
-we arrive at the code in ScrollToImpl that updates the visual viewport offset on the presshell https://searchfox.org/mozilla-central/rev/9b0bdcc37419e6765223358a31a4a54d62e1cd97/layout/generic/nsGfxScrollFrame.cpp#3232
-mLastScrollOrigin is Other, which can clobber apz, so we enter the if and add the difference between the before/after layout scroll position from this ScrollToImpl call (20000-0 = 20000) to the visual viewport offset (20000), getting 40000, which gets clamped to the max ~38000.
-and now the layout scroll position and the visual viewport offset have been detached when they shouldn't be.
If we reset the visual viewport offset when the scroll frame is destroyed we avoid the problem here.
Differential Revision: https://phabricator.services.mozilla.com/D139792
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.
Differential Revision: https://phabricator.services.mozilla.com/D137168
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.
Differential Revision: https://phabricator.services.mozilla.com/D137165
This patch makes us use the same code path for scroll snapping of
zero-velocity OnPanEnd and followed-by-momentum OnPanEnd.
This is preferable because mFollowedByMomentum is unreliable.
Depends on D135921
Differential Revision: https://phabricator.services.mozilla.com/D135922
ScrollSnapUtils::GetSnapPointForDestination has different behavior based on
the scroll unit. More specifically, the difference is in CalcSnapPoints::AddEdge:
Unless the unit is DEVICE_PIXELS, this function will only look for snap
points in the scroll direction. For example, if we are currently at position 40,
our destination is 50, and we have snap points at 30 and 100, then we'll only
consider snap point 100, even though snap point 30 would be closer to the destination.
Furthermore, if the destination is at the same position as the current position,
then we will not snap at all.
This behavior makes some sense for discrete-tick wheel scrolling, but I don't think
it is appropriate for touchpad scrolling. ScrollSnapToDestination is called from
OnPanMomentumStart and from AttemptFling; in both these contexts I think it makes
more sense to take all snap points into account.
With this change, it becomes possible to call ScrollSnapToDestination even when
our current velocity is zero. Without this change, we wouldn't scroll snap at
all in that case.
Depends on D135920
Differential Revision: https://phabricator.services.mozilla.com/D135921
Scroll snapping uses an MSD smooth scroll to animate to the target position.
During the scroll snap animation, any synthesized momentum events
from the OS should be ignored; the scroll snap animation has taken over.
It is possible that this patch has unfortunate consequences when mixing
keyboard and touchpad scrolling. For example, pressing space to scroll down
by a page and then doing a fling with the touchpad might now have different
results.
Differential Revision: https://phabricator.services.mozilla.com/D135920
I had assumed in bug 1750858 that we would only need to modify RenderBufferTextureHost.
It turns out macOS will also still use the ffmpeg decoder and so RenderExternalTextureHost
must report cropped YCbCr sizes as well.
Differential Revision: https://phabricator.services.mozilla.com/D139679
RecvClearCachedResources() does not handle parent commands. They are handled by a different transaction. It caused the problem. To avoid the problem, modifying RecvClearCachedResources() as to handle parent commands.
Differential Revision: https://phabricator.services.mozilla.com/D139568
* Add support for local scale factors to a surface, allowing it to
be rasterized in root coordinate space. This allows snapping to
work across surfaces where the surface transform is a fractional
offset.
* Calculate scaling factors per rasterized surface and propagate
them. Ensures correct scale factor calculations when dealing with
nested preserve-3d contexts with 90-degree axis rotations.
* Support determining exact surface device rect for 2d surfaces
with fractional surface transforms.
* Fix line decoration cache key size calculations based on world
scaling factor.
* Remove `get_clipped_device_rect` usage for calculating clip-mask
surface allocations, use `surface.get_surface_rect` instead. The
prior method doesn't correctly account for expanded local regions
from the current dirty rect, resulting in invalidation issues in
some animated edge cases. Also unifies the way clip-mask surface
allocations work with the way general render target surface
allocations work.
Differential Revision: https://phabricator.services.mozilla.com/D138982
