This seems weird, and dangerous, but we have e.g.: https://searchfox.org/mozilla-central/source/toolkit/crashreporter/breakpad-windows-libxul/moz.build which compiles a bunch of stuff for FINAL_LIBRARY = 'xul'. It disables STL wrapping, which seems sketchy as well. The wrapper disabling is probably necessary because of: https://searchfox.org/mozilla-central/source/toolkit/crashreporter/client/moz.build which pulls in different breakpad bits for each platform. On windows, that's done with: https://searchfox.org/mozilla-central/source/toolkit/crashreporter/client/moz.build#29 which pulls in the bits we were defining for xul, above. It seems not so great to have this situation, but it obviously works. Would anything be radically wrong if we: a) compiled this code for libxul and the crashreporter separately (which seems bad); or b) just made the crashreporter link against mozglue? (The wider context here is that I'm trying to make some linker order file bits work for our clang-cl builds, and it's annoying to have code compiled for libxul that also gets linked elsewhere...and specifically in this case, doesn't get linked against mozglue.)

I haven't written this code but it seems to me it's doing two separate things that happen to be together just because It Works™ (or worked until now). The first thing is provide the bits of breakpad that the crashreporter client needs (namely the HTTP upload code and possibly some bits from common) then it also provides the parts of breakpad that we do use in libxul. There's no point in having those two parts lumped together because the crashreporter client does not link with libxul and we don't need HTTP upload support in libxul. So we can probably get rid of this file and move the required bits into toolkit/crashreporter/client/moz.build and toolkit/crashreporter/moz.build.

This makes one less place where we link code compiled for libxul into a place that doesn't link mozglue, and is cleaner to boot. We don't need the BREAKPAD_NO_TERMINATE_THREAD define that breakpad-windows-libxul defines because we're not including the handler code in the crashreporter binary.

Comment on attachment 8989314 [details] [diff] [review] make the windows crashreporter not dependent on libxul files Review of attachment 8989314 [details] [diff] [review]: ----------------------------------------------------------------- This looks fine, but can you remove the few extraneous bits from breakpad-windows-libxul/moz.build now? The sender and http_upload.cc bits are definitely not needed in libxul: https://dxr.mozilla.org/mozilla-central/rev/9c02d2ecf22050bfee5d70c04a359d8aaff6eb91/toolkit/crashreporter/breakpad-windows-libxul/moz.build#7 Can you also file a followup to do a more thorough cleanup here? It may be that we can do a lot more cleanup, but I don't want to block you on that for now. (Specifically a lot of this complexity is to support the "inject a crash reporter into the flash player" which I suspect we could probably get rid of at this point.)

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/be04a499d0d0 make the windows crashreporter not dependent on libxul files; r=ted.mielczarek