1742985 - Pinch zooming doesn't work after GPU process crash

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[Jamie Nicol [:jnicol]]

This is especially noticeable now that I have been testing the GPU process on Android, as pinch zooming is very common. But it also affects windows, and linux if you force-enable the GPU process.

STR:

1. Open any page, eg mozilla.org
2. Pinch zooming works as expected
3. Trigger a GPU process crash
4. Attempt to pinch zoom, but it doesn't work.

With some logging I have determined that AsyncPanZoomController::UpdateZoomConstraints() does not get called after the GPU process crash, meaning mZoomConstraints.mAllowZoom remains false, and as a result pinch gestures are ignored by the pan zoom controller.

UpdateZoomConstraints appears to get called in the first place (prior to GPU process crash) due to a first paint event in ZoomConstraintsClient. Perhaps we need to find a way to trigger that event from BrowserChild::ReinitRendering(), or something like that.

Comment 1

[Timothy Nikkel (:tnikkel)]

There's a patch in https://phabricator.services.mozilla.com/D113608 for refreshing the zoom constraints when a tab is dragged to a new tab, probably doesn't fix this, but a similar type of problem.

Comment 2

[Jamie Nicol [:jnicol]]

Attachment: Bug 1742985 - Ensure ZoomConstraints get refreshed after GPU process restart. r?tnikkel

Following a GPU process restart ZoomConstraints do not currently get
set for the newly recreated APZCTreeManagers, meaning it is no longer
possible to asynchronously zoom pages.

To solve this, we make ZoomConstraintsClient observe a new
"compositor-reinitialized" topic. We send this notification in
GPUProcessManager::HandleProcessLost() to notify
ZoomConstraintsClients for parent process documents, and in
ContentChild::RecvReinitRendering() for documents in their respective
content processes. This must be performed after the compositor has
been reinitialized so that the APZCTreeManagerChild is able to send
the constraints to the APZCTreeManagerParent in the compositor
process.

Comment 3

[Jamie Nicol [:jnicol]]

Attachment: Bug 1742985 - Add more methods to nsIGfxInfo for GPU process testing. r?tnikkel

Add KillGPUProcessForTests, which kills the GPU process without
generating a crash dump (unlike the existing CrashGPUProcessForTests).

Additionally add EnsureGPUProcessReadyForTests, which returns a
promise that resolves to true when the GPU process is enabled and
ready, and false if it is disabled. If called while the GPU process is
being (re)started, it will not resolve until it has finished launching
(or was disabled due to error).

Finally, make GPUProcessHost::IsConnected check whether the process
handle is valid. This ensures it returns false immediately following a
call to KillProcess but prior to the GPUChild being destroyed. This
means tests can call EnsureGPUProcessReadyForTests immediately after
KillGPUProcessForTests or CrashGPUProcessForTests, and it will
reliably wait for the new process to launch, as intended.

Depends on D135207

Comment 4

[Jamie Nicol [:jnicol]]

Attachment: Bug 1742985 - Add test to ensure zooming works correctly following GPU process restart. r?tnikkel

Depends on D135328

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/441e0b2582cc
Ensure ZoomConstraints get refreshed after GPU process restart. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/6bdd63376651
Add more methods to nsIGfxInfo for GPU process testing. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/9de7d3c3aa74
Add test to ensure zooming works correctly following GPU process restart. r=tnikkel

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/441e0b2582cc
https://hg.mozilla.org/mozilla-central/rev/6bdd63376651
https://hg.mozilla.org/mozilla-central/rev/9de7d3c3aa74