Closed
Bug 968803
Opened 10 years ago
Closed 10 years ago
RUN_ON_THREAD should be able to statically determine dispatch flags for return-valued runnables
Categories
(Core :: WebRTC, defect)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: froydnj, Assigned: froydnj)
Details
Attachments
(2 files, 1 obsolete file)
4.05 KB,
patch
|
ekr
:
review+
|
Details | Diff | Splinter Review |
60.16 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
This is sort of like bug 825234, I think.
Assignee | ||
Comment 1•10 years ago
|
||
This is not strictly necessary, but I think it makes the next patch slightly nicer, and runnable_utils.py needs a bit of love anyway.
Attachment #8371578 -
Flags: review?(ekr)
Assignee | ||
Comment 2•10 years ago
|
||
The main benefit here is removing the virtual function that we really only care about in debug builds. I thought there would be more cases that returned a result, but such cases only really occur in testcases that do their own Dispatch() calls anyway. I feel better knowing that these errors will be caught at compile time, rather than runtime, though.
Attachment #8371582 -
Flags: review?(ekr)
Assignee | ||
Comment 3•10 years ago
|
||
2-months-and-counting ping.
Updated•10 years ago
|
Attachment #8371578 -
Flags: review?(ekr) → review+
Comment 4•10 years ago
|
||
Comment on attachment 8371582 [details] [diff] [review] part 2 - statically type runnable classes that return a result Review of attachment 8371582 [details] [diff] [review]: ----------------------------------------------------------------- I'd like to understand my question below about RefPtr before r+ ::: media/mtransport/runnable_utils.h @@ +21,5 @@ > + NoResult, > + ReturnsResult > +}; > + > +// Temporary hack. Really we want to have a template which will do this Is this comment still necessary? @@ +25,5 @@ > +// Temporary hack. Really we want to have a template which will do this > +static inline nsresult > +RunOnThreadInternal(nsIEventTarget *thread, nsIRunnable *runnable, uint32_t flags) > +{ > + nsCOMPtr<nsIRunnable> runnable_ref(runnable); Why did this become an nsCOMPtr?
Attachment #8371582 -
Flags: review?(ekr) → review-
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Eric Rescorla (:ekr) from comment #4) > ::: media/mtransport/runnable_utils.h > @@ +21,5 @@ > > + NoResult, > > + ReturnsResult > > +}; > > + > > +// Temporary hack. Really we want to have a template which will do this > > Is this comment still necessary? Hm. Probably not. I can remove it. > @@ +25,5 @@ > > +// Temporary hack. Really we want to have a template which will do this > > +static inline nsresult > > +RunOnThreadInternal(nsIEventTarget *thread, nsIRunnable *runnable, uint32_t flags) > > +{ > > + nsCOMPtr<nsIRunnable> runnable_ref(runnable); > > Why did this become an nsCOMPtr? Because you're supposed to use nsCOMPtr with interface types and nsRefPtr/RefPtr with concrete types.
Assignee | ||
Comment 8•10 years ago
|
||
Comment removed. Applying r+ as per comment 7.
Attachment #8371582 -
Attachment is obsolete: true
Attachment #8415309 -
Flags: review+
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b30e471400c5 https://hg.mozilla.org/integration/mozilla-inbound/rev/8a6c3ab28bbc
Flags: in-testsuite-
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b30e471400c5 https://hg.mozilla.org/mozilla-central/rev/8a6c3ab28bbc
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•