846660 - java.lang.InstantiationException: can''t instantiate class char at java.lang.Class.newInstanceImpl(Native Method) on Samsung Galaxy SIII and Note running JB

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Keyboards and IME, defect)

Description

[Scoobidiver (away)]

It's #50 crasher in 20.0.

Here are affected devices during the last days:
* February 27:
Samsung GT-I9300 	5
Samsung GT-N8000 	4
Samsung GT-I8190 	1
* February 26:
Samsung GT-I8190 	5
* February 25:
Samsung GT-N8000 	1

Here is a crash report: bp-b1875217-190f-4f87-b238-5eddf2130301.

java.lang.InstantiationException: can't instantiate class char
	at java.lang.Class.newInstanceImpl(Native Method)
	at java.lang.Class.newInstance(Class.java:1319)
	at org.mozilla.gecko.GeckoEditable.invoke(GeckoEditable.java:751)
	at $Proxy0.charAt(Native Method)
	at android.text.Selection.setSelection(Selection.java:81)
	at android.view.inputmethod.BaseInputConnection.setSelection(BaseInputConnection.java:497)
	at com.android.internal.view.IInputConnectionWrapper.executeMessage(IInputConnectionWrapper.java:288)
	at com.android.internal.view.IInputConnectionWrapper$MyHandler.handleMessage(IInputConnectionWrapper.java:77)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4921)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1038)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:805)
	at dalvik.system.NativeStart.main(Native Method)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.InstantiationException%3A+can%27%27t+instantiate+class+char+at+java.lang.Class.newInstanceImpl%28Native+Method%29

Comment 1

[Chris Peterson [:cpeterson]]

jchen: do you know why this bug only seems to affect Firefox 20? The "can't instantiate class char" exception is quite strange.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Chris Peterson (:cpeterson) from comment #1)
> jchen: do you know why this bug only seems to affect Firefox 20? The "can't
> instantiate class char" exception is quite strange.

Hmm, looking at the docs, it does say you cannot instantiate primitive types. I think that code was introduced in 20 so only 20 is affected.

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Use default values for primitive types and fix setSelection for Samsung keyboards (v1)

This patch fixes the bug where we were trying to instantiate primitive types through Class.newInstance() which is not possible. Only char and int are encountered so only those two are checked.

This patch also fixes the underlying bug where the Samsung keyboard can call setSelection with offsets < 0. We simply ignore these calls, considering the default BaseInputConnection.setSelection implementation ignores calls with offsets that go past the length (but that imeplementation doesn't check for offsets < 0 for some reason)

Comment 4

[Chris Peterson [:cpeterson]]

Comment on attachment 748967 [details] [diff] [review]
Use default values for primitive types and fix setSelection for Samsung keyboards (v1)

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

LGTM. Should we uplift this fix to Aurora?

::: mobile/android/base/GeckoInputConnection.java
@@ +687,5 @@
> +    public boolean setSelection(int start, int end) {
> +        if (start < 0 || end < 0) {
> +            // Some keyboards (e.g. Samsung) can call setSelection with
> +            // negative offsets. In that case we ignore the call, similar to how
> +            // BaseInputConnection.setSelection ignores offsets that go past the length.

Alternatively, we could clamp negative offsets to 0. I worry that the VKB will get confused if the selection has not changed. That said, I think we should use your current approach because it's similar to BaseInputConnection.setSelection()'s error handling.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Yeah we should probably uplift this.

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 748967 [details] [diff] [review]
Use default values for primitive types and fix setSelection for Samsung keyboards (v1)

[Approval Request Comment]

Bug caused by (feature/regressing bug #): Bug 817386

User impact if declined: Occasional crash, especially on Samsung devices

Testing completed (on m-c, etc.): Locally

Risk to taking this patch (and alternatives if risky): Very little; patch only works around the crash

String or IDL/UUID changes made by this patch: None

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/92a9b801e5d0

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/92a9b801e5d0

Comment 9

[Alex Keybl [:akeybl]]

Comment on attachment 748967 [details] [diff] [review]
Use default values for primitive types and fix setSelection for Samsung keyboards (v1)

Appropriate for uplift, but no need to track.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1b4fe2e4a025