Closed Bug 575901 Opened 13 years ago Closed 13 years ago

DeCOMtaminate nsIStyleRuleProcessor method signatures

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla2.0b2

People

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

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Patch
21.72 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
Remove nsresult return type from RulesMatching updated to tip
19.40 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
Attached patch Patch — — Splinter 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: 13 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.