This is why bug 1236108 is not merging to aurora or beta cleanly.

Since the minidump path can be overridden programmatically in the chrome process, using that path as the base for .extra files won't work since content is unaware of it. This patch changes everything to use the temp path when MOZ_CONTENT_SANDBOX is not defined or when sandboxing is disabled via pref. It also moves the derivation of the content temp path out of exception context on Windows and Mac, as I found out that those functions touch the heap. I also noticed that xpcshell is not sandbox-aware when utilized as a parent process. I've filed bug 1257098 to take care of that, but this patch includes a hack for the immediate term. Review commit: https://reviewboard.mozilla.org/r/40833/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40833/

Comment on attachment 8731785 [details] MozReview Request: Bug 1256541: Fix incorrect generation of path for child process .extra files when content sandboxing is disabled; r?bsmedberg https://reviewboard.mozilla.org/r/40833/#review38747

Thanks for fixing this, Aaron. I've marked it blocking all of the OOM | Unknowns we have that aren't related to the JS engine.

Backed this out in https://hg.mozilla.org/integration/mozilla-inbound/rev/0974ea6ae83b for crashes like https://treeherder.mozilla.org/logviewer.html#?job_id=24573582&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/faf8710ec82947959292a72c11790a340f70b083 Bug 1256541: Fix incorrect generation of path for child process .extra files when content sandboxing is disabled; r=bsmedberg

I made some false assumptions with that assertion that was firing. I removed it as the code can safely deal with that case.

Can you request uplift? Then we can also uplift bug 1236108. :)

Comment on attachment 8731785 [details] MozReview Request: Bug 1256541: Fix incorrect generation of path for child process .extra files when content sandboxing is disabled; r?bsmedberg (Sheriff note: this needs to be landed by aklotz) Approval Request Comment [Feature/regressing bug #]: Crash reporter OOM annotations for e10s content processes [User impact if declined]: Ability to analyze content OOM crashes would be impaired [Describe test coverage new/current, TreeHerder]: Existing tests in xpcshell and browser-chrome [Risks and why]: Low, this patch simplifies existing code which is mostly related to calculation of target directories for crash dump annotations. [String/UUID change made/needed]: None

Comment on attachment 8731785 [details] MozReview Request: Bug 1256541: Fix incorrect generation of path for child process .extra files when content sandboxing is disabled; r?bsmedberg Useful for e10s crash reports. Please uplift to aurora and beta

Hitting conflicts on both aurora and beta.

I need to land this myself.

https://hg.mozilla.org/releases/mozilla-aurora/rev/044150f51647 https://hg.mozilla.org/releases/mozilla-beta/rev/72fb48644f42

I think this one just broke VS2015 compilation in the beta tree: c:/seamonkey/comm-beta/mozilla/toolkit/crashreporter/nsExceptionHandler.cpp(1198): error C2664: 'size_t CrashReporter::BuildTempPath (wchar_t *,size_t)': cannot convert argument 1 from 'nsAString_internal::char_iterator' to 'wchar_t *'mozmake[7]: Entering directory 'c:/seabuild/release/comm-beta-15/obj-x86_64-pc-mingw32/intl/icu/target/tools/gencmn'

On aurora too. c:/seamonkey/comm-aurora/mozilla/toolkit/crashreporter/nsExceptionHandler.cpp(1215): error C2664: 'size_t CrashReporter::BuildTempPath(wchar_t *,size_t)': cannot convert argument 1 from 'nsAString_internal::char_iterator' to 'wchar_t *' c:/seamonkey/comm-aurora/mozilla/toolkit/crashreporter/nsExceptionHandler.cpp(1215): note: Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast wwc fixed it: >> size_t actualLen = BuildTempPath(wwc(aResult.BeginWriting()), XP_PATH_MAX);

Feel free to file a follow-up bug to fix this if you like, but since our official VC2015 support is effective for Version 48, I'm not personally inclined to request uplift for this change.

Doesn't make much sense to file a bug if it's likely not being checked in. Will just put private fixes in my trees. Thanks for your time FRG