583636 - NSPR logging for cookies doesn't work

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[Nickolay_Ponomarev]

Following the instructions from http://www.mozilla.org/projects/netlib/cookies/cookie-log.html :

export NSPR_LOG_MODULES=cookie:4
export NSPR_LOG_FILE=/path/to/log
/path/to/firefox-bin

produces an empty file.

This regressed between 
2010-06-30-03-mozilla-central
2010-07-01-03-mozilla-central

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=580205b95ceb&tochange=82edf5bd1abe

probably the e10s merge...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Is this in an opt build?  Are IPC headers including prlog.h somewhere or something?  :(

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

None of the core IPC code (ipc/glue/*) or the C++ it generates uses prlog, but "IPC code" encompasses a fair amount of stuff, some of it might be using prlog or pulling in other headers that do.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Well, the relevant headers cookies include are "mozilla/net/NeckoCommon.h" and "mozilla/net/CookieServiceChild.h" plus whatever those include...

Comment 4

[Boris Zbarsky [:bzbarsky]]

OK, those don't include prlog.h (or more precisely the .i for the cookie service shows prlog.h being included where it should be).  So the problem is really somewhere in cookie-land....

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

So, this does fix the bug for me. I'm not sure why bz's testing showed a different result; I see prlog.h included from ipc/chromium/src/base/logging.h as well as dist/include/mozilla/BlockingResourceBase.h

Comment 6

[[e-mail]]

Comment on attachment 463171 [details] [diff] [review]
patch

r=dwitte

Comment 7

[Benjamin Smedberg]

We should really be defining FORCE_PR_LOGGING in the makefile for a directory, not in any of the headers or .cpp files. See e.g. http://mxr.mozilla.org/mozilla-central/source/dom/plugins/Makefile.in#152

Comment 8

[[e-mail]]

I guess that means we'd have to do it for all of netwerk/, since netwerk/cookie doesn't have its own Makefile anymore... I imagine that'd be OK though.

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #8)
> I guess that means we'd have to do it for all of netwerk/, since netwerk/cookie
> doesn't have its own Makefile anymore... I imagine that'd be OK though.

What's http://mxr.mozilla.org/mozilla-central/source/netwerk/cookie/Makefile.in then?

We could do it for all of netwerk, but that will enable several log modules that weren't previously enabled. Do we still care about codesize? :-)

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

Also, that DOM makefile will enable logging even when --disable-logging was specified, is that intentional?

Comment 11

[[e-mail]]

(In reply to comment #9)
> What's http://mxr.mozilla.org/mozilla-central/source/netwerk/cookie/Makefile.in
> then?

Oh, right, I thought we'd removed that but we only moved netwerk/cookie/src to netwerk/cookie...

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 463171 [details] [diff] [review]
patch

OK, I'll stay with this patch because in a makefile I can't check for MOZ_LOGGING.

Asking for approval - a trivial patch to make sure that cookie logging is enabled in nightlies/releases to make investigating bugs possible.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

http://hg.mozilla.org/mozilla-central/rev/ad8051aa2e3a