1550291 - Fennec crash reporter triggers Android Q warning about background activity starts

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Kevin Brosnan [Ex-Mozilla]]

When GeckoView or Fennec crashes on Android Q a toast notification appears warning of a background activity start. Looks like we might need to use the notification shade to preserve the crash reporter functions. https://developer.android.com/preview/privacy/background-activity-starts

Comment 1

[Chris Peterson [:cpeterson]]

Here's the Google bug about this new background activity restriction:
https://issuetracker.google.com/issues/128511873

Many "me too" comments and duplicate bugs, but no signal from Google that they might back off this new restriction in Android Q's final release.

Comment 2

[Devin Reams]

Chris, would the GV team help determine the right approach here? Stefan mentioned they are starting to talk about it re: Fenix too.

Please let me know how the Android team can help support this.

Comment 3

[Chris Peterson [:cpeterson]]

(In reply to Devin Reams (dreams) from comment #2)

Chris, would the GV team help determine the right approach here? Stefan mentioned they are starting to talk about it re: Fenix too.

Sure. I'll send this bug back to GV triage so the GV team can discuss next steps.

Comment 4

[Chris Peterson [:cpeterson]]

Devin, to be compatible with the upcoming Android Q release (expected in ~Q3), Fennec's crash reporter needs to start a foreground service instead of an Activity.

Fenix/A-C will need to do the same, but it has different code. I filed a new bug for Fenix/A-C: https://github.com/mozilla-mobile/android-components/issues/3097

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Thanks. We'll go with that.

Comment 6

[Chris Peterson [:cpeterson]]

Thanks! btw, Sebastian had some comments (in the Fenix GitHub issue) about Android's startForegroundService API on Android API level 26+. I don't know if this is relevant for Fennec, too.

https://github.com/mozilla-mobile/android-components/issues/3097#issuecomment-495349717

Comment 7

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1550291 - Add new "Crash handler" notification channel; r?snorp

Comment 8

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1550291 - Android Q: Use the overlay permission or a foreground notification to start the crash handler; r?VladBaicu

Android Q doesn't allow starting Activities from background so we'll use a
foreground notification from where users could start CrashReporterActivity.

Depends on D33028

Comment 9

[Kevin Brosnan [Ex-Mozilla]]

Google released Q beta 4 with the finalized API changes and the background execution change is still part of Q. https://android-developers.googleblog.com/2019/06/android-q-beta-4-and-final-apis.html

Comment 10

[Jan Henning [:JanH]]

But according to https://issuetracker.google.com/issues/128511873#comment81, activities can be launched from the background if you hold the SYSTEM_ALERT_WINDOW permission, which we already require for the tab queue in any case. Though we still have to handle the case where we haven't (yet) granted that permission by the user, too...

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d7f44087b9a5
Add new "Crash handler" notification channel; r=JanH
https://hg.mozilla.org/integration/autoland/rev/53931ee8bb19
Android Q: Use the overlay permission or a foreground notification to start the crash handler; r=VladBaicu

Comment 12

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/d7f44087b9a5
https://hg.mozilla.org/mozilla-central/rev/53931ee8bb19

Comment 13

[Ryan VanderMeulen [:RyanVM]]

We're planning to spin a Fennec 68b10 build on Thursday, so it would be great if we could get an uplift request in time for that.

Comment 14

[Petru-Mugurel Lingurar [:petru]]

Prerequisites for testing this:

• latest Android Q installed (>= beta 4)
• force a crash using about:crashparent

There are two scenarios with slightly different behaviors that needs to be tested:

1. The crash reporter starts right away

• Display over other apps permission is given to our app.
  User would easiest do this when activating Tab queue.

2. The crash reporter can be started from a notification posted upon a crash

• If Tab queue is disabled (that Display over other apps special permission is not allowed for us) a notification would be posted.
• Tapping the notification would open the crash reporter.
• Tapping the Ignore button will dismiss the notification, users loosing so the ability to submit a crash report for the recent crash.

Once started, the crash reporter should offer the same experience as previously, nothing should be changed.

Comment 15

[Sorina Florean [:sflorean]]

Tested with Google Pixel (Android Q- latest) on 69.0a1 from Treeherder (CI of mozilla-central) and here are my findings following the steps provided by Petru:

• everything worked as expected: the crash reporter starts right away if "display over other apps" is on
• a notification is displayed if "display over other apps" is off.

Found this scenario where the tabs aren't restored after the crash:

1. Go to Settings -> Advanced -> Restore tabs -> set to "don't restore after quitting Nightly".
2. Force a crash using about:crashparent.
3. Tap on "Ignore" from the notification.
4. Open Fennec.

Results: the tabs aren't restored after the crash due to the fact that no crash report was submitted.

Note: on older Android versions since we don't have notification we can't test this but the behavior is to restore the tabs after a crash with "Restore tabs - don't restore after quitting Nightly" on.
Should I file a new bug? Or the issue will be fixed here? @Petru

Comment 16

[Petru-Mugurel Lingurar [:petru]]

Thanks Sorina!
Will investigate this issue in this ticket.

Comment 17

[Jan Henning [:JanH]]

This is the bit that let's GeckoApp know we've crashed the next time it starts:
https://dxr.mozilla.org/mozilla-central/rev/43b2d57b25cc16d0571e6bef6a414abe24457154/mobile/android/base/java/org/mozilla/gecko/CrashReporterActivity.java#207-213

Maybe moving this to the point where we decide to either directly launch the Crash Reporter UI, respectively alternatively only show the notification is sufficient?

Comment 18

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1550291 - Ensure we save CRASHED status to be able to restore previous tabs; r?JanH

Default app behavior for when a crash occurs involves starting an Activity that
lets users submit a crash report and maybe provide additional info.
In the event of a crash we want to restore for the user all previous tabs, this
Activity being the component that saves the CRASHED status that will trigger
the restore process.

On Android Q, when a crash notification is posted but ignored by the user, upon
starting the app would not know it crashed and it would not try restoring tabs.
Executing the "save CRASHED status" logic whenever a crash occurs, irrespective
of if we'll show the Activity or just post a notification ensures users will
always have their previous tabs restored.

Comment 19

[Chris Peterson [:cpeterson]]

I'm reopening this bug because Petru is landing some follow-up fixes.

68=affected because we'll want to uplift all these Android Q fixes to Fennec 68 Beta.

Comment 20

[Petru-Mugurel Lingurar [:petru]]

I've added a new patch. Please also land D34826.

Comment 21

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9068424 [details]
Bug 1550291 - Add new "Crash handler" notification channel; r?snorp

Beta/Release Uplift Approval Request

• User impact if declined: Crash Reporter is not working on Android Q devices.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: The steps from comment #14 and comment #15.
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): We change how the crash reporter starts and saves the "CRASHED" status so that the app would know to restore tabs after a crash.
• String changes made/needed: crash_handler_notifications_channel,crash_notification_title, crash_notification_message, crash_notification_negative_button_text

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/35ee61053c39
Ensure we save CRASHED status to be able to restore previous tabs; r=AndreiLazar

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9068424 [details]
Bug 1550291 - Add new "Crash handler" notification channel; r?snorp

Fixes crash reporting on Android Q devices. Let's get this uplifted now so it's in tomorrow's Nightly builds for QA to verify.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a74afc1a7a21
https://hg.mozilla.org/releases/mozilla-beta/rev/22cfa63e8563
https://hg.mozilla.org/releases/mozilla-beta/rev/9ac627048305

Comment 25

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/35ee61053c39

Comment 26

[Eliza Balazs]

Hi!
I tested this on Beta Nightly 68.0a1 (2019-06-13), Nightly 68.0a1 (2019-06-14) with Google Pixel (Android Q) and I could not reproduce the issue. The tabs are restored when Restore tabs is set to "Don't restore after quitting Nightly" and the "Ignore" button is selected from the pop-up.
This is not fixed on Beta 68.0b10 and I will leave the qe-verify flag until we verify this on the next Beta build.
Due to my findings I will mark this issue as fixed on Firefox 69.
Thanks!

Comment 27

[Chris Peterson [:cpeterson]]

Thanks, Eliza!

I'll set the firefox68 status flag to affected as a reminder that we still need to uplift the fix for restoring tabs (comment 25).

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Comment 25 was included in the Beta uplift in comment 24.
https://hg.mozilla.org/releases/mozilla-beta/rev/9ac627048305

Comment 29

[Eliza Balazs]

Hi!
Verified as fixed on Beta 68.0b11 with Google Pixel (Android Q) and I could not reproduce the issue. The tabs are restored when Restore tabs is set to "Don't restore after quitting Nightly" and the "Ignore" button is selected from the pop-up.
Due to my findings I will mark this issue as fixed on Firefox 68.
Thanks!