873072 - Launching settings from Android system notification crashes if Gecko has been killed.

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

E/AndroidRuntime( 6532): FATAL EXCEPTION: main
E/AndroidRuntime( 6532): java.lang.RuntimeException: Unable to resume activity {org.mozilla.fennec/org.mozilla.gecko.GeckoPreferences}: java.lang.NullPointerException
E/AndroidRuntime( 6532): 	at android.app.ActivityThread.performResumeActivity(ActivityThread.java:2742)
E/AndroidRuntime( 6532): 	at android.app.ActivityThread.handleResumeActivity(ActivityThread.java:2771)
E/AndroidRuntime( 6532): 	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2235)
E/AndroidRuntime( 6532): 	at android.app.ActivityThread.access$600(ActivityThread.java:141)
E/AndroidRuntime( 6532): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1234)
E/AndroidRuntime( 6532): 	at android.os.Handler.dispatchMessage(Handler.java:99)
E/AndroidRuntime( 6532): 	at android.os.Looper.loop(Looper.java:137)
E/AndroidRuntime( 6532): 	at android.app.ActivityThread.main(ActivityThread.java:5041)
E/AndroidRuntime( 6532): 	at java.lang.reflect.Method.invokeNative(Native Method)
E/AndroidRuntime( 6532): 	at java.lang.reflect.Method.invoke(Method.java:511)
E/AndroidRuntime( 6532): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:793)
E/AndroidRuntime( 6532): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:560)
E/AndroidRuntime( 6532): 	at dalvik.system.NativeStart.main(Native Method)
E/AndroidRuntime( 6532): Caused by: java.lang.NullPointerException
E/AndroidRuntime( 6532): 	at org.mozilla.gecko.GeckoConnectivityReceiver.start(GeckoConnectivityReceiver.java:49)
E/AndroidRuntime( 6532): 	at org.mozilla.gecko.GeckoApplication.onActivityResume(GeckoApplication.java:73)
E/AndroidRuntime( 6532): 	at org.mozilla.gecko.GeckoPreferences.onResume(GeckoPreferences.java:161)
E/AndroidRuntime( 6532): 	at android.app.Instrumentation.callActivityOnResume(Instrumentation.java:1185)
E/AndroidRuntime( 6532): 	at android.app.Activity.performResume(Activity.java:5182)
E/AndroidRuntime( 6532): 	at android.app.ActivityThread.performResumeActivity(ActivityThread.java:2732)
E/AndroidRuntime( 6532): 	... 12 more
W/ActivityManager(  526):   Force finishing activity org.mozilla.fennec/org.mozilla.gecko.GeckoPreferences
W/ActivityManager(  526): Activity pause timeout for ActivityRecord{42a990c8 u0 org.mozilla.fennec/org.mozilla.gecko.GeckoPreferences}

Steps:

i) Start Nightly, get notification about data & stats 
ii) Swipe off Nightly to remove it from the foreground
iii) Attempt to access data & stats from the notification shortcut

--
LG Nexus 4 (Android 4.2.2)
Nightly (05/16)

Comment 1

[Scoobidiver (away)]

Aurora 23 affected?

Comment 2

[Aaron Train [:aaronmt]]

(In reply to Scoobidiver from comment #1)
> Aurora 23 affected?

Yes.

Comment 3

[Chenxia Liu [:liuche]]

This is happening because we're trying to start an Activity that has a dependency on Gecko, which may be killed after being backgrounded.

It looks like the solution will be to launch GeckoApp with a flag, and have GeckoApp bring up the Data Choices screen.

Sriram, I think you've done some work with the Gecko/Activities interface (about:home, maybe?) - is there a better way to initialize Gecko than an intent to GeckoApp, or does this the best approach?

Comment 4

[Sriram Ramasubramanian [:sriram]]

(In reply to Chenxia Liu [:liuche] from comment #3)
> This is happening because we're trying to start an Activity that has a
> dependency on Gecko, which may be killed after being backgrounded.
> 
> It looks like the solution will be to launch GeckoApp with a flag, and have
> GeckoApp bring up the Data Choices screen.
> 
> Sriram, I think you've done some work with the Gecko/Activities interface
> (about:home, maybe?) - is there a better way to initialize Gecko than an
> intent to GeckoApp, or does this the best approach?

Since Preferences screen is dependent on Gecko, I would say, start GeckoApp with a new ACTION ("OPEN_PREFERENCES" or something), which will start gecko in the background and open the Preferences activity.
  - This would ensure that, when someone taps a back from the Settings screen, they will see the browser, which is ready to use.

The other hack would be to refactor "isGeckoRunning()" to a separate service or something, and then an activity like Preferences screen can start Gecko, if its not running already. (Note: Services are battery killers. They won't stop until a phone is rebooted.
Note 2: May be we might need it for "GeckoView (like WebView)", so that GeckoView can know the start of Gecko being available in memory or not).

Comment 5

[Chenxia Liu [:liuche]]

Attachment: Patch: use GeckoApp to launch GeckoPreferences v1

Comment 6

[Chenxia Liu [:liuche]]

Attachment: Patch: use GeckoApp to launch GeckoPreferences v2

Launch data choices preferences pane through GeckoApp, in case Gecko has been killed.

try: https://tbpl.mozilla.org/?tree=Try&rev=ee45859e2aea

Comment 7

[Chenxia Liu [:liuche]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/91cc9c75c3d1

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/91cc9c75c3d1

Comment 9

[Chenxia Liu [:liuche]]

Attachment: Aurora patch: Launch settings through GeckoApp

Try build for aurora patch: https://tbpl.mozilla.org/?tree=Try&rev=06a6b0a346f0

Will request approval after it runs.

Comment 10

[Chenxia Liu [:liuche]]

Attachment: Aurora patch: Launch settings through GeckoApp v2

try build w/ tests: https://tbpl.mozilla.org/?tree=Try&rev=f9736f00fab5

Comment 11

[Chenxia Liu [:liuche]]

Comment on attachment 763018 [details] [diff] [review]
Aurora patch: Launch settings through GeckoApp v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 833625
User impact if declined: users with low memory may see crashes when trying to accept the one-time notification of Mozilla data reporting policies
Testing completed (on m-c, etc.): no problems on m-c since 6/13 landing, all green try build: https://tbpl.mozilla.org/?tree=Try&rev=f9736f00fab5
Risk to taking this patch (and alternatives if risky): changing the code path if a notification is clicked; may be slightly slower to display datareporting screen than original approach because of extra step to launch activity
String or IDL/UUID changes made by this patch: none

Comment 12

[Chenxia Liu [:liuche]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2c7dc0066cd2

Comment 13

[Teodora Vermesan (:TeoVermesan)]

Verified fixed on:
Build: Firefox for Android 23.0b8 (2013-07-23)
Device: LG Nexus 4
OS: Android 4.2.2

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

This needs to be backed out from FF23 beta in order to deal with the regression in bug 896992 - please prepare a patch for uplift to beta and we can look for a forward fix in FF24.

Comment 15

[Chenxia Liu [:liuche]]

Attachment: Patch: backout of patch on beta

Lukas - I think this is due to having the "Don't keep activities" developer option checked, so I'll try to make a patch for fixing that (fairly rare) case and upload it this evening.

Comment 16

[Chenxia Liu [:liuche]]

Comment on attachment 780717 [details] [diff] [review]
Patch: backout of patch on beta

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 896992
User impact if declined: Users who enable "Don't keep activities" in the Developer Options and then kill Firefox after a first launch and then launch Firefox from the notification will get into an infinite loop
Testing completed (on m-c, etc.): try build: https://tbpl.mozilla.org/?tree=Try&rev=031c9fc7e4a7
Risk to taking this patch (and alternatives if risky): Original bug will be present
String or IDL/UUID changes made by this patch: none

Comment 17

[Chenxia Liu [:liuche]]

Lukas, I've uploaded some one-line fixes that resolve this problem under bug 896992. If those get r+, I'll request uplift for them to beta and aurora.

Comment 18

[Teodora Vermesan (:TeoVermesan)]

Verified fixed on:
Build: Firefox for Android 24.0a2 (2013-07-24)
Device: LG Nexus 4
OS: Android 4.2.2

Comment 19

[Chenxia Liu [:liuche]]

Lukas, the patch for the fix turns out to be more than a one-liner, and since it touches startup (which is brittle already), we should just go with the original plan: land the backout patch for beta and fix the bug on aurora.

Comment 20

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 780717 [details] [diff] [review]
Patch: backout of patch on beta

Sounds good, let's do that.

Comment 21

[Chenxia Liu [:liuche]]

https://tbpl.mozilla.org/?tree=Mozilla-Beta&rev=3cf37a4bbb88

Comment 22

[Lukas Blakk [:lsblakk] use ?needinfo]

Can QA verify this has been disabled correctly on 23?

Comment 23

[Lukas Blakk [:lsblakk] use ?needinfo]

This is still affected on FF23 and we're shipping with it due to lack of time, bug 896992 can track fixing the looping this caused for FF24

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Can we call this status-firefox23:wontfix then?

Comment 25

[bhavana bajaj [:bajaj]]

Can QA please help with verification on Fx24 once  https://bugzilla.mozilla.org/show_bug.cgi?id=896992 lands ?

Comment 26

[Aaron Train [:aaronmt]]

Confirming that the crash is back on mozilla-beta; fixed on mozilla-aurora and mozilla-beta.

Comment 27

[Aaron Train [:aaronmt]]

(^central)

Comment 28

[Flaviu Cos]

Removing verifyme flag since it was verified by Aaron in comment 26.