372831 - provide API to submit extra data with minidump

Status: RESOLVED FIXED

Description

[Ted Mielczarek [:ted] [:ted.mielczarek]]

<bsmedberg> luser: What I really want is an AnnotateMiniDump(const char *key,
            const char *data) API where we can save all kinds of interesting
            info at runtime (before a crash happens)

<bsmedberg> which will get forwarded to the server along with the minidump
<bsmedberg> we can use that to provide, e.g. the list of enabled extensions

<mento> ok, what we expect you to do in that case is write your own file using
          the same basename and different extension when we call the minidump
          callback

Comment 1

[James Ross]

Will this be available to extensions via JS as well?

Comment 2

[Benjamin Smedberg]

Well... yes and no. Extensions could use it, but unless the server knows what to do with the data it will be ignored. We're not prepared to store and report arbitrary metadata on the server.

Comment 3

[Ted Mielczarek [:ted] [:ted.mielczarek]]

The meat of this is going to be handled in bug 366970, but I'll do the scriptable portion here.

Comment 4

[Ted Mielczarek [:ted] [:ted.mielczarek]]

Created attachment 260646 [details] [diff] [review]
add scriptable API

Adds nsICrashReporter interface.  Only one method exists currently: annotateCrashReport(), which just delegates to the existing function of the same name.  The interface is implemented on the singleton nsXULAppInfo, so you access it via Components.classes["@mozilla.org/xre/app-info;1"].getService(Components.interfaces.nsICrashReporter).

Comment 5

[Benjamin Smedberg]

Comment on attachment 260646 [details] [diff] [review]
add scriptable API

>Index: toolkit/airbag/nsICrashReporter.idl

>+  /**
>+   * Add some extra data to be submitted with a crash report.
>+   * @param key
>+   *        Name of the data to be added.

>+   * @throw NS_ERROR_INVALID_ARG if key contains invalid characters

It would be nice to say what characters those are ;-) r=me with that change.

We might also want to register another contractid for this component, so that people don't have to ask for it through the app-info contractid.

Comment 6

[Ted Mielczarek [:ted] [:ted.mielczarek]]

Created attachment 261024 [details] [diff] [review]
xpcom api + bsmedberg's comments + unit test

Ok, this isn't quite finished, but I wanted to get it up before I left.  Should address bsmedberg's comments, plus I added a unit test (in C++).  The new contractID doesn't quite work, I need to look at that.

The unit test doesn't actually test the XPCOM API, mostly because the implementation lives in libxul, but it's a good start.

Comment 7

[Ted Mielczarek [:ted] [:ted.mielczarek]]

Comment on attachment 261024 [details] [diff] [review]
xpcom api + bsmedberg's comments + unit test

Rob, can you take a look at the unit test for me?

Comment 8

[Robert Sayre]

Comment on attachment 261024 [details] [diff] [review]
xpcom api + bsmedberg's comments + unit test

>Index: toolkit/airbag/test/Makefile.in
>===================================================================
>+#
>+# The Original Code is Oracle Corporation code.
>+#
>+# The Initial Developer of the Original Code is
>+#   Ted Mielczarek <[e-mail]>

Is this right?

>+
>+DEPTH		= ../../..
>+topsrcdir	= @top_srcdir@
>+srcdir		= @srcdir@
>+VPATH		= @srcdir@

nit: extra tab before srcdir?

Comment 9

[Mark Mentovai]

> nit: extra tab before srcdir?

Artifact of the extra characters at the beginning of the line - Ted's spacing is OK here:

DEPTH		= ../../..
topsrcdir	= @top_srcdir@
srcdir		= @srcdir@
VPATH		= @srcdir@

Comment 10

[Ted Mielczarek [:ted] [:ted.mielczarek]]

Created attachment 261139 [details] [diff] [review]
with fixed contract id

Ok, this is all of the above, plus it registers in the right place, so you can use a different contract id.

Comment 11

[Ted Mielczarek [:ted] [:ted.mielczarek]]

(In reply to comment #8)
> (From update of attachment 261024 [details] [diff] [review])
> >Index: toolkit/airbag/test/Makefile.in
> >===================================================================
> >+#
> >+# The Original Code is Oracle Corporation code.
> >+#
> >+# The Initial Developer of the Original Code is
> >+#   Ted Mielczarek <[e-mail]>
> 
> Is this right?

Oops, copy and pasted from vlad's storage tests.  Fixed that and checked in.

Comment 12

[Mark Banner (:standard8)]

I think the test on this bug is causing breakage on FF's WINNT 5.1 qm-winxp01 dep unit test tinderbox. 

+        $(DEPTH)/toolkit/airbag/$(LIB_PREFIX)airbagexception_s.$(LIB_SUFFIX) \

This library (toolkit/airbag/test/Makefile.in) doesn't exist, possibly due to the bug 368205 sync up?

Comment 13

[Ted Mielczarek [:ted] [:ted.mielczarek]]

My bad, I think I did this in an older tree, then didn't fix that before checking in.  Fix checked in.

Comment 14

[Ted Mielczarek [:ted] [:ted.mielczarek]]

Yeah, I haven't had any caffeine yet.  Checked in another fix.