Closed Bug 1088938 Opened 6 years ago Closed 6 years ago
Allow annotations set using Crash
Reporter::Annotate Crash Report() to be removed subsequently
Right now crash log annotations (added using AnnotateCrashReport or AppendCrashNotesToCrashReport) can't be removed. So, unless you *cause* a crash just after adding an annotation, they're limited to things that are always relevant, no matter how we crash (since we don't know whether or not we'll crash where we might expect to). It'd be very convenient to also be able to make "speculative" annotations -- ones that are relevant if we crash where we think we might, but aren't at all relevant if we don't crash there (if we crash later, somewhere else, for different reasons). For example we have a case, in bug 1086977, where we crash whenever a particular plugin is loaded, but we're not entirely sure what it's filename is. If we log all plugin filenames, most of our annotations will be irrelevant. But the situation changes if we're able to remove an annotation after it's become irrelevant -- for example if we can log every plugin's filename, then remove that annotation if we don't crash where we expect to. Right now can do something like this with AnnotateCrashReport(). We can add a "speculative" note under a key just before we might crash, and then change it to something like "please ignore" afterwards if we don't crash. But it'd be even better if calling AnnotateCrashReport() with the same key and "" for data caused the previous annotation to be removed entirely.
This works in my tests.
Assignee: nobody → smichaud
Status: NEW → ASSIGNED
Attachment #8511363 - Flags: review?(ted)
Talking on IRC, Ted, you suggested I add a call to explicitly remove crash report annotations -- to make it clearer how this can be done.
Comment on attachment 8512229 [details] [diff] [review] Fix rev1 Review of attachment 8512229 [details] [diff] [review]: ----------------------------------------------------------------- This is fine. I still think you could just remove the entry from the hashtable, but it's not that big of a deal.
Attachment #8512229 - Flags: review?(ted) → review+
Comment on attachment 8512229 [details] [diff] [review] Fix rev1 Landed on mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/712679cd32c7
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.