1537958 - Visual viewport resize/scroll events can get lost

Status: RESOLVED FIXED

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

Description

[Kartikaya Gupta (email:[e-mail])]

The mechanism added in bug 1478776 to dispatch the visual viewport scroll/resize events has a flaw because the VisualViewport object is owned by the nsGlobalWindow, and therefore has a lifetime longer than that of the presshell/refreshdriver it uses to dispatch events. So for example the VisualViewport object can set mResizeEvent to new resize event which registers itself with a refresh driver. Then that presShell gets torn down, and so the event never fires, and mResizeEvent is never cleared. Subsequent attempts at firing a resize event will also not work because mResizeEvent is already non-null.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

FWIW I'm seeing this while running ./mach mochitest --verify gfx/layers/apz/test/mochitest/test_group_zoom.html on an android x86 emulator, while investigating intermittent failures in that test. At least some of the failures on file might be caused by this.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=b7b5ca8d73393e5d5082347807a9a6b61ed10f96

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1537958 - Discard and reschedule visual viewport events if the prescontext changes. r?botond

It seems that in some scenarios, the lifetime of the visual viewport
object exceeds the lifetime of the prescontext. In particular, the
prescontext produced by GetPresContext() can be torn down and a new one
installed. This can leave the visual viewport in a wedged state where it
has scheduled events to be dispatched, but they will never actually be
dispatched, and then subsequent events will not get scheduled. This
patch checks to see if the prescontext has changed, and if so, discards
the old events and reschedules new ones.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=eb5a3d92d6563fa367ec1c765262d89fcce59c9e uses a weakptr and is looking much greener

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4d0388ce7f1a
Discard and reschedule visual viewport events if the prescontext changes. r=botond

Comment 6

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/4d0388ce7f1a

Comment 7

[Kartikaya Gupta (email:[e-mail])]

I'm happy to let this ride the trains given the slightly risky nature of the patch