1207750 - preference/envvar to crash on cairo error

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

It's sometimes useful to be able to set a preference or environment variable to crash cairo the instant an error occurs, marking the root case of the error handling path or subsequent problems much further down the line.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Jeff, any objections in principle?  I'd probably start with an environment variable approach...

Comment 2

[Jeff Muizelaar [:jrmuizel]]

No objections in principle. I've typically just set a break point on _cairo_error to accomplish this in the past.

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> No objections in principle. I've typically just set a break point on
> _cairo_error to accomplish this in the past.

Right, but that may not be something we can ask random people "out there" to do - on the other hand, we can have them set an environment variable, run the steps that cause the problem, submit the crash report...

Comment 4

[Jeff Muizelaar [:jrmuizel]]

(In reply to Milan Sreckovic [:milan] from comment #3)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> > No objections in principle. I've typically just set a break point on
> > _cairo_error to accomplish this in the past.
> 
> Right, but that may not be something we can ask random people "out there" to
> do - on the other hand, we can have them set an environment variable, run
> the steps that cause the problem, submit the crash report...

This is a very compelling argument.

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: (wip) Crash on cairo error

Something like this.  Questions - how do we want to crash (I took the null pointer assignment from Bas' recent special build to exactly deal with this in the field), and it feels like this should be inside of an #ifdef that is specific to our builds.  Thoughts?

Comment 6

[Bas Schouten (:bas.schouten)]

Comment on attachment 8669080 [details] [diff] [review]
(wip) Crash on cairo error

Review of attachment 8669080 [details] [diff] [review]:
-----------------------------------------------------------------

I like this idea.

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Crash on cairo error. r=bschouten

May as well do a review; same as the WIP patch, except I added the #ifdef around the code.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8670431 [details] [diff] [review]
Crash on cairo error. r=bschouten

Review of attachment 8670431 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/cairo/cairo/src/cairo.c
@@ +172,5 @@
>      CAIRO_ENSURE_UNIQUE;
>      assert (_cairo_status_is_error (status));
>  
> +#ifdef MOZILLA_VERSION
> +    static int abortOnError = -1;

abort_on_error would be the preferred casing for this variable.

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Crash on cairo error. r=bschouten, r=jrmuizelaar

Change the variable name.  Build only try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aedb56b75bbd

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/253dbc53a583

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/253dbc53a583