Closed Bug 848102 Opened 12 years ago Closed 12 years ago

CheckForLastRunCrash shouldn't move dump if MOZ_CRASHREPORTER_NO_REPORT is set

Categories

(Toolkit :: Crash Reporting, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: ahal, Assigned: ahal)

Details

Attachments

(1 file, 1 obsolete file)

On B2G minidumps get submitted even if MOZ_CRASHREPORTER_NO_REPORT is set. I'm not very familiar with breakpad, but from what I understand from ted.. B2G calls [1] from [2] which does [3]. The b2g automated tests set MOZ_CRASHREPORTER_NO_REPORT, but the minidumps get moved anyway meaning the harness can't use them with minidump_stackwalker. [1] http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#2390 [2] http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#108 [3] http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#2435
We're not really setting MOZ_CRASHREPORTER_NO_REPORT for B2G, because the way we launch B2G causes us to ignore environment variables. We launch B2G using b2g.sh: http://mxr.mozilla.org/mozilla-central/source/build/mobile/b2gautomation.py#314 In order to set environment variables that would actually affect the B2G process, we'd have to modify this file before executing it. Is there a pref we could set that would have the same effect?
I think we should at least be able to work around this problem (filed bug 848124). I don't really care if we use the environment variable, a pref or whatever. But both this bug and bug 848124 are things that should be fixed whether or not they break b2g minidump stackwalking so I'm inclined to go the route that fixes them along the way.
Pretty sure this is all we need. I don't know how to test this on b2g, but at least it doesn't change behaviour for desktop
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Attachment #721837 - Flags: review?(ted)
Actually the call to GetExtraFileForMinidump should also be wrapped in the if statement.
Attachment #721837 - Attachment is obsolete: true
Attachment #721837 - Flags: review?(ted)
Attachment #721841 - Flags: review?(ted)
I managed to test this out with b2g. It looks like the b2g process is not automagically restarted which means the minidumps remain in profile/minidumps after the test run. They do however get removed from profile/minidumps the next time b2g.sh is run (even if MOZ_CRASHREPORTER_NO_REPORT is set). So I *think* this bug doesn't actually block bug 843296, but it should still be fixed.
I'll leave it up to ted whether this patch is still worth taking or not.
No longer blocks: 843296
Comment on attachment 721841 [details] [diff] [review] Patch 2.0 - don't copy minidumps to pending directory if MOZ_CRASHREPORTER_NO_REPORT is set Normally I would be all about fixing this for the sake of correctness, but if we don't need this for automation let's just leave it be. We can always revisit if need be.
Attachment #721841 - Flags: review?(ted)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: