992406 - crash in mozilla::gfx::SurfaceToPackedBGRA(mozilla::gfx::DataSourceSurface*)

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

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

This bug was filed from the Socorro interface and is 
report bp-543d31c1-f064-4132-9405-11a8a2140403.
=============================================================
This was the #22 topcrasher today for Firefox 31.0a1, with 73 out of 7220 crashes.  It first appeared 2014-03-27. 

Stack: 

0 	xul.dll 	mozilla::gfx::SurfaceToPackedBGRA(mozilla::gfx::DataSourceSurface *) 	gfx/2d/DataSurfaceHelpers.cpp
1 	xul.dll 	mozilla::widget::AsyncFaviconDataReady::OnComplete(nsIURI *,unsigned int,unsigned char const *,nsACString_internal const &) 	widget/windows/WinUtils.cpp
2 	xul.dll 	mozilla::places::NotifyIconObservers::Run() 	toolkit/components/places/AsyncFaviconHelpers.cpp
3 	xul.dll 	nsThread::ProcessNextEvent(bool,bool *) 	xpcom/threads/nsThread.cpp
4 	xul.dll 	NS_ProcessNextEvent(nsIThread *,bool) 	xpcom/glue/nsThreadUtils.cpp
5 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) 	ipc/glue/MessagePump.cpp
6 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
7 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
8 	xul.dll 	nsBaseAppShell::Run() 	widget/xpwidgets/nsBaseAppShell.cpp
9 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp
10 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp
11 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp
12 	xul.dll 	XREMain::XRE_main(int,char * * const,nsXREAppData const *) 	toolkit/xre/nsAppRunner.cpp
13 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp
14 	firefox.exe 	do_main 	browser/app/nsBrowserApp.cpp
15 	firefox.exe 	NS_internal_main(int,char * *) 	browser/app/nsBrowserApp.cpp
16 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp
17 	firefox.exe 	__tmainCRTStartup 	f:/dd/vctools/crt_bld/self_x86/crt/src/crtexe.c
18 	kernel32.dll 	BaseThreadInitThunk 	
19 	ntdll.dll 	__RtlUserThreadStart 	
20 	ntdll.dll 	_RtlUserThreadStart

More Reports:
https://crash-stats.mozilla.com/report/list?product=Firefox&range_unit=days&version=Firefox%3A31.0a1&signature=mozilla%3A%3Agfx%3A%3ASurfaceToPackedBGRA%28mozilla%3A%3Agfx%3A%3ADataSourceSurface*%29&date=2014-04-04&range_value=14#tab-sigsummary

Comment 1

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

Jeff, where should we start looking based on the stack?

Comment 2

[Jeff Muizelaar [:jrmuizel]]

This code was introduced in bug 981430

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: patch

Pretty clearly a null-deref on dataSurface. The crash reports show that this is not an OOM issue though. Bas, it seems a bit concerning that GetDataSurface can be failing. Any thoughts on that?

Comment 4

[Jonathan Watt [:jwatt]]

There was a question for you in comment 3. ;)

Comment 5

[u279076]

This has risen to #13 for Firefox 31, now accounting for 1.07% of all Firefox 31 browser crashes in the last week. At this rate I expect this to become a topcrash very soon.

Comment 6

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/191b0a9e6664

Comment 7

[Jonathan Watt [:jwatt]]

Comment on attachment 8405920 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 981430
User impact if declined: topcrash
Testing completed (on m-c, etc.): landed on m-i, trivial patch safe for immediate uplift
Risk to taking this patch (and alternatives if risky): pretty much zero
String or IDL/UUID changes made by this patch: none

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/191b0a9e6664

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6baeb7902e34

Comment 10

[Bogdan Maris, Desktop Test Engineering]

No new crashes are registered in Socorro on 30 and 31 so marking this as verified.

Firefox 30 beta:
https://crash-stats.mozilla.com/query/?product=Firefox&version=Firefox%3A30.0b&range_value=2&range_unit=weeks&date=05%2F06%2F2014+09%3A00%3A00&query_search=signature&query_type=contains&query=mozilla%3A%3Agfx%3A%3ASurfaceToPackedBGRA%28mozilla%3A%3Agfx%3A%3ADataSourceSurface*%29&reason=&release_channels=&build_id=&process_type=any&hang_type=any

Firefox 31 Aurora:
https://crash-stats.mozilla.com/query/?product=Firefox&version=Firefox%3A31.0a2&range_value=2&range_unit=weeks&date=05%2F06%2F2014+09%3A00%3A00&query_search=signature&query_type=contains&query=mozilla%3A%3Agfx%3A%3ASurfaceToPackedBGRA%28mozilla%3A%3Agfx%3A%3ADataSourceSurface*%29&reason=&release_channels=&build_id=&process_type=any&hang_type=any

Comment 11

[Bas Schouten (:bas.schouten)]

(In reply to Jonathan Watt [:jwatt] (vacation Dec 22 - Jan 5) from comment #3)
> Created attachment 8405920 [details] [diff] [review]
> patch
> 
> Pretty clearly a null-deref on dataSurface. The crash reports show that this
> is not an OOM issue though. Bas, it seems a bit concerning that
> GetDataSurface can be failing. Any thoughts on that?

This should be able to occur on device loss and things like that.