This probably worked until the dlopen stuff landed, and the changes in bug 603592 don't do anything to make it work. Hopefully we can fix it. Shouldn't block enabling this because we don't have content process crash reporting enabled yet, and I doubt plugin crashes are a big problem on Android right now. mwu: will plugin-container have the same libraries mapped at the same addresses as fennec? If so, we can probably just cheat and use the data we have in the parent process. If not, we'll have to remote the data somehow.

The plugin container should contain exactly the same libraries at the same addresses. However, I think we can just report the mappings like we do in the parent process case.

Yeah, but the caveat there is that we then have to convey that information across to the parent process somehow. (I guess we could just go whole-hog and make the API that reports them use IPC in child processes.)

Ted, I'm assuming this falls on your plate, if not please talk to me.

untested but making this available if people want to try it.

That's not actually going to fix this, since the minidump generation is done in the chrome process, and we're not remoting CrashReporter::AddLibraryMapping, so the library mapping info won't be available when we write the minidump.

(In reply to comment #5) > That's not actually going to fix this, since the minidump generation is done in > the chrome process, and we're not remoting CrashReporter::AddLibraryMapping, so > the library mapping info won't be available when we write the minidump. Yes, I know. I'm just putting this part of the patch up if you or someone else wants to try remoting that data.

Okay, thanks.

Can't really do anything here until bug 581335 is done.

We have plugins disabled on android, so we can't start with that?

Oh, good question. I have no idea.

Apparently we do build with plugins enabled for Android, they're just preffed off. I should be able to work with that.

Comment on attachment 487290 [details] [diff] [review] Report mapping to crash reporter in the child process I'm not sure if we want this yet, I need to figure out how we're going to remote it first. Thanks for the patch, though, I'll wind up using it somehow.

(In reply to comment #11) > Apparently we do build with plugins enabled for Android, they're just preffed > off. I should be able to work with that. Except the test plugin doesn't currently work on Android, and we crash loading plugins right now (bug 611683). Difficult to do anything useful here at the moment.

Should be possible to kill -11 the content process then submit the report through about:crashes, right?

Yes, although it won't be a useful report until I fix this bug. Our libraries just show up as /dev/AShmem.

Oh, I see what you meant. Yes, I can use that to test this.

This patch adds an IPDL protocol and uses it to remote the library mapping info from the content process, and uses it for writing child minidumps. It's an awful lot of complexity, but it works. This depends on a small Breakpad patch as well, which I'll attach in a moment.

Turns out CrashGenerationServer didn't have everything it needed to allow the writeDumps setting to work, and to allow you to write dumps from the callback function.

Comment on attachment 491240 [details] [diff] [review] Remote AddLibraryMapping from the child process, and use the results for child process minidump generation I think I missed a bunch of license headers as well, I'll add those before landing.

I'll give these all a spin through the tryserver as well to make sure I didn't break any other platforms.

Comment on attachment 491240 [details] [diff] [review] Remote AddLibraryMapping from the child process, and use the results for child process minidump generation >diff --git a/dom/ipc/CrashReporterChild.cpp b/dom/ipc/CrashReporterChild.cpp >new file mode 100644 >--- /dev/null >+++ b/dom/ipc/CrashReporterChild.cpp >@@ -0,0 +1,16 @@ >+#include "CrashReporterChild.h" >+ >+namespace mozilla { >+namespace dom { >+CrashReporterChild::CrashReporterChild() >+{ >+ MOZ_COUNT_CTOR(CrashReporterChild); >+} >+ >+CrashReporterChild::~CrashReporterChild() >+{ >+ MOZ_COUNT_DTOR(CrashReporterChild); >+} >+ >+} // namespace dom >+} // namespace mozilla Might as well inline these in the header for now, save a file. >diff --git a/dom/ipc/PCrashReporter.ipdl b/dom/ipc/PCrashReporter.ipdl >new file mode 100644 >--- /dev/null >+++ b/dom/ipc/PCrashReporter.ipdl I think you need a license header here. >diff --git a/toolkit/crashreporter/nsExceptionHandler.cpp b/toolkit/crashreporter/nsExceptionHandler.cpp >--- a/toolkit/crashreporter/nsExceptionHandler.cpp >+++ b/toolkit/crashreporter/nsExceptionHandler.cpp >@@ -248,20 +251,21 @@ > // so the embedding will provide a list of shared > // libraries that are mapped into anonymous mappings. > typedef struct { > std::string name; > std::string debug_id; > uintptr_t start_address; > size_t length; > size_t file_offset; > } mapping_info; > static std::vector<mapping_info> library_mappings; >+static std::map<PRUint32,google_breakpad::MappingList> child_library_mappings; |typedef std::map<PRUint32,google_breakpad::MappingList> MappingMap;| or something, then |static MappingMap child_library_mappings;|. >+#if defined(__ANDROID__) >+ // Do dump generation here since the CrashGenerationServer doesn't >+ // have access to the library mappings. >+ std::map<PRUint32,google_breakpad::MappingList>::iterator iter = >+ child_library_mappings.find(aClientInfo->pid_); |MappingMap::const_iterator| plz. >+ if (iter == child_library_mappings.end()) >+ return; Would like to at least warn here, this condition indicates fail somewhere (or NYI).

Patch ready for checkin, probably shouldn't land until the blocking bug does.

With comments addressed, ready for checkin.

The content process bits of bug 592768 got punted to bug 581335.

Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/aa662b68d604 http://hg.mozilla.org/mozilla-central/rev/55b7f3d63b7a

And a bustage fix for good luck: http://hg.mozilla.org/mozilla-central/rev/871cdedc077b (broke the Maemo QT builds, which are --enable-ipc but --disable-crashreporter)