Status
()
People
(Reporter: froydnj, Assigned: froydnj)
Tracking
Firefox Tracking Flags
(firefox53 fixed)
Details
Attachments
(3 attachments)
2.24 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
4.07 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
6.21 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
I probably won't catch everything, but I have a patch series to fix a goodly number of places.
![]() |
(Assignee) | |
Comment 1•a year ago
|
||
Created attachment 8823387 [details] [diff] [review] make ThreadInfo::mName a UniqueFreePtr Smart pointers are better than raw pointers.
Attachment #8823387 -
Flags: review?(mstange)
![]() |
(Assignee) | |
Comment 2•a year ago
|
||
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.
Attachment #8823388 -
Flags: review?(mstange)
![]() |
(Assignee) | |
Comment 3•a year ago
|
||
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.
Attachment #8823389 -
Flags: review?(mstange)
Updated•a year ago
|
Attachment #8823387 -
Flags: review?(mstange) → review+
Comment 4•a year ago
|
||
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.
Attachment #8823388 -
Flags: review?(mstange) → review+
Comment 5•a year ago
|
||
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.
Attachment #8823389 -
Flags: review?(mstange) → review+
Pushed by nfroyd@mozilla.com: 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•a year ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/41e0e99837f9 https://hg.mozilla.org/mozilla-central/rev/07f764d10078 https://hg.mozilla.org/mozilla-central/rev/e1a7c28606f1
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•