Closed Bug 372831 Opened 14 years ago Closed 14 years ago

provide API to submit extra data with minidump

Categories

(Toolkit :: Crash Reporting, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: ted, Assigned: ted)

References

Details

Attachments

(1 file, 2 obsolete files)

<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
Blocks: 366973
Blocks: 375083
Will this be available to extensions via JS as well?
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.
The meat of this is going to be handled in bug 366970, but I'll do the scriptable portion here.
Assignee: nobody → ted.mielczarek
Attached patch add scriptable API (obsolete) — Splinter Review
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).
Attachment #260646 - Flags: first-review?(benjamin)
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.
Attachment #260646 - Flags: first-review?(benjamin) → first-review+
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.
Attachment #260646 - Attachment is obsolete: true
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?
Attachment #261024 - Flags: second-review?(sayrer)
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 <ted.mielczarek@gmail.com>

Is this right?

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

nit: extra tab before srcdir?
Attachment #261024 - Flags: second-review?(sayrer) → second-review+
> 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@
Ok, this is all of the above, plus it registers in the right place, so you can use a different contract id.
Attachment #261024 - Attachment is obsolete: true
Attachment #261139 - Flags: first-review?(benjamin)
Attachment #261139 - Flags: first-review?(benjamin) → first-review+
(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 <ted.mielczarek@gmail.com>
> 
> Is this right?

Oops, copy and pasted from vlad's storage tests.  Fixed that and checked in.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
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?
My bad, I think I did this in an older tree, then didn't fix that before checking in.  Fix checked in.
Yeah, I haven't had any caffeine yet.  Checked in another fix.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.