85290 - PAC: removed console logging

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Christopher Blizzard (:blizzard)]

The PAC module dumps a lot of debug messages to the console.  Here's a patch to
remove them:

Index: nsProxyAutoConfig.js
===================================================================
RCS file: /cvsroot/mozilla/netwerk/base/src/nsProxyAutoConfig.js,v
retrieving revision 1.9
diff -u -r1.9 nsProxyAutoConfig.js
--- nsProxyAutoConfig.js        2001/04/26 14:06:22     1.9
+++ nsProxyAutoConfig.js        2001/06/11 21:40:27
@@ -35,11 +35,6 @@
 const nsIIOService = Components.interfaces['nsIIOService'];
 const nsIDNSService = Components.interfaces.nsIDNSService;
 
-function debug(msg)
-{
-    dump("\nPAC:" + msg + "\n\n");
-}
-
 // implementor of nsIProxyAutoConfig
 function nsProxyAutoConfig() {};
 
@@ -70,7 +65,6 @@
         var uri = url.QueryInterface(Components.interfaces.nsIURI);
         // Call the original function-
         var proxy = LocalFindProxyForURL(uri.spec, uri.host);
-        debug("Proxy = " + proxy);
         if (proxy == "DIRECT") {
             host.value = null;
             type.value = "direct";
@@ -89,7 +83,6 @@
     },
 
     LoadPACFromURL: function(uri, ioService) {
-        debug("Loading PAC from " + uri.spec);
         this.done = false;
         var channel = ioService.newChannelFromURI(uri);
         pacURL = uri.spec;

Comment 1

[Christopher Blizzard (:blizzard)]

I'm looking for rubber stamps on this really obvious change.

Comment 2

[tor]

rs=tor

Comment 3

[Gervase Markham [:gerv]]

r=gerv

Gerv

Comment 4

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 5

[Christopher Blizzard (:blizzard)]

*** Bug 80885 has been marked as a duplicate of this bug. ***

Comment 6

[Christopher Blizzard (:blizzard)]

Checked in.

Comment 7

[Christopher Blizzard (:blizzard)]

I ended up removing two other dump() statements in the registration bits of the
module, too.

Comment 8

[benc]

Was this necessary? Given the number of bugs in PAC right now, and the
difficulty in verifying the behavior, this is going to make qa difficult.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

If every other module did this, qa using printfs would also be difficult because
you'd be seeing tons of other junk from everyone else.

Comment 10

[benc]

changed summary

Comment 11

[benc]

Other modules probably also have other methods of troubleshooting. Without this,
PAC users are going to be flying blind if they are trying to figure out where
the connections are going, esp on Mac (which doesn't have netstat) and somewhat
on Windows (because netstat sucks and doesn't show every open connection).

The point is, even if the output was too verbose, there should have been *SOME*
discussion rather than slamming the code in. Heck, there are almost a dozen PAC
bugs that actually block reall functionality, and nobody is fixing any of those,
but two coders have run out and tried to "fix" this problem.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

printing to the console in release builds causes a number of problems.  I don't
know the bug number offhand that describe these problems, but they can include
filling up the disk or memory with the text printed.  So our policy is basically
that we shouldn't print in release builds.

Comment 13

[benc]

Where is that conversation happening? If people are going to remove features
that other people are using, the solution would be ideally implemented before
the removal occurs.

Right now, when people need to analyze their PAC file problems, I tell them to
downgrade to Mozilla 0.9 so they can see the output of conosole..

Comment 14

[Ben Bucksch (:BenB)]

all: what is the right way to get the information? benc has a real problem.

Why can't dumb() be smart enough to not print on teh console in release builds?

Comment 15

[benc]

We might be able to solve this if we implement alert() bug 86846. We could
document that people should put in break points.

david: which bug documents console filling up disk and memory?

Comment 16

[benc]

VERIFIED: Win and Linux, mozilla for the last year.

I never use console logging on Mac, but this was fixed for Mac OS classic, which
is not a primary test platform for me anymore.