Clients of nsIAppShell::RunInStableState() use one-shot nsIRunnables and don't need to keep ownership of the nsIRunnable. It makes sense then to transfer ownership to the app shell. This was requested for an nsContentUtils helper in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1171785#c5 The existing bare pointer API could also work with minimum ref count manipulation if the caller never adds a ref after new. However, such bare pointer APIs do not make it clear that the implementation will always add a ref and release (which is required to trigger deletion of a runnable with no references). Our reference logging macros also fail to detect objects lost this way and so live objects with no references is not a pattern I want to encourage. Accepting already_AddRefed<nsIRunnable> in the API would make transfer of ownership clear. Contrarily, clients of RunBeforeNextEvent() in dom/indexedDB use a different model where nsIRunnable just to provides a callback on a long lived object and so passing ownership is not so helpful.
Created attachment 8617665 [details] [diff] [review] change RunInStableState API to receive ownership of nsIRunnable
Attachment #8617665 - Flags: review?(roc)
Attachment #8617665 - Flags: review?(roc) → review+
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.