Closed Bug 899894 Opened 6 years ago Closed 6 years ago

don't null check nsStyleSet::ResolveBlah return values

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: heycam, Assigned: heycam)

Details

Attachments

(1 file)

Attached patch null-checksSplinter Review
None of the nsStyleSet::ResolveBlah functions can ever return null, so we can avoid null checking their return values.
Attachment #783556 - Flags: review?(dbaron)
Comment on attachment 783556 [details] [diff] [review]
null-checks

In ElementRestyler::RestyleSelf, maybe MOZ_ASSERT(newExtraContext)
too?

Could you also remove the null-checks *inside* nsStyleSet::GetContext?


(I wonder if we should pursue using an auto-rewriting tool to remove
more null checks like these?)

r=dbaron
Attachment #783556 - Flags: review?(dbaron) → review+
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #2)
> Could you also remove the null-checks *inside* nsStyleSet::GetContext?

Which ones?

> (I wonder if we should pursue using an auto-rewriting tool to remove
> more null checks like these?)

Yeah, might be nice to have some tool that knows you've introduced an unnecessary null check into your patch.
Flags: needinfo?(dbaron)
(In reply to Cameron McCormack (:heycam) from comment #3)
> (In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from
> comment #2)
> > Could you also remove the null-checks *inside* nsStyleSet::GetContext?
> 
> Which ones?

Oh, I was looking at a tree without https://hg.mozilla.org/mozilla-central/rev/0261e8137818 .  Oops.
Flags: needinfo?(dbaron)
https://hg.mozilla.org/mozilla-central/rev/c732306ad9af
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.