Status

()

Core
Printing: Output
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: dbaron, Assigned: dcone (gone))

Tracking

({mlk})

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tind-mlk])

Attachments

(3 attachments)

done's checkin on 2001-11-01 14:14 or 14:17 caused an nsPrintOptions object to
show up on the tinderbox leak stats.
Keywords: mlk
Whiteboard: [tind-mlk]

Comment 1

17 years ago
Created attachment 22443 [details] [diff] [review]
[patch] fix leak and do some cleaning up
(Assignee)

Comment 2

17 years ago
I will fix my bug, but I will leave all the rest of the cleanup to the module 
owner of nsAppRunner.cpp.
Hmmm.   Registering services in nsViewerApp / nsAppRunner seems like the
wrong thing to do.  Why isn't being listed in CreateInstance/components in
gfx/src/{mac,windows,gtk}/nsGfxFactory{Mac,Windows,GTK}.cpp enough?  Isn't
what's enough for CreateInstance enough for GetService?  See
http://lxr.mozilla.org/mozilla/source/xpcom/doc/xpcom-brownbag-1.html
http://lxr.mozilla.org/mozilla/source/xpcom/doc/xpcom-code-faq.html#ComponentManager%20Vs%20ServiceManager

Comment 4

17 years ago
dcone: no problem :-)
(Assignee)

Comment 5

17 years ago
Created attachment 22462 [details] [diff] [review]
Patch to fix leak.
Could you explain why you need this RegisterService call at all.  I don't see
why.  Taking it out of nsAppRunner/nsViewerApp would also fix the leak.  (And do
you have the same leak in both places?)
(Assignee)

Comment 7

17 years ago
David, because we wanted this to be a singleton accessible from more than just 
GFX.
I read the documents you pointed at and I believe we fulfill those requirments.

I don't think you need to call RegisterService to get that to happen.  You
shouldn't need to change nsAppRunner to make that kind of change.  The
ServiceManager will call CreateInstance for you.  I think you only need to use
RegisterService if:
 * the ServiceManager can't call CreateInstance
 * you want to override the object that would be created by CreateInstance
(Assignee)

Comment 9

17 years ago
Ok.. that would be great actually.. let me do some research and I will get it to 
the right thing..
(Assignee)

Comment 10

17 years ago
Created attachment 22658 [details] [diff] [review]
Patch to fix leak..
(Assignee)

Comment 11

17 years ago
The above patch fixes the leak by taking the RegisterService of the 
nsIPrintOptions out.  I confirmed that a singleton will still be constructed.
Thanks for the help Dave..

Comment 12

17 years ago
sr=buster
r=dbaron on that, although you should also do the same for viewer
(Assignee)

Comment 14

17 years ago
I did not start the service for viewer.. so nothing needs to be done there.
Um...ok, I guess I'll just let viewer be weird since it looks weird already...

jag:  Are you going to file another bug on that cleanup?

Comment 16

17 years ago
Yeah, bug 65660.
(Assignee)

Updated

17 years ago
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 17

17 years ago
Checked in the fix.

Comment 18

17 years ago
verified tha tthe patch went in in version 1.256 in lxr.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.