Closed Bug 1305490 Opened 3 years ago Closed 3 years ago

D3D11TextureData::UpdateFromSurface ASSERT crashes child when used by plugin

Categories

(Core :: Graphics: Layers, defect, P3)

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: handyman, Assigned: sotaro)

References

Details

(Keywords: crash, Whiteboard: [gfx-noted])

Attachments

(3 files, 1 obsolete file)

Attached file pluginproc.txt
STR:

0. Launch Firefox x64
1. Go to https://www.fenoxo.com/play/playtits.html

Expected Result:
The plugin plays fine.

Actual Result:
CRASH!

--------

Running a debug build and connecting the debugger to the child and plugin processes shows that this assert triggers in the child process when the crash happens:

https://dxr.mozilla.org/mozilla-central/rev/29beaebdfaccbdaeb4c1ee5a43a9795ab015ef49/gfx/layers/d3d11/TextureD3D11.cpp#657

I've attached the plugin and content process stacks at the time of the crash.  The pretty clearly show the communication that led to this situation.
Attached file contentproc.txt
Has STR: --- → yes
Keywords: crash
OS: Unspecified → Windows
Priority: -- → P3
Whiteboard: [gfx-noted]
Assignee: nobody → sotaro.ikeda.g
The assert was added by bug 1289640.
Blocks: 1289640
(In reply to David Parks [:handyman] from comment #0)
> Created attachment 8794957 [details]
> pluginproc.txt
> 
> STR:
> 
> 0. Launch Firefox x64
> 1. Go to https://www.fenoxo.com/play/playtits.html

I could reproduce the crash when async plugin rendering was used by flash plugin.
When TextureData::UpdateFromSurface() is not supported, TextureClient::UpdateFromSurface() fallbacks to DrawTarget approach on main thread. Plugin ipc runs on main thread, then the DrawTarget approach seems to work in this case.
  https://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/TextureClient.cpp#628
Attachment #8797913 - Flags: review?(matt.woodrow)
Confirmed that attachment 8797913 [details] [diff] [review] addressed the crash.
Comment on attachment 8797913 [details] [diff] [review]
patch - Remove assert in D3D11TextureData::UpdateFromSurface()

Review of attachment 8797913 [details] [diff] [review]:
-----------------------------------------------------------------

This will fix the crash, but we still won't be able to upload.

Can you please instead wait until the patch from bug 1292923 lands and then pass the ALLOC_UPDATE_FROM_SURFACE flag to make sure we get an appropriate TextureClient.
Attachment #8797913 - Flags: review?(matt.woodrow) → review-
OK, thanks!
Depends on: 1292923
Attachment #8797913 - Attachment is obsolete: true
Attachment #8799629 - Flags: review?(matt.woodrow)
Attachment #8799629 - Flags: review?(matt.woodrow) → review+
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ecca76e7b94
Use ALLOC_UPDATE_FROM_SURFACE flag r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/1ecca76e7b94
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Depends on: 1311952
Depends on: 1312988
It looks like this change introduced a regression that broke Farmville 2 Flash game on Facebook from rendering correctly. Marked that down as https://bugzilla.mozilla.org/show_bug.cgi?id=1317984.
Depends on: 1317984
You need to log in before you can comment on or make changes to this bug.