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
