provide API to submit extra data with minidump

RESOLVED FIXED

Status

()

RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: ted, Assigned: ted)

Tracking

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

<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
(Assignee)

Updated

12 years ago
Blocks: 366973

Updated

12 years ago
Blocks: 375083

Comment 1

12 years ago
Will this be available to extensions via JS as well?

Comment 2

12 years ago
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)

Updated

12 years ago
Assignee: nobody → 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).
Attachment #260646 - Flags: first-review?(benjamin)

Comment 5

12 years ago
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+
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.
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 8

12 years ago
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+

Comment 9

12 years ago
> 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@
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.
Attachment #261024 - Attachment is obsolete: true
Attachment #261139 - Flags: first-review?(benjamin)

Updated

12 years ago
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
Last Resolved: 12 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.
(Assignee)

Updated

12 years ago
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.