nightlies only. Operating System Percentage Windows 8.1 48.28 % Windows 7 40.30 % Windows 8 8.19 % first crash from https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Alayers%3A%3ACairoImage%3A%3AGetTextureClient%28mozilla%3A%3Alayers%3A%3ACompositableClient*%29&product=Firefox&query_type=is_exactly&range_unit=weeks&process_type=any&hang_type=any&date=2014-06-02+12%3A00%3A00&range_value=4#tab-correlations is report bp-7160211f-77ef-499a-be88-35a612140505 buildid 20140504030205 ============================================================= 0 xul.dll mozilla::layers::CairoImage::GetTextureClient(mozilla::layers::CompositableClient *) gfx/layers/ImageContainer.cpp 1 xul.dll mozilla::layers::ImageClientSingle::UpdateImageInternal(mozilla::layers::ImageContainer *,unsigned int,bool *) gfx/layers/client/ImageClient.cpp 2 xul.dll mozilla::layers::ImageClientSingle::UpdateImage(mozilla::layers::ImageContainer *,unsigned int) gfx/layers/client/ImageClient.cpp 3 xul.dll mozilla::layers::ClientImageLayer::RenderLayer() gfx/layers/client/ClientImageLayer.cpp 4 xul.dll mozilla::layers::ClientContainerLayer::RenderLayer() gfx/layers/client/ClientContainerLayer.h 5 xul.dll mozilla::layers::ClientContainerLayer::RenderLayer() gfx/layers/client/ClientContainerLayer.h 6 xul.dll mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::ThebesLayer *,gfxContext *,nsIntRegion const &,mozilla::layers::DrawRegionClip,nsIntRegion const &,void *),void *,mozilla::layers::LayerManager::EndTransactionFlags) gfx/layers/client/ClientLayerManager.cpp 7 xul.dll mozilla::layers::ClientLayerManager::EndTransaction(void (*)(mozilla::layers::ThebesLayer *,gfxContext *,nsIntRegion const &,mozilla::layers::DrawRegionClip,nsIntRegion const &,void *),void *,mozilla::layers::LayerManager::EndTransactionFlags) gfx/layers/client/ClientLayerManager.cpp 8 xul.dll nsDisplayList::PaintForFrame(nsDisplayListBuilder *,nsRenderingContext *,nsIFrame *,unsigned int) layout/base/nsDisplayList.cpp 9 xul.dll nsLayoutUtils::PaintFrame(nsRenderingContext *,nsIFrame *,nsRegion const &,unsigned int,unsigned int) layout/base/nsLayoutUtils.cpp 10 xul.dll PresShell::Paint(nsView *,nsRegion const &,unsigned int) layout/base/nsPresShell.cpp 11 xul.dll nsViewManager::ProcessPendingUpdatesPaint(nsIWidget *) view/src/nsViewManager.cpp 12 xul.dll nsViewManager::ProcessPendingUpdatesForView(nsView *,bool) view/src/nsViewManager.cpp 13 xul.dll nsViewManager::ProcessPendingUpdates() view/src/nsViewManager.cpp

(In reply to Wayne Mery (:wsmwk) from comment #0) > nightlies only. I suspect OMTC Windows as the cause/trigger. When did this start?

from comment 0, approx buildid 20140504030205

This code is OMTC-only so it at least cannot have happenned on windows before we enabled OMTC. What's happenning is that we just successfully locked a TextureClient, but failed to get a DrawTarget out of it, which is a bit worrying because in a lot of places we don't check that the DrawTarget can be null if the TextureClient was sucessfully locked. This crash should be easy to fix (just a null-check) but I would like to know what caused us to fail to create a DrawTarget out of a valid and locked TextureClient.

It's kinda bad that we need to null-check here even if the TextureClient was locked sucessfully. I have ideas to properly ensure that a locked TextureClient will always provide a good DrawTarget, but in the mean time let's add a null-check here.

D3D TextureClients are the only ones I've seen fail to provide DrawTargets while being locked so far, so here's a patch that makes sure they either can provide a DrawTarget ot fail to lock (if OPEN_WRITE). I'll refactor TextureClient to handle that at a higher level rather than doing that for all of the other TextureClient implementations. In the mean time let's assume that GetAsDrawTarget can (rarely) fail.

Comment on attachment 8437884 [details] [diff] [review] null-check the DrawTarget in ImageClient::GetTextureClient Review of attachment 8437884 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/ImageContainer.cpp @@ +653,5 @@ > } > > textureClient->Unlock(); > > + if (failed) { Do we have an AutoLockTextureClient helper? Would make this code cleaner, since we could just immediately return when GetAsDrawTarget() fails.

Comment on attachment 8437891 [details] [diff] [review] Make sure that D3D TextureClients can provide access to a DrawTarget if they are locked in write-mode. Review of attachment 8437891 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/d3d9/TextureD3D9.cpp @@ +587,5 @@ > mIsLocked = true; > > + // Make sure that successful write-lock means we will have a DrawTarget to > + // write into. > + if (aMode && OpenMode::OPEN_WRITE) { Why is it ok to be locked in READ_WRITE or READ and not be able to return a DT?

(In reply to Matt Woodrow (:mattwoodrow) from comment #7) > Comment on attachment 8437891 [details] [diff] [review] > Make sure that D3D TextureClients can provide access to a DrawTarget if they > are locked in write-mode. > > Review of attachment 8437891 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/layers/d3d9/TextureD3D9.cpp > @@ +587,5 @@ > > mIsLocked = true; > > > > + // Make sure that successful write-lock means we will have a DrawTarget to > > + // write into. > > + if (aMode && OpenMode::OPEN_WRITE) { > > Why is it ok to be locked in READ_WRITE or READ and not be able to return a > DT? creating/destroying Draw targets can have a non-negligible cost, depending on the backend, There isn't really a good reason to get a DrawTarget out of a TextureClient that you don't want to write into. (In reply to Matt Woodrow (:mattwoodrow) from comment #6) > Do we have an AutoLockTextureClient helper? Would make this code cleaner, > since we could just immediately return when GetAsDrawTarget() fails. Right, I'll update the patch.

Addressed the comment and landed the first patch: https://hg.mozilla.org/integration/mozilla-inbound/rev/337592467861

Comment on attachment 8437891 [details] [diff] [review] Make sure that D3D TextureClients can provide access to a DrawTarget if they are locked in write-mode. Review of attachment 8437891 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/d3d11/TextureD3D11.cpp @@ +185,5 @@ > + } > + > + // Make sure that successful write-lock means we will have a DrawTarget to > + // write into. > + if (aMode && OpenMode::OPEN_WRITE) { One '&' here, not two. Same for the other file. I think we should aim to make GetAsDrawTarget only valid when OPEN_WRITE is specified too, if that isn't already the case. Maybe assert the lock mode in that function.

fixed the 2nd patch and landed it: https://hg.mozilla.org/integration/mozilla-inbound/rev/3fb0089cfb28

(In reply to Nicolas Silva [:nical] from comment #12) > fixed the 2nd patch and landed it: > https://hg.mozilla.org/integration/mozilla-inbound/rev/3fb0089cfb28 After this, when omtc is on, h264 playing is broken.

Could this patch of caused HTML5 play on Youtube to present audio only, and no video ?

3 crashes in Socorro with [@ mozilla::layers::CairoImage::GetTextureClient(mozilla::layers::CompositableClient*)] signature on 33.0b in the last month - 2 for 33 beta 1 and 1 crash for beta 3: https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Alayers%3A%3ACairoImage%3A%3AGetTextureClient%28mozilla%3A%3Alayers%3A%3ACompositableClient%2A%29&product=Firefox&query_type=contains&range_unit=weeks&process_type=any&version=Firefox%3A33.0b&hang_type=any&date=2014-09-23+08%3A00%3A00&range_value=4#tab-reports No crash reports for 34.0a2 and 35.0a1 versions. Also, I see no issues (as pointed in comment 13 and comment 15) with Firefox 33 beta 6 (Build ID: 20140922173023) on Windows 7 64-bit and Windows 8 32-bit.