Closed Bug 1264592 Opened 8 years ago Closed 8 years ago

Intermittent test_tap.html | Document element didn't get painted - got true, expected false

Categories

(Core :: Panning and Zooming, defect)

48 Branch
Unspecified
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: cbook, Assigned: kats)

References

()

Details

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

Attachments

(1 file)

https://treeherder.mozilla.org/logviewer.html#?job_id=25784709&repo=mozilla-inbound

 01:44:22 INFO - 243 INFO TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/mochitest/test_tap.html | Document element didn't get painted - got true, expected false
Component: Graphics: Layers → Panning and Zooming
OS: Unspecified → Android
Version: unspecified → 48 Branch
Assignee: nobody → bugmail.mozilla
I pushed a try patch with some logging to https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ea2c82448af&selectedJob=20159622, there's an instance of this failure that has a log. I haven't had time to analyze the log output yet.
I don't think I added enough logging to diagnose this fully. Some more logging:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bfca2b5a17e
So it looks like there's two paints happening in the period where there's supposed to be no paints. At least one of the paints is because of Fennec's thumbnailing code. I don't know if there's a way to disable that, but it probably shouldn't be marking elements as painted either.
Try push which includes logging and also a patch to make the thumbnailing code not screw with the test:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=253e1ecde9ea
^ that looks pretty green, so I'm going to chalk this intermittent entirely up to the thumbnailing code. Patch coming up.
Could we use aBuilder->IsPaintingToWindow() instead?
That should be fine, I didn't know what the exact semantics of IsPaintingToWindow() and NS_FRAME_PAINTED_THEBES are.
NS_FRAME_PAINTED_THEBES is used by checkAndClearPaintedState, which is only used for tests (some mochitests + the magic reftest 'reftest-no-paint' class). I'd be surprised if a test was relying on NS_FRAME_PAINTED_THEBES being added for a non-IsPaintingToWindow paint.
I'm not 100% sure about the semantics of IsPaintingToWindow though. If you confirm that fennec thumbnails don't set it and you have a green try push, that should be fine though.
Comment on attachment 8749178 [details]
MozReview Request: Bug 1264592 - When doing thumbnails for fennec, don't mark frames as painted because it can interfere with tests. r?mstange

Dropping flag for now.
Attachment #8749178 - Flags: review?(mstange)
Comment on attachment 8749178 [details]
MozReview Request: Bug 1264592 - When doing thumbnails for fennec, don't mark frames as painted because it can interfere with tests. r?mstange

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50793/diff/1-2/
Attachment #8749178 - Flags: review?(mstange)
Comment on attachment 8749178 [details]
MozReview Request: Bug 1264592 - When doing thumbnails for fennec, don't mark frames as painted because it can interfere with tests. r?mstange

https://reviewboard.mozilla.org/r/50793/#review47801
Attachment #8749178 - Flags: review?(mstange) → review+
https://hg.mozilla.org/mozilla-central/rev/a1df2f5386b3
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Looks like the fix worked, I don't see any new occurrences of this in Orangefactor after the fix landed.
Comment on attachment 8749178 [details]
MozReview Request: Bug 1264592 - When doing thumbnails for fennec, don't mark frames as painted because it can interfere with tests. r?mstange

Approval Request Comment
[Feature/regressing bug #]: test added in bug 1257641 fails intermittently on fennec
[User impact if declined]: none. in theory this is a test-only change, but the code modified might be being used by add-ons via the DOMWindowUtils API. No non-test code in Firefox itself uses it, but I figured I'd request approval for uplift just to be safe.
[Describe test coverage new/current, TreeHerder]: there was a test that was failing intermittently before but isn't now
[Risks and why]: about as close to zero as it can get :)
[String/UUID change made/needed]: none
Attachment #8749178 - Flags: approval-mozilla-aurora?
Comment on attachment 8749178 [details]
MozReview Request: Bug 1264592 - When doing thumbnails for fennec, don't mark frames as painted because it can interfere with tests. r?mstange

Another APZ scroll fix, should only affect test failures, ok to uplift.
Attachment #8749178 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.