Closed
Bug 1034921
Opened 10 years ago
Closed 10 years ago
Remove dangerous public destructor of nsFoo in TestThreadUtils.cpp
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: bjacob, Assigned: anujagarwal464, Mentored)
References
Details
Attachments
(1 file, 1 obsolete file)
|
(deleted),
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
In bug 1028588 we removed dangerous public destructors of XPCOM-refcounted classes outside of a finite whitelist, see HasDangerousPublicDestructor. Now we are going over the entries in this whitelist.
One of them is: nsFoo in TestThreadUtils.cpp
|
||
Updated•10 years ago
|
Mentor: continuation
|
Assignee | |
Updated•10 years ago
|
Assignee: nobody → anujagarwal464
|
|
Assignee | |
Comment 1•10 years ago
|
||
Attachment #8483420 -
Flags: feedback?(continuation)
|
|
||
Comment 2•10 years ago
|
||
Comment on attachment 8483420 [details] [diff] [review]
bug1034921.diff
Review of attachment 8483420 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me! Thanks for the patch.
Attachment #8483420 -
Flags: review?(nfroyd)
Attachment #8483420 -
Flags: feedback?(continuation)
Attachment #8483420 -
Flags: feedback+
|
||
Updated•10 years ago
|
Attachment #8483420 -
Flags: review?(nfroyd) → review+
|
|
Assignee | |
Updated•10 years ago
|
Keywords: checkin-needed
|
|
||
Comment 3•10 years ago
|
||
You need to post a successful try run for checkin-needed.
Keywords: checkin-needed
|
|
Assignee | |
Comment 4•10 years ago
|
||
|
|
||
Comment 5•10 years ago
|
||
Try run failed. It looks like you need to keep the "virtual" on ~nsFoo. In theory, if there was a subclass of nsFoo, then having it be non-virtual wouldn't call the right destructor on when deleting something with type nsFoo.
|
|
Assignee | |
Comment 6•10 years ago
|
||
Attachment #8483420 -
Attachment is obsolete: true
|
|
||
Comment 7•10 years ago
|
||
Comment on attachment 8484174 [details] [diff] [review]
bug1034921.diff
That try run is green. This probably doesn't really need a re-review (all it does is make the dtor virtual) but here we go anyways.
Attachment #8484174 -
Flags: review?(nfroyd)
|
|
||
Updated•10 years ago
|
Attachment #8484174 -
Flags: review?(nfroyd) → review+
|
|
||
Updated•10 years ago
|
Keywords: checkin-needed
|
||
Comment 8•10 years ago
|
||
Keywords: checkin-needed
|
||
Comment 9•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
|
||
Updated•10 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•