1000922 - Use nsMainThreadPtrHandle instead of passing around already_AddRefed and calling forget everywhere

Status: RESOLVED FIXED

(Toolkit Graveyard :: OS.File, defect)

Description

[Josh Matthews [:jdm]]

The code in NativeOSFileInternals.cpp looks more complicated than necessary in my quick skim. As far as I can tell, we're passing around references to main-thread-only callback interfaces as already_AddRefed, storing them in nsCOMPtr and then forgetting them before a release happens, all to avoid forbidden refcounting on XPConnect wrappers. We can do this more simply and correctly by storing nsMainThreadPtrHandle types in the nsRunnable derivatives instead, and that will remove the need for all of the forget calls too.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Oh, we have such a thing as nsMainThreadPtrHandle? I couldn't find anything such when I wrote NativeOSFileInternals.cpp. Definitely, this can be used to simplify the code.

Comment 2

[Alessio Placitelli [:Dexter]]

I could work on that if needed, even though it looks like a good bug for mentoring!

(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?" - I'll be away on April 9th-16th) from comment #1)
> Oh, we have such a thing as nsMainThreadPtrHandle? I couldn't find anything
> such when I wrote NativeOSFileInternals.cpp. Definitely, this can be used to
> simplify the code.

Comment 3

[Josh Matthews [:jdm]]

You're always welcome to mentor it if you'd like!

Comment 4

[Chris Manchester (limited bugmail, email directly)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6316edec7d21

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1000922 - Use nsMainThreadPtrHandle instead of already_AddRefed and forget for callbacks in NativeOSFileInternals.cpp r=jdm

Bug 1000922 - Use nsMainThreadPtrHandle instead of already_AddRefed and forget for callbacks in NativeOSFileInternals.cpp r=jdm

Comment 6

[Josh Matthews [:jdm]]

Comment on attachment 8653024 [details]
MozReview Request: Bug 1000922 - Use nsMainThreadPtrHandle instead of already_AddRefed and forget for callbacks in NativeOSFileInternals.cpp r=jdm

https://reviewboard.mozilla.org/r/16445/#review15771

Looks good! Thanks for doing this work!

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b3d5b3dad0e

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8b3d5b3dad0e