797023 - Work - Headless crash reporting in metrofx doesn't report if the browser crashes on startup

Status: RESOLVED WONTFIX

(Firefox for Metro Graveyard :: General, defect, P2)

Description

[Jim Mathies [:jimm]]

We submit crashes from within the browser vs. launching an external program. Hence, if the browser crashes on startup, we'll never get a crash report. 


reference bug 797013

Comment 1

[Jim Mathies [:jimm]]

I wonder if we could switch from using browser.js submission to launching the desktop client in some sort of silent mode.

Comment 2

[Jim Mathies [:jimm]]

Attachment: crashreporter cleanup

This eliminates the else block, no logic changes.

Comment 3

[Jim Mathies [:jimm]]

Attachment: cr silent mode v.1

This adds a silent mode to crash reporter.

Comment 4

[Jim Mathies [:jimm]]

Attachment: apprunner v.1

This handles launching cr in silent mode early in startup for metro.

Comment 5

[Jim Mathies [:jimm]]

I have a front end patch for this as well. However, beofre I get to that there is an unresolved problem here. Processes that are integrated into the metro ui receive undocumented process management behavior. A couple issues:

- despite being launched as an independent process, crashreporter still gets latched into metro ui process suspend/resume behavior. So for example if if you have crashreporter spawned off and metrofx gets suspended, crashreporter gets suspended also. This can cause problems with sending reports.

- also, the crashreporter process will get torn down unceremoniously if metrofx exits for any reason, including, a startup crash.

There must be a way to separate crashreporter from metro's process management, but afaict it's not document anywhere.

Comment 6

[Brian R. Bondy [:bbondy]]

we could disable the suspending of the crash reporting process completely. Here's some example code:
http://dxr.mozilla.org/mozilla-central/widget/windows/winrt/MetroAppShell.cpp#l268

> There must be a way to separate crashreporter from metro's process management, but afaict it's not document anywhere.

What about using cmd.exe to start it or something like that?
Maybe there's a createprocess or shellexecute flag as well.

Comment 7

[Jim Mathies [:jimm]]

(In reply to Brian R. Bondy [:bbondy] from comment #6)
> we could disable the suspending of the crash reporting process completely.
> Here's some example code:
> http://dxr.mozilla.org/mozilla-central/widget/windows/winrt/MetroAppShell.
> cpp#l268
> 
> > There must be a way to separate crashreporter from metro's process management, but afaict it's not document anywhere.
> 
> What about using cmd.exe to start it or something like that?
> Maybe there's a createprocess or shellexecute flag as well.

I was looking around for something like that but didn't find anything. The old desktop enabled browser doc doesn't mention it either. I'm wondering if a support request might be the best route to take.

Comment 8

[Jim Mathies [:jimm]]

Attachment: client v.1

Got this working using shell execute. (hat tip bbondy)

Comment 9

[Jim Mathies [:jimm]]

Attachment: client v.1

Comment 10

[Jim Mathies [:jimm]]

Comment on attachment 718570 [details] [diff] [review]
client v.1

This is the silent submit part in crashreporter.exe.

Comment 11

[Jim Mathies [:jimm]]

Attachment: apprunner/exception handler bits v.1

Comment 12

[Jim Mathies [:jimm]]

Comment on attachment 718572 [details] [diff] [review]
apprunner/exception handler bits v.1

sorry ted, if there's someone you can delegate this to, please do.

Comment 13

[Jim Mathies [:jimm]]

Attachment: front end v.1

Comment 14

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #13)
> Created attachment 718573 [details] [diff] [review]
> front end v.1

Note the other little bit of front end code to this is over in bug 844619.

Comment 15

[Brian R. Bondy [:bbondy]]

Comment on attachment 718573 [details] [diff] [review]
front end v.1

Review of attachment 718573 [details] [diff] [review]:
-----------------------------------------------------------------

yay for removal only patch

Comment 16

[Jim Mathies [:jimm]]

Attachment: crashreporter cleanup -w

Makes it a bit easier to see what changed.

Comment 17

[(not currently active) Ted Mielczarek]

This is WONTFIX per the discussion in bug 831972?

Comment 18

[Jim Mathies [:jimm]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #17)
> This is WONTFIX per the discussion in bug 831972?

I think so. We could still land it, tie it to a registry key so we have it for debugging. If the user sets a reg key that's the same as opting in. However from the discussion in bug 831972, it's clear our normal submit method will have to be an about page where the user approves every submit.

Would you be up for landing this even if it's not going to be used on a regular basis?

Comment 19

[(not currently active) Ted Mielczarek]

The crashreporter code is already pretty complicated, I'd rather avoid additional features we don't actually need.