1230740 - crash in mozilla::gfx::GetCairoSurfaceForSourceSurface

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[[:philipp]]

[Tracking Requested - why for this release]:

This bug was filed from the Socorro interface and is 
report bp-d687bf78-6bbe-4809-8462-ddf282151203.
=============================================================
Crashing Thread
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::gfx::GetCairoSurfaceForSourceSurface(mozilla::gfx::SourceSurface*, bool, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) 	gfx/2d/DrawTargetCairo.cpp
1 	xul.dll 	mozilla::gfx::AutoClearDeviceOffset::Init(mozilla::gfx::SourceSurface*) 	gfx/2d/DrawTargetCairo.cpp
2 	xul.dll 	mozilla::gfx::DrawTargetCairo::DrawSurface(mozilla::gfx::SourceSurface*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::DrawSurfaceOptions const&, mozilla::gfx::DrawOptions const&) 	gfx/2d/DrawTargetCairo.cpp
3 	xul.dll 	mozilla::layers::RotatedBuffer::DrawBufferQuadrant(mozilla::gfx::DrawTarget*, mozilla::layers::RotatedBuffer::XSide, mozilla::layers::RotatedBuffer::YSide, mozilla::layers::RotatedBuffer::ContextSource, float, mozilla::gfx::CompositionOp, mozilla::gfx::SourceSurface*, mozilla::gfx::Matrix const*) 	gfx/layers/RotatedBuffer.cpp
4 	xul.dll 	mozilla::layers::RotatedBuffer::DrawBufferWithRotation(mozilla::gfx::DrawTarget*, mozilla::layers::RotatedBuffer::ContextSource, float, mozilla::gfx::CompositionOp, mozilla::gfx::SourceSurface*, mozilla::gfx::Matrix const*) 	gfx/layers/RotatedBuffer.cpp
5 	xul.dll 	mozilla::layers::RotatedContentBuffer::BeginPaint(mozilla::layers::PaintedLayer*, unsigned int) 	gfx/layers/RotatedBuffer.cpp
6 	xul.dll 	mozilla::layers::ContentClientRemoteBuffer::BeginPaintBuffer(mozilla::layers::PaintedLayer*, unsigned int) 	gfx/layers/client/ContentClient.h
7 	xul.dll 	mozilla::layers::ClientPaintedLayer::PaintThebes() 	gfx/layers/client/ClientPaintedLayer.cpp
8 	xul.dll 	mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) 	gfx/layers/client/ClientPaintedLayer.cpp
9 	xul.dll 	mozilla::layers::ClientContainerLayer::RenderLayer() 	gfx/layers/client/ClientContainerLayer.h
10 	xul.dll 	mozilla::layers::ClientLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) 	gfx/layers/client/ClientLayerManager.h
11 	xul.dll 	mozilla::layers::ClientContainerLayer::RenderLayer() 	gfx/layers/client/ClientContainerLayer.h
12 	xul.dll 	mozilla::layers::ClientLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) 	gfx/layers/client/ClientLayerManager.h
13 	xul.dll 	mozilla::layers::ClientContainerLayer::RenderLayer() 	gfx/layers/client/ClientContainerLayer.h
14 	xul.dll 	mozilla::layers::ClientLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) 	gfx/layers/client/ClientLayerManager.h
15 	xul.dll 	mozilla::layers::ClientContainerLayer::RenderLayer() 	gfx/layers/client/ClientContainerLayer.h
16 	xul.dll 	mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) 	gfx/layers/client/ClientLayerManager.cpp
17 	xul.dll 	mozilla::layers::ClientLayerManager::EndTransaction(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) 	gfx/layers/client/ClientLayerManager.cpp
18 	xul.dll 	nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) 	layout/base/nsDisplayList.cpp
19 	xul.dll 	nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) 	layout/base/nsLayoutUtils.cpp
20 	xul.dll 	PresShell::Paint(nsView*, nsRegion const&, unsigned int) 	layout/base/nsPresShell.cpp

this signature has sprung up in beta 8 in the 43 cycle, so it is probably an effect of the fixes landed in bug 1200021 - it's around #10 on the top score board for 43.0b8. it's not contained to a particular gpu vendor/driver.

nominating for tracking, since it sprung up late in the cycle & so that we keep an eye on its impact.

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking for 43, Milan if you have a chance, looks like there is some fallout from bug 1200021.

Comment 2

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

I will take a look - do we know if the crash from bug 1200021 got any better, and if this one is worse than that one?  Given the timing, chances are the backout is the best way to do it, and continue to live with the devil we know from bug 1200021.

Comment 3

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

Attachment: Beta patch - wallpaper over nullptr crashes. r=bas

If we decide not to back out bug 1200021, here's a speculative patch.

Comment 4

[[:philipp]]

the volume of this crash doesn't look much worse than bug 1200021 did.

Comment 5

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

Comment on attachment 8696670 [details] [diff] [review]
Beta patch - wallpaper over nullptr crashes. r=bas

Approval Request Comment
Do we have time for a beta, with this patch, and time enough to back it out (together with bug 1200021 patch) if this doesn't help?

Comment 6

[Bob Clary [:bc] (inactive)]

Reproduced with bughunter.

reproducible with x86 bp-dc44efd6-5ecb-4383-b99c-6088c2151210 and x86_64 bp-0ddc6396-5261-4924-94db-0319c2151210 on Windows 7 at https://s0.2mdn.net/1019344/1443037910075/selfie-300x600/index.html

To reproduce, reload the page multiple times. The browser may hang and not show a crash report, but if you kill it and start again and go to about:crashes you will see the crash report.

Comment 7

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

Great - I'll take a look on Monday when I have Win 7 access.

Comment 8

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

Attachment: Wallpaper over nullptr crashes. Carry r=bas

Comment 9

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

Having trouble reproducing; Bob, what's your about:support graphics section look like?

Comment 10

[Bob Clary [:bc] (inactive)]

Windows ESX VM:

Graphics
--------

Adapter Description: RDPDD Chained DD
Adapter Description (GPU #2): VMware SVGA 3D
Adapter Drivers: RDPDD
Adapter Drivers (GPU #2): vm3dum vm3dgl
Adapter RAM: Unknown
Adapter RAM (GPU #2): 16
Asynchronous Pan/Zoom: wheel input enabled
Device ID: 0x0000
Device ID (GPU #2): 0x0405
Direct2D Enabled: Blocked for your graphics card because of unresolved driver issues.
DirectWrite Enabled: false (6.2.9200.17568)
Driver Date (GPU #2): 11-11-2013
Driver Version (GPU #2): 7.14.1.2032
GPU #2 Active: false
GPU Accelerated Windows: 0/1 Basic (OMTC) Blocked for your graphics card because of unresolved driver issues.
Subsys ID: 00000000
Subsys ID (GPU #2): 040515ad
Supports Hardware H264 Decoding: No; Hardware video decoding disabled or blacklisted
Vendor ID: 0x0000
Vendor ID (GPU #2): 0x15ad
WebGL Renderer: Blocked for your graphics card because of unresolved driver issues.
windowLayerManagerRemote: true
AzureCanvasBackend: skia
AzureContentBackend: cairo
AzureFallbackCanvasBackend: cairo
AzureSkiaAccelerated: 0

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8696670 [details] [diff] [review]
Beta patch - wallpaper over nullptr crashes. r=bas

Crash fix, taking it. Let's uplift to Beta44.

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Neither patch here appears to apply cleanly to beta 44. Can we get a rebased patch if this is still needed for the new beta 44?

Comment 13

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

This is a follow up patch to bug 1200021; that was never put on 44 (we did 45 and 43.)  I would suggest a few days of collecting 45 information in aurora, and if it looks like the combination of this and bug 1200021 took care of the problem, we can uplift to 44.
Yes, no, maybe?

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8696670 [details] [diff] [review]
Beta patch - wallpaper over nullptr crashes. r=bas

Going with Milan's recommendation to wait for Aurora45 data before uplifting this to Beta44. Nom'ing for Aurora uplift.

Comment 15

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

We can track issues specific to 44 in bug 1233182 because it depends on another bug.

Comment 16

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

Let's land the main patch at this point, and then figure out 45 (in this bug) and 44 (in bug 1233182.)

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/99e72f7e3843

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/99e72f7e3843

Comment 19

[u279076]

Just a note that this is currently the #14 topcrash in Release with 1877 crashes reported against Firefox 43.0.1, accounting for 0.91% of crashes on Release. I'm not sure what the current uplift strategy is but if this is low risk we should uplift.

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8696670 [details] [diff] [review]
Beta patch - wallpaper over nullptr crashes. r=bas

Taking it in aurora to test the impact.

Comment 21

[u279076]

From bug 1216909 (depends on this bug among others)
===================================================
Currently the signature sits at the following ranks in each branch:
> Nightly (46): 0 crashes over the last week
>  Aurora (45): 0 crashes over the last week
>    Beta (44): 726 crashes over the last week or #20 with 0.82%
> Release (43): 0 crashes over the last week

Given these stats I'm not sure the Nightly/Aurora data is going to be useful. We may want to consider uplifting to Beta sooner rather than later so if there are any negative impacts we hear about them sooner and can back out as necessary.

Comment 22

[u279076]

> Given these stats I'm not sure the Nightly/Aurora data is going to be useful.

Actually I take this bit back. I just noticed the signature in this bug is different (the stats above are for the signature that depends on this bug). We *do* actually have stats for *this* signature (182 and 98 in Nightly and Aurora respectively). So at the end of the day we should be able to measure the impact of this change.

However, I would still like to ask that we keep an eye on bug 1216909 and its dependencies (this bug being one of them) as that is a topcrash that currently affects Beta and Beta alone. I'd like to see that get fixed as part of Firefox 44's release.

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3a5e87559868

Comment 24

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Milan, do you feel we are at a safe place where we can consider uplifting this fix to Beta44 to ultimately address top crash bug 1216909? If we do not have conclusive data on how much it has helped, is it worthwhile uplifting this for 44.0b6 and watch it for a week? We can backout if things go downhill or keep it otherwise. Please let me know.

Comment 25

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

I don't see crashes in Aurora after the 28th, so it seems to have helped.  I don't trust my search skills completely, so Anthony can confirm, but, yes, I'd uplift this to Beta.

Comment 26

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

Comment on attachment 8696670 [details] [diff] [review]
Beta patch - wallpaper over nullptr crashes. r=bas

Will take care of uplifting this to beta/44 as a part of bug 1233182.

Comment 27

[u279076]

Sorry it took so long to come back to this, bugmail started going to spam after my holidays. Firefox 45.0a2 has 10 crashes over the last week with a build ID on or before 20151228004010, no crashes with anything more recent.