Bug 7531

Summary: hang in SVGPolygonElementImpl::toPathData in polygon test case
Product: WebKit Reporter: Alex Jones <the.decryptor>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Major CC: alice.barraclough, a, darin
Priority: P1 Keywords: InRadar, SVGHitList
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://www.croczilla.com/svg/samples/polygons3/polygons3.xml
Attachments:
Description Flags
Sample
none
Proposed patch for solving slowness problem eric: review+

Description Alex Jones 2006-02-28 22:01:09 PST
When viewing the compound document, hovering over the elements will become more sluggish until suddenly WebKit will go to 100% CPU usage and freeze, i have tried the page on a few builds of WebKit and each and every one has had the same problem.

Source of the file is http://www.croczilla.com/svg/samples/ (called "Polygons 3")
Comment 1 Gustaaf Groenendaal (MysteryQuest) 2006-03-04 16:00:56 PST
Confirmed. Set severity to major since this can be considered as a crash.
Comment 2 Eric Seidel (no email) 2006-03-04 16:41:07 PST
Created attachment 6858 [details]
Sample
Comment 3 Darin Adler 2006-03-05 16:32:03 PST
The problem here is about indexing into a linked list. The points list is an SVGPointListImpl, which is implemented as a linked list. But the code in SVGPolygonElementImpl::toPathData indexes into it as if it was a vector.

This can be fixed by either changing it into a vector, or iterating through it in the proper manner for a linked list.
Comment 4 Darin Adler 2006-03-05 16:40:47 PST
The DOMList class is a curious one, and the underlying cause of this problem. It offers no way to iterate the list other than by index, but it's implemented with a linked list! The name "List" makes it sound like a linked list, but it's interface makes it more like a vector. The linked list implementation does make remove and insert operations fast.

I think that DOMList should be changed to use a Vector. But it also might be best to simply eliminate this class. The current implementation is very strange. The only function in the class that does any ref/deref on the items in the list is clear(), which calls deref on all the items in the list. That's very strange since the deref doesn't balance anything done elsewhere in the class. This needs to be cleaned up.
Comment 5 Eric Seidel (no email) 2006-03-06 01:53:09 PST
DOMList should just be killed.
Comment 6 Alexander Kellett 2006-03-07 12:28:04 PST
steps:
1. merge domlist into svglist
2. reimplement to use vector
3. update code to use saner interface if appropriate 
sounds good?

to me it should from a bird eyes view look seems like the layout is never finishing, every call is causing a full re-layout. is this really meant to be happen?
Comment 7 Alice Liu 2006-03-20 07:26:49 PST
<rdar://problem/4483863>
Comment 8 Rob Buis 2006-04-08 13:22:04 PDT
Created attachment 7584 [details]
Proposed patch for solving slowness problem

DOMList may need redesigning, however I think this patch
solves the real problem at hand.
Comment 9 Eric Seidel (no email) 2006-04-08 13:24:15 PDT
Comment on attachment 7584 [details]
Proposed patch for solving slowness problem

Looks good. r=me