Closed
Bug 372831
Opened 16 years ago
Closed 16 years ago
provide API to submit extra data with minidump
Categories
(Toolkit :: Crash Reporting, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: ted, Assigned: ted)
References
Details
Attachments
(1 file, 2 obsolete files)
14.93 KB,
patch
|
benjamin
:
first-review+
|
Details | Diff | Splinter Review |
<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•16 years ago
|
||
Will this be available to extensions via JS as well?
Comment 2•16 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.
Assignee | ||
Comment 3•16 years ago
|
||
The meat of this is going to be handled in bug 366970, but I'll do the scriptable portion here.
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → ted.mielczarek
Assignee | ||
Comment 4•16 years ago
|
||
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•16 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+
Assignee | ||
Comment 6•16 years ago
|
||
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
Assignee | ||
Comment 7•16 years ago
|
||
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•16 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•16 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@
Assignee | ||
Comment 10•16 years ago
|
||
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•16 years ago
|
Attachment #261139 -
Flags: first-review?(benjamin) → first-review+
Assignee | ||
Comment 11•16 years ago
|
||
(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: 16 years ago
Resolution: --- → FIXED
Comment 12•16 years ago
|
||
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?
Assignee | ||
Comment 13•16 years ago
|
||
My bad, I think I did this in an older tree, then didn't fix that before checking in. Fix checked in.
Assignee | ||
Comment 14•16 years ago
|
||
Yeah, I haven't had any caffeine yet. Checked in another fix.
Assignee | ||
Updated•16 years ago
|
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•