1499415 - Crash in java.lang.NullPointerException: at org.mozilla.gecko.BrowserApp.onPrepareOptionsMenu(BrowserApp.java)

Status: RESOLVED FIXED

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

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-b37bc271-142a-43b9-b5af-9e5240181016.
=============================================================

This signature has been seen before, most recently in Bug 869827: https://bit.ly/2pXoTzY. Overall this is the #10 crash in 62.0.3. Filing because it needs a new bug to see if we can do anything about the Null Pointer Exception being called below. Only 1 crash in 64 in the last 7 days, and visible in 63 beta but in low volume.

java.lang.NullPointerException: Attempt to invoke virtual method 'boolean org.mozilla.gecko.extensions.ExtensionPermissionsHelper.getShowUpdateIcon()' on a null object reference
	at org.mozilla.gecko.BrowserApp.onPrepareOptionsMenu(BrowserApp.java:3904)
	at org.mozilla.gecko.GeckoApp.invalidateOptionsMenu(GeckoApp.java:421)
	at org.mozilla.gecko.BrowserApp.refreshChrome(BrowserApp.java:1884)
	at org.mozilla.gecko.GeckoApp.onLocaleReady(GeckoApp.java:1330)
	at org.mozilla.gecko.BrowserApp.onLocaleReady(BrowserApp.java:2975)
	at org.mozilla.gecko.GeckoApp$10$1.run(GeckoApp.java:1262)
	at android.os.Handler.handleCallback(Handler.java:815)
	at android.os.Handler.dispatchMessage(Handler.java:104)
	at android.os.Looper.loop(Looper.java:207)
	at android.app.ActivityThread.main(ActivityThread.java:5728)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:888)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:749)

Comment 1

[Mike Taylor [:miketaylr]]

Susheel, do you have someone who can take a look at this crash? Thanks.

Comment 2

[:sdaswani]

Looks pretty low volume, so will put on our low priority list.

Comment 3

[Liz Henry (:lizzard) (relman/hg->git project)]

It's the #10 topcrash on release, but doesn't show up in high volume on other channels. 
Since 63 release is next week, marking as wontfix.

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1499415 - Check mExtensionPermissionsHelper for null before using it; r?sdaswani

`mExtensionPermissionsHelper` is initialized in BrowserApp's onCreate() and
nullified in it's onDestroy().
GeckoApp's onCreate() can finish after BrowserApp's onDestroy() because it uses
background initialization.
`mExtensionPermissionsHelper` will be checked for null before using it and
if null it's safe to ignore it's method's results as it only can be null if the
app was already closed.

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Looking through the code looks like this might happen when the app is opened and closed immediately after.
GeckoApp's onCreate() uses background threads to complete it's initialization and from one such thread this crash originates [1].

When the app is closed BroserApp.java nullifies that ExtensionPermissionHelper instance[2].
But the background initialization of GeckoApp might still be running and eventually [3] a call to onPrepareOptionsMenu in BrowserApp will be made which will try to use that already nullified ExtensionPermissionHelper instance.

Because `mExtensionPermissionsHelper` is only nullified in BrowserApp's onDestroy() the safe and simple solution would be to just add a null check before trying to execute something on it.


[1] https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java#1211
[2] https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#1491
[3] https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java#1264

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2294c830e11
Check mExtensionPermissionsHelper for null before using it; r=sdaswani

Comment 7

[Andreea Pavel [:apavel]]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 9

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9022623 [details]
Bug 1499415 - Check mExtensionPermissionsHelper for null before using it; r?sdaswani

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: App crash

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

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): Simple null check added

String changes made/needed:

Comment 10

[Julien Cristau [:jcristau]]

Comment on attachment 9022623 [details]
Bug 1499415 - Check mExtensionPermissionsHelper for null before using it; r?sdaswani

fennec crash fix, approved for 64.0b9

Comment 11

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e578f064314f