1130011 - Touch listeners in bootstrap.js can slow down APZ responsiveness

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

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

With parent-process APZ and (hopefully soon) the input-thread work landing, it's more important than ever to not have spurious touchstart and touchmove listeners covering large parts of the gaia UI. Because gecko cannot know ahead of time if the listeners are going to call preventDefault() or not on the event, it must wait for the listeners to run before it can do any processing on those events, such as panning/clicking. Therefore adding touchstart and touchmove listeners on things like the root window in gaia will directly have a negative performance impact when it comes to responsiveness.

This bug is specifically about the touchstart listeners registered in bootstrap.js [1] for (according to the comment) working around some hardware issue on some devices. I would like to see this removed, and if necessary, replaced with a workaround in the widget/gonk code. If that is not doable, then I would rather see a 1x1 div positioned at 0,0 to eat these touch events, rather than have the listener registered on the window, which will intercept *all* touch events.

[1] https://github.com/mozilla-b2g/gaia/blob/ded85e94a325d1546d5c46fbe68153ddd7671181/apps/system/js/bootstrap.js#L270

Comment 1

[GitHub Autolander Bot]

Attachment: [PullReq] etiennesegonzac:bug-1130011 to mozilla-b2g:master

Comment 2

[Etienne Segonzac (:etienne)]

Comment on attachment 8559912 [details] [review]
[PullReq] etiennesegonzac:bug-1130011 to mozilla-b2g:master

Yep, confirmed that it's not happening on the flame, I vote for removing it entirely.

And if we ever reproduce it on a device adding a 1by1 div sounds like a good fix.

Comment 3

[Kevin Grandon :kgrandon]

Comment on attachment 8559912 [details] [review]
[PullReq] etiennesegonzac:bug-1130011 to mozilla-b2g:master

Sounds good to me.

Comment 4

[GitHub Autolander Bot]

https://github.com/mozilla-b2g/gaia/pull/27979

The pull request could not be applied to the integration branch. Please try again after current integration is complete.

Comment 5

[GitHub Autolander Bot]

https://github.com/mozilla-b2g/gaia/pull/27979

The pull request could not be applied to the integration branch. Please try again after current integration is complete.

Comment 6

[Kevin Grandon :kgrandon]

Sorry, this is happening because the tree is closed and autolander doesn't know any better =( Please wait for the tree to re-open and try again.

I've filed bug 1130433 to address this.

Comment 7

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/33e864db8d1576eeef537c26af89f7a53b85213a

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Reverted for the same Gaia unit test failures that were visible in the Gaia Try runs.
Master: https://github.com/mozilla-b2g/gaia/commit/a0e71e61922bde009a3b6714cbe965021d3279f1

https://treeherder.mozilla.org/logviewer.html#?job_id=499746&repo=gaia-try

Comment 9

[Kevin Grandon :kgrandon]

Sorry, autolander does not yet run *all* test suites, so we still need to check gaia-try. I've filed bug 1130549 to make it more clear on where we stand on enabling these suites.

Comment 10

[GitHub Autolander Bot]

Attachment: [PullReq] etiennesegonzac:bug-1130011 to mozilla-b2g:master

Comment 11

[Etienne Segonzac (:etienne)]

Comment on attachment 8561272 [details] [review]
[PullReq] etiennesegonzac:bug-1130011 to mozilla-b2g:master

Carrying the r+, sorry for the mess.

Comment 12

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/09e5f0b133b6d6021548b3e3fe8326d9e9a1420c

Comment 13

[Gregor Wagner [:gwagner]]

Do we want this on 2.2 as well or is it too late now?

Comment 14

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

It's probably better to leave it out at this point. By itself it won't do anything, we'd need to uplift bug 1130016 with it. And think there were some regressions from this (that we fixed and uplifted) but still, there is some risk it will expose other bugs in 2.2 that I'd rather not deal with.