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

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Jamie Nicol [:jnicol]]

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?

Comment 1

[Jamie Nicol [:jnicol]]

Attachment: Bug 1644732 - Don't call NotifyNotUsed() from ~RenderAndroidSurfaceTextureHostOGL(). r?sotaro

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

Comment 2

[Sotaro Ikeda [:sotaro]]

(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!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5222e4d5c6dd
Don't call NotifyNotUsed() from ~RenderAndroidSurfaceTextureHostOGL(). r=sotaro

Comment 4

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5222e4d5c6dd