Summary: | [GTK] Crash when GlyphPage::fill is called with more than 256 bytes of data | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | doug turner <dougt> | ||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | alp | ||||
Priority: | P2 | Keywords: | Gtk | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Mac | ||||||
OS: | OS X 10.4 | ||||||
Attachments: |
|
Description
doug turner
2007-11-19 12:35:34 PST
Created attachment 17405 [details]
not a real fix
I am not sure if this is the right approach.. in fact, i am pretty sure that it isn't.
The ASSERT should be checked in so that we do not hit this problem in the future. As for the second part, it seams that the caller to fill() needs to be notified that only some of the data could be processed.
*** Bug 14446 has been marked as a duplicate of this bug. *** (In reply to comment #1) > Created an attachment (id=17405) [edit] > not a real fix > > I am not sure if this is the right approach.. in fact, i am pretty sure that it > isn't. > This is straight from the Windows port (WebCore/platform/win/GlyphPageTreeNodeWin.cpp): bool GlyphPage::fill(UChar* buffer, unsigned bufferLength, const FontData* fontData) { // The bufferLength will be greater than the glyph page size if the buffer has Unicode supplementary characters. // We won't support this for now. if (bufferLength > GlyphPage::size) return false; bool haveGlyphs = false; CGGlyph localGlyphBuffer[GlyphPage::size]; wkGetGlyphs(fontData->platformData().cgFont(), buffer, localGlyphBuffer, bufferLength); for (unsigned i = 0; i < GlyphPage::size; i++) { Glyph glyph = localGlyphBuffer[i]; if (!glyph) setGlyphDataForIndex(i, 0, 0); else { setGlyphDataForIndex(i, glyph, fontData); haveGlyphs = true; } } return haveGlyphs; } Looks like this strategy is just fine for now, and fixes a real crasher bug. Nice catch! Looking at this line from Win: for (unsigned i = 0; i < GlyphPage::size; i++) We do this instead: for (unsigned i = 0; i < bufferLength; i++) Both yield the same result AFAICT. I wonder which is more correct. Comment on attachment 17405 [details]
not a real fix
This fix is good enough and fixes some major crashers. The Win port does the same thing.
The return needs to be moved before the face is locked, but otherwise looks good.
Landed in r27914. If there are further issues we can open a new bug, or RFE for better Unicode handling and/or complex text support. It's too bad there isn't a way to share the common code. |