Add a SetCount() method to nsCOMArray and nsVoidArray

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

This bug is about adding a SetCount() method to nsCOMArray and nsVoidArray to allow the number of elements in these types of arrays to be easily set. I'm specifically talking about changing the number of elements in these arrays, not about (just) changing their capacity.

Note that nsTArray supports this via it's SetLength() method.

Some background: I'm rewriting the code that handles SVG length list attributes, etc, in SVG. I'm splitting the current heavyweight all-in-one DOM/internal lists into fast and simple internal lists, and DOM tearoff counterpart lists that are created on demand to wrap their corresponding internal list. When a list attribute changes, the DOM list's length must be kept in sync with its internal counterpart. I'm using nsCOMArray in the DOM wrapper list classes, so I need to be able to change the number of elements they contain, removing excess elements when the list's length is decreased, or filling the extra elements with null when it's increasing (elements are created lazily on demand, so that's exactly what I want). This is all easy for the internal lists where I'm using nsTArray, but not for nsCOMArray.
Posted patch patch (obsolete) — Splinter Review
Assignee: nobody → jwatt
Status: NEW → ASSIGNED
Attachment #431477 - Flags: review?
Attachment #431477 - Flags: review?(dbaron)
Attachment #431477 - Flags: review?(benjamin)
Attachment #431477 - Flags: review?
Changed the tab in nsCOMArray_base::SetCount to spaces locally.
It's worth noting that both nsCOMArray and nsVoidArray have ctors that allow you to pass an initial count to null fill an initial number of elements, so a SetCount() doesn't seem amiss.
Comment on attachment 431477 [details] [diff] [review]
patch

Benjamin's review should be sufficient here; you don't need mine.
Attachment #431477 - Flags: review?(dbaron)
Okay. Benjamin thought you should look it over too, but I'm happy either way.
Comment on attachment 431477 [details] [diff] [review]
patch

No tests?
Attachment #431477 - Flags: review?(benjamin) → review-
Posted patch patch + testSplinter Review
Sorry, you're quire right. Here's a fixed patch with a test that I've actually run. I should know better by now than to think something's too simple to screw up. Can I blame it on Q1 deadline pressure? :-(

(FWIW (not much) the tests I've been writing for bug 515116 would have caught this once I got round to running them.)
Attachment #431477 - Attachment is obsolete: true
Attachment #432290 - Flags: review?(benjamin)
Blocks: 515116
Attachment #432290 - Flags: review?(benjamin) → review+
Pushed http://hg.mozilla.org/mozilla-central/rev/6dcd5017b9be
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.