Closed Bug 1500565 Opened 3 years ago Closed 3 years ago

apz_test_native_event_utils.js's synthesizeNativeTouchSequences intermittently causes Assertion failure: !mTargetDuration.IsZero(), at /gfx/layers/apz/src/AndroidFlingPhysics.cpp:158

Categories

(Core :: Panning and Zooming, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: JanH, Assigned: botond)

Details

Attachments

(1 file)

Curious. Looking at how mTargetDuration is computed, it seems the only way it would be zero is if the incoming mVelocity is zero, but we assert that three lines earlier.
Hmm... in the Try push from comment 0, I retriggered the Android debug test chunk that runs test_session_scroll_visual_viewport.html (chunk c8) 10 times, but it didn't run into the assertion failure. I wonder if it's something that only happens locally, or perhaps it's just a really low-frequency intermittent?
For me it wasn't *that* low-frequency when running locally, but it's possible that the timings on the emulator are different. Maybe this happens if the simulated events are sent too rapidly?
Ok, I pulled the patches from Try and ran them locally, and triggered the bug after several retries.

When mTargetDuration is computed as 0, the incoming value of mVelocity was "inf". This suggests that we synthesized two touch events with different positions but the same timestamp.

Looking at the implementation of synthesizeNativeTouchSequences(), the events are not assigned a timestamp (and indeed, the nsIDOMWindowUtils.sendNativeTouchPoint() API that they use doesn't have such an option). Rather, the events pick up the current time as their timestamp in widget code.

This means that if the synthesization code runs sufficiently fast, we can end up with two subsequent events with the same timestamp. (This also explains why we don't see the bug in automation: the emulator is probably too slow for that to happen.)

Some possible fix options:

  1) Have synthesizeNativeTouchSequences() explicitly wait 1 ms or so
     between each touch point, so ensure they have distinct timestamps.

     This would be straightforward, but could slow down tests too much.

  2) Allow the synthesizing code to choose timestamps for the events
     (that don't have to correspond to wallclock-time).

     This would be the most general solution, but it would require
     plumbing the timestamp through a lot of places (nsIDOMWindowUtils,
     nsIWidget, various widget backends).

  3) Check for a velocity of "inf" in APZ code and don't start a fling
     in that case. Presumably, the tests that trigger this don't
     actually need a fling to take place, and if they do, they can
     do something else like option (1) above.

I'm leaning towards option (3), but I'm open to arguments otherwise.
The posted fix implements option (3).
Assignee: nobody → botond
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/58b7faab3b8d
Avoid start a fling animation with infinite velocity. r=kats
https://hg.mozilla.org/mozilla-central/rev/58b7faab3b8d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Thank you.
No worries, thanks for working on bug 1498812!
You need to log in before you can comment on or make changes to this bug.