1456978 - Avoid getting stuck in the TooManyPendingFrames state with async scene building

Status: RESOLVED FIXED

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

Description

[Kartikaya Gupta (email:[e-mail])]

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

Comment 1

[Kartikaya Gupta (email:[e-mail])]

I have gecko-side patches that go on top of https://github.com/servo/webrender/pull/2691

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1456978 - Rename function to a more appropriate name.

Review commit: https://reviewboard.mozilla.org/r/239742/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/239742/

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1456978 - Remove redundant construction of WindowId.

Review commit: https://reviewboard.mozilla.org/r/239744/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/239744/

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1456978 - Ensure that all generate_frame transactions end up clearing the pending frame count.

This prevents us from getting stuck in the frame throttling code where
TooManyPendingFrames always returns true.

Review commit: https://reviewboard.mozilla.org/r/239746/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/239746/

Comment 5

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8971016 [details]
Bug 1456978 - Rename function to a more appropriate name.

https://reviewboard.mozilla.org/r/239742/#review245552

Comment 6

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8971017 [details]
Bug 1456978 - Remove redundant construction of WindowId.

https://reviewboard.mozilla.org/r/239744/#review245554

Comment 7

[Sotaro Ikeda [:sotaro]]

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 :)

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8971016 [details]
Bug 1456978 - Rename function to a more appropriate name.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/239742/diff/1-2/

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8971017 [details]
Bug 1456978 - Remove redundant construction of WindowId.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/239744/diff/1-2/

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8971018 [details]
Bug 1456978 - Ensure that all generate_frame transactions end up clearing the pending frame count.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/239746/diff/1-2/

Comment 11

[Kartikaya Gupta (email:[e-mail])]

^ rebased on top of the WR update. Kicked off a try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=01648141e8a8d943d050f35295431329a136b746

Comment 12

[Pulsebot]

Pushed by [e-mail]:
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

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ca63e4de5479
https://hg.mozilla.org/mozilla-central/rev/d5e9d984dcc6
https://hg.mozilla.org/mozilla-central/rev/704dd8298b15