1390452 - Handle TDR when video is playing

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Kevin Chen[:kechen] (UTC + 8)]

There are chances that compositor process will crash after TDR when there is a H.264 video playing.

STR:
1. Open firefox
2. Open a website/file that contains a H.264 video.
3. Open command line as administrator and type "dxcap -forcetdr" to trigger device reset.

Expected result:
Video might go black but the browser survives.

Actual result:
The compositor process will crash.
If compositor process is on GPU process then no critical dammage, GPU process will relaunch itself; however, if it is on UI process, then firefox crashes.

Comment 1

[Kevin Chen[:kechen] (UTC + 8)]

Attachment: Bug 1390452 - Check texture compatibility when ensuring the texture source;

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

Comment 2

[Kevin Chen[:kechen] (UTC + 8)]

My deduction is that compositor use the invalid textures which causes the crash.
The thing is that contents' textures are owned by layers, so when TDR is triggered and the layer tree is recreated, the old textures will be discarded too.
However, in video case, the textures are not owned by the layer system; therefore the compositor will still have chance to use old textures even the layer system is recreated.

Comment 3

[Kevin Chen[:kechen] (UTC + 8)]

Attachment: crash log

The crash log looks a like some crashes in bug 1346007.

Comment 4

[Bas Schouten (:bas.schouten)]

Comment on attachment 8898166 [details]
Bug 1390452 - Check texture compatibility when ensuring the texture source;

https://reviewboard.mozilla.org/r/169532/#review174884

::: gfx/layers/d3d11/TextureD3D11.cpp:937
(Diff revision 1)
>    }
>  
> +  MOZ_ASSERT(mTexture);
> +  RefPtr<ID3D11Device> device;
> +  mTexture->GetDevice(getter_AddRefs(device));
> +  if (device != DeviceManagerDx::Get()->GetCompositorDevice()) {

These should somehow be integrated into OpenSharedHandle, then the call above here (in both cases), can just call OpenSharedHandle (no longer check !mTexture directly), and OpenSharedHandle will just ensure mTexture is either valid for the right device, or attempt to (re-)open the texture for the right device. In any case it will return false if a valid mTexture isn't present. Feel free to rename it to something like EnsureTexture() or something like that.

Comment 5

[Kevin Chen[:kechen] (UTC + 8)]

Comment on attachment 8898166 [details]
Bug 1390452 - Check texture compatibility when ensuring the texture source;

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

Comment 6

[Bas Schouten (:bas.schouten)]

Comment on attachment 8898166 [details]
Bug 1390452 - Check texture compatibility when ensuring the texture source;

https://reviewboard.mozilla.org/r/169532/#review175742

::: gfx/layers/d3d11/TextureD3D11.cpp:755
(Diff revision 2)
> +    mTexture->GetDevice(getter_AddRefs(device));
> +    if (device == DeviceManagerDx::Get()->GetCompositorDevice()) {
> +      NS_WARNING("Incompatible texture.");
> +      return true;
> +    }
> +	mTexture = nullptr;

nit: Tab!

::: gfx/layers/d3d11/TextureD3D11.cpp:1025
(Diff revision 2)
>  {
> -  RefPtr<ID3D11Device> device = GetDevice();
> -  if (!device) {
> +  RefPtr<ID3D11Device> device;
> +  if (mTextures[0]) {
> +    mTextures[0]->GetDevice(getter_AddRefs(device));
> +    if (device == DeviceManagerDx::Get()->GetCompositorDevice()) {
> +	    NS_WARNING("Incompatible texture.");

nit: More tabs!

::: gfx/layers/d3d11/TextureD3D11.cpp:1028
(Diff revision 2)
> +    mTextures[0]->GetDevice(getter_AddRefs(device));
> +    if (device == DeviceManagerDx::Get()->GetCompositorDevice()) {
> +	    NS_WARNING("Incompatible texture.");
> +	    return true;
> +    }
> +	mTextures[0] = nullptr;

nit: Another tab

Comment 7

[Kevin Chen[:kechen] (UTC + 8)]

Comment on attachment 8898166 [details]
Bug 1390452 - Check texture compatibility when ensuring the texture source;

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

Comment 8

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 2 changes to 2 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: Rev c821b8357ee3 needs "Bug N" or "No bug" in the commit message.
remote: Kevin Chen <[e-mail]>
remote: Bug - 1390452 Check texture compabability when ensuring the texture source; r=bas
remote: 
remote: MozReview-Commit-ID: LjFvMezb1TV
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.c_commitmessage hook failed
abort: push failed on remote

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please fix the "compabability" typo in the commit message while you're revising it anyway.

Comment 10

[Kevin Chen[:kechen] (UTC + 8)]

Comment on attachment 8898166 [details]
Bug 1390452 - Check texture compatibility when ensuring the texture source;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/169532/diff/3-4/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39c513a8195d
Check texture compatibility when ensuring the texture source; r=bas

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/39c513a8195d