Closed
Bug 451709
Opened 16 years ago
Closed 16 years ago
include obj-c exception info in crash reports
Categories
(Core :: General, defect)
Tracking
()
VERIFIED
FIXED
mozilla1.9.1b1
People
(Reporter: jaas, Assigned: jaas)
Details
(Keywords: verified1.9.0.5, verified1.9.1)
Attachments
(1 file, 5 obsolete files)
14.30 KB,
patch
|
mconnor
:
approval1.9.0.4-
|
Details | Diff | Splinter Review |
Now that bug 444103 has landed we can include Obj-C exception information in crash reports.
Flags: blocking1.9.1+
Attachment #335059 -
Flags: review?(ted.mielczarek)
Updated•16 years ago
|
Attachment #335059 -
Flags: review?(ted.mielczarek) → review+
> +#include "nsString.h"
Could you make that 'nsStringGlue.h' instead? This file is exported in the SDK and external folks can't use the internal string headers.
Attachment #335059 -
Attachment is obsolete: true
Attachment #335229 -
Flags: review?(bent.mozilla)
Comment on attachment 335229 [details] [diff] [review] fix v1.1 >+ if (nameBuffer) >+ NS_Free(nameBuffer); >+ if (reasonBuffer) >+ NS_Free(reasonBuffer); Those ifs are unnecessary as you've already checked them. r=me with those gone!
Attachment #335229 -
Flags: review?(bent.mozilla) → review+
Thanks Ben!
Attachment #335229 -
Attachment is obsolete: true
Attachment #335368 -
Flags: superreview?(roc)
This is good, but can't we get a stack trace from the Obj-C exception which points back into the framework that threw the exception? Also, is it really wise to have all this code in static nsObjCExceptionLog? Can't we share it across files somehow?
Attachment #335368 -
Attachment is obsolete: true
Attachment #335368 -
Flags: superreview?(roc)
Attachment #335563 -
Attachment is obsolete: true
Attachment #335752 -
Flags: review?(ted.mielczarek)
Comment 9•16 years ago
|
||
Comment on attachment 335752 [details] [diff] [review] fix v1.4 As I said on IRC, I'm not wild about passing the exception object through the interface as a void*, but I don't have a huge objection to it either, so if this is better for you, then go for it.
Attachment #335752 -
Flags: review?(ted.mielczarek) → review+
Attachment #335752 -
Flags: superreview?(roc)
Comment on attachment 335752 [details] [diff] [review] fix v1.4 >+ unichar* nameBuffer = (unichar*)NS_Alloc(sizeof(unichar) * ([name length] + 1)); > ... >+ [name getCharacters:nameBuffer]; The apple docs say that [name getCharacters:nameBuffer] only fills nameBuffer up to [name length] so I think you want to null-terminate yourself.
Comment on attachment 335752 [details] [diff] [review] fix v1.4 Rev the IID on nsICrashReporter.idl
Attachment #335752 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Comment 12•16 years ago
|
||
Attachment #335752 -
Attachment is obsolete: true
Assignee | ||
Comment 13•16 years ago
|
||
landed on trunk
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment on attachment 335838 [details] [diff] [review] fix v1.5 Requesting approval for 1.9.0.x for this patch, since it (in conjunction with bug 444103, on which it depends) would really help decipher the extremely high number of Mac crashes that are lumped under nsObjCExceptionLogAbort(NSException*).
Attachment #335838 -
Flags: approval1.9.0.4?
Updated•16 years ago
|
Attachment #335838 -
Flags: approval1.9.0.5?
Attachment #335838 -
Flags: approval1.9.0.4?
Attachment #335838 -
Flags: approval1.9.0.4-
Comment 15•16 years ago
|
||
Comment on attachment 335838 [details] [diff] [review] fix v1.5 bug 444103 isn't landing for 1.9.0.4, so this definitely can't land now.
Comment 16•16 years ago
|
||
Comment on attachment 335838 [details] [diff] [review] fix v1.5 I'm rolling up a combination branch patch over in bug 444103. I'll request approval over there.
Attachment #335838 -
Flags: approval1.9.0.5?
Comment 17•16 years ago
|
||
Fixed in combination with branch checkin of bug 444103. I have an extension that can be used to test this, I'll try to make a litmus test out of it.
Keywords: fixed1.9.0.5
Comment 18•16 years ago
|
||
Steps to test: 1) Install the Crash Me Now! Advanced extension: http://ted.mielczarek.org/code/mozilla/crashme-advanced.xpi and restart your browser 2) Click the "Tools -> Crash me! -> ObjC Exception!" menu item. Your browser should crash, and the crash reporter should appear. 3) Click the "Details" button. A panel should appear containing lot of text. Verify that this text is included: "Notes: Obj-C Exception data: Test Exception: Catch this!"
Flags: in-litmus?
Comment 19•16 years ago
|
||
Ted, does it mean we don't have to use crashme.xpi anymore or is the advanced version only functional on OS X?
Hardware: PC → All
Comment 20•16 years ago
|
||
The new version works on Win/Mac/Linux just like the old one. The only difference is that the advanced version gives you an option of how to crash. You can use either one.
Comment 21•16 years ago
|
||
I've asked because it would be easier to maintain only one version of crashme on Litmus. Because there is a sub menu available now all the other tests have to be updated as well. Which will confuse users with the simpler version because they cannot see this sub menu. I'm not sure if this is the right place to discuss but let Tracy or Marcia decide about what to do on Litmus.
Comment 22•16 years ago
|
||
Ah, yeah, I can see that being annoying. The old version of crashme is still available, but I'm using the same extension ID for both, so you can't install them side-by-side. If you've got a better idea I'm open to suggestions. I could produce another simple extension that just provides the "ObjC Exception" functionality if that would make life simpler.
Comment 23•16 years ago
|
||
Verified fixed with the official 1.9.0.5 build (Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.5) Gecko/2008120121 Firefox/3.0.5). The data is now included.
Keywords: fixed1.9.0.5 → verified1.9.0.5
Comment 24•16 years ago
|
||
Also verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20081204 Shiretoko/3.1b3pre ID:20081204020603 Setting 1.9.1b1 as TM because a2 isn't selectable anymore.
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.9.1b1
Comment 25•16 years ago
|
||
(In reply to comment #22) > Ah, yeah, I can see that being annoying. The old version of crashme is still > available, but I'm using the same extension ID for both, so you can't install > them side-by-side. If you've got a better idea I'm open to suggestions. I could > produce another simple extension that just provides the "ObjC Exception" > functionality if that would make life simpler. Tracy, could we replace all instances of the old crashme extension with the new one on Litmus?
Updated•16 years ago
|
Keywords: fixed1.9.1
Updated•16 years ago
|
Keywords: fixed1.9.1 → verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•