61243 - [nsTraceRefcnt]move dynamic_cast out of NSCAP_* macros

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

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

I've never liked having the dynamic_cast in the macros for nsCOMPtr logging.
When I first wrote it, I couldn't get it to work another way, but the trick
seems to use nsISupports* instead of void* for the function argument, and then
do the dynamic_cast within the function.

I have this in my tree, but it needs to be tested...

Comment 1

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

Attachment: patch, that also fixes part of bug 60415

Comment 2

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

Attachment: patch for this bug, bug 62006, and bug 60415

Comment 3

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

Attachment: all of the above, plus fix longstanding PL hashtable leaks in nsTraceRefcnt::Shutdown

Comment 4

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

Attachment: same as previous, but with ASSERT_ACTIVITY_IS_LEGAL not containing the ';'

Comment 5

[Chris Waterson]

Looks good to me. r=waterson

Comment 6

[Brendan Eich [:brendan]]

Just curious why you want stub static methods for

+NS_COM void
+nsTraceRefcnt::Startup()
+{
+#ifdef NS_BUILD_REFCNT_LOGGING
+  SetActivityIsLegal(PR_TRUE);
+#endif
+}

when NS_BUILD_REFCNT_LOGGING is not defined.  Isn't the whole source file
excluded from compilation (or if not, shouldn't its contents be #ifdef'd out by
one giant #ifdef)?

School me, the answer can't stop sr=[e-mail]

/be

Comment 7

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

I did it that way because everything else in the file was done that way, and I
didn't want to change the whole file.  I'm not sure why kipp originally wrote it
that way, but I've stuck to the style.  Perhaps he wanted to avoid
optimized-only build bustage that would result from nsTraceRefcnt functions
being called when NS_BUILD_REFCNT_LOGGING is defined.

So do you think we should change it to comment out the whole file (and most of
nsTraceRefcnt.h) if NS_BUILD_REFCNT_LOGGING is not defined?  It might require a
bit of #ifdef-ing in various places, although such things should probably not be
in release builds anyway.

Comment 8

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

I checked in this fix way back on 2000-12-23 at 08:43 PST.  The remaining
leaks of nsTraceRefcnt's hashtables are covered by bug 63665 (with patch)
and Brendan's comments are covered by bug 66159.  Marking FIXED.