Closed Bug 473911 Opened 16 years ago Closed 16 years ago

Crash due to too much recursion in nsCSSDocumentRule::URL::~URL

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: jruderman, Assigned: Waldo)

References

Details

(Keywords: crash, testcase)

Attachments

(2 files, 1 obsolete file)

http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSRules.h#209 ~URL() { delete next; } Should probably be converted into a loop, either in the destructor or in whatever causes the destructor to run.
Attached patch Loop inside ~URL (obsolete) — Splinter Review
Replacing all delete calls with ->Destroy() calls would perhaps be aesthetically better (and would avoid a next-null check if the compiler's not super-clever), but then you can't have automatic destruction of nsCSSDocumentRule::mURLs, and the patch would touch more lines. I prefer this given the more-lines concern, but your call which is better.
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Attachment #357333 - Flags: review?(dbaron)
Rather than adding a Destroy function, could you just use the NS_CSS_DELETE_LIST_MEMBER macro? This also has the advantage of making it easier to find all the places where we do this.
Also, it seems like the test should instead go in layout/style/crashtests/
Attached patch Done and doneSplinter Review
Attachment #357333 - Attachment is obsolete: true
Attachment #357437 - Flags: review?(dbaron)
Comment on attachment 357437 [details] [diff] [review] Done and done r+sr=dbaron
Attachment #357437 - Flags: superreview+
Attachment #357437 - Flags: review?(dbaron)
Attachment #357437 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Depends on: 477490
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: