I will claim that the most common behavior of developers is to leave XPCOM_DEBUG_BREAK alone and not set it to any particular value. I bet most people haven't even heard of this or know what it does.
With that env var unset, in Debug mode, NS_ASSERTION will print to stderr and otherwise do nothing. In non-Debug mode, it will just do nothing. Is that the best behavior for this? Should perhaps (most of) these claimed assertions really be MOZ_ASSERT? Hence this proposal. Now switching them assertions does reduce the flexibility here. Presently, by controlling XPCOM_DEBUG_BREAK, you can make NS_ASSERTION suspend the process, print out a stack trace, abort, print the stack and then abort, or trigger a breakpoint in a debugger. One full test run on linux64 produced over 1500 instances of 72 unique assertions - while some of those are expected assertions because of the expectAssertions API, they can't all be; so I'm skeptical that the DEBUG_BREAK env var feature is actually usable without triggering instances unrelated to what you're actually working on. It's probably much easier to drop in a NS_DebugBreak(NS_DEBUG_BREAK). And because some tests use the expectAssertions API (about 70), we couldn't completely remove this until that got refactored to some new mechanism - perhaps just minimizing NS_ASSERTION into a newly renamed thing that was used just for this purpose. But I believe the majority of ~5600 assertions today can be moved to MOZ_ASSERT without consequence and that this would more accurately reflect a developer's intention when they wrote the code. (I think. The name at least implies it.) And for the ones that we're triggering unintentionally, and can't become MOZ_ASSERT today - perhaps we can file issues, get them investigated and changed to a NS_WARN_IF or a Log or some other behavior? For the rest, we can cut over all of the ones except the ones we've seen triggered on Taskcluster based on some heuristic of choosing taskcluster runs. (Which I'd be happy to hear suggestions for if anyone has thoughts on something more sophisticated than 'the last week of -central runs'.) PS: This is separate from a proposal to deal with NS_WARNING_ASSERTION which could come next... _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
