1004294 - [AccessFu] Explore by touch in B2G is sluggish

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Eitan Isaacson [:eeejay]]

From some profiling, it seems like the bottleneck is the message sending, not the actual moveToPoint traversal.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Improve explore by touch performance.

Here is a WIP. Marco, want to give it a spin? Extra bonus if someone tests this on Android, may need sighted assistance to confirm that finger is actually on correct element in Android.

Comment 2

[Marco Zehe (:MarcoZ)]

Comment on attachment 8415660 [details] [diff] [review]
Improve explore by touch performance.

I actually tested Android first, because that build setup is currently working for me, whereas B2G isn't, or rather, I forgot how to do this magical invisible VM thing Yura installed for me. :-)

And on Android, things are definitely not looking so good. :-( Exploring by touch is sluggish or lags behind. It feels like TalkBack events get queued up and lag behind by one or two items minimum. Only if I pause exploration has AccessFu a chance to catch up.

Minusing feedback based on this.

Comment 3

[Eitan Isaacson [:eeejay]]

Attachment: Improve explore by touch performance.

Unfortunately the top-level chrome window in Android has some peculiarities that make it hard to send mousemove events. Specifically, its viewport "size" is smaller than the content size that is displayed, because the resolution is independant, or something. Anyway, I was forced to use separate code paths for android and the rest.

Comment 4

[Marco Zehe (:MarcoZ)]

Comment on attachment 8424141 [details] [diff] [review]
Improve explore by touch performance.

f=me for this one! Works nicely on both FF OS and Android, the latter on both Nexus 7 2012 and 2013. Nice speed improvement on all tested devices!

Comment 5

[Eitan Isaacson [:eeejay]]

Attachment: Improve explore by touch performance.

Oops, I forgot to stage the changes in the patch. This is the correct one.

Comment 6

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8424966 [details] [diff] [review]
Improve explore by touch performance.

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

AFAICT there's a name collision in CC (+ a couple of comments).

::: accessible/src/jsat/AccessFu.jsm
@@ +917,5 @@
>      aEvent.stopPropagation();
>    },
>  
>    moveToPoint: function moveToPoint(aRule, aX, aY) {
> +    if (Utils.MozBuildApp === 'mobile/android') {

Need a comment here (similar to the comment #3)

@@ +923,5 @@
> +      mm.sendAsyncMessage('AccessFu:MoveToPoint',
> +        {rule: aRule, x: aX, y: aY, origin: 'top'});
> +    } else {
> +      let win = Utils.win;
> +      let wu = win.QueryInterface(Ci.nsIInterfaceRequestor).

Since we already have a getter for dpi, that resolves nsIDOMWindowUtils, lets have a getter for that in utils.

@@ +924,5 @@
> +        {rule: aRule, x: aX, y: aY, origin: 'top'});
> +    } else {
> +      let win = Utils.win;
> +      let wu = win.QueryInterface(Ci.nsIInterfaceRequestor).
> +      getInterface(Ci.nsIDOMWindowUtils);

Nit: indentation.

::: accessible/src/jsat/ContentControl.jsm
@@ +132,5 @@
> +        { json: { x: aEvent.screenX, y: aEvent.screenY, rule: 'Simple' } });
> +    }
> +  },
> +
> +  handleMoveToPoint: function cc_handleMoveToPoint(aEvent) {

There's already a handleMoveToPoint method in cc.

Comment 7

[Eitan Isaacson [:eeejay]]

Attachment: Improve explore by touch performance.

Comment 8

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8425754 [details] [diff] [review]
Improve explore by touch performance.

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

Great, r=me with just 1 comment.

::: accessible/src/jsat/AccessFu.jsm
@@ +1006,5 @@
>      let horizontal = aDetails.horizontal;
>      let page = aDetails.page;
>      let p = AccessFu.adjustContentBounds(aDetails.bounds, Utils.CurrentBrowser,
>                                           true, true).center();
>      let wu = Utils.win.QueryInterface(Ci.nsIInterfaceRequestor).

wu is now not necessary.

Comment 9

[Eitan Isaacson [:eeejay]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2219d94afa5a

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2219d94afa5a