1897567 - GeckoView should not ask APZ for an InputResult for events other than MotionEvent.ACTION_DOWN

Status: RESOLVED FIXED

(Firefox for Android :: Browser Engine, task, P2)

Description

[Botond Ballo [:botond]]

In bug 1891094, I discovered that, since bug 1807073, GeckoView sometimes asks APZ for an InputResult for motion events of type MotionEvent.ACTION_MOVE, not just of type MotionEvent.ACTION_DOWN.

This is something that APZ does not expect, and I suspect that it may be the cause of some of the pull-to-refresh bugs blocking bug 1882722.

I'm filing this bug to track revising the GeckoView code to go back to only asking APZ for an InputResult on ACTION_DOWN events. More precisely, the revision will likely need to be made in the android-components layer, to only call the onTouchEventForDetailResult GeckoView API for ACTION_DOWN events (likely involving a modification of this call site.

Alternatively, if we find that asking for an InputResult for ACTION_MOVE events is sometimes necessary, we need to decide what the expected result is in various scenarios, and then audit the APZ code to make sure it sends the expected result.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

My best guess here is probably we need to do the same thing what this code block does in the MotionEvent.ACTION_DOWN case.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1897567 - Invoke onTouchEventForDetailResult just once for each touch block. r?botond!,tthibaud!

Comment 3

[Hiroyuki Ikezoe (:hiro)]

D213695 is a minimum change but it fixes bug 1891094. What I am concerned is, I am not confident enough not to regress, normally our existing automated test cases makes me confident, but unfortunately in this specific case, changes in Fenix (to be precise, it's in android-components) there's no tests. I can't event add the reduced test case I attached in bug 1891094 comment 21. :/

Comment 4

[Polly [:polly]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

D213695 is a minimum change but it fixes bug 1891094. What I am concerned is, I am not confident enough not to regress, normally our existing automated test cases makes me confident, but unfortunately in this specific case, changes in Fenix (to be precise, it's in android-components) there's no tests. I can't event add the reduced test case I attached in bug 1891094 comment 21. :/

i would love to help progress this work if possible!
But without existing tests, it sounds very difficult to be sure that we haven't broken anything.
Do you have any ideas on how we could gain confidence that this patch works well?
eg. a list of bugs / scenarios that we could manually retest?

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Thanks Polly. But the comment is outdated. I realized that some of tests exist in android-components directory and I have modified them in D213695.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de69f60eb1f8
Invoke onTouchEventForDetailResult just once for each touch block. r=botond,tthibaud,android-reviewers

Comment 7

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/de69f60eb1f8