439343 - Crash [@ _moz_cairo_surface_set_device_offset ] with text-shadow, large text-indent, font-size and letter-spacing

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build for me.
Without the use of text-shadow, it doesn't crash for me, so I guess this is a regression from bug 10713, somehow.

http://crash-stats.mozilla.com/report/index/5d51bf54-3b0a-11dd-8795-001cc45a2c28?p=1
0  	xul.dll  	_moz_cairo_surface_set_device_offset  	 cairo-surface.c:823
1 	xul.dll 	gfxASurface::SetDeviceOffset 	gfxASurface.cpp:220
2 	xul.dll 	nsContextBoxBlur::Init 	nsCSSRendering.cpp:4813
3 	xul.dll 	nsTextFrame::PaintOneShadow 	nsTextFrameThebes.cpp:3995
4 	xul.dll 	nsTextFrame::PaintText 	nsTextFrameThebes.cpp:4279
5 	xul.dll 	nsDisplayText::Paint 	nsTextFrameThebes.cpp:3537
6 	xul.dll 	nsDisplayList::Paint 	nsDisplayList.cpp:296
7 	xul.dll 	nsDisplayClip::Paint 	nsDisplayList.cpp:880
8 	xul.dll 	nsDisplayList::Paint 	nsDisplayList.cpp:296
9 	xul.dll 	nsDisplayClip::Paint 	nsDisplayList.cpp:880
10 	xul.dll 	nsDisplayList::Paint 	nsDisplayList.cpp:296
11 	xul.dll 	nsLayoutUtils::PaintFrame 	nsLayoutUtils.cpp:988
12 	xul.dll 	PresShell::Paint 	nsPresShell.cpp:5413
13 	xul.dll 	nsViewManager::RenderViews 	nsViewManager.cpp:614
14 	xul.dll 	nsViewManager::Refresh 	nsViewManager.cpp:502
15 	xul.dll 	xul.dll@0x30975d 	
16 	xul.dll 	HandleEvent 	nsView.cpp:168
17 	xul.dll 	nsWindow::DispatchEvent 	nsWindow.cpp:985
18 	xul.dll 	nsWindow::DispatchWindowEvent 	nsWindow.cpp:1010
19 	xul.dll 	xul.dll@0x2fb0ec 	
20 	xul.dll 	nsWindow::ProcessMessage 	nsWindow.cpp:4288
21 	xul.dll 	nsWindow::WindowProc 	nsWindow.cpp:1200
22 	user32.dll 	InternalCallWinProc 	
23 	user32.dll 	UserCallWinProcCheckWow 	
24 	user32.dll 	DispatchClientMessage 	
25 	user32.dll 	__fnDWORD 	
26 	ntdll.dll 	KiUserCallbackDispatcher 	
27 	xul.dll 	nsCxPusher::Push 	
28 	user32.dll 	DispatchMessageW 	
29 	xul.dll 	nsAppShell::ProcessNextNativeEvent 	nsAppShell.cpp:148

Comment 1

[Michael Ventnor]

Looks like a Cairo bug. I'll try to see if there are any sore spots; vlad, could you also try to have a look?

Comment 2

[Michael Ventnor]

Attachment: Patch

Got it. We're getting an invalid surface due to the ridiculous values, and simple GFX API's don't do safety checking obviously for perf reasons. Cairo was dereferencing a null pointer.

This fixes it by making it public whether the surface is valid or not.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 325209 [details] [diff] [review]
Patch

I'll let Vlad take this to make sure he's OK with the API.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Just use CairoStatus() that's already there -- see http://hg.mozilla.org/mozilla-central/index.cgi/file/5034371d2778/gfx/thebes/src/gfxASurface.cpp#l272 .  Check mImageSurface->CairoStatus() != 0 to see if there's an error/invalid surface.  Not ideal, but it's what's used in a few other places now.. we don't have anything good in place for dealing with errors there.

Comment 5

[Michael Ventnor]

Attachment: Patch 2

Comment 6

[Michael Ventnor]

Comment on attachment 325225 [details] [diff] [review]
Patch 2

Throwing to roc since no API change is needed anymore.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 325225 [details] [diff] [review]
Patch 2

+  if (!mImageSurface || mImageSurface->CairoStatus() != 0)

lose the "!= 0"

Comment 8

[Michael Ventnor]

Attachment: Patch 3

Comment 9

[Reed Loden [:reed]]

Pushed as changeset 15714:1b378971708e.

Comment 10

[Martijn Wargers (dead)]

Well, the testcase doesn't crash anymore, but it is now massively slow with current trunk build and using a lot of memory.

Comment 11

[Martijn Wargers (dead)]

Anyway, no crash anymore, so this is fixed.
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a1pre) Gecko/2008071504
Minefield/3.1a1pre

Comment 12

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3cb00994014f

Comment 13

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/3cb00994014f