On Android we have long since calculated the compositor widget size
from the Surface rather than using the main thread widget size. This
is to avoid a trip via the main thread in response to a
ResumeAndResize event on the UI thread.
AndroidCompositorWidget::mClientSize therefore gets calculated when
the compositor is resumed.
However, it is possible in some circumstances for the compositor to
receive a display list prior to it being resumed. In this bug's case,
SyncResumeAndResize is getting called before the UI thread has been
notified that the compositor has been initialized. It therefore cannot
resume the compositor, and we instead resume the compositor on the
subsequent NotifyCompositorCreated call. This starts a race between
the main thread paint and NotifyCompositorCreated being scheduled on
the UI thread then resuming the compositor via
PUiCompositorController. If we receive
WebRenderBridgeParent::RecvSetDisplayList prior to
UiCompositorControllerParent::RecvResumeAndResize, then
AndroidCompositorWidget::mClientSize will be zero. This results in us
setting zero-sized webrender scene rect, leading to webrender exiting
early during rendering, resulting in a black screen.
To fix this, allow the main thread to set the AndroidCompositorWidget
size, in addition to the UI thread being able to set it. We do so by
adding a NotifyClientSizeChanged method to
PlatformCompositorWidgetDelegate, called from
nsWindow::OnSizeChanged. The cross-process implementation of this uses
an IPDL call on PCompositorWidget, which shares a top-level protocol
with PWebRenderBridge, meaning calls are guaranteed to occur in
order. This means a resize event on the main thread is guaranteed to
set the CompositorWidget size before the display list from the
subsequent paint is received.
Differential Revision: https://phabricator.services.mozilla.com/D144594
This adds new version of the GeckoView API
GeckoDisplay.surfaceChanged(), which takes a single argument of a new
type GeckoDisplay.SurfaceInfo. As well as containing fields for each
the the existing surfaceChanged() arguments, this has an additional
SurfaceControl field. This must be provided when rendering in to a
SurfaceView on SDK level 29 or greater. On earlier SDK levels, or when
rendering in to a TextureView or SurfaceTexture, this can be
null. SurfaceViewWrapper and GeckoView classes are updated to handle
this correctly. The old surfaceChanged() methods have been deprecated,
and tests have been updated to use the new version.
When provided, the SurfaceControl is passed along with the Surface
through to the widget and, when enabled, over to the GPU process. The
compositor widget then creates a child Surface from that
SurfaceControl, and renders in to that child Surface rather than the
parent one.
This works around a bug on Android 12 where following the GPU process
dying the Surface was left in an unusable state, meaning subsequent
attempts to initialize a compositor would fail. Because the Surface is
now created by the GPU process it gets destroyed when the process
dies, therefore a new Surface can successfully be created when we
reinitialize the compositor.
Differential Revision: https://phabricator.services.mozilla.com/D143485
For example, the SDK class android.media.MediaDrm$KeyStatus will now
be defined as MediaDrm::KeyStatus rather than just KeyStatus.
Not only does this avoid polluting the top-level namespace, but it
also avoids a bug where invalid type names were generated if the
nested class contains a method with a parameter or return of the outer
class' type.
Differential Revision: https://phabricator.services.mozilla.com/D143043
In bug 1756700 we delayed initializing the compositor on android. A
consequence of this is that when
LayerViewSupport::SetDefaultClearColor() gets called
mUiCompositorControllerChild is still null, meaning we skip setting
the compositor's clear color. As a result, dark-mode fenix users have
once again started seeing white flashes while waiting for the page to
load.
Additionally, when the compositor is re-initialized (following a
fallback to software rendering, or a GPU process restart) we do not
set the clear color for the new compositor, which again can lead to
white flashes.
To fix both of these issues, this patch makes us cache the color value
passed to LayerViewSupport::SetDefaultClearColor(). Then whenever
LayerViewSupport::NotifyCompositorCreated() is called, we call
UiCompositorControllerChild::SetDefaultClearColor() to ensure the new
compositor uses the correct clear color.
Differential Revision: https://phabricator.services.mozilla.com/D141730
In bug 1756700 we delayed initializing the compositor on android. A
consequence of this is that when
LayerViewSupport::SetDefaultClearColor() gets called
mUiCompositorControllerChild is still null, meaning we skip setting
the compositor's clear color. As a result, dark-mode fenix users have
once again started seeing white flashes while waiting for the page to
load.
Additionally, when the compositor is re-initialized (following a
fallback to software rendering, or a GPU process restart) we do not
set the clear color for the new compositor, which again can lead to
white flashes.
To fix both of these issues, this patch makes us cache the color value
passed to LayerViewSupport::SetDefaultClearColor(). Then whenever
LayerViewSupport::NotifyCompositorCreated() is called, we call
UiCompositorControllerChild::SetDefaultClearColor() to ensure the new
compositor uses the correct clear color.
Differential Revision: https://phabricator.services.mozilla.com/D141730
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
With the later patch in this series to delay initializing the
compositor, we started crashing in ScreenshotTest#giantScreenshot due
to webrender's window size sanity check.
This check panics early if we attempt to render an area larger than
webrender can handle (rather than panicing internally in
webrender). However, this test deliberately creates a 999999x999999
sized window, to ensure that attempting to allocate a bitmap this size
for a screenshot results in an out of memory exception.
Previously this test only succeeded because we created the compositor
early with a default size of 0x0, whereas now we create it after the
widget has its very large size. Additionally, the test completes
before we have a chance to render anything, otherwise it would indeed
have crashed.
To ensure users do not hit the panic in the wild, in bug 1653649 we
added the necessary limit to the default widget size constraints,
ensuring we never create widgets that are too large. On Android,
however, we do not use the size constraints, so this had no effect.
This patch starts applying size constraints to android widgets,
meaning we do not attempt to render too large an area, and webrender
does not panic. The test still attempts to allocate a large bitmap,
and therefore still throws an out of memory exception and passes.
Differential Revision: https://phabricator.services.mozilla.com/D140050
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
If the android system kills the GPU process to free memory while the
app is in the background, then we want to avoid immediately restarting
the GPU process.
To achieve this, we make GPUProcessManager keep track of whether it is
in the foreground or background. If HandleProcessLost() gets called
while in the background then we destroy the existing compositor
sessions as before, but return early instead of immediately
relaunching the process. If the process has not been launched when the
app later gets foregrounded then we do so then.
The final part of HandleProcessLost(), which reinitializes the content
bridges and emits the "compositor-reinitialized" signal, has been
moved to a new function ReinitializeRendering(). If the GPU process
has been disabled, this gets called as-before at the end of
HandleProcessLost(). When the GPU process is enabled, however, we now
call it from OnProcessLaunchComplete(), so that it gets called
regardless of whether the process is launched immediately or after a
delay.
While we're here, rename the functions RebuildRemoteSessions() and
RebuildInProcessSessions() to DestroyRemoteCompositorSessions() and
DestroyInProcessCompositorSessions(), to better reflect what they
actually do: the "rebuilding" part occurs later on. Also update the
mega-comment documenting the restart sequence, as it was somewhat
outdated.
In case a caller of EnsureGPUReady() gets called before the foreground
signal arrives (eg in nsBaseWidget::CreateCompositorSession() due to a
refresh tick paint), make EnsureGPUReady() launch the GPU process
itself if the GPU process is enabled but not yet launched. As a
consequence, to avoid launching the GPU process unnecessarily, change
a couple callers of EnsureGPUReady() to simply check whether the
process is enabled instead.
Additionally, guard against a null pointer deref if the compositor has
been destroyed when the widget receives a memory pressure event. This
is now more likely to occur as there may be a gap between the
compositor being destroyed and recreated.
Differential Revision: https://phabricator.services.mozilla.com/D139042
Rather than using a separate function
APZCTreeManager::AddInputBlockCallback(), allow an optional callback
argument to be passed to APZInputBridge::ReceiveInputEvent().
This avoids a race condition where the input block is processed before
the callback has a chance to be added to the input queue, meaning the
callback will never be fired. With the GPU process enabled the added
latency of adding the callback cross-process meant this occured
frequently, causing intermittent junit test failures.
This works similarily to before, with the in-process APZCTreeManager
implementation simply calling InputQueue::AddInputBlockCallback() and
the InputQueue will fire the callback when the input block has been
processed. For the cross-process implementation, APZInputBridgeChild
maintains a local map of the callbacks, and APZInputBridgeParent adds
an intermediate callback to its local InputQueue. This intermediate
callback will call APZInputBridgeParent::SendCallInputBlockCallback(),
which tells the APZInputBridgeChild to fire the real callback.
Care must be taken in both APZInputBridgeChild and Parent to only add
their respective callbacks if we determine that it is required, eg not
already handled by the root APZ.
Differential Revision: https://phabricator.services.mozilla.com/D138801
In bug 1750569 we attempted to ensure that following a GPU process
crash outstanding screen pixels requests would be fulfilled. While
this usually worked there was a race condition between sending the
request to the new compositor and the content process sending the
display list to the new compositor, which meant that sometimes we
would screenshot an empty screen instead of the page content.
As a GPU process crash is an extraordinary circumstance and
screenshots are non-critical, the best solution is to simply return an
error if a GPU process crash occurs while there is an outstanding
request (or if a new request is made whilst the GPU process is
restarting). This patch also updates the junit test to check for this
error rather than expecting a screenshot to be returned.
Differential Revision: https://phabricator.services.mozilla.com/D138323
WebRender retains about 50MBs of memory for every window. When switching
between lots of tabs on Android (where 1 tab = 1 window), this can cause
problems as the app will consume a significant amount of memory.
To avoid this problem, we send a memory pressure event whenever a session is
deactivated, which signals to WebRender that it should deallocate the memory.
This message is sent on a delay of 10s to avoid interfering with tab switching,
and we cancel the message if the tab becomes active again before we fire the
memory pressure event.
Co-Authored-By: Cathy Lu <calu@mozilla.com>
Co-Authored-By: Jonathan Almeida [:jonalmeida] <jonalmeida942@gmail.com>
Differential Revision: https://phabricator.services.mozilla.com/D136965
If the GPU process crashes while a screen pixels request is
outstanding, then currently that request will never be completed. This
patch makes it so that we check for outstanding requests when the
compositor is (re)initialized, and send those requests to the new
compositor if they exist.
This includes a refactoring of how the LayerViewSupport class accesses
the UiCompositorControllerChild from the UI thread. Previously it
looked it up through the nsWindow's mCompositorSession whenever it was
required. However, since the compositor session can now be destroyed
and created we instead notify the UI thread whenever that occurs with
LayerViewSupport::NotifyCompositorSessionLost and
LayerViewSupport::NotifyCompositorCreated. The latter of these takes a
reference to the UiCompositorControllerChild, which can safely be used
by the LayerViewSupport on the UI thread.
Differential Revision: https://phabricator.services.mozilla.com/D136151
Since bug 1747116 landed, if the compositor is reinitialized whilst
the Android Surface is invalid, we avoid crashing when querying the
window size and instead keep the compositor in a paused state.
However, in this case we will believe the widget size is 0x0 until the
compositor is eventually resumed. If webrender receives a display list
during this time, it will set an empty view rect. This means when the
compositor is subsequently resumed webrender believes it has nothing
to render, and we get stuck in a state where nothing is ever rendered
to the screen.
This patch initializes the AndroidCompositorWidget with an initial
size, which avoids the problem.
Differential Revision: https://phabricator.services.mozilla.com/D135711
If AndroidCompositorWidget's surface reference points to a surface
that has been destroyed, we can end up with a null ANativeWindow
pointer. This can result in crashes when using it to query the window
size.
This patch makes it so that we use the native window to query the size
only when the surface has changed rather than for every call to
GetClientSize(). This allows us to guard against a null pointer in a
single place. If we have a null pointer then return false from
OnCompositorSurfaceChanged(). CompositorBridgeParent::ResumeComposition()
will handle that by not resuming the compositor, like it already does
if WebRenderBridgeParent::Resume() fails.
Additonally, when we receive a pause event from GeckoView ensure that
we always set the mCompositorPaused flag to true, even if the
UiCompositorController is null. This avoids a possible cause of the
situation described above - if we receive a pause event (eg the app is
minimised) during compositor reinitialization (while the
UiCompositorController is temporarily null) we would not set that flag
to true, and would therefore resume compositing in to an invalid
surface.
Depends on D135117
Differential Revision: https://phabricator.services.mozilla.com/D135118
This patch ensures that, following a GPU process crash, we
re-initialize the compositor and resume painting on Android.
nsWindow::GetWindowRenderer() is made to always reinitialize the
window renderer if there is none, like on other platforms. We
therefore no longer need to track whether webrender is being disabled,
as this is no longer a special case.
Previously we started the compositor as initially paused in
nsBaseWidget::CreateCompositorSession only if the widget did not yet
have a surface. Now we must unconditionally (re)start it as initially
paused, as even though the widget in the parent process may have a
surface, we will not have been able to send it to the GPU process
yet. We will send the surface to the compositor once control flow
returns to nsWindow::CreateLayerManager, where we will also now resume
the compositor if required.
Finally, we must ensure that we manually trigger a paint, both in the
parent and content processes. On other platforms this occurs
automatically following a GPU process loss through various refresh
driver events. On Android, however, nothing causes the refresh driver
to paint by itself, and we cannot receive input without first
initializing our APZ controllers, which does not happen until the
compositor receives a display list. We therefore must manually
schedule a paint. We do so from nsWindow::NotifyCompositorSessionLost
for the parent process, and BrowserChild::ReinitRendering for content
processes.
Differential Revision: https://phabricator.services.mozilla.com/D131232
Declare a GPU process and corresponding Service in the
AndroidManifest. This is of a new class GeckoServiceGpuProcess which
inherits from GeckoServiceChildProcess, and provides a binder
interface ICompositorSurfaceManager which allows the parent process to
set the compositor Surface for a given widget ID, and the compositor
in the GPU process to look up the Surface for a widget ID. The
ICompositorSurfaceManager interface is exposed to the parent process
through a new method getCompositorSurfaceManager() in the
IChildProcess interface.
Add a new connection type for GPU processes to GeckoProcessManager,
along with a function to look up the GPU process connection and fetch
the ICompositorSurfaceManager binder. When the GPU process is launched
we store the returned binder in the GPUProcessHost, and when each
widget's compositor is created we store a reference to the binder in
the UiCompositorControllerChild.
Each nsWindow is given a unique ID, and whenever the Surface changes
due to an Android resume event, it sends the new surface for that ID
to the GPU process (if enabled) by calling
ICompositorSurfaceManager.onSurfaceChanged().
Stop inheriting AndroidCompositorWidget from InProcessCompositorWidget
and instead inherit from CompositorWidget directly. This class holds a
reference to the Surface that will be rendered in to. The
CompositorBridgeParent notifies the CompositorWidget whenever it has
been resumed, allowing it to fetch the new Surface. For the
cross-process CompositorWidgetParent implementation it fetches that
Surface from the CompositorSurfaceManagerService, whereas the
InProcessAndroidCompositorWidget can read it directly from the real
widget.
AndroidCompositorWidget::GetClientSize() can now calculate its size
from the Surface, rather than racily reading the value from the
nsWindow. This means RenderCompositorEGL and RenderCompositorOGLSWGL
can now use GetClientSize() again rather than querying their own size
from the Surface.
With this patch, setting layers.gpu-process.enabled to true will cause
us to launch a GPU process and render from it. We do not yet
gracefully recover from a GPU process crash, nor can we render
anything using SurfaceTextures (eg video or webgl). Those will come in
future patches.
Differential Revision: https://phabricator.services.mozilla.com/D131231
Most of our JNI code either holds strong references to Java objects, i.e. the
Gecko Garbage Collector is responsible for clearning the object, or weak
references to Java code, i.e. the Java GC is responsible for clearing the
object.
Some objects, however, are special and need manual care. These objects
implement the `onWeakNonIntrusiveDetach` method, which is manually triggered by
calling `Detach`.
My understanding is that these objects need to be cleared from either the Java
side (when the GC destroys the GeckoSession) or from the C++ side, when the
window associated to a GeckoSession is closed.
This is done to avoid holding off to a window for longer than necessary, since
it's very expensive to do so.
The intermittent subject of this bug was caused by us not clearing the
Compositor object when the window closes on the Gecko side, letting the Java
side call `syncPauseCompositor` on a dead JNI object.
This is the before code of the Compositor's `onWeakNonIntrusiveDetach`:
```
void OnWeakNonIntrusiveDetach(already_AddRefed<Runnable> aDisposer) {
RefPtr<Runnable> disposer = aDisposer;
if (RefPtr<nsThread> uiThread = GetAndroidUiThread()) {
// ...
uiThread->Dispatch(NS_NewRunnableFunction(
"LayerViewSupport::OnWeakNonIntrusiveDetach",
[compositor, disposer = std::move(disposer),
results = &mCapturePixelsResults, window = mWindow]() mutable {
if (auto accWindow = window.Access()) {
// ...
compositor->OnCompositorDetached();
}
disposer->Run();
}));
}
```
As you can see from the above, the `OnCompositorDetached` method, which informs
the Java layer that the compositor should not be called anymore, was only
invoked when `window.Access()` returns successfully, i.e. when we have a
window that is alive.
Normally, a Compositor object always has a window associated to it, as there's
a strong link between the GeckoSession and the Compositor object on the Java
side.
There are, however, some edge cases (mostly during shutdown) where the Java
code holds a reference to the Compositor but not the GeckoSession. In those
cases, we would fail the following check
```
if (auto accWindow = window.Access()) {
```
And fail to call `OnCompositorDetached`.
We don't really need the window to call `OnCompositorDetached`, as explained
above, so we can just move the call outside the `if` statement to fix this
problem.
Differential Revision: https://phabricator.services.mozilla.com/D130101
* Per advice from Emilio on Matrix, I consolidated overflow checks into `ScrollFrameHelper::GetOverflowState()`.
* In `ScrollFrameHelper::ReflowFinished` we detect the condition requring the app to expand the toolbar.
(Hiro, I know that you suggested a second place to detect this. If you feel that it is important enough to add that,
we'd prefer filing a follow-up bug in Layout for that case that your team can follow up on.)
* We then propagate the notification through `PresShell`, up through `PBrowser`, through the `nsWindow`, then into the `GeckoSession`
* We invoke a new method on the `ContentDelegate`. This seemed like the reasonable delegate to use given other existing
callbacks in the similar vein (such as going fullscreen), but let me know if this should go elsewhere.
* We update GVE and JUnit tests to test this.
Differential Revision: https://phabricator.services.mozilla.com/D120499
While I evaluated multiple approaches to this problem, I selected two:
The first is to handle the case in `GeckoSession.load` directly from within
Java, as this allows us to avoid a Java to Native string conversion, as well as
giving us better opportunities to report the exact nature of the failure back to
the embedding app.
We add a new error code specifically for this case.
In the case where no `NavigationDelegate` is registered, I used
`GeckoSession.load` throwing an exception as a crude way of notifying the app.
In the case where the load request originates from within web content, we deny
the request from within `nsWindow` to avoid requiring any conversion of the
URI spec from native into Java.
Differential Revision: https://phabricator.services.mozilla.com/D109427
We do want APZEventResult to have a valid mHandledResult in the case of
nsEventStatus_eConsumeDoDefault.
Note that when we call SetStatusAsConsumeDoDefault() with a InputBlockState,
in ReceiveScrollWheelInput() for example, we need to keep the block alive there,
so each block is now RefPtr-ed instead of a raw pointer in such functions (the
raw pointer is sometimes the active one (mActiveWheelBlock etc.) which will be
discarded in ProcessQueue()).
Differential Revision: https://phabricator.services.mozilla.com/D103417
