1479039 - Implement focus and accessible focus cache

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Eitan Isaacson [:eeejay]]

To optimize TalkBack performance we can mutate a sparse tree on each accessible event before it is dispatched. This change will replace the AccessFu Presentation/OutputGenerator modules.

Going forward, AccessFu would only be dealing with input and presentation will all be native. Maybe further down the road the controller will be native as well.

Comment 1

[Eitan Isaacson [:eeejay]]

Store a cache of the focused or accessible focused node and it's parentage. This cache will get invalidated on any focus/accessible focus change.

Unlike the viewport cache in bug 1502187, this cache will have few children, but be highly specific on each child: bounds, text, boolean flags, collection/range info, etc.

A union of both caches (with this one probably taking precedence) will give accessible services all the info they generally need.

Comment 2

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1479039 - Move PersistentPropertiesToArray to nsAccUtils. r?Jamie!

We currently convert the proxy output to XPCOM property bags and
override Attributes(). Since we will be using the arrays a lot more,
switch to them.

Depends on D11210

Comment 3

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1479039 - Refactor ToBundle to have a multiple argument version. r?Jamie!

Depends on D11211

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1479039 - Extend BatchData struct and added focus path cache. r?yzen!

Depends on D11213

Comment 5

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1479039 - Implement java portion of focus path cache. r?snorp!,yzen!

Depends on D11214

Comment 6

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1479039 - Update cached focus path bounds on scroll. r?snorp!,yzen!

Depends on D11215

Comment 7

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1479039 - Update SessionAccessibility cache and state from events. r?snorp!,yzen!

Depends on D11216

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce20e58d20a2
Move PersistentPropertiesToArray to nsAccUtils. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/9e5f05367dde
Refactor ToBundle to have a multiple argument version. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/ef87e3565a4b
Extend BatchData struct and added focus path cache. r=yzen
https://hg.mozilla.org/integration/autoland/rev/90cde1d999d3
Implement java portion of focus path cache. r=snorp,yzen
https://hg.mozilla.org/integration/autoland/rev/b47fdd1194c7
Update cached focus path bounds on scroll. r=snorp,yzen
https://hg.mozilla.org/integration/autoland/rev/234f5f3db94f
Update SessionAccessibility cache and state from events. r=snorp,yzen

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Let's see how this does in Nightly - from bug 1499163 we may want to consider uplift to 64.

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ce20e58d20a2
https://hg.mozilla.org/mozilla-central/rev/9e5f05367dde
https://hg.mozilla.org/mozilla-central/rev/ef87e3565a4b
https://hg.mozilla.org/mozilla-central/rev/90cde1d999d3
https://hg.mozilla.org/mozilla-central/rev/b47fdd1194c7
https://hg.mozilla.org/mozilla-central/rev/234f5f3db94f

Comment 11

[Chris Peterson [:cpeterson]]

Based on bug 1499163 comment 26 and 27, it sounds like we'll want to uplift this fix to Fennec 64 Beta.

Comment 12

[Eitan Isaacson [:eeejay]]

Comment on attachment 9023397 [details]
Bug 1479039 - Move PersistentPropertiesToArray to nsAccUtils. r?Jamie!

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1479037

User impact if declined: Users may potentially experience slowdowns if they have accessibility enabled (screen reader, password manager, etc)

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: Bug 1501496, Bug 1502187, Bug 1507026, Bug 1506709

Risk to taking this patch: Medium

Why is the change risky/not risky? (and alternatives if risky): This feature (caching) is turned off by default, so this should not impact users directly. A followup uplift will turn this feature on by default and we can monitor it in nightly for a bit longer.

String changes made/needed:

Comment 13

[Eitan Isaacson [:eeejay]]

Comment on attachment 9023399 [details]
Bug 1479039 - Refactor ToBundle to have a multiple argument version. r?Jamie!

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: 

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): 

String changes made/needed:

Comment 14

[Julien Cristau [:jcristau]]

Comment on attachment 9023397 [details]
Bug 1479039 - Move PersistentPropertiesToArray to nsAccUtils. r?Jamie!

ok let's get this in for 64.0b12

Comment 15

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2da43796c5bd
https://hg.mozilla.org/releases/mozilla-beta/rev/e3c4ebd80791
https://hg.mozilla.org/releases/mozilla-beta/rev/f5d4bc87f0fd
https://hg.mozilla.org/releases/mozilla-beta/rev/44d90df54ca9
https://hg.mozilla.org/releases/mozilla-beta/rev/857f5b5b5777
https://hg.mozilla.org/releases/mozilla-beta/rev/078a827710bf

Comment 16

[Chris Peterson [:cpeterson]]

status-geckoview64=fixed

Comment 17

[Kevin Brosnan [Ex-Mozilla]]

Release Note Request (optional, but appreciated)
[Why is this notable]: Performance fixes for users who use external password managers on Android.
[Affects Firefox for Android]: Yes
[Suggested wording]: Users who use password manager apps will no longer see hangs due to these apps
[Links (documentation, blog post, etc)]:

Comment 18

[Julien Cristau [:jcristau]]

comment 17