1061032 - crash in java.lang.NullPointerException: at org.mozilla.gecko.ChromeCast$2.onConnected(ChromeCast.java)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Screencasting, defect)

Description

[Aaron Train [:aaronmt]]

This bug was filed from the Socorro interface and is 
report bp-3b539201-693c-4048-9780-c56032140830.
=============================================================

java.lang.NullPointerException
	at org.mozilla.gecko.ChromeCast$2.onConnected(ChromeCast.java:196)
	at com.google.android.gms.internal.hd.c(Unknown Source)
	at com.google.android.gms.common.api.c.eF(Unknown Source)
	at com.google.android.gms.common.api.c.d(Unknown Source)
	at com.google.android.gms.common.api.c$2.onConnected(Unknown Source)
	at com.google.android.gms.internal.hd.c(Unknown Source)
	at com.google.android.gms.internal.hd.ck(Unknown Source)
	at com.google.android.gms.internal.hc$h.b(Unknown Source)
	at com.google.android.gms.internal.hc$h.d(Unknown Source)
	at com.google.android.gms.internal.hc$b.fq(Unknown Source)
	at com.google.android.gms.internal.hc$a.handleMessage(Unknown Source)
	at android.os.Handler.dispatchMessage(Handler.java:102)
	at android.os.Looper.loop(Looper.java:146)
	at android.app.ActivityThread.main(ActivityThread.java:5487)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:515)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1283)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1099)
	at dalvik.system.NativeStart.main(Native Method)

Comment 1

[Wesley Johnston (:wesj)]

Do we still see this?

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

Yes, 1 crash each on B6 and B7.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: chromecast3

Comment 4

[Wesley Johnston (:wesj)]

Attachment: Patch

At first glance this looks impossible. I have to guess that sometime between when we call api.connect() and the onConnected callback is called, someone calls stop? That's the only place we set the apiClient to null (but note, stop has an apiClient.isConnected() call in it that would have to pass first). Maybe something reconnects automatically for us at some point after stop fires and that causes this to refire? I

Comment 5

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/fx-team/rev/a259ef6f66d5

Comment 6

[Wesley Johnston (:wesj)]

Comment on attachment 8499045 [details] [diff] [review]
Patch

I realize that I missed the last beta here (I think?). Noming anyway because this is tracking. If we don't care about beta, someone should decide and remove it.

Approval Request Comment
[Feature/regressing bug #]: Chromecast stuff
[User impact if declined]: Possible crash somehow? We don't have str.
[Describe test coverage new/current, TBPL]: None. Can't test Chromecast stuff very well
[Risks and why]: Low risk. Just a null check
[String/UUID change made/needed]: none.

Comment 7

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8499045 [details] [diff] [review]
Patch

It's too late for 33 as we have already built the release candidate. Let's take the fix in 34 once it's landed on m-c.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8499045 [details] [diff] [review]
Patch

Sadly, we've been burned when adding a simple NPE check in Fennec before. We're early enough in the cycle that I think we can take this check. Do we have chromecasts that we can use to actually test this case?

Aurora+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/057a88b514d5

I have a Chromecast if you need any extra help with testing.

Comment 11

[Wesley Johnston (:wesj)]

We have plenty. We just don't have any STR for these bugs.

Comment 12

[u279076]

Calling this verified fixed since there are 0 crash reports in Socorro for this in 34/35 over the last week. Additionally, I'm marking this qe-verify- since we don't have STR for QE to verify.