Bug 15584

Summary: REGRESSION(r26696): GtkLauncher segfaults on WebCore::WidthIterator::advance
Product: WebKit Reporter: Jan Alonzo <jmalonzo>
Component: TextAssignee: Mark Rowe (bdash) <mrowe>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, mrowe
Priority: P2 Keywords: Gtk
Version: 523.x (Safari 3)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
segfault backtrace
none
Reduction
none
Patch hyatt: review+

Jan Alonzo
Reported 2007-10-20 18:58:39 PDT
GtkLauncher crashes on WebCore::WidthIterator::advance with a segmentation fault. This happens on select sites like google.com, wikipedia, but it doesn't seem to crash on http://planet.gnome.org. Steps to reproduce: 1) Launch GtkLauncher with ./WebKitBuild/Debug/WebKitTools/GtkLauncher/GtkLauncher 2) Crash.
Attachments
segfault backtrace (6.07 KB, text/plain)
2007-10-20 19:00 PDT, Jan Alonzo 		no flags
Details
Reduction (114 bytes, text/html)
2007-10-20 19:04 PDT, Mark Rowe (bdash) 		no flags
Details
Patch (1.61 KB, patch)
2007-10-20 19:45 PDT, Mark Rowe (bdash) 		hyatt: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Jan Alonzo
Comment 1 2007-10-20 19:00:47 PDT
Created attachment 16751 [details] segfault backtrace backtrace
Mark Rowe (bdash)
Comment 2 2007-10-20 19:04:14 PDT
Created attachment 16752 [details] Reduction I don't see the crash on launch as described, but I can reproduce this when searching Google for "bdash". I've attached a reduction of the page that demonstrates the crash. It appears to be a single Unicode character that is causing the problem.
Mark Rowe (bdash)
Comment 3 2007-10-20 19:15:07 PDT
This was introduced in http://trac.webkit.org/projects/webkit/changeset/26696.
Mark Rowe (bdash)
Comment 4 2007-10-20 19:45:05 PDT
Created attachment 16755 [details] Patch
Darin Adler
Comment 5 2007-10-20 19:47:59 PDT
Comment on attachment 16755 [details] Patch Good fix. But we really should structure this so we don't call glyphDataForCharacter twice; it can be an expensive operation. If references make this too tricky, you can use a const GlyphData*.
Dave Hyatt
Comment 6 2007-10-20 19:49:37 PDT
Comment on attachment 16755 [details] Patch r=me, although darin should maybe look at this when he gets a chance.
Mark Rowe (bdash)
Comment 7 2007-10-20 19:51:47 PDT
Landed in r26837.
Mark Rowe (bdash)
Comment 8 2007-10-20 19:58:04 PDT
Darin, GlyphPage::glyphDataForCharacter is always an array lookup so I wouldn't consider it an expensive operation. I landed it after Dave reviewed it on IRC, but I can go ahead and make the change you suggested if you would like.
Darin Adler
Comment 9 2007-10-21 09:42:07 PDT
(In reply to comment #8) > Darin, GlyphPage::glyphDataForCharacter is always an array lookup so I wouldn't > consider it an expensive operation. I landed it after Dave reviewed it on IRC, > but I can go ahead and make the change you suggested if you would like. OK. I can live with this the way it is, I guess.
Note You need to log in before you can comment on or make changes to this bug.