Perhaps keep these fatal (or at least NS_ASSERTION and cleanup code) for production releases; or we could turn them off in Beta. These provide boot-and-suspenders against sec bugs and get us better error reports in nightly/aurora

Comment on attachment 717955 [details] [diff] [review] Make some DataChannels assertions fatal in opt/release Now includes the control of the assertions from configure.in. We'd leave them on by default through Aurora and likely beta, perhaps into the first release or two of production. Some we make make hard MOZ_CRASHes permanently where the security implications are important enough.

Comment on attachment 717955 [details] [diff] [review] Make some DataChannels assertions fatal in opt/release Review of attachment 717955 [details] [diff] [review]: ----------------------------------------------------------------- It's not really clear to me why these asserts and not others should be fatal. It would be helpful to document which ones are made so because of security implications, otherwise we'll never know if it's safe to make them non-fatal again.

Comment on attachment 717955 [details] [diff] [review] Make some DataChannels assertions fatal in opt/release Review of attachment 717955 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +5283,5 @@ > +dnl ======================================================== > +MOZ_ARG_DISABLE_BOOL(webrtc, > +[ --disable-webrtc-assertions Disable WebRTC non-DEBUG assertions], > + MOZ_WEBRTC_ASSERT_ALWAYS=, > + MOZ_WEBRTC_ASSERT_ALWAYS=1) Do we really need this to be a configure option? Doesn't seem worthwhile. Maybe just change the initial assignment to only assign if unset? ::: netwerk/sctp/datachannel/DataChannel.cpp @@ +60,5 @@ > +#define ASSERT_WEBRTC(x) do { if (!(x)) { MOZ_CRASH(); } } while 0 > +#endif > +#else > +#define ASSERT_WEBRTC(x) MOZ_ASSERT((x)) > +#endif I think this would read more simply if you did: #ifdef DEBUG #define ASSERT_WEBRTC(x) MOZ_ASSERT((x)) #elif defined(MOZ_WEBRTC_ASSERT_ALWAYS) #define ASSERT_WEBRTC(x) do { if (!(x)) { MOZ_CRASH(); } } while 0 #endif

https://hg.mozilla.org/integration/mozilla-inbound/rev/01de5071069b Silly mistake while merging

https://hg.mozilla.org/mozilla-central/rev/a4ae083f9caf https://hg.mozilla.org/mozilla-central/rev/01de5071069b