774422 - Can't launch app from install notification

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), defect)

Description

[Wesley Johnston (:wesj)]

There's a race going on here between the NotificationHandler bringing our app to the front and Gecko starting the new webapp intent. Apparently us coming to the front is winning now.

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/NotificationHandler.java.in#50

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch

This changes the order that we dispatch these so the "bring App to the front" intent is sent before the webapp one.

Some part of me worries that this is just moving the race without solving the problem, but the Gecko bit is the only cross thread piece here and firing it after we call startActivity(AppIntent) should ensure that that intent is always dispatched second... I think?

Comment 2

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d679f29a5cf1

Comment 3

[Ed Morley [:emorley]]

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

Comment 4

[Aaron Train [:aaronmt]]

Looks good. V-F on M-C (Nightly 07/17), GN 4.1.1

Does this need to be uplifted to Aurora?

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Aaron Train [:aaronmt] from comment #4)
> Looks good. V-F on M-C (Nightly 07/17), GN 4.1.1
> 
> Does this need to be uplifted to Aurora?

I think so. Wes, please nom.

Comment 6

[Wesley Johnston (:wesj)]

Comment on attachment 642693 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This code has been here a long time, but something got faster and started triggering this race. No idea what it was.
User impact if declined: Notifications that launch intents might not work.
Testing completed (on m-c, etc.): Landed on mc Jul 17.
Risk to taking this patch (and alternatives if risky): Low risk. Just reordering existing code.
String or UUID changes made by this patch:  None.

Comment 7

[Alex Keybl [:akeybl]]

Comment on attachment 642693 [details] [diff] [review]
Patch

[Triage Comment]
Approving for Aurora 16. Is this low-risk/high-reward enough for uplift to Beta 15?

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Alex Keybl [:akeybl] from comment #7)
> Comment on attachment 642693 [details] [diff] [review]
> Patch
> 
> [Triage Comment]
> Approving for Aurora 16. Is this low-risk/high-reward enough for uplift to
> Beta 15?

Nah. WebApps is not going to be enabled on Fx15 (Beta)

Comment 9

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/31111ab0eb7a