Closed Bug 1172377 Opened 5 years ago Closed 5 years ago

change nsIAppShell::RunInStableState API to receive ownership of nsIRunnable

Categories

(Core :: Widget, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: karlt, Assigned: karlt)

References

Details

Attachments

(1 file)

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.
See Also: → 1155059
https://hg.mozilla.org/mozilla-central/rev/208891d8e388
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.