Handle TDR when video is playing

RESOLVED FIXED in Firefox 57

Status

()

RESOLVED FIXED
a year ago
2 months ago

People

(Reporter: kechen, Assigned: kechen)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla57
All
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

a year ago
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.
(Assignee)

Updated

a year ago
Has STR: --- → yes
Comment hidden (mozreview-request)
(Assignee)

Comment 2

a year ago
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.
(Assignee)

Updated

a year ago
Blocks: 1388995
(Assignee)

Comment 3

a year ago
Created attachment 8898232 [details]
crash log

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

Comment 4

a year ago
mozreview-review
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.
Attachment #8898166 - Flags: review?(bas) → review-
Comment hidden (mozreview-request)

Comment 6

a year ago
mozreview-review
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
Attachment #8898166 - Flags: review?(bas) → review+
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 8

a year ago
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 <kechen@mozilla.com>
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
Please fix the "compabability" typo in the commit message while you're revising it anyway.
Flags: needinfo?(kechen)
Keywords: checkin-needed
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Flags: needinfo?(kechen)
Keywords: checkin-needed

Comment 11

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/39c513a8195d
Check texture compatibility when ensuring the texture source; r=bas
Keywords: checkin-needed

Comment 12

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/39c513a8195d
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57

Updated

2 months ago
Blocks: 1188006
You need to log in before you can comment on or make changes to this bug.