1088938 - Allow annotations set using CrashReporter::AnnotateCrashReport() to be removed subsequently

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Steven Michaud [:smichaud] (Retired)]

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.

Comment 1

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix

This works in my tests.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix rev1

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 3

[(not currently active) Ted Mielczarek]

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.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8512229 [details] [diff] [review]
Fix rev1

Landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/712679cd32c7

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/712679cd32c7