Closed Bug 1063775 Opened 10 years ago Closed 10 years ago

Remove null checks after 'new' (infallible allocation) in StyleAnimationValue.cpp

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(2 files)

I noticed we have a lot of null checks after 'new' in StyleAnimationValue.cpp, mostly for nsCSSValue & friends.

Since 'new' is infallible, we should just drop these null-checks.

(Unless we're planning on switching to fallible allocation for these objects, for some reason, and converting nsCSSParser.cpp to be aware of that as well [it currently assumes we have infallible allocation].  I don't think that's going to happen, though.)
Attached patch fix v1Splinter Review
Attachment #8485217 - Flags: review?(dbaron)
(There are also some calls to e.g. nsCSSValueList->Clone() which we null-check in StyleAnimationValue.cpp; they seem to be infallible as well, since they use unchecked "new" under the hood, and appear to be infallible by extension. I haven't removed those checks here, but I may in a subsequent patch.)
Comment on attachment 8485217 [details] [diff] [review]
fix v1

r=dbaron

(Don't forget about the followup.  Although I suppose it's not critical either.)
Attachment #8485217 - Flags: review?(dbaron) → review+
Thanks! Here's the followup patch to document & assert that the nsCSSValue*List Clone() methods are infallible, and remove their null-checks in StyleAnimationValue.cpp.

(I couldn't find any other usages of these functions in layout/style, so I think that's the only spot that needs fixing.)
Attachment #8491201 - Flags: review?(dbaron)
(The assertions are more for documentation than for actual value-checking. I put them right next to the sole "return" statement, as proof/reminder that we can't return null -- but really, if 'result' were null, we would have crashed earlier in the function.)
https://hg.mozilla.org/mozilla-central/rev/2077fa33653d
https://hg.mozilla.org/mozilla-central/rev/308462c1b78a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: