1322863 - use UniquePtr for managing pointers where possible.

Status: RESOLVED FIXED

Description

[Nathan Froyd [:froydnj] (out until April 30)]

I probably won't catch everything, but I have a patch series to fix a goodly number of places.

Comment 1

[Nathan Froyd [:froydnj] (out until April 30)]

Created attachment 8823387 [details] [diff] [review]
make ThreadInfo::mName a UniqueFreePtr

Smart pointers are better than raw pointers.

Comment 2

[Nathan Froyd [:froydnj] (out until April 30)]

Created attachment 8823388 [details] [diff] [review]
make ThreadInfo manage mProfile with UniquePtr

Smart pointers are better than raw pointers.

I think I like this patch, but there is also some weirdness that ThreadInfo owns
ThreadProfile objects, and ThreadProfile objects can own ThreadInfo objects
(though apparently not at the same time?).  We should sort that out.

Comment 3

[Nathan Froyd [:froydnj] (out until April 30)]

Created attachment 8823389 [details] [diff] [review]
manage Sampler::PlatformData with UniquePtr

Smart pointers are better than raw pointers, and this makes clients of
PlatformData slightly simpler because they don't have to manage
destruction themselves: the new UniquePtr-derived type handles all of
that for us.

Comment 4

[Markus Stange [:mstange]]

Comment on attachment 8823388 [details] [diff] [review]
make ThreadInfo manage mProfile with UniquePtr

Review of attachment 8823388 [details] [diff] [review]:
-----------------------------------------------------------------

I don't know about the the ThreadInfo vs ThreadProfile ownership situation, but this patch seems like it should be safe either way.

Comment 5

[Markus Stange [:mstange]]

Comment on attachment 8823389 [details] [diff] [review]
manage Sampler::PlatformData with UniquePtr

Review of attachment 8823389 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.
It still feels somewhat unnecessary complex - we're abstracting something out that's known at compile time, right?
It may also be unused, because I don't see any callers of GetProfiledThread.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/41e0e99837f9
part 1 - make ThreadInfo::mName a UniqueFreePtr; r=mstange
https://hg.mozilla.org/integration/mozilla-inbound/rev/07f764d10078
part 2 - make ThreadInfo manage mProfile with UniquePtr; r=mstange
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1a7c28606f1
part 3 - manage Sampler::PlatformData with UniquePtr; r=mstange

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/41e0e99837f9
https://hg.mozilla.org/mozilla-central/rev/07f764d10078
https://hg.mozilla.org/mozilla-central/rev/e1a7c28606f1