change nsIAppShell::RunInStableState API to receive ownership of nsIRunnable

RESOLVED FIXED in Firefox 41

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
mozilla41
Points:
---

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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.
(Assignee)

Updated

3 years ago
See Also: → bug 1155059
(Assignee)

Comment 1

3 years ago
Created attachment 8617665 [details] [diff] [review]
change RunInStableState API to receive ownership of nsIRunnable
Attachment #8617665 - Flags: review?(roc)
https://hg.mozilla.org/mozilla-central/rev/208891d8e388
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.