Open Bug 639725 Opened 14 years ago Updated 2 years ago

plugin-side X11 error messages no longer in crash report app notes

Categories

(Toolkit :: Crash Reporting, defect)

x86_64
Linux
defect

Tracking

()

People

(Reporter: karlt, Unassigned)

References

Details

Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=58101a16aff7&tochange=30239e4cebd8 Looks like http://hg.mozilla.org/mozilla-central/rev/bcae411bc333 This is currently making it difficult to distinguish regular old fashioned plugin X errors from the wave of GLX errors that has arrived with wmode=direct on youtube.
Did that ever work? We don't remote AnnotateCrashReport yet (bug 581341).
Ah, I forgot all about that. cjones' changes were necessary because in content processes we run a lot of existing code that tries to do crashreporter things, and we didn't want to have to sprinkle ProcessType checks all over. I think the right solution is just to make the CrashReporter APIs check processtype internally and remote themselves correctly, and switch your annotation there to just use AnnotateCrashReport directly, so that it should work from any type of process.
Thanks for your comments. It does make sense that we centralize processtype checks. One issue is that it is harder to remote notes from a non-main thread, and I assume NS_DebugBreak for example could be called from any thread. (Is CrashReporter::AppendAppNotesToCrashReport() actually safe from a non-main thread?) I guess we can just have a is-main-thread check and no-op if not. Something to bear in mind is they we may want something on 2.0, which may be averse to significant changes. However, with a new release model, I don't know whether we'll continue to support older releases, so this may not end up being an issue.
Depends on: 581341
For a simpler fix, we could perhaps just put the process type checks in all the crashreporter APIs, and bail out for non-chrome processes (except for GetEnabled(), which we could fix to work correctly), so that calling AnnotateCrashReport() would be harmless in content processes. Then this callsite ought to work again, and we can leave remoting the other APIs for that other bug. Does that sound reasonable?
I don't have a feel for how many APIs would need to have the checks added, but that sounds reasonable to me.
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.h (as ugly as it has become) is the set of public APIs exposed curerntly.
No longer blocks: nsITimer-fail
Blocks: 671916
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.