Bug 15584 - REGRESSION(r26696): GtkLauncher segfaults on WebCore::WidthIterator::advance
: REGRESSION(r26696): GtkLauncher segfaults on WebCore::WidthIterator::advance
Status: RESOLVED FIXED
: WebKit
Text
: 523.x (Safari 3)
: PC Linux
: P2 Normal
Assigned To:
:
: Gtk
:
:
  Show dependency treegraph
 
Reported: 2007-10-20 18:58 PST by
Modified: 2007-10-21 09:42 PST (History)


Attachments
segfault backtrace (6.07 KB, text/plain)
2007-10-20 19:00 PST, Jan Alonzo
no flags Details
Reduction (114 bytes, text/html)
2007-10-20 19:04 PST, Mark Rowe (bdash)
no flags Details
Patch (1.61 KB, patch)
2007-10-20 19:45 PST, Mark Rowe (bdash)
hyatt: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2007-10-20 18:58:39 PST
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.
------- Comment #1 From 2007-10-20 19:00:47 PST -------
Created an attachment (id=16751) [details]
segfauly backtrace

backtrace 
------- Comment #2 From 2007-10-20 19:04:14 PST -------
Created an attachment (id=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.
------- Comment #3 From 2007-10-20 19:15:07 PST -------
This was introduced in http://trac.webkit.org/projects/webkit/changeset/26696.
------- Comment #4 From 2007-10-20 19:45:05 PST -------
Created an attachment (id=16755) [details]
Patch
------- Comment #5 From 2007-10-20 19:47:59 PST -------
(From update of attachment 16755 [details])
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*.
------- Comment #6 From 2007-10-20 19:49:37 PST -------
(From update of attachment 16755 [details])
r=me, although darin should maybe look at this when he gets a chance.
------- Comment #7 From 2007-10-20 19:51:47 PST -------
Landed in r26837.
------- Comment #8 From 2007-10-20 19:58:04 PST -------
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.
------- Comment #9 From 2007-10-21 09:42:07 PST -------
(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.