888374 - crash [@ android.content.res.Resources$NotFoundException: Resource ID #0x0 at android.content.res.Resources.getValue(Resources.java) ] Clicking link in about:healthreport for Choose what I share

Status: RESOLVED FIXED

(Firefox Health Report Graveyard :: Client: Android, defect)

Description

[Arun Balachandran Ganesan [:abc]]

On Nightly, in about:healthreport if you tap on 'Choose what I share' at the bottom of the page it should link to Settings > Firefox Health Report, but it causes a crash.

Comment 1

[Matt Brandt [:mbrandt]]

Here's the traceback -- let me know if this isn't sufficient and additional information would be helpful.

E/GeckoConsole( 6626): AboutHealthReport: showing settings.
W/ResourceType( 6626): No package identifier when getting value for resource number 0x00000000
D/AndroidRuntime( 6626): Shutting down VM
W/dalvikvm( 6626): threadid=1: thread exiting with uncaught exception (group=0x40a541f8)
E/GeckoAppShell( 6626): >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 1 ("main")
E/GeckoAppShell( 6626): android.content.res.Resources$NotFoundException: Resource ID #0x0
E/GeckoAppShell( 6626): 	at android.content.res.Resources.getValue(Resources.java:1049)
E/GeckoAppShell( 6626): 	at android.content.res.Resources.loadXmlResourceParser(Resources.java:2165)
E/GeckoAppShell( 6626): 	at android.content.res.Resources.getXml(Resources.java:937)
E/GeckoAppShell( 6626): 	at android.preference.GenericInflater.inflate(GenericInflater.java:261)
E/GeckoAppShell( 6626): 	at android.preference.PreferenceManager.inflateFromResource(PreferenceManager.java:269)
E/GeckoAppShell( 6626): 	at android.preference.PreferenceFragment.addPreferencesFromResource(PreferenceFragment.java:278)
E/GeckoAppShell( 6626): 	at org.mozilla.gecko.GeckoPreferenceFragment.onCreate(GeckoPreferenceFragment.java:31)
E/GeckoAppShell( 6626): 	at android.app.FragmentManagerImpl.moveToState(FragmentManager.java:795)
E/GeckoAppShell( 6626): 	at android.app.FragmentManagerImpl.moveToState(FragmentManager.java:1032)
E/GeckoAppShell( 6626): 	at android.app.BackStackRecord.run(BackStackRecord.java:622)
E/GeckoAppShell( 6626): 	at android.app.FragmentManagerImpl.execPendingActions(FragmentManager.java:1382)
E/GeckoAppShell( 6626): 	at android.app.Activity.performStart(Activity.java:4474)
E/GeckoAppShell( 6626): 	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2042)
E/GeckoAppShell( 6626): 	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2104)
E/GeckoAppShell( 6626): 	at android.app.ActivityThread.access$600(ActivityThread.java:132)
E/GeckoAppShell( 6626): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1157)
E/GeckoAppShell( 6626): 	at android.os.Handler.dispatchMessage(Handler.java:99)
E/GeckoAppShell( 6626): 	at android.os.Looper.loop(Looper.java:137)
E/GeckoAppShell( 6626): 	at android.app.ActivityThread.main(ActivityThread.java:4575)
E/GeckoAppShell( 6626): 	at java.lang.reflect.Method.invokeNative(Native Method)
E/GeckoAppShell( 6626): 	at java.lang.reflect.Method.invoke(Method.java:511)
E/GeckoAppShell( 6626): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:789)
E/GeckoAppShell( 6626): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:556)
E/GeckoAppShell( 6626): 	at dalvik.system.NativeStart.main(Native Method)
E/Gecko   ( 6626): mozalloc_abort: Redirecting call to abort() to mozalloc_abort
D/Zygote  ( 1872): Process 6626 exited cleanly (11)
I/ActivityManager( 1998): Process org.mozilla.fennec (pid 6626) has died.

Comment 2

[Matt Brandt [:mbrandt]]

reproduced on Android 4.0.4

Comment 3

[Chenxia Liu [:liuche]]

I'm pretty sure this is me, I missed this replacing this reference when I landed bug 872329, which removes the preferences_datareporting resource.

Comment 4

[Chenxia Liu [:liuche]]

Attachment: Patch: Reference new resource

Build for testing here: http://people.mozilla.com/~liuche/bug-888374/fix-resource-crash.apk

Tracy, can you test on your different versions?

Comment 5

[Tracy Walker [:tracy]]

crash report from 3.1 tablet: https://crash-stats.mozilla.com/report/index/b0331c05-a704-47ce-9bd4-30f2f2130628

Comment 6

[Tracy Walker [:tracy]]

test build looks good on multiple devices. no longer crashing with it.

Comment 7

[Nick Alexander :nalexander [he/him]]

Comment on attachment 769064 [details] [diff] [review]
Patch: Reference new resource

Review of attachment 769064 [details] [diff] [review]:
-----------------------------------------------------------------

Can you add some sanity checking to GeckoPreferenceFragment.java:31 so that we don't crash on bad input?  r+ with that.

Comment 8

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #7)
> Comment on attachment 769064 [details] [diff] [review]
> Patch: Reference new resource
> 
> Review of attachment 769064 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can you add some sanity checking to GeckoPreferenceFragment.java:31 so that
> we don't crash on bad input?  r+ with that.

Oh, and load the default settings if the specified resource is not loadable.

Comment 9

[Chenxia Liu [:liuche]]

Attachment: Patch: rename ref to deleted resource and handle unknown resources

Added a check for a bad resource id, and display the default settings screen if the resource is invalid.

Comment 10

[Nick Alexander :nalexander [he/him]]

Comment on attachment 769215 [details] [diff] [review]
Patch: rename ref to deleted resource and handle unknown resources

Review of attachment 769215 [details] [diff] [review]:
-----------------------------------------------------------------

nits, answer Q for yourself, then r=me.

::: mobile/android/base/GeckoPreferenceFragment.java
@@ +24,5 @@
>  
>      @Override
>      public void onCreate(Bundle savedInstanceState) {
>          super.onCreate(savedInstanceState);
> +        String resourceName = getArguments().getString("resource");

Q: what happens if resourceName is null?

@@ +32,5 @@
> +        if (res != 0) {
> +            addPreferencesFromResource(res);
> +        } else {
> +            // Invalid resource. Use default resource.
> +            Log.e(LOGTAG, "Failed to find resource: " + resourceName);

nit: make the log message just one line.

@@ +40,5 @@
> +            resourceName = isMultiPane ? "preferences_customize_tablet" : "preferences";
> +            res = getActivity().getResources().getIdentifier(resourceName,
> +                                                             "xml",
> +                                                             getActivity().getPackageName());
> +            addPreferencesFromResource(res);

nit: call this just once, outside of the if block.

::: mobile/android/base/GeckoPreferences.java
@@ +95,5 @@
>              if (intentExtras != null && intentExtras.containsKey(INTENT_EXTRA_RESOURCES)) {
>                  String resourceName = intentExtras.getString(INTENT_EXTRA_RESOURCES);
>                  int resource = getResources().getIdentifier(resourceName, "xml", getPackageName());
> +                if (resource != 0) {
> +                    addPreferencesFromResource(resource);

Again, call this just once outside of the block.

@@ +98,5 @@
> +                if (resource != 0) {
> +                    addPreferencesFromResource(resource);
> +                } else {
> +                    // 0 is an invalid resource; use the default preferences screen.
> +                    Log.e(LOGTAG, "Failed to find resource: " + resourceName);

nit: just one line.

@@ +106,2 @@
>              } else {
>                  addPreferencesFromResource(R.xml.preferences);

In fact, maybe just call once in the function?

Comment 11

[Chenxia Liu [:liuche]]

Attachment: Patch: rename ref to deleted resource and handle unknown resources v2

Addressed nits and added handling of null resourceName.

Comment 12

[Chenxia Liu [:liuche]]

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

Tracy: 24 is not affected, so we don't need aurora uplift, correct?

Comment 13

[Phil Ringnalda (:philor)]

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

Comment 14

[Tracy Walker [:tracy]]

The link in question is not even clickable in aurora. I believe other work needs to be checked into aurora to make the link work there. When that is complete, I imagine this fix would also have to be uplifted to 24. Or is FHR not supposed to work on aurora(24), Nick?

Comment 15

[Nick Alexander :nalexander [he/him]]

(In reply to Tracy Walker [:tracy] from comment #14)
> The link in question is not even clickable in aurora. I believe other work
> needs to be checked into aurora to make the link work there. When that is
> complete, I imagine this fix would also have to be uplifted to 24. Or is FHR
> not supposed to work on aurora(24), Nick?

Tracy, sorry to be so slow replying to this.  Lost in the shuffle.

This made it to beta on June 24th (2 weeks ago): https://hg.mozilla.org/releases/mozilla-beta/rev/89d6bdd9e08f

and the current jelly (at https://fhr.cdn.mozilla.net/en-US/mobile/js/mobile.js) has the link logic, so this should work in current Aurora.