1502187 - Introduce viewport accessible caching in android

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Eitan Isaacson [:eeejay]]

The goal of this caching scheme is to serve these cases:
1. Password managers care about the visible content, specifically if there is a pair of plan and password editboxes.
2. Switch access cares about all the interactive object in view
3. BrailleBack cares about the geometries of the adjacent accessibles.

For those cases we can offer a lightweight cache of all the accessibles in the viewport. For now, we will only provide hierarchy, bounds, className and boolean flags. Because it is lightweight invalidating and repopulating the entire cache is relatively cheap. We will invalidate on every scroll (with a throttled interval), and on initial tree creation, and on any tree mutation. We my add more events that invalidate the cache in the future.

Because the cache is invalidated frequently , this should be good enough for accessible services that are not heavily dependent on events or need low latency, like a screen reader.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1502187 - Add Batch protocol to PDocAccessible. r?Jamie!

This protocol is meant to be used by platform wrappers to push bulk data
to the chrome process.

Depends on D9689

Comment 2

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1502187 - Add FULL_ACCESSIBILITY_TREE to GeckoSessionSettings. r?jchen!

Defaults now to 'true', when caching is good enough I'll switch it off.

Generally, this would be used for apps/cases when full accessible tree
is needed like UIAutomator.

Depends on D9864

Comment 3

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1502187 - Implement native part of viewport caching. r?Jamie!

Depends on D9865

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1502187 - Implement Java part of viewport tree caching. r?jchen!

Depends on D9866

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a184f7e259c
Add Batch protocol to PDocAccessible. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/70def5ae1d11
Add FULL_ACCESSIBILITY_TREE to GeckoSessionSettings. r=jchen
https://hg.mozilla.org/integration/autoland/rev/89413425620a
Implement native part of viewport caching. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/fe3f1d254a1f
Implement Java part of viewport tree caching. r=jchen

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7a184f7e259c
https://hg.mozilla.org/mozilla-central/rev/70def5ae1d11
https://hg.mozilla.org/mozilla-central/rev/89413425620a
https://hg.mozilla.org/mozilla-central/rev/fe3f1d254a1f

Comment 8

[Eitan Isaacson [:eeejay]]

Comment on attachment 9020198 [details]
Bug 1502187 - Add Batch protocol to PDocAccessible. 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 1479039, 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 9

[Eitan Isaacson [:eeejay]]

Comment on attachment 9020199 [details]
Bug 1502187 - Add FULL_ACCESSIBILITY_TREE to GeckoSessionSettings. r?jchen!

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1479037

User impact if declined: See previous patch request

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 1479039, Bug 1507026, Bug 1506709

Risk to taking this patch: Medium

Why is the change risky/not risky? (and alternatives if risky): See previous patch request

String changes made/needed:

Comment 10

[Eitan Isaacson [:eeejay]]

Comment on attachment 9020200 [details]
Bug 1502187 - Implement native part of viewport caching. r?Jamie!

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1479037

User impact if declined: See previous

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 1479039, Bug 1507026, Bug 1506709

Risk to taking this patch: Medium

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

String changes made/needed:

Comment 11

[Eitan Isaacson [:eeejay]]

Comment on attachment 9020201 [details]
Bug 1502187 - Implement Java part of viewport tree caching. r?jchen!

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1479037

User impact if declined: See previous

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 1479039, Bug 1507026, Bug 1506709

Risk to taking this patch: Medium

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

String changes made/needed:

Comment 12

[Julien Cristau [:jcristau]]

I'm rather uneasy about taking this set of changes late in the beta cycle.  Is there a viable alternative such as backing out the regressing change?  How many fennec users enable accessibility?

Comment 13

[Eitan Isaacson [:eeejay]]

About 1/4 of our users have a11y enabled, if I am reading telemetry correctly[1]. The absolute majority of those users have a11y enabled because of some anti-malware app (or malware), or a password manager. From their perspective Fennec has a noticeable performance regression in 64. Only a tiny percentage of the a11y users are screen reader, or other assistive technology, users. I am almost certain that this, and the other uplifts I asked for, would have no impact outside of Android/GeckoView.

If we were to back out the patches that introduced this regression (bug 1479037), it would be equally disruptive if not more. It would mean reintroducing a few thousand lines of code.

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2018-11-12&include_spill=0&keys=__none__!__none__!__none__&max_channel_version=beta%252F64&measure=A11Y_INSTANTIATED_FLAG&min_channel_version=null&processType=*&product=Fennec&sanitize=1&sort_keys=submissions&start_date=2018-10-22&table=0&trim=1&use_submission_date=0

Comment 14

[Chris Peterson [:cpeterson]]

64=affected and [geckoview:p1] because this bug blocks Focus 8.0.

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 9020201 [details]
Bug 1502187 - Implement Java part of viewport tree caching. r?jchen!

a11y changes for 64.0b12

Comment 16

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f96dbab7b6a3
https://hg.mozilla.org/releases/mozilla-beta/rev/f75f849333f5
https://hg.mozilla.org/releases/mozilla-beta/rev/b3869ac5fad2
https://hg.mozilla.org/releases/mozilla-beta/rev/26a874969a1e

Comment 17

[Chris Peterson [:cpeterson]]

status-geckoview64=fixed