1305490 - D3D11TextureData::UpdateFromSurface ASSERT crashes child when used by plugin

Status: RESOLVED FIXED

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

Description

[David Parks [:handyman]]

Attachment: 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.

Comment 1

[David Parks [:handyman]]

Attachment: contentproc.txt

Comment 2

[Sotaro Ikeda [:sotaro]]

The assert was added by bug 1289640.

Comment 3

[Sotaro Ikeda [:sotaro]]

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

Comment 4

[Sotaro Ikeda [:sotaro]]

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

Comment 5

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Remove assert in D3D11TextureData::UpdateFromSurface()

Comment 6

[Sotaro Ikeda [:sotaro]]

Confirmed that attachment 8797913 [details] [diff] [review] addressed the crash.

Comment 7

[Matt Woodrow (:mattwoodrow)]

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.

Comment 8

[Sotaro Ikeda [:sotaro]]

OK, thanks!

Comment 9

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Use ALLOC_UPDATE_FROM_SURFACE flag

Comment 10

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ccbecd7811f2ce9a739664ca414bc8aaa3d7654

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ecca76e7b94
Use ALLOC_UPDATE_FROM_SURFACE flag r=mattwoodrow

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1ecca76e7b94

Comment 13

[Jukka Jylänki]

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.