Closed Bug 1644732 Opened 4 months ago Closed 4 months ago

Assertion failure: mPrepareStatus == STATUS_PREPARED in RenderAndroidSurfaceTextureHostOGL::NotifyNotUsed when running webgl

Categories

(Core :: Graphics: WebRender, defect)

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox79 --- fixed

People

(Reporter: jnicol, Assigned: jnicol)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

After applying the fix from bug 1644678, we run in to this assertion.

We are failing the assertion because mPrepareStatus == STATUS_NONE. The reason it is STATUS_NONE is because NotifyNotUsed() is being called twice. The first time through the regular mechanism of TextureHost::ReleaseCompositableRef() -> WebRenderTextureHost::NotifyNotUsed() -> RenderThread::NotifyNotUsed() -> RenderAndroidSurfaceTextureHost::NotifyNotUsed(). This performs the required cleanup (calling mSurfTex->ReleaseTexImage()) then sets mPrepareStatus = STATUS_NONE. Then the second time it is called from the destructor ~RenderAndroidSurfaceTextureHost() -> DeleteTextureHandle() -> NotifyNotUsed(). Since we have just set mPrepareStatus to STATUS_NONE, the assertion fails the second time.

Sotaro, can we just remove the call to DeleteTextureHandle() from the destructor? Aren't we guaranteed that NotifyNotUsed() will be called the regular way?

If there is a chance that NotifyNotUsed() might not be called before the destructor, then we can add a return guard to the top of NotifyNotUsed, ie return early if mPrepareStatus == STATUS_NONE?

Flags: needinfo?(sotaro.ikeda.g)

Doing so means it is called twice, which causes an assertion failure.

(In reply to Jamie Nicol [:jnicol] from comment #0)

After applying the fix from bug 1644678, we run in to this assertion.

We are failing the assertion because mPrepareStatus == STATUS_NONE. The reason it is STATUS_NONE is because NotifyNotUsed() is being called twice. The first time through the regular mechanism of TextureHost::ReleaseCompositableRef() -> WebRenderTextureHost::NotifyNotUsed() -> RenderThread::NotifyNotUsed() -> RenderAndroidSurfaceTextureHost::NotifyNotUsed(). This performs the required cleanup (calling mSurfTex->ReleaseTexImage()) then sets mPrepareStatus = STATUS_NONE. Then the second time it is called from the destructor ~RenderAndroidSurfaceTextureHost() -> DeleteTextureHandle() -> NotifyNotUsed(). Since we have just set mPrepareStatus to STATUS_NONE, the assertion fails the second time.

Sotaro, can we just remove the call to DeleteTextureHandle() from the destructor? Aren't we guaranteed that NotifyNotUsed() will be called the regular way?

It is not necessary to call DeleteTextureHandle() from the destructor. And your patch looks good!

Flags: needinfo?(sotaro.ikeda.g)
Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5222e4d5c6dd
Don't call NotifyNotUsed() from ~RenderAndroidSurfaceTextureHostOGL(). r=sotaro
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79
You need to log in before you can comment on or make changes to this bug.