Closed Bug 1157848 Opened 6 years ago Closed 6 years ago

Fix leak in OwningNonNull constructor from already_AddRefed

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla40
Tracking Flags:
Tracking Status
firefox40 --- fixed

People

(Reporter: ayg, Assigned: ayg)

References

Details

Attachments

(1 file)

Patch
1.14 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review 
Constructing OwningNonNull from already_AddRefed in bug 1153649 part 1 is totally broken and leaks.  It does .take() and then assigns the result to an nsRefPtr.
Flags: in-testsuite-
Summary: OwningNonNull seems to sometimes cause leaks → Fix leak in OwningNonNull constructor from already_AddRefed
Attached patch Patch — — Splinter Review 
I'm assuming you're okay with taking this because you took the original one.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d3eb6ab4913
Attachment #8596752 - Flags: review?(bzbarsky)
Comment on attachment 8596752 [details] [diff] [review]
Patch

Don't you need Move(aValue)?

Anyway, r=me assuming this actually compiles when used.
Attachment #8596752 - Flags: review?(bzbarsky) → review+
Seems to have passed try.  Beats me why.  I suspect it's something to do with the fact that it's a template argument, there are magical rules for that.
https://hg.mozilla.org/mozilla-central/rev/889f95f88740
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.