Closed Bug 1649494 Opened 7 months ago Closed 5 months ago

Crash in [@ mozilla::widget::AndroidAlerts::ShowPersistentNotification]

Categories

(GeckoView :: General, defect, P1)

Unspecified
Android

Tracking

(firefox-esr68 unaffected, firefox-esr78 unaffected, firefox79 wontfix, firefox80 wontfix, firefox81 verified, firefox82 verified)

VERIFIED FIXED
82 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox79 --- wontfix
firefox80 --- wontfix
firefox81 --- verified
firefox82 --- verified

People

(Reporter: agi, Assigned: agi)

References

(Regression)

Details

(Keywords: crash, Whiteboard: [geckoview:m82])

Crash Data

Attachments

(1 file)

First report for this bug is 6/15, so either something we or AC did recently.

This bug is for crash report bp-140bf268-ae89-4d3b-b210-5eafa0200630.

Top 10 frames of crashing thread:

0 libxul.so mozilla::widget::AndroidAlerts::ShowPersistentNotification widget/android/AndroidAlerts.cpp:86
1 libxul.so nsAlertsService::ShowPersistentNotification toolkit/components/alerts/nsAlertsService.cpp:222
2 libxul.so nsAlertsService::ShowAlertNotification toolkit/components/alerts/nsAlertsService.cpp:193
3 libxul.so NS_InvokeByIndex xpcom/reflect/xptcall/md/unix/xptcinvoke_arm.cpp:167
4 libxul.so XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:1140
5 libxul.so XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:946
6 libxul.so js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:576
7 libxul.so Interpret js/src/vm/Interpreter.cpp:639
8 libxul.so js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:611
9 libxul.so js::jit::DoCallFallback js/src/jit/BaselineIC.cpp:2992
Priority: -- → P2
Severity: -- → S3
Priority: P2 → P1
Whiteboard: [geckoview:m82]
Duplicate of this bug: 1658462
Assignee: nobody → agi

Agi and I chatted on Matrix about this, and we evaluated the option of explicitly specifying the extension URL at https://searchfox.org/mozilla-central/rev/73a14f1b367948faa571ed2fe5d7eb29460787c1/toolkit/components/extensions/parent/ext-notifications.js#47-50 .We decided against that in favor of the above patch, because adding the moz-extension://UUID URL to the notification would cause "UUID" to appear in the notification, in the spot where the domain name would usually appear.

If we ever want to add the URL, we need to add a way to get the extension name (or somewhere similarly human-readable) to appear instead.

Pushed by asferro@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a4f5d187f2bc
Don't assume a notification has a URL. r=aklotz
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

Please nominate this for Beta approval when you get a chance.

Flags: needinfo?(agi)

Comment on attachment 9171958 [details]
Bug 1649494 - Don't assume a notification has a URL. a=RyanVM

Beta/Release Uplift Approval Request

  • User impact if declined: Crash when extensions create notifications (e.g. search by image).
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The fix is basically a null check.
  • String changes made/needed:
Flags: needinfo?(agi)
Attachment #9171958 - Flags: approval-mozilla-beta?

Comment on attachment 9171958 [details]
Bug 1649494 - Don't assume a notification has a URL. a=RyanVM

Approved for 81.0b4.

Attachment #9171958 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

There's a conflict on mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md when trying to uplift this to beta.

Agi, could, you, please, take a look?

Flags: needinfo?(agi)
Attachment #9171958 - Attachment description: Bug 1649494 - Don't assume a notification has a URL. → Bug 1649494 - Don't assume a notification has a URL. a=RyanVM

This patch should now apply to Beta. I could swear I had access to push to beta directly but looks like I don't? https://phabricator.services.mozilla.com/D88165

Flags: needinfo?(agi)

We can still reproduce this on Fenix Beta 81.1.0 with the steps from https://github.com/mozilla-mobile/fenix/issues/12910:
Fixed on Nightly 9/2.

QA Contact: oana.horvath

The uplift in comment 11 didn't land in time for the 81.1.0 build.

FYI, the 81.1.0-beta.2 build should contain this fix.

Flags: needinfo?(oana.horvath)

Thanks Ryan, we have verified the fix on the 81.1.0-beta.2 build, no longer crashing.

Status: RESOLVED → VERIFIED
Flags: needinfo?(oana.horvath)

It seems, as a result a new bug appeared: https://github.com/mozilla-mobile/fenix/issues/14993

You need to log in before you can comment on or make changes to this bug.