Closed Bug 1278009 Opened 9 years ago Closed 9 years ago

Intermittent test_group_touchevents.html | helper_touch_action.html | After fourth diagonal drag, with manipulation - x axis - got 10, expected 50

Categories

(Core :: Panning and Zooming, defect, P3)

Product:
Core
Component:
Panning and Zooming
Version:
49 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla50
Tracking Flags:
Tracking Status
firefox49 --- fixed
firefox50 --- fixed

People

(Reporter: KWierso, Assigned: kats)

References

Details

(Keywords: intermittent-failure, Whiteboard: [gfx-noted])

Attachments

(1 file)

Patch
3.50 KB, patch
botond
: review+
Details | Diff | Splinter Review
Whiteboard: [gfx-noted]
Component: Graphics: Layers → Panning and Zooming
Presumably regression from bug 1275604 which added this test.
Assignee: nobody → bugmail.mozilla
Blocks: 1275604
The Android failures seem to be just because the test takes a long time and times out. We should bump up the timeout on the test based on the number of subtests it's running.
From the logging output it looks like some of the touch events just don't get through, we're in the middle of synthesizing a horizontal drag when the events just stop. There's also a crash stack further down in the log which seems to indicate that the the parent process main thread is stuck in a call to WidgetToScreenOffset which requires some roundtripping to the X server and waiting for a reply. Might be some sort of race condition where if we call this function too frequently a reply gets lost? That would explain why this is only really happening on Linux.
But anyway the failures in the try push are not the same as the failures in orangefactor, so maybe I should just ignore these.
Another try push to try and isolate the Linux x64 TC M-e10s-8 job which seems to have the lion's share of the failures reported in OF (and which, inconveniently, I can't add a job for on my existing try push). https://treeherder.mozilla.org/#/jobs?repo=try&revision=9effe4110ac1
I haven't been able to reproduce this failure in over 100 runs of the APZ mochitests with rr chaos mode. The try push above is also turning up quite empty so far. I might have to land more logging into the tree.
Priority: -- → P3
Tried another try push with logging, still no repros: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dcb4e039d24a I suspect the logging is adjusting the timing sufficiently that the test always passes. But I'm surprised that I wasn't able to reproduce it with rr chaos mode.
Ok, I trimmed down the logging and got some useful output in https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ca3f34cf260&selectedJob=23449599 It seems to indicate that after setting the touch-action to 'manipulation' at [1], the next touch drag still uses 'none' as the touch-action value. So either the waitForAllPaints/flushApzRepaints wasn't sufficient to flush the event regions to the compositor, or the compositor hit-test didn't pick it up properly. [1] http://searchfox.org/mozilla-central/rev/a7c8e9f3cc323fd707659175a46826ad12899cd1/gfx/layers/apz/test/mochitest/helper_touch_action.html#82
Looks like the event regions don't get flushed to the compositor based on the logging in https://treeherder.mozilla.org/#/jobs?repo=try&revision=25f3ba340c22&selectedJob=23502841
With the patches on bug 1288187 I can repro a similar failure on OS X pretty trivially. I believe the problem is that after changing the touchAction property from script, we need to do a flush before the style difference algorithm is run and schedules a paint. In other words, the way the test is written now, it's possible that no paint occurs between changing the touchAction property, and the following events getting dispatched. One is guaranteed to happen eventually, but if we want to force it to happen there we need to use waitForAllPaintsFlushed instead of waitForAllPaints. Patch coming up.
Comment on attachment 8773338 [details] [diff] [review] Patch Try push is looking good.
Attachment #8773338 - Flags: review?(botond)
Attachment #8773338 - Flags: review?(botond) → review+
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1828ce0a42d8 After changing the touch-action property, ensure we flush layout before waiting for the paint. r=botond
https://hg.mozilla.org/mozilla-central/rev/1828ce0a42d8
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: