856794 - fix callers of infallible nsTArray::EnsureLengthAtleast() to not look at the return type, in SetBackgroundList / SetBackgroundPairList

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: bug 856794 - don't check the return value of nsTArray::EnsureLengthAtleast()

Comment 2

[Daniel Holbert [:dholbert]]

Comment on attachment 732040 [details] [diff] [review]
bug 856794 - don't check the return value of nsTArray::EnsureLengthAtleast()

Looks like this code was added in bug 322475 and bug 576044 by zwol.

Probably worth getting his input on this.  If this code is a way for web developers to easily make us allocate an arbitrarily large amount of memory[1], then we may want to switch to a fallible array and keep the alloc-failed fallback code.

--> kicking review over to zwol.

[1] (I'm not sure offhand if this is the case or not, for this code. But that's my rough internal heuristic for "should we be using fallible memory-allocation?")

Comment 3

[Zack Weinberg (:zwol)]

Comment on attachment 732040 [details] [diff] [review]
bug 856794 - don't check the return value of nsTArray::EnsureLengthAtleast()

In general, CSS can only make the parser / rule processor allocate memory linear in the size of the style sheet, and on that basis we haven't really even tried to make it robust against out-of-memory, despite appearances.  I wrote a patch at one point to rely on infallible malloc from the parser proper; don't remember if it ever landed.

So I think this is safe, but I'm going to forward review to Cameron, because I haven't looked at nsRuleNode in quite some time and I might not know something important.

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 732040 [details] [diff] [review]
bug 856794 - don't check the return value of nsTArray::EnsureLengthAtleast()

Yeah this seems safe to me.

Comment 5

[Trevor Saunders (:tbsaunde)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/0e12c03b2915

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0e12c03b2915