Closed Bug 1456978 Opened 2 years ago Closed 2 years ago

Avoid getting stuck in the TooManyPendingFrames state with async scene building

Categories

(Core :: Graphics: WebRender, enhancement, P1)

Other Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: kats, Assigned: kats)

References

(Blocks 1 open bug)

Details

(Whiteboard: [gfx-noted])

Attachments

(3 files)

Right now if I add GenerateFrame transactions that bypass the scene builder, they end up hitting the codepath at [1] during startup. This has a much worse effect than what the TODO implies. What actually happens is that op.scroll and op.composite are both false for this transaction, so the new_document_ready callback at [2] is never invoked for the transaction, which leads to the C++ side having a mismatch with mPendingTransaction count. This results in us always going down the codepath at [3] and this basically hoses composition.

The TooManyPendingFrames mechanism for frame throttling is pretty brittle in general and it would be good to replace it with something more robust. However, in the meantime, we should at least fix the existing mechanism so it doesn't fall into this situation.

[1] https://searchfox.org/mozilla-central/rev/36dec78aecc40539ecc8d78e91612e38810f963c/gfx/webrender/src/render_backend.rs#991
[2] https://searchfox.org/mozilla-central/rev/36dec78aecc40539ecc8d78e91612e38810f963c/gfx/webrender/src/render_backend.rs#1038
[3] https://searchfox.org/mozilla-central/rev/36dec78aecc40539ecc8d78e91612e38810f963c/gfx/layers/wr/WebRenderBridgeParent.cpp#1250
Attachment #8971018 - Flags: review?(sotaro.ikeda.g)
Comment on attachment 8971016 [details]
Bug 1456978 - Rename function to a more appropriate name.

https://reviewboard.mozilla.org/r/239742/#review245552
Attachment #8971016 - Flags: review?(sotaro.ikeda.g) → review+
Comment on attachment 8971017 [details]
Bug 1456978 - Remove redundant construction of WindowId.

https://reviewboard.mozilla.org/r/239744/#review245554
Attachment #8971017 - Flags: review?(sotaro.ikeda.g) → review+
Comment on attachment 8971018 [details]
Bug 1456978 - Ensure that all generate_frame transactions end up clearing the pending frame count.

https://reviewboard.mozilla.org/r/239746/#review245580

Looks good :)
Attachment #8971018 - Flags: review?(sotaro.ikeda.g) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ca63e4de5479
Rename function to a more appropriate name. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/d5e9d984dcc6
Remove redundant construction of WindowId. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/704dd8298b15
Ensure that all generate_frame transactions end up clearing the pending frame count. r=sotaro
You need to log in before you can comment on or make changes to this bug.