1388466 - Ensure APZ keyboard handling works properly with compositor resets

Status: RESOLVED FIXED

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

Description

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

(In reply to Martin Stránský from bug 1377950 comment #19)
> There's also a crash after the restart when keyboard is hit:
> 
> Thread 1 "firefox" received signal SIGSEGV, Segmentation fault.
> 0x00007fffe0b6860f in mozilla::layers::FocusState::IsCurrent
> (this=0x7fffb4eae8a8)
>     at /home/komat/tmp676-trunk-gtk3/src/gfx/layers/apz/src/FocusState.cpp:31
> 31	  MOZ_ASSERT(mLastContentProcessedEvent <= mLastAPZProcessedEvent);


This seems like a bug. If the GPU process restarts mLastAPZProcessedEvent can get reset to a smaller value than mLastContentProcessedEvent. We should have some mechanism for handling this better.

Comment 1

[Ryan Hunt [:rhunt]]

I wasn't able to reproduce this when debugging bug 1377950. But it seems entirely possible. I'm going to try it on windows too to see if I can reproduce it.

I'm guessing we can reset the content process focus number to 0 when the compositor/APZCTreeManager is deleted and brought back.

Comment 2

[Ryan Hunt [:rhunt]]

I should also note that this isn't just with the compositor process, and could theoretically happen with a device reset, which bug 1377950 kind of is.

Comment 3

[Martin Stránský [:stransky] (ni? me)]

It's 100% reproductible for me:

1) launch browser with only one tab with some internal content inside (about: or "session restore" page after browser start)
2) restart compositor (ALT+F2, type r on gnome-shell)
3) hit ALT+L on browser (I tried to open a new page after compositor restart)

I got that when working on Bug 1377950 - when only internal page is loaded the the compositor restart does not "blank" it, instead I get the keyboard ASSERT() crash.

Comment 4

[Ryan Hunt [:rhunt]]

(In reply to Martin Stránský from comment #3)
> It's 100% reproductible for me:
> 
> 1) launch browser with only one tab with some internal content inside
> (about: or "session restore" page after browser start)
> 2) restart compositor (ALT+F2, type r on gnome-shell)
> 3) hit ALT+L on browser (I tried to open a new page after compositor restart)
> 
> I got that when working on Bug 1377950 - when only internal page is loaded
> the the compositor restart does not "blank" it, instead I get the keyboard
> ASSERT() crash.

Okay those instructions work for me, I'll work on a fix now.

Comment 5

[Ryan Hunt [:rhunt]]

Attachment: Bug 1388466 - Sync focus sequence numbers when the compositor is reset.

When a compositor is recreated, either from a driver reset or a GPU process crash, the
focus sequence number in content persists, and when APZ is recreated, content will have
a higher sequence number.

There are two ways of handling this: either resetting all of content's focus sequence
numbers to zero to match the new focus state, or updating the new focus state to the
latest sequence number in content.

This commit does the latter by detecting when the focus state was newly created and
syncing it's sequence number to content if content is ahead.

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

Comment 6

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

Comment on attachment 8895979 [details]
Bug 1388466 - Sync focus sequence numbers when the compositor is reset.

https://reviewboard.mozilla.org/r/167252/#review172482

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bcc26dc569d
Sync focus sequence numbers when the compositor is reset. r=kats

Comment 8

[Ryan Hunt [:rhunt]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=33140d5eb28bac5ddd189469edbc2c83d5e92b5c

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1bcc26dc569d