Closed Bug 585787 Opened 14 years ago Closed 14 years ago

Replace global compares with compartment compares in quickstubs once the latter are fast

Categories

(Core :: XPConnect, defect)

Product:
Core
Component:
XPConnect
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla2.0b8

People

(Reporter: bzbarsky, Assigned: jst)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

Fix, both places covered, and with assertions for real this time.
(deleted), patch
mrbkap
: review+
bzbarsky
: approval2.0+
Details | Diff | Splinter Review 
See comments in the patch for bug 585783.
Blocks: 599438
Attached patch Fix. (obsolete) (deleted) — — Splinter Review 

        Attachment #485229 -
        Flags: review?(mrbkap)

    
    

    
  
Why is the right thing cx->compartment and not obj->compartment() ?

    
    

    
  
Comment on attachment 485229 [details] [diff] [review]
Fix.

There's one more place that needs this change, new patch coming up.

        Attachment #485229 -
        Attachment is obsolete: true

        Attachment #485229 -
        Flags: review?(mrbkap)

    
  
Assignee: nobody → jst
Status: NEW → ASSIGNED

    
    

    
  
obj->compartment has to be equal to cx->compartment and cx->compartment is cheaper to read. We should assert this for extra safety.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Fix, both places covered, and with assertions. (obsolete) (deleted)
        — 
          — Splinter Review
      

    


  

        Attachment #485232 -
        Flags: review?(mrbkap)

    
    

    
  
The assertions went awol....

Andreas, thanks for explaining.

    
    

    
  
Comment on attachment 485232 [details] [diff] [review]
Fix, both places covered, and with assertions.

Duh, forgot to qref, new patch coming up.

        Attachment #485232 -
        Attachment is obsolete: true

        Attachment #485232 -
        Flags: review?(mrbkap)

        Attachment #485310 -
        Flags: review?(mrbkap) → review+

    
    

    
  
Can we land this, please?  I'd really like to merge the patch for bug 590612 on top of this, instead of just conflicting with this change....

    
  

        Attachment #485310 -
        Flags: approval2.0?

    
  

        Attachment #485310 -
        Flags: approval2.0? → approval2.0+

    
    

    
  
Pushed http://hg.mozilla.org/mozilla-central/rev/410c9f17f0fc
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Peter's patch merged to (obsolete) (deleted)
        — 
          — Splinter Review
      

    


  

    
    

    
  
Comment on attachment 490890 [details] [diff] [review]
Peter's patch merged to

Er... ignore this.  bzexport fail.

        Attachment #490890 -
        Attachment is obsolete: true

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: