Closed Bug 1434996 Opened 3 years ago Closed 3 years ago
Intermittent PID 12769 | Assertion failure: a
Transaction Id == 1 || a Transaction Id > Last Pending Transaction Id(), at /builds/worker/workspace/build/src/gfx/layers/wr/Web Render Bridge Parent .cpp:1269
59 bytes, text/x-review-board-request
Filed by: kgupta [at] mozilla.com https://treeherder.mozilla.org/logviewer.html#?job_id=159861895&repo=try https://queue.taskcluster.net/v1/task/SydPAQ6LTJmL7NVQ4z7F2w/runs/0/artifacts/public/logs/live_backing.log This happened on a try push where I tried turning on the web-platform-tests
3 years ago
I can reproduce this locally, investigating.
Assignee: nobody → bugmail
Based on my investigation so far I think the problem is that if we go back/forward in the bfcache, then we end up reusing a prescontext and refreshdriver from the bfcache. This means we can go from a refresh driver with a high transaction id value to a refresh driver with a lower transaction id value (that is still nonzero), and that causes the assertion failure because the next transaction that the compositor sees will be > 1 but also less than the last transaction id it received. The test that's hitting this crash does back/forward navigation  and when that navigation happens I see a refresh driver with a nonzero transaction id getting installed at . I don't know if we can rely on the transaction ids increasing monotonically given this. Or maybe we should reset the transaction id counter when we pull a prescontext out of the bfcache.  https://searchfox.org/mozilla-central/rev/a5abf843f8fac8530aa5de6fb40e16547bb4a47a/testing/web-platform/tests/service-workers/service-worker/navigate-window.https.html#117,119  https://searchfox.org/mozilla-central/rev/a5abf843f8fac8530aa5de6fb40e16547bb4a47a/layout/painting/nsDisplayList.cpp#2485
Or we can try to use the isFirstPaint flag instead of aTransactionId==1 as the check. That seems to fix it locally. https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a3b6e9e54e765f0fc4b66f314f9d55403859df1
There's a debug M-e10s-5 failure in the above try push that is failing the modified assertion. Looking into it.
Ugh, looks like we sometimes do a paint with the paint suppression turned on? We send a paint after going through  with mIsFirstPaint=true and mPaintingSuppressed=true, so the isFirstPaint flag doesn't get to the compositor side. This seems like another bug but I don't want to mess with that right now. I have different idea about how to fix this that is probably going to be more robust.  https://searchfox.org/mozilla-central/rev/a5abf843f8fac8530aa5de6fb40e16547bb4a47a/layout/base/PresShell.cpp#6393
(In reply to Kartikaya Gupta (email:email@example.com) from comment #6) > I have different idea about how to fix this that is probably > going to be more robust. This idea was to make the transaction id a class which stored the presShellId as well as the transaction id. Then a changing presShellId would indicate that the refresh driver changed and the compositor could use that to do it's special magic. But then I discovered that since we implemented WebRenderLayerManager, the ClientLayerManager::SetTransactionIdAllocator implementation has grown (in bug 1283947) and it looks like it handles exactly this kind of problem. So I'm copying over that code and let's see how it does: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a42562d78cf286e4d9c74a7adeeda6f98220430a
That's looking better. I think it might fix some of the mochitests as well. Kicked off another try push with some of the skipped mochitests re-enabled to see if they are still failing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a18532c149a0aebfbfbd8cc6932d854d357291e2
Comment on attachment 8949833 [details] Bug 1434996 - Update WebRenderLayerManager::SetTransactionIdAllocator to match ClientLayerManager. https://reviewboard.mozilla.org/r/219144/#review224958 Good catch!
Attachment #8949833 - Flags: review?(sotaro.ikeda.g) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/c0a2f744ef5c Update WebRenderLayerManager::SetTransactionIdAllocator to match ClientLayerManager. r=sotaro
You need to log in before you can comment on or make changes to this bug.