java.lang.NullPointerException: at org.mozilla.gecko.GeckoInputConnection$2.run(GeckoInputConnection.java)

RESOLVED FIXED in Firefox 17

Status

()

Firefox for Android
General
P1
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Scoobidiver (away), Assigned: jchen)

Tracking

({crash, regression, topcrash})

17 Branch
Firefox 19
ARM
Android
crash, regression, topcrash
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox17+ fixed, firefox18+ fixed, fennec17+)

Details

(Whiteboard: [native-crash], crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
There's one crash in 18.0a1/20120927114117: bp-f1634d1c-0f3b-4b17-b754-c3a5d2120928.

java.lang.NullPointerException
	at org.mozilla.gecko.GeckoInputConnection$2.run(GeckoInputConnection.java:514)
	at android.os.Handler.handleCallback(Handler.java:587)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:130)
	at android.app.ActivityThread.main(ActivityThread.java:3683)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:507)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:839)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:597)
	at dalvik.system.NativeStart.main(Native Method)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.GeckoInputConnection%242.run%28GeckoInputConnection.java%29
(Reporter)

Comment 1

5 years ago
It was uplifted to Aurora. The regression ranges might be:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ca4af4af5334&tochange=b038e9e2023f
http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=846561f033db&tochange=a4880b03634f
It might be a regression from bug 793943, bug 792677, bug 793935, bug 793354, bug 788589
Keywords: regression
Version: Trunk → Firefox 17
(Reporter)

Comment 2

5 years ago
It's #4 top crasher in 17.0b1.
tracking-firefox17: --- → ?
Keywords: topcrash
(Reporter)

Updated

5 years ago
tracking-fennec: --- → ?
This is our #4 topcrash on Beta 17.

This looks like fallout from bug 788600 (which backed out bug 769520). GeckoInputConnection thinks it hasCompositionString(), but then getComposingSpan() returns null and we crash.
Priority: -- → P1
(Reporter)

Comment 4

5 years ago
(In reply to Chris Peterson (:cpeterson) from comment #3)
> This looks like fallout from bug 788600.
That bug landed in 15.0.1 while this bug appeared in 17.0.
Assigning to Chris to investigate, this is a topcrasher so we'll definitely track for 17.
Assignee: nobody → cpeterson
tracking-firefox17: ? → +
jchen, do you want to take a look at this topcrash bug? I think I have a band-aid fix for this crash, but I don't have time to test it and uplift it for Beta 17 because I'm supposed to be working on B2G/Gaia bugs.
(Assignee)

Comment 7

5 years ago
I can take it. Are there STR? I see that the keyboard has to be fullscreen but otherwise I'm not sure how to trigger it.
Status: NEW → ASSIGNED
(Assignee)

Comment 8

5 years ago
Created attachment 672391 [details] [diff] [review]
Patch to avoid crash scenario

It is better to check for null composing span in this case
Attachment #672391 - Flags: review?(blassey.bugs)
(Assignee)

Updated

5 years ago
Assignee: cpeterson → nchen
Comment on attachment 672391 [details] [diff] [review]
Patch to avoid crash scenario

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

::: mobile/android/base/GeckoInputConnection.java
@@ +509,5 @@
>                  if (imm != null && imm.isFullscreenMode()) {
>                      int newStart;
>                      int newEnd;
> +                    Span span = getComposingSpan();
> +                    if (span != null) {

should this be:
if (span != null && hasCompositionString()) {
Attachment #672391 - Flags: review?(blassey.bugs) → review+
(Assignee)

Comment 10

5 years ago
(In reply to Brad Lassey [:blassey] from comment #9)
> Comment on attachment 672391 [details] [diff] [review]
> Patch to avoid crash scenario
> 
> Review of attachment 672391 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/GeckoInputConnection.java
> @@ +509,5 @@
> >                  if (imm != null && imm.isFullscreenMode()) {
> >                      int newStart;
> >                      int newEnd;
> > +                    Span span = getComposingSpan();
> > +                    if (span != null) {
> 
> should this be:
> if (span != null && hasCompositionString()) {

I checked with cpeterson and he said it's redundant to check both getComposingSpan and hasCompositionString.
(In reply to Jim Chen [:jchen :nchen] from comment #10)
> I checked with cpeterson and he said it's redundant to check both
> getComposingSpan and hasCompositionString.

They *ought* to be redundant, but I am not confident they are. getComposingSpan queries the IME and hasCompositionString just tracks composition state from events.

I think we should check both span != null && hasCompositionString() until we can test that they are redundant (and then remove hasCompositionString).
(Assignee)

Comment 12

5 years ago
Created attachment 672759 [details] [diff] [review]
Patch to avoid crash scenario (v2)

Okay added the hasCompositionString check. Carried over r+. Thanks, Brad and Chris!
Attachment #672391 - Attachment is obsolete: true
Attachment #672759 - Flags: review+
jchen, this is a topcrash for Beta 17, so we will want your fix uplifted to Aurora and Beta.
status-firefox17: --- → affected
status-firefox18: --- → affected
status-firefox19: --- → affected
(Assignee)

Comment 14

5 years ago
Please check into m-c for now.
Keywords: checkin-needed
tracking-fennec: ? → 17+
https://hg.mozilla.org/integration/mozilla-inbound/rev/977d17351049
Flags: in-testsuite-
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/977d17351049
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
(Reporter)

Updated

5 years ago
status-firefox19: affected → ---
Can we get an uplift nomination here?  Having this in the next Beta (ie: land by tomorrow) would let us gather more data and see the crash volume for this signature hopefully decrease.
tracking-firefox18: --- → +
(Assignee)

Comment 18

5 years ago
Comment on attachment 672759 [details] [diff] [review]
Patch to avoid crash scenario (v2)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): race condition; missing null check
User impact if declined: topcrash for current Beta
Testing completed (on m-c, etc.): none, because there is no definite STR for the crash. can only be tested by looking at crashstats
Risk to taking this patch (and alternatives if risky): very small risk. changes in patch only applies to cases where we used to crash
String or UUID changes made by this patch: none
Attachment #672759 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 19

5 years ago
Comment on attachment 672759 [details] [diff] [review]
Patch to avoid crash scenario (v2)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): race condition; missing null check
User impact if declined: topcrash for current Beta
Testing completed (on m-c, etc.): none, because there is no definite STR for the crash. can only be tested by looking at crashstats
Risk to taking this patch (and alternatives if risky): very small risk. changes in patch only apply to cases where we used to crash
String or UUID changes made by this patch: none
Attachment #672759 - Flags: approval-mozilla-beta?
Attachment #672759 - Flags: approval-mozilla-beta?
Attachment #672759 - Flags: approval-mozilla-beta+
Attachment #672759 - Flags: approval-mozilla-aurora?
Attachment #672759 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 20

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/f67f7ed3d53a

https://hg.mozilla.org/releases/mozilla-beta/rev/9576d3860ac3
(Reporter)

Updated

5 years ago
status-firefox17: affected → fixed
status-firefox18: affected → fixed
You need to log in before you can comment on or make changes to this bug.