DeCOMtaminate nsIStyleRuleProcessor method signatures

RESOLVED FIXED in mozilla2.0b2

Status

()

defect
--
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: craig.topper, Assigned: craig.topper)

Tracking

(Blocks 1 bug)

Trunk
mozilla2.0b2
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

9 years ago
Posted patch PatchSplinter Review
Converted an outparam on one method. The others I just changed NS_IMETHOD to virtual nsresult because the implementations can return something other than NS_OK. Though I don't think the caller of RulesMatching checks the result. So I'm willing to do further changes to remove the return type with some help.
Attachment #455078 - Flags: review?(dbaron)
> Though I don't think the caller of RulesMatching checks the result.

Indeed.  I think we should nix those return types.  What help do you need?
(Assignee)

Comment 2

9 years ago
There's an OOM check in ElementTransitions::EnsureStyleRuleFor that causes RulesMatching to return NS_ERROR_OUT_OF_MEMORY. Can I just remove the OOM check from that method?
Yes, operator new never returns null on trunk.
(Assignee)

Comment 4

9 years ago
Removed the OOM check so I could remove the nsresult return type.
Attachment #455612 - Flags: review?(dbaron)
(Assignee)

Comment 5

9 years ago
Attachment #455612 - Attachment is obsolete: true
Attachment #455891 - Flags: review?(dbaron)
Attachment #455612 - Flags: review?(dbaron)
(Assignee)

Updated

9 years ago
Blocks: deCOM
(Assignee)

Updated

9 years ago
Blocks: 576831
(Assignee)

Updated

9 years ago
Attachment #455078 - Flags: review?(dbaron) → review?(bzbarsky)
(Assignee)

Updated

9 years ago
Attachment #455891 - Flags: review?(dbaron) → review?(bzbarsky)
Comment on attachment 455078 [details] [diff] [review]
Patch

r=bzbarsky
Attachment #455078 - Flags: review?(bzbarsky) → review+
Comment on attachment 455891 [details] [diff] [review]
Remove nsresult return type from RulesMatching updated to tip

This is fine, but one followup that might be worth doing is to implement empty version of the various RulesMatching methods directly on nsIStyleRuleProcessor and remove all the empty subclass impls  Might make icache a little happier.
Attachment #455891 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/02e762e39d42
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
OS: Windows 98 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b2
You need to log in before you can comment on or make changes to this bug.