377918 - Crash [@ gfxContext::CurrentSurface] with tab character (new textframe)

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Simon Montagu :smontagu]

Top of stack:

thebes.dll!gfxContext::CurrentSurface(double * dx=0x00000000, double * dy=0x00000000)  Line 78 + 0x3 bytes
thebes.dll!gfxContext::CurrentSurface()  Line 91 + 0x10 bytes
thebes.dll!SetupContextFont(gfxContext * aContext=0x00000000, gfxWindowsFont * aFont=0x034ac890)  Line 635 + 0xc bytes
thebes.dll!gfxWindowsFontGroup::InitTextRunGDI(gfxContext * aContext=0x00000000, gfxTextRun * aRun=0x034acf40, const char * aString=0x01f7dc37, unsigned int aLength=0x00000001)  Line 723 + 0x10 bytes
thebes.dll!gfxWindowsFontGroup::MakeTextRun(const unsigned char * aString=0x01f7dc37, unsigned int aLength=0x00000001, gfxTextRunFactory::Parameters * aParams=0x0012c784)  Line 623
thebes.dll!gfxFontGroup::GetSpecialStringTextRun(gfxFontGroup::SpecialString aString=STRING_SPACE, gfxTextRun * aTemplate=0x03eef6c0)  Line 551 + 0x18 bytes
gklayout.dll!GetSpecialString(gfxFontGroup * aFontGroup=0x03eef5b8, gfxFontGroup::SpecialString aSpecial=STRING_SPACE, gfxTextRun * aTextRun=0x03eef6c0)  Line 1172
gklayout.dll!PropertyProvider::GetSpacing(unsigned int aStart=0x00000000, unsigned int aLength=0x00000001, gfxFont::Spacing * aSpacing=0x0012c9d8)  Line 2006 + 0x1d bytes
thebes.dll!gfxTextRun::GetAdjustedSpacing(unsigned int aStart=0x00000000, unsigned int aEnd=0x00000001, gfxTextRun::PropertyProvider * aProvider=0x0012d1d8, gfxFont::Spacing * aSpacing=0x0012c9d8)  Line 753
thebes.dll!gfxTextRun::BreakAndMeasureText(unsigned int aStart=0x00000000, unsigned int aMaxLength=0x00000001, int aLineBreakBefore=0x00000001, double aWidth=58860.000000000000, gfxTextRun::PropertyProvider * aProvider=0x0012d1d8, int aSuppressInitialBreak=0x00000001, gfxFont::RunMetrics * aMetrics=0x0012d2b8, int aTightBoundingBox=0x00000000, int * aUsedHyphenation=0x0012d2b4, unsigned int * aLastBreak=0x0012d274)  Line 1193
gklayout.dll!nsTextFrame::Reflow(nsPresContext * aPresContext=0x03e68c60, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0x03f07ef8)  Line 4762 + 0x64 bytes

MakeTextRun on Windows needs a gfxContext, and GetSpecialStringTextRun doesn't provide one.

Comment 1

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

I'm thinking of moving GetSpecialStringTextRun out of gfxFontGroup altogether and just have it live in nsTextFrame(Thebes). When the new textrun cache goes in, we won't need to specially cache special strings any more.

Comment 2

[Simon Montagu :smontagu]

Attachment: Assert on aParams->mContext

Ok, but should we at least put in an assertion?

Comment 3

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

Comment on attachment 263584 [details] [diff] [review]
Assert on aParams->mContext

sure

Comment 4

[Simon Montagu :smontagu]

Attachment 263584 [details] [diff] checked in. The crash should be fixed by bug 375760.

Comment 6

[Simon Montagu :smontagu]

No crash in today's build.

Comment 7

[Simon Montagu :smontagu]

Attachment: reftest

This is my first reftest, so I'd like review

Comment 8

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

What are you trying to test here? We don't normally create reftests to test crashes.

Comment 9

[Simon Montagu :smontagu]

I'm trying to kill two birds with one stone and test that there isn't a visible glyph for the tab character.

Comment 10

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

Comment on attachment 270190 [details] [diff] [review]
reftest

ok

Comment 11

[Eli Friedman]

(In reply to comment #8)
> What are you trying to test here? We don't normally create reftests to test
> crashes.

How else are you going to test crashes?

Comment 12

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

I dunno if there's an approved method, but mochitests would be better than reftests, because they avoid having to load a reference page and build a compressed PNG for each page.