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)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(2 files)
10.00 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
4.68 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
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.)
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8485217 -
Flags: review?(dbaron)
Assignee | ||
Comment 2•10 years ago
|
||
(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+
Assignee | ||
Comment 4•10 years ago
|
||
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)
Assignee | ||
Comment 5•10 years ago
|
||
(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.)
Attachment #8491201 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2077fa33653d https://hg.mozilla.org/integration/mozilla-inbound/rev/308462c1b78a
Flags: in-testsuite-
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.
Description
•