Closed Bug 451590 Opened 16 years ago Closed 16 years ago

nsNavHistory needs nsIClassInfo with THREADSAFE property

Categories

(Toolkit :: Places, defect)

Product:
Toolkit
Component:
Places
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.1b1

People

(Reporter: sdwilsh, Assigned: sdwilsh)

References

Details

Attachments

(1 file)

v1.0
27.61 KB, patch
dietrich
: review+
bent.mozilla
: review+
Details | Diff | Splinter Review 
since you can access the database connection on more than one thread.  We also should add an assertion in every other method to ensure that we are on the main thread only (we can slowly start to make things more threadsafe if we so choose too).
Assignee: nobody → sdwilsh
Summary: Places needs nsIClassInfo with THREADSAFE property → nsNavHistory needs nsIClassInfo with THREADSAFE property
Attached patch v1.0 — — Splinter Review 

        Attachment #335835 -
        Flags: review?(dietrich)

    
  
Whiteboard: [has patch][needs review dietrich]

        Attachment #335835 -
        Flags: review?(dietrich) → review+

    
    

    
  
Comment on attachment 335835 [details] [diff] [review]
v1.0


>+
>+// We don't care about flattening everything
>+NS_IMPL_CI_INTERFACE_GETTER5(
>+  nsNavHistory
>+, nsINavHistoryService
>+, nsIGlobalHistory3
>+, nsIGlobalHistory2
>+, nsIDownloadHistory
>+, nsIBrowserHistory
>+)
> 

do you also need nsICharsetResolver here?

the rest looks straightforward enough, but please get follow-on review from bent or someone more familiar w/ the thread utils code.

    
    

    
  
(In reply to comment #2)
> do you also need nsICharsetResolver here?
I didn't want to flatten that interface - if we really needed to get to it in script, we could still QI (it didn't seem useful to expose by default).

    
  

        Attachment #335835 -
        Flags: review?(bent.mozilla)

    
  
Whiteboard: [has patch][needs review dietrich] → [has patch][needs review bent]

    
    

    
  
Comment on attachment 335835 [details] [diff] [review]
v1.0

Looks good to me.

        Attachment #335835 -
        Flags: review?(bent.mozilla) → review+

    
  
Whiteboard: [has patch][needs review bent] → [has patch][has review][can land]

    
    

    
  
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/8cdeae0c853b
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [has patch][has review][can land]
Target Milestone: --- → mozilla1.9.1b1

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: