Bug 15584

Summary:

REGRESSION(r26696): GtkLauncher segfaults on WebCore::WidthIterator::advance

Product:

WebKit

Reporter:

Jan Alonzo <jmalonzo>

Component:

Text

Assignee:

Mark Rowe (bdash) <mrowe>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, mrowe

Priority:

Keywords:

Gtk

Version:

523.x (Safari 3)

Hardware:

OS:

Linux

Attachments:

Description	Flags
segfault backtrace	none
Reduction	none
Patch	hyatt: review+

Description Jan Alonzo 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.

Comment 1 Jan Alonzo 2007-10-20 19:00:47 PDT

Created attachment 16751 [details]
segfault backtrace

backtrace

Comment 2 Mark Rowe (bdash) 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.

Comment 3 Mark Rowe (bdash) 2007-10-20 19:15:07 PDT

This was introduced in http://trac.webkit.org/projects/webkit/changeset/26696.

Comment 4 Mark Rowe (bdash) 2007-10-20 19:45:05 PDT

Created attachment 16755 [details]
Patch

Comment 5 Darin Adler 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*.

Comment 6 Dave Hyatt 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.

Comment 7 Mark Rowe (bdash) 2007-10-20 19:51:47 PDT

Landed in r26837.

Comment 8 Mark Rowe (bdash) 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.

Comment 9 Darin Adler 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.