Make MOZ_CC_LOG_DIRECTORY work again

RESOLVED FIXED in mozilla23

Status

()

Core
XPCOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

Trunk
mozilla23
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 736441 [details] [diff] [review]
Patch (v1)
Attachment #736441 - Flags: review?(continuation)
Comment on attachment 736441 [details] [diff] [review]
Patch (v1)

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

Thanks for fixing this!
Attachment #736441 - Flags: review?(continuation) → review+
(Assignee)

Updated

5 years ago
Assignee: nobody → ehsan
https://hg.mozilla.org/mozilla-central/rev/aa2d9eb84633
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
What is it exactly that this patch does, and what's the right fix to this code for bug 859817 to work?  You did exactly the opposite of what my patches try to do.  This will build with bug 859817 if I add a dont_AddRef, but I'd be very interested to know why an nsCOMPtr and getter_AddRefs doesn't work here.
Flags: needinfo?(ehsan)
(Assignee)

Comment 5

5 years ago
getter_AddRefs nulls out the nsCOMPtr before passing it as nsIFoo** to the callee.  This means that here, nsMemoryInfoDumper::OpenTempFile would never see a non-null argument (note that its second argument is in/out) and therefore would always disregard the nsIFile* opened from MOZ_CC_LOG_DIRECTORY.

I think the right way to fix this is to make OpenTempFile not expect an in/out argument, and just make it return an already_AddRefed<nsIFile>.  But I'm not sure why this code is breaking your changes in bug 859817 in the first place, since it theory I should only be using raw interface management, right?
Flags: needinfo?(ehsan)
I figured that must be the reason after I commented.  The thing that broke stuff is "return logFile;" -- it needs to be "return dont_AddRef(logFile);" or something.  I'll see if I can fix it properly, or alternatively I'll work around it.
(Assignee)

Comment 7

5 years ago
(In reply to comment #6)
> I figured that must be the reason after I commented.  The thing that broke
> stuff is "return logFile;" -- it needs to be "return dont_AddRef(logFile);" or
> something.  I'll see if I can fix it properly, or alternatively I'll work
> around it.

I see, let me know if you want me to fix it for you, would be happy to!
I'm just adding a dont_AddRef() in the return for now, since properly fixing the function looks like too much trouble for my purposes.  That works for me -- if you change this code back to the way it was, the cast can be removed.
You need to log in before you can comment on or make changes to this bug.