1483329 - Add crash handling API

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P1)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

Focus and FxR want to report crashes themselves, so they can prompt the user or report to multiple places. The current toggle to enable GeckoView to send the crash report itself is not adequate.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Alright, the plan here is as follows:

Remove the current crash-related stuff from GeckoRuntime.

Add a new method to GeckoRuntimeSettings that allows you to set a Class for a Service which will be started in response to crashes. We'll document the Intent details such as the action and all of the extras it will receive. In the main process (app process) it will only catch native crashes. The app is on its own for dealing with unhandled exceptions. For child processes we'll try to report any type of crash. In this case the app will receive a flag indicating that it was not a fatal crash. The onCrash() method in ContentDelegate remains as a way for the app to recover from such a crash.

The crash handling service is started as a foreground service, since background services are not allowed on Oreo+. This means the Service has to call startForeground()[1] (which requires a Notification) or it will be killed pretty quickly[2]. Experimentation has revealed that you can start a new activity (in a new process) or schedule work with JobScheduler without having to all startForeground().

The future ideal situation is that Gecko's main process will be separate from the app process. Once that is complete, none of the Gecko crash handling will conflict with the app at all. We aren't there yet, but hopefully this plan at least allows the app some flexibility in the reporting.

[1] https://developer.android.com/reference/android/app/Service.html#startForeground(int,%20android.app.Notification)
[2] https://developer.android.com/about/versions/oreo/background#services

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1483329 - Remove existing crash-related methods in GeckoRuntimeSettings r=jchen

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1483329 - Remove job id methods from GeckoRuntimeSettings r=jchen

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1483329 - Add crash handling API to GeckoRuntime r=jchen,esawin

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1483329 - Hook Fennec up to new crash handling API r=jchen

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1483329 - Hook up geckoview_example to new crash reporting API r=jchen

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1483329 - Report content process crashes via GeckoRuntime r=jchen

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1483329 - Add tests for GeckoRuntime crash handling API r=jchen

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1483329 - Hook up GeckoSessionTestRule and TestRunnerActivity to crash reporting r=jchen

Comment 10

[Dylan Roeh (:droeh) (he/him)]

Comment on attachment 9003880 [details]
Bug 1483329 - Add crash handling API to GeckoRuntime r=jchen,esawin

Dylan Roeh (:droeh) has approved the revision.

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9003879 [details]
Bug 1483329 - Remove job id methods from GeckoRuntimeSettings r=jchen

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 12

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9003878 [details]
Bug 1483329 - Remove existing crash-related methods in GeckoRuntimeSettings r=jchen

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 13

[Eugen Sawin [:esawin]]

Comment on attachment 9003880 [details]
Bug 1483329 - Add crash handling API to GeckoRuntime r=jchen,esawin

Eugen Sawin [:esawin] on leave has approved the revision.

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9003883 [details]
Bug 1483329 - Report content process crashes via GeckoRuntime r=jchen

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9003884 [details]
Bug 1483329 - Add tests for GeckoRuntime crash handling API r=jchen

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 16

[Chris Peterson [:cpeterson]]

status-geckoview62=wontfix because we don't need to uplift the GV crash reporter to GECKOVIEW_62_RELBRANCH. Focus will disable GV crash reporting in https://github.com/mozilla-mobile/focus-android/issues/3217. Even without GV crash reporting, we can still monitor GV's crash volume because Focus submits Sentry crash reports for GeckoSession.onCrash() callbacks like https://github.com/mozilla-mobile/focus-android/issues/2900.

Comment 17

[Chris Peterson [:cpeterson]]

[geckoview:klar:p2] because the GV crash reporter is not a Focus+GV release blocker. As noted above in comment 16, Focus can disable GV's Breakpad crash reporting and rely on Sentry crash reporting to avoid PII issues.

Comment 18

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9003885 [details]
Bug 1483329 - Hook up GeckoSessionTestRule and TestRunnerActivity to crash reporting r=jchen

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 19

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9003882 [details]
Bug 1483329 - Hook up geckoview_example to new crash reporting API r=jchen

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 20

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9003881 [details]
Bug 1483329 - Hook Fennec up to new crash handling API r=jchen

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 21

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9003880 [details]
Bug 1483329 - Add crash handling API to GeckoRuntime r=jchen,esawin

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b4e37c0e3a4
Remove existing crash-related methods in GeckoRuntimeSettings r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/03ec9a21d6b7
Remove job id methods from GeckoRuntimeSettings r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/730ce6aee5d2
Add crash handling API to GeckoRuntime r=jchen,esawin
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd805c5472d5
Hook Fennec up to new crash handling API r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/a417022fbfd4
Hook up geckoview_example to new crash reporting API r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8e0d1e76a4f
Report content process crashes via GeckoRuntime r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e53cff1c083
Add tests for GeckoRuntime crash handling API r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/47ee3d56910d
Hook up GeckoSessionTestRule and TestRunnerActivity to crash reporting r=jchen

Comment 23

[Chris Peterson [:cpeterson]]

status-firefox63=wontfix because James says this is API is too big to uplift.

Comment 24

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/0b4e37c0e3a4
https://hg.mozilla.org/mozilla-central/rev/03ec9a21d6b7
https://hg.mozilla.org/mozilla-central/rev/730ce6aee5d2
https://hg.mozilla.org/mozilla-central/rev/cd805c5472d5
https://hg.mozilla.org/mozilla-central/rev/a417022fbfd4
https://hg.mozilla.org/mozilla-central/rev/e8e0d1e76a4f
https://hg.mozilla.org/mozilla-central/rev/2e53cff1c083
https://hg.mozilla.org/mozilla-central/rev/47ee3d56910d