937157 - Remove some ancient or unused debugging #defines

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Comment 1

[Bobby Holley (:bholley)]

Yes please! I don't know of anyone who builds as "xpc_hacker" anymore.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: remove some

I removed some jband debug flags, and some other flags that aren't used, plus a few other misc things.  We appear to be vulnerable to LEADING_UPPERCASE_ACCESS_ERRORS.

Comment 3

[Andrew McCreight [:mccr8]]

(In reply to Bobby Holley (:bholley) from comment #1)
> Yes please! I don't know of anyone who builds as "xpc_hacker" anymore.

I just did some basic low-risk removal.  I can remove more of the DEBUG_xpc_hacker stuff after surveying people if you'd like.

Comment 4

[Bobby Holley (:bholley)]

(In reply to Andrew McCreight [:mccr8] from comment #3)
> I just did some basic low-risk removal.  I can remove more of the
> DEBUG_xpc_hacker stuff after surveying people if you'd like.

I don't think a survey is necessary. I'm 90% certain that DEBUG_xpc_hacker builds wouldn't even compile anymore. Feel free to remove them if you feel so inclined. We can always add them back if igor comes out of the woodwork and claims that we broke his workflow. ;-)

Comment 5

[Andrew McCreight [:mccr8]]

In email and IRC, I spoke with dbaron, jst, timeless, and bz, and none of them have used any of this stuff in forever, so I'll work on some patches to remove more of it.  (Ms2ger asked mrbkap, and he isn't using any of this either.)

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: remove some more

Comment 7

[Bobby Holley (:bholley)]

Comment on attachment 830919 [details] [diff] [review]
remove some more

Review of attachment 830919 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/src/XPCWrappedNative.cpp
@@ -3013,5 @@
>  
>  /***************************************************************************/
>  
> -#ifdef XPC_CHECK_CLASSINFO_CLAIMS
> -static void DEBUG_CheckClassInfoClaims(XPCWrappedNative* wrapper)

This seems like it could still be useful to run #ifdef DEBUG and have it MOZ_ASSERT if it finds problems.

Comment 8

[Andrew McCreight [:mccr8]]

Landed in 13 pieces as part of this push:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=5cb70c98cbb6

I left in XPC_CHECK_CLASSINFO_CLAIMS.  I filed bug 937157 for enabling this in debug builds.

L64 debug try run was green: https://tbpl.mozilla.org/?tree=Try&rev=8ab149fedc09

Comment 9

[Andrew McCreight [:mccr8]]

Looks like around 800 lines removed in total.

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3df299e18f29
https://hg.mozilla.org/mozilla-central/rev/c0c75fb6ff17
https://hg.mozilla.org/mozilla-central/rev/6d16fdc91ac7
https://hg.mozilla.org/mozilla-central/rev/52c7a0f15fef
https://hg.mozilla.org/mozilla-central/rev/b83b785c1736
https://hg.mozilla.org/mozilla-central/rev/da04bbb2bad4
https://hg.mozilla.org/mozilla-central/rev/0bbf398ade29
https://hg.mozilla.org/mozilla-central/rev/3f46c9ffdc9a
https://hg.mozilla.org/mozilla-central/rev/53edeffc65c2
https://hg.mozilla.org/mozilla-central/rev/f54d82a391bc
https://hg.mozilla.org/mozilla-central/rev/9ae2e02e7022
https://hg.mozilla.org/mozilla-central/rev/2c12e321f22a
https://hg.mozilla.org/mozilla-central/rev/b73ce52d676f