Closed
Bug 451590
Opened 16 years ago
Closed 16 years ago
nsNavHistory needs nsIClassInfo with THREADSAFE property
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla1.9.1b1
People
(Reporter: sdwilsh, Assigned: sdwilsh)
References
Details
Attachments
(1 file)
|
(deleted),
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 | |
Updated•16 years ago
|
Assignee: nobody → sdwilsh
|
|
Assignee | |
Updated•16 years ago
|
Summary: Places needs nsIClassInfo with THREADSAFE property → nsNavHistory needs nsIClassInfo with THREADSAFE property
|
|
Assignee | |
Comment 1•16 years ago
|
||
Attachment #335835 -
Flags: review?(dietrich)
|
|
Assignee | |
Updated•16 years ago
|
Whiteboard: [has patch][needs review dietrich]
|
||
Updated•16 years ago
|
Attachment #335835 -
Flags: review?(dietrich) → review+
|
|
||
Comment 2•16 years ago
|
||
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.
|
|
Assignee | |
Comment 3•16 years ago
|
||
(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).
|
|
Assignee | |
Updated•16 years ago
|
Attachment #335835 -
Flags: review?(bent.mozilla)
|
|
Assignee | |
Updated•16 years ago
|
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+
|
|
Assignee | |
Updated•16 years ago
|
Whiteboard: [has patch][needs review bent] → [has patch][has review][can land]
|
|
Assignee | |
Comment 5•16 years ago
|
||
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.
Description
•