1135066 - SEGV in mozilla::layers::BufferTextureClient::AllocateForSurface

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

+++ This bug was initially created as a clone of Bug #1095925 +++

The test case from bug 1095925 crashes OS X with a different signature.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Check for failed surface creation. r=jmuizelaar

Note: This does not crash on Windows.

Comment 2

[Markus Stange [:mstange]]

Doesn't this mean we'll hit the MOZ_ASSERT(mCGContext) further down? We're only trying to create a CG DrawTarget because our existing one is something else, so if we continue with the old value of dt then borrowing a CGContext from that won't work, BorrowedCGContext::BorrowCGContextFromDrawTarget will return null.

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

Yeah, you're right; we get out the other end in the release build, but there are asserts that fire in debug.

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Catch failed CreateDrawTarget. r=mstange

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1981a2467ef3

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e4a81270915c

Based on bug 1095925, I assume this is also wontfix for esr31. I assume we want this on Aurora/Beta however?

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e4a81270915c

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8573492 [details] [diff] [review]
Catch failed CreateDrawTarget. r=mstange

Let me know if you think this is critical enough for beta uplift, otherwise I think we're good with just Aurora, and mostly because it will become ESR.

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: OS X sec-moderate in 38 (eventually ESR)
[Describe test coverage new/current, TreeHerder]: None; will check if we can create an automated test for this but not land it until this fix releases.
[Risks and why]: Low.  Avoid dereferencing null pointer.
[String/UUID change made/needed]: n/a

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Automated test for catching this crash. r=mstange

Don't know about the timing for landing this - I imagine only after this fix is in the release?

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8573492 [details] [diff] [review]
Catch failed CreateDrawTarget. r=mstange

I wouldn't mind this on beta just for completeness if the patch applies cleanly.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8573492 [details] [diff] [review]
Catch failed CreateDrawTarget. r=mstange

Per comment 10.

Comment 12

[Milan Sreckovic [:milan] (needinfo for best results)]

For the automated test: https://treeherder.mozilla.org/#/jobs?repo=try&revision=75ac8b25170d

The test does fail before the fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e61d5ddd732

Comment 13

[Milan Sreckovic [:milan] (needinfo for best results)]

Leave NI on me to remember to land the test once the fix is in all the trains.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/26960651c5d9

Comment 15

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8573492 [details] [diff] [review]
Catch failed CreateDrawTarget. r=mstange

While this is a straightforward patch, I don't think "completeness" is enough justification to take the change in Beta at this point. Let's let this ride the 38 train.