94922 - Implement NSPR Logging for plugin module (and clean up tabs)

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P3)

Description

[Peter Lubczynski]

It would be very helpful for debugging (on platforms like OSX and somes UNIXs
which have a hard time loading symbols for shared libraries in a debugger) if
some sort of cross-platform TRACE or LOGGING were implemented in the mozilla
plugins module like was done in imaging or networking. I think that by using
this it will be easier to find the cause of bugs and prevent regressions. This
kind of extra debug output could also be used in the future for automated
regression tests.

The following patch starts to implement NSPR logging for:
* ns4xPlugin.cpp
* ns4xPluginInstance.cpp
* nsPluginHostImpl.cpp

I'd like to get this chunk of patch in for now before moving on to more detail
in these files and others.

There are three kinds of logs, each having several levels:
* General Plugin Logging
* 4.x NPP API
* 4.x NPN API
Basic directions for use are included in nsPluginLogging.h

While I was working on these files, I noticed a LARGE number of tabs. There were
so many of them, (eek!) I couldn't stop myself from fixing as many as I could!
By no means is this code cleanup complete, but it does make things a little bit
more readable. To see diffs without whitespace changes, use cvs -w.

Comment 1

[Peter Lubczynski]

Attachment: first try @ implementing NSPR logging plus code cleanup (monster patch) v.1

Comment 2

[Peter Lubczynski]

Attachment: new patch, with quick-use macros v.2

Comment 3

[av (gone)]

From MSVC Help:
"A #define without a token-string removes occurrences of identifier from the 
source file. The identifier remains defined and can be tested using the #if 
defined and #ifdef directives."

How about this:

#ifdef PLUGIN_LOGGING
 #define NPN_PLUGIN_LOG(a, b)              \
   PR_BEGIN_MACRO                          \
   PR_LOG(nsPluginLogging::gNPNLog, a, b); \
   PR_LogFlush();                          \
   PR_END_MACRO
#else
 #define NPN_PLUGIN_LOG(a, b) //!! nothing here
#endif

Comment 4

[av (gone)]

Peter, why do you define your own logging levels?

Comment 5

[Peter Lubczynski]

I wanted more control and I didn't like the way NSPR defined them. I was
thinking of later specifying a "network" or "event" log level than just noisy,
normal, and basic.

Comment 6

[av (gone)]

Also, I would take bolder action and remove all printfs. I think, edburn's 
printfs were added for some local purposes and probably no longer needed. As 
well as my range request output.

Comment 7

[av (gone)]

r=av

Comment 8

[Peter Lubczynski]

Attachment: updated patch, v.3

Comment 9

[Peter Lubczynski]

The updated patch:

* removes #debug_edburns printf()'s 
* adds more NOISY logging for streams
* conflicts resolved

Marc, can you super review?

Comment 10

[av (gone)]

What about my 2001-08-14 12:14 comment?

Comment 11

[Peter Lubczynski]

Ahh..sorry..new patch in just a bit....

Comment 12

[Peter Lubczynski]

Attachment: updated patch again v.4

Comment 13

[Marc Attinasi]

Peter, can you attach a patch without the whitespace changes please? It will be
much easier to review...

Comment 14

[Peter Lubczynski]

Attachment: real patch v.5

Comment 15

[Peter Lubczynski]

Attachment: special version of last patch without whitespace (cvs diff -N -u9 -w)

Comment 16

[av (gone)]

r=av on last modification of the patch.

Comment 17

[Marc Attinasi]

* nsPluginLogging.h: note the line that says

\ No newline at end of file

you want to fix that.

* There is an #if 0 you might want to remove in ns4xPlugin.cpp

Looks like a very nice cleanup, and somce useful logging! So, assuming that you
compiled with and without NSPR_LOGGING and PLUGIN_LOGGING defined, then sr=attinasi.

Comment 18

[edburns]

Looks good. r=edburns

Comment 19

[Peter Lubczynski]

Patch checked in, marking FIXED.

Comment 20

[shrirang khanzode]

marking verif(stamped)