1314839 - Handle asserts using MOZ_ASSERT in stylo

Status: RESOLVED WONTFIX

(Core :: CSS Parsing and Computation, defect, P5)

Description

[Manish Goregaokar [:manishearth]]

Stylo currently has a bunch of expect()s (and other things) littered around. These will cause aborts.

We should give stylo the ability to call the MOZ_ASSERT codepath (with a `moz_panic!` or something), and try to convert as many non-safety-related panics as possible to moz_panics which recover gracefully. For example, https://reviewboard.mozilla.org/r/89688/diff/2#11 has an expect() in it when we find out that the iterator isn't as long as it should be. While this should never happen, if it does we can still recover gracefully-ish by just stopping there and nulling the next pointer of the cssvaluelist (and reporting an error via moz_panic, which only crashes in debug mode).

Comment 1

[Manish Goregaokar [:manishearth]]

It was just pointed out to me that moz_assert does nothing at all in release mode (I thought that it would log the error somewhere).

In that case we should still make our debug asserts use the moz_assert backtrace printing codepath, I guess? Or are we okay with how Rust backtraces work?

Comment 2

[Bobby Holley (:bholley)]

We should lean towards being abort-happy for now. Aborting means that the crash reporter will give us stacks when we screw up, which is really useful when we turn something like this on for nightly.

Once this is riding the trains to release, we can see if there are any particularly problematic abort points and add graceful recovering for those as needed. But for now I think we shouldn't worry about it.

Comment 3

[Chris Peterson [:cpeterson]]

status-firefox57=wontfix unless someone thinks this bug should block 57

Comment 4

[Cameron McCormack (:heycam)]

Given comment 2, I think this is WONTFIX.