1376538 - APZ: Transmit focus updates more frequently

Status: RESOLVED FIXED

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

Description

[Ryan Hunt [:rhunt]]

Async keyboard scrolling can only happen when APZ has the current focus target. We currently transmit a focus update in nsDisplayList::PaintRoot, but we should be able to transmit more frequently than that.

(see Botond's comment in bug 1351783 comment 168)

Comment 1

[Botond Ballo [:botond]]

The description of this bug made me realize that I actually made a mistake in bug 1351783 comment 168:

(In reply to Botond Ballo [:botond] from comment #168)
> (In reply to Botond Ballo [:botond] from comment #55)
> > > +Bug 1351783 part 10 - Create and sync the current FocusTarget on each layer transaction r?kats
> > > +
> > > +This commit modifies PresShell and nsDisplayList to send a FocusTarget update on
> > > +every layer transaction. Ideally we would like to send updates as often as possible,
> > > +but this seems like it works well. This can be iterated on later, if necessary.
> > 
> > As a lead for future iteration on this: look into "empty transactions" (bug
> > 1257641 is a good place to look), and make sure a focus update is sent with
> > empty transactions too.
> 
> It looks like empty transactions still go through PresShell::Paint(), so
> they will already pick up the focus update with the code as written.

The focus update is sent in nsDisplayList::PaintRoot(), not PresShell::Paint(), is nsDisplayList::PaintRoot() is *not* called for empty transactions (because for empty transactions we early-exit PresShell::Paint() here [1] or here [2], while PaintRoot() is called from PaintFrame() [3]).

So we will need two code changes in this bug:

  1) Sending the focus target as part of an empty transaction

  2) Scheduling an empty transaction when the focus target changes

[1] http://searchfox.org/mozilla-central/rev/b425854d9bbd49d5caf9baef3686e49ec91c17ec/layout/base/PresShell.cpp#6356
[2] http://searchfox.org/mozilla-central/rev/b425854d9bbd49d5caf9baef3686e49ec91c17ec/layout/base/PresShell.cpp#6401
[3] http://searchfox.org/mozilla-central/rev/b425854d9bbd49d5caf9baef3686e49ec91c17ec/layout/base/PresShell.cpp#6426

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: focus-send.patch

I don't really know why we were calling layerManager->SetFocusTarget in nsDisplayList. There isn't any reason for it anymore (there was before), so this patch fixes that, which simplifies some things.

This patch then requests an empty transaction whenever we get a new focus sequence number so that we get focus updates quicker. This can potentially happen frequently, so it's important that we don't spam IPC with updates. I'm told by Botond that using an empty transaction could be a good fit for this.

Comment 3

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

Comment on attachment 8882001 [details] [diff] [review]
focus-send.patch

Review of attachment 8882001 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Might be good to amend the commit message to specifically mention empty transactions.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ec5e890bcbd
Transmit APZ focus target updates on empty transactions. r=kats

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8ec5e890bcbd