1562831 - Firefox crash report is displayed twice after about:crashparent

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Session Restore, defect)

Description

[Eliza Balazs]

Attachment: tabcrash.txt

Environment:
Device:
OnePlus 5T (Android 9);
Xiaomi Mi Pad 2 (Android 5.1)

Build:
Nightly 68.0a1 (2019-06-28);
Beta 68.0b14;
Release 68.0.

Steps to reproduce:

1. Visit 2, 3 pages in different tabs and go to about:crashparent;
2. From the Firefox crash report choose "Restart Nightly";
3. Observe the restored tabs.

Expected result:
The tabs opened at step 1 are restored.

Actual result:
Firefox crash report is displayed again.

Notes:

• This is reproducible with "Always restore"/ "Don't restore after quitting Nightly" set in Restore Tabs.
• Not reproducible on Release 67.0.3.
• Video: https://drive.google.com/file/d/1P3b7IdvNSTF2X1ddJraY4hXxeOmR40wS/view

Comment 1

[Marcia Knous [:marcia]]

I can check, but I think we may have recently changed something with about:crashparent. If this is only happening in this scenario it is not bad - it would be bad if it is happening every time we crash.

Comment 2

[Petru-Mugurel Lingurar [:petru]]

Seems like this issue is a regression of bug 1550291.
Although the majority of devices are exposed to it, it seems to be reproducible on just a few, but indeed, for every time we crash - the Crash Reporter is restarted and shown a second time.

Comment 3

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1562831 - Stop the Crash Service cleanly to prevent it's restart; r?VladBaicu

The service would be restarted after System.exit(0) which would show the crash
feedback form again to the user.

That System.exit(0) was initially used to prevent a silent ANR because of the
Service being started from background on Android Oreo+ without a foreground
notification.

To overcome all this we'll also use a foreground notification on Android Oreo+
but with NotificationManager.IMPORTANCE_LOW to be non-intrusive.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9c61c4e5b1d
Stop the Crash Service cleanly to prevent it's restart; r=VladBaicu

Comment 5

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/d9c61c4e5b1d

Comment 6

[Eliza Balazs]

Hi!
Verified as fixed on Nightly 69.0a1 (2019-07-04) from mozilla-central with

• Google Pixel (Android Q);
• OnePlus 5T (Android 9);
• Nexus 6P (Android 8.1.0);
• Motorola Nexus 6 (Android 7.1.1).

I will mark this as verified on Firefox 69.
Thanks!

Comment 7

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9075646 [details]
Bug 1562831 - Stop the Crash Service cleanly to prevent it's restart; r?VladBaicu

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: High importance fix for a regression introduced in 68.
• User impact if declined: Some users may experience the crash reporter being shown two times after a crash.
• Fix Landed on Version: 69
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Medium risk since the change affects the crash reporter flow on all Android versions.
• String or UUID changes made by this patch:

Beta/Release Uplift Approval Request

• User impact if declined: High importance fix for a regression introduced in 68.
  Some users may experience the crash reporter being shown two times after a crash.
• 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: Simulate a crash on different devices with different Android versions. Confirm the crash reporter appear as expected.
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Medium risk since the change affects the crash reporter flow on all Android versions.
• String changes made/needed:

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 9075646 [details]
Bug 1562831 - Stop the Crash Service cleanly to prevent it's restart; r?VladBaicu

Not necessary on release, and I'd rather have it bake a bit on nightly/beta rather than take it directly into 68.0, so leaving the esr68 request open until next week.

Comment 9

[Petru-Mugurel Lingurar [:petru]]

(In reply to Julien Cristau [:jcristau] from comment #8)

Not necessary on release, and I'd rather have it bake a bit on nightly/beta rather than take it directly into 68.0, so leaving the esr68 request open until next week.

I was just hoping for it to be included in the next dot release.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9075646 [details]
Bug 1562831 - Stop the Crash Service cleanly to prevent it's restart; r?VladBaicu

Fixes a regression in Fennec causing crash reports to show up twice. Approved for Fennec 68.1b1.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/161aae250e42

Comment 12

[Diana Rus]

Hi, verified as fixed on firefox ESR 68.1b1 (Build 3 )

• Sony Xperia Z3 (Android 5.1.1);
• Redmi Note 3 (Android 6.0.1);
• One Plus A2005 (Android 6.0.1);
• Sony Xperia Z5 (Android 7.0)
• Samsung Galaxy S9 (Android 8.1.0);
• Pixel XL (Android Q);

I will mark this as verified on Firefox 68 ESR. The ticket will be marked as verified.
Thanks!

Comment 13

[Chris Peterson [:cpeterson]]

Approved for Fennec 68.1b1.