Closed Bug 1130011 Opened 5 years ago Closed 5 years ago

Touch listeners in bootstrap.js can slow down APZ responsiveness

Categories

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

All
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-v2.2 affected, b2g-master fixed)

RESOLVED FIXED
Tracking Status
b2g-v2.2 --- affected
b2g-master --- fixed

People

(Reporter: kats, Assigned: etienne)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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
See Also: → 1130016
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.
Attachment #8559912 - Flags: review?(kgrandon)
Comment on attachment 8559912 [details] [review]
[PullReq] etiennesegonzac:bug-1130011 to mozilla-b2g:master

Sounds good to me.
Attachment #8559912 - Flags: review?(kgrandon) → review+
Keywords: checkin-needed
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.
Keywords: checkin-needed
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.
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.
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
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
Assignee: nobody → etienne
Status: RESOLVED → REOPENED
Flags: needinfo?(etienne)
Resolution: FIXED → ---
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.
Attachment #8559912 - Attachment is obsolete: true
Flags: needinfo?(etienne)
Comment on attachment 8561272 [details] [review]
[PullReq] etiennesegonzac:bug-1130011 to mozilla-b2g:master

Carrying the r+, sorry for the mess.
Attachment #8561272 - Flags: review+
Keywords: checkin-needed
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Depends on: 1135938
Depends on: 1134493
Do we want this on 2.2 as well or is it too late now?
Flags: needinfo?(bugmail.mozilla)
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.
Flags: needinfo?(bugmail.mozilla)
You need to log in before you can comment on or make changes to this bug.