Closed Bug 575901 Opened 10 years ago Closed 10 years ago

DeCOMtaminate nsIStyleRuleProcessor method signatures

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla2.0b2

People

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

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Attached 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?
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.
Removed the OOM check so I could remove the nsresult return type.
Attachment #455612 - Flags: review?(dbaron)
Attachment #455612 - Attachment is obsolete: true
Attachment #455891 - Flags: review?(dbaron)
Attachment #455612 - Flags: review?(dbaron)
Blocks: deCOM
Blocks: 576831
Attachment #455078 - Flags: review?(dbaron) → review?(bzbarsky)
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+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/02e762e39d42
Status: ASSIGNED → RESOLVED
Closed: 10 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.