1428036 - stylo: Add the CSS string which is parsed to crash report annotations

Status: NEW

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

Description

[Calixte Denizet (:calixte)]

Since reproduce crashes is not always easy (even with urls), it could interesting to add the parsed string (or just a slice) to crash report (when it happens somewhere in the parser) to help developers to understand what the issue is.
:emilio, is it feasible ?

Comment 1

[Emilio Cobos Álvarez (:emilio)]

I'm not too familiar with the crash reporting functionality, :jryans is a bit more, so he can maybe comment.

Not sure what amount of crashes come from CSS parsing for this to be worth, but we can probably stash a stack pointer to the input from `ParserInput::new(..)` (or from a wrapper type) in nightlies, and clean it up on destruction.

What do you think jryans?

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I would imagine some setup similar to how we capture the Rust panic message could work.

In the Rust panic case, Rust code exposes an FFI method[1] that can be called at any time to access a possible panic reason.  This is called during exception handling for both the parent[2] and child[3] processes to insert the data into the crash report.

So, the parser would stash the input like Emilio mentions and expose some similar FFI method for use at crash time to access it.

I think it's likely though that the parser input would be considered PII (personally identifiable) since it could reveal the URL that was visited.  So, it would likely mean that this parser input field would only be made available to Mozilla staff with special access to PII fields on crash reports.  (Perhaps everyone involved has this access, so it may not be a problem here.)

[1]: https://searchfox.org/mozilla-central/rev/652fbd6270de0d3ec424d2b88f8375ff546f949f/toolkit/library/rust/shared/lib.rs#77
[2]: https://searchfox.org/mozilla-central/rev/652fbd6270de0d3ec424d2b88f8375ff546f949f/toolkit/crashreporter/nsExceptionHandler.cpp#1085
[3]: https://searchfox.org/mozilla-central/rev/652fbd6270de0d3ec424d2b88f8375ff546f949f/toolkit/crashreporter/nsExceptionHandler.cpp#1321

Comment 3

[Chris Peterson [:cpeterson]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #2)
> I think it's likely though that the parser input would be considered PII
> (personally identifiable) since it could reveal the URL that was visited. 
> So, it would likely mean that this parser input field would only be made
> available to Mozilla staff with special access to PII fields on crash
> reports.  (Perhaps everyone involved has this access, so it may not be a
> problem here.)

Since the parser input could be PII, the patch will also need a review from a data steward:

https://wiki.mozilla.org/Firefox/Data_Collection