973728 - crash in java.lang.NoSuchMethodError: android.content.SharedPreferences$Editor.apply at org.mozilla.gecko.webapp.Allocator.putPackageName(Allocator.java)

Status: RESOLVED FIXED

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

Description

[Aaron Train [:aaronmt]]

This bug was filed from the Socorro interface and is 
report bp-7e0d764a-27d1-48c7-a779-ec5962140216.
=============================================================

java.lang.NoSuchMethodError: android.content.SharedPreferences$Editor.apply
	at org.mozilla.gecko.webapp.Allocator.putPackageName(Allocator.java:89)
	at org.mozilla.gecko.webapp.Allocator.findOrAllocatePackage(Allocator.java:79)
	at org.mozilla.gecko.webapp.Dispatcher.onCreate(Dispatcher.java:41)
	at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1047)
	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2627)
	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2679)
	at android.app.ActivityThread.access$2300(ActivityThread.java:125)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2033)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:123)
	at android.app.ActivityThread.main(ActivityThread.java:4627)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:521)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:868)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:626)
	at dalvik.system.NativeStart.main(Native Method)

Comment 1

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

SharedPreferences.apply is not supported in Froyo. mobile/android/base/webapp/Allocator.java has several places that use apply()

We typically use commit() in a background thread, which is not as clean :(

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: use Editor.commit() on API level 8

This patch uses Editor.commit() when the API level is 8.

Mark: you suggested that I use commit() on a background thread, as some other Fennec code does.  But Allocator is an API that exposes higher-level methods for changing prefs, like putPackageName and updateColor.  So its consumers may think they can access the changed prefs immediately (f.e. immediately call getOrigin after calling putOrigin), which wouldn't be guaranteed by a background-thread-based implementation.

We could unrefactor the code, making consumers edit/commit prefs directly, to avoid that danger.  But that would be a larger change, and we'd lose the benefit of the abstraction.

Thus this patch uses commit() on the main thread when the API level is 8, which subjects old devices to the associated performance hit; but otherwise uses apply(), for newer devices.  Since most devices are newer, this maintains the performance of apply() for most devices, while still fixing the crash for the few remaining older ones, which feels like the right tradeoff.

I haven't tested this on an older device yet, as I don't have access to one.  But it works as expected on a newer one.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

James wasn't sure this would work, so I hunted down a 2.2 device and tested it there.  Android does (i)nform the log about apply() not being found:

03-24 15:43:42.601   2155     2155               dalvikvm  I  Could not find method android.content.SharedPreferences$Editor.apply, referenced from method org.mozilla.gecko.webapp.Allocator.save

But it doesn't crash, and the app installs, launches, and runs as expected.

Also, I see a lot of similar info and error messages in the log, f.e.:

03-24 15:43:42.546   2155     2155               dalvikvm  I  Could not find method android.hardware.Camera.getNumberOfCameras, referenced from method org.mozilla.gecko.GeckoAppShell.initCamera
03-24 15:43:42.546   2155     2155               dalvikvm  I  Could not find method android.hardware.Camera.open, referenced from method org.mozilla.gecko.GeckoAppShell.initCamera
03-24 15:43:42.546   2155     2155               dalvikvm  E  Could not find class 'android.view.TextureView', referenced from method org.mozilla.gecko.GeckoAppShell.initCamera

So this appears to be a common way to conditionally access classes and methods in Fennec, and it appears to work despite such messages being logged.

Comment 4

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

Comment on attachment 8395712 [details] [diff] [review]
patch v1: use Editor.commit() on API level 8

I can live with this.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/362a9771b602

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/362a9771b602