Bug 26470 – [Cairo] Zero-height image pattern Causes Crash on Windows, improper Behavior on GTK

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brent Fulgham]

The Windows Cairo build of WebKit will crash when running the LayoutTest "fast/gradients/border-image-gradient-sides-and-corners.html".

The failure occurs down inside cairo, but is triggered by a NaN value generated in Image::drawTiled (see Image.cpp line 170):

vPhase -= fmodf(dstRect.height(), scale.height() * srcRect.height() / 2.0f);

The fmodf (at least under Windows) generates a silent NaN, yielding a vPhase of -1.0#IND00.  Later, this triggers a divide-by-zero deep in the cairo logic.

This same test works properly under Safari.

[Brent Fulgham]

The GTK+ build of WebKit does not crash, but seems to generate different gradient images each time the screen refreshes.  This is sort of a neat effect, but certainly not the intended behavior!

[Gustavo Noronha (kov)]

As I discussed on IRC with Brent, the NaN also happens in GTK+, but the only problem I can see with the test is that the gradients are drawn incorrectly, and seem to have different colors each time they are rendered - no crash here. I'm attaching screenshots - I did nothing but take the screenshots, I'm beting that taking the shot causes the window to be re-exposed.

[Gustavo Noronha (kov)]

Created attachment 31415 [details]
first shot

[Gustavo Noronha (kov)]

Created attachment 31416 [details]
shot 2

[Gustavo Noronha (kov)]

Created attachment 31417 [details]
shot 3

[Brent Fulgham]

Running the same test in a Mac OS X Debug build, I can see that the vPhase calculation also produces a NaN value.  This is eventually processed in the ImageCG.cpp (Image::drawPattern) method, which works with the NaN as-is, uses it in the various transform calculations, then uses them to generate the pattern.

CGPatternCreate seems to handle the NaN matrix without any errors:
matrix = { a = 0.5, b = 0, c = 0, d = 0.5, tx = -nan(0x400000), ty = -nan(0x400000) } 

[Brent Fulgham]

Created attachment 31422 [details]
Comparison of Safari and WinCairo after patch

[Brent Fulgham]

Created attachment 31425 [details]
Avoid NaN crash

[Brent Fulgham]

Patch confirmed to work on both Windows Cairo build, and webkitgtk.

[Gustavo Noronha (kov)]

Comment on attachment 31425 [details]
Avoid NaN crash

Looks right to me (and also trusting krit's judgement).

[Brent Fulgham]

Landed in http://trac.webkit.org/changeset/44771.