This bug was filed from the Socorro interface and is report bp-6f45a537-55b9-4a7f-a342-54cc72130901. ============================================================= Very frequent when trying the pdf.js add-on in Fennec.

Seems like a bug around SurfaceStreamHostOGL which is used to composite Skia/GL 2D canvases on Linux. Not clear to me who should look into this bug, let's ask milan. Skia/GL people would be :gw280 or :snorp or me, while Layers people would be :nical or Bas or :nrc or me, I guess. Basically, anyone in the gfx team is a reasonable choice.

:snorp is on vacation this week, George could take a look - do we have any crash stats on this?

It's the #9 crasher on Fennec Nightly but the number of crashes is not a lot. It does happen very frequently when testing pdf.js on Fennec. Since I've been looking at pdf.js stuff, I will go ahead and debug this and see if I can find anything. If not, I will bug Snorp or George about it later.

The crash happens when attempting to release an already destroyed SurfaceStreamHostOGL::mStreamGL, which was added in bug 894405. The comments in bug 894405 kind of predicted this regression :( I think this is a regression from bug 904620. That bug took out the mNeedsUpdate flag which means GLContext::AddRef [1] is only called for some updates, whereas GLContext::Release is called for all updates on the other side [2], causing a mismatch. [1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/CanvasClient.cpp#218 [2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/TextureHostOGL.cpp#648 I'm not sure what's the correct way to fix it. Can you take this, Matt?

Thanks for debugging this Jim. It looks like we're recreating the TextureHost object most times, so we're releasing the GLContext more than we AddRef it. This is a known bug that should be fixed by new textures, so I'll leave it for now. Even with that fixed, it's still at least valid for the host side to recreate textures, so I don't think we can have these checks work reliably. The cost of a few extra refs should be fine.

Comment on attachment 801324 [details] [diff] [review] Don't try to minimize the number of AddRef/Release pairs Review of attachment 801324 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/client/CanvasClient.cpp @@ +206,5 @@ > #endif > } else { > SurfaceStreamHandle handle = stream->GetShareHandle(); > SurfaceDescriptor *desc = mDeprecatedTextureClient->GetDescriptor(); > + *desc = SurfaceStreamDescriptor(handle, false); This is an ugly as hell way to set the descriptor on the texture client. Is there a SetDescriptor we can use?

Comment on attachment 801324 [details] [diff] [review] Don't try to minimize the number of AddRef/Release pairs [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 904620 User impact if declined: Can't uplift 904620 Testing completed (on m-c, etc.): Been on m-c/aurora. Risk to taking this patch (and alternatives if risky): Very low risk. String or IDL/UUID changes made by this patch: None