1570869 - Make RenderThread's mWindowInfo state management easier to understand

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, task, P3)

Description

[Markus Stange [:mstange]]

I was reading RenderThread.cpp and had some trouble understanding how mWindowInfo's members are managed: Which parts of it belong to the same frame, whether the rendering count can become > 1, what exactly the role of the doc frame counting is, etc.
I've found some code modifications that should make things easier to understand.

Comment 1

[Markus Stange [:mstange]]

Attachment: Bug 1570869 - Combine pending frame info in RenderThread's WindowInfo. r?nical

Comment 2

[Markus Stange [:mstange]]

Attachment: Bug 1570869 - Inline IncRenderingFrameCount into HandleFrame. r?nical

IncRenderingFrame only had one caller. Inlining it into HandleFrame makes it clearer
how the values in mWindowInfos are mutated and in what order calls happen.
This also renames HandleFrame to HandleFrameOneDoc, because we're expecting one call
per document before we actually trigger the render.

Depends on D40370

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 1570869 - Remove mPendingCount in favor of an accessor. r?nical

Depends on D40371

Comment 4

[Markus Stange [:mstange]]

Attachment: Bug 1570869 - Replace mRenderingCount with a boolean, to make it clear that we are only ever rendering at most one frame. r?nical

The only place that increments mRenderingCount, HandleFrameOneDoc, also synchronously calls FrameRenderingComplete
at the end of the function, which decrements mRenderingCount again. So it can never grow beyond 1.

Depends on D40372

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1570869 - Fold mRender, mDocFramesSeen and mDocFrameCounts into PendingFrameInfo. r?nical

This makes it clear that these belong to a single frame and makes some assumptions explicit.
For example, in the old code, mDocFrameCounts.size() was the same as mPendingFrames.size()
when a pending frame was added, but then the sizes differed during rendering because a frame's
mDocFrameCount would be popped at the beginning of rendering while mPendingFrames would be
popped at the end of rendering.
This modification also makes some clearing of values unnecessary. A new frame always starts out
with cleared values for mDocFramesSeen and mFrameNeedsRender.

This patch also combines the two locks in HandleFrameOneDoc.

Depends on D40373

Comment 6

[Markus Stange [:mstange]]

Attachment: Bug 1570869 - Inline FrameRenderingComplete into HandleFrameOneDoc. r?nical

Depends on D40374

Comment 7

[Markus Stange [:mstange]]

Attachment: Bug 1570869 - Add some comments to WindowInfo and reorder members a little. r?nical

Depends on D40375

Comment 8

[Nicolas Silva [:nical]]

Thanks for simplifying this code!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/61b89c3a2aaf
Combine pending frame info in RenderThread's WindowInfo. r=nical
https://hg.mozilla.org/integration/autoland/rev/e04baaedbcec
Inline IncRenderingFrameCount into HandleFrame. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/4e9fed7a845a
Remove mPendingCount in favor of an accessor. r=nical
https://hg.mozilla.org/integration/autoland/rev/acb46795e741
Replace mRenderingCount with a boolean, to make it clear that we are only ever rendering at most one frame. r=nical
https://hg.mozilla.org/integration/autoland/rev/c393a002b81a
Fold mRender, mDocFramesSeen and mDocFrameCounts into PendingFrameInfo. r=nical
https://hg.mozilla.org/integration/autoland/rev/378d13e14823
Inline FrameRenderingComplete into HandleFrameOneDoc. r=nical
https://hg.mozilla.org/integration/autoland/rev/267e2feb994f
Add some comments to WindowInfo and reorder members a little. r=nical

Comment 10

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/61b89c3a2aaf
https://hg.mozilla.org/mozilla-central/rev/e04baaedbcec
https://hg.mozilla.org/mozilla-central/rev/4e9fed7a845a
https://hg.mozilla.org/mozilla-central/rev/acb46795e741
https://hg.mozilla.org/mozilla-central/rev/c393a002b81a
https://hg.mozilla.org/mozilla-central/rev/378d13e14823
https://hg.mozilla.org/mozilla-central/rev/267e2feb994f