1435788 - Assertion failure: mLastContentProcessedEvent <= mLastAPZProcessedEvent

Status: RESOLVED FIXED

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

Description

[Aria Beingessner [:Gankra]]

Attachment: crash.txt

Steps to reproduce (that I have definitely checked):

* Launch a debug build of firefox 
* Open a second tab
* Snap that tab to a new window
* Visit another page in the new window/tab (e.g. twitter.com)

Instant 100% Reliable Crash

I am currently seeing this on a local build of 404db9dad9c2517de8cf708779bee2a091020fab (central as of a few hours ago) with some minor patches that shouldn't affect this subsystem. 

This happens with or without webrender.

Comment 1

[Ryan Hunt [:rhunt] (on leave until early May)]

Is this still happening?

Comment 2

[Aria Beingessner [:Gankra]]

(sorry for slow response)

Yep, still reproduces with this morning's central.

Comment 3

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

Most likely we just need to reset one of these keyboard event counters when a tab gets moved to another window. Somewhere in CompositorBridgeParent::RecvAdoptChild is probably the place to do that.

Comment 4

[Ryan Hunt [:rhunt] (on leave until early May)]

I agree. We do try and detect this situation and recover from it when we receive an update, [1] but maybe we should do that in other places.

Unfortunately I was only able to reproduce this once and lost the chance to see where this was happening, by trying it again and then it never reproduced again.

Could you give me a stack trace of where this is crashing? That might help.

[1] https://searchfox.org/mozilla-central/rev/74b7ffee403c7ffd05b8b476c411cbf11d134eb9/gfx/layers/apz/src/FocusState.cpp#129

Comment 5

[Aria Beingessner [:Gankra]]

Attachment: stacktrace.txt

this is what spews on my terminal (I think the WR-related stuff is unrelated, but maybe not?)

Comment 6

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

There's some patches in bug 1441916 that /might/ fix this (low probability). I tried reproducing without those patches and couldn't, so I can't confirm if it fixes it or not.

Comment 7

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

I just hit this on a Windows local debug build after killing the GPU process via about:support, and then reloading about:support by pressing Ctrl+r on the new GPU process. (The keyboard input is probably important here). Seems to be reproducible, I'll see if I can debug it.

Comment 8

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

Actually this might be because I have my changes for bug 1441324 applied.

Comment 9

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

Attachment: Bug 1435788 - Allow handling focus-changing events prior to the first update on GPU process restart.

If the GPU process restarts, the mLastAPZProcessedEvent gets reset to 1.
The code in Update() checks for this special case and resets the value
to the incoming content process sequence number. However, we before that
first call to Update() on the sampler thread, the FocusState might get calls
to ReceiveFocusChangingEvent(), which would be triggered by input events on
the controller thread. These input events would advance
mLastAPZProcessedEvent which would bypass the special case handling in
Update(). This can leave mLastAPZProcessedEvent at a lower value than
mLastContentProcessedEvent which triggers assertion failures.

This patch ensures that calls to ReceiveFocusChangingEvent() during this
initial state doesn't increment mLastAPZProcessedEvent, and so allows
the special case handling in Update() to work as expected. It also adds
the special case handling to the branch where the first Update() call
results in no focus target being selected.

Review commit: https://reviewboard.mozilla.org/r/227704/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/227704/

Comment 10

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4afbb0ebd35953d2bf8b7967c718618a4d21fb2 includes this patch. Pretty sure you can get the same problem when opening a new window, because it creates a new APZCTreeManager with a new FocusState with a APZ sequence number of 1, and that can get incremented by input events before the content sequence number arrives. So although I encountered the problem with a GPU process reset, the patch should fix Alexis' STR as well.

Comment 11

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8958790 [details]
Bug 1435788 - Allow handling focus-changing events prior to the first update on GPU process restart.

https://reviewboard.mozilla.org/r/227704/#review233480

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22f4a3eab719
Allow handling focus-changing events prior to the first update on GPU process restart. r=rhunt

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/22f4a3eab719

Comment 14

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

Alexis, if you get a chance it would be good to check this fixes it for you.