1077645 - Startup crash due to some Facebook-related intent extras

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

Google Play has some startup crashes that appear to be caused by Facebook putting broken stuff into the Intent bundle. I think this is the intent.getString("args") call going south. We should be able to just catch the exception there.

java.lang.RuntimeException: Unable to start activity ComponentInfo{org.mozilla.firefox_beta/org.mozilla.gecko.BrowserApp}: android.os.BadParcelableException: ClassNotFoundException when unmarshalling: com.facebook.events.common.ActionSource
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2585)
at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2641)
at android.app.ActivityThread.access$800(ActivityThread.java:156)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1355)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:157)
at android.app.ActivityThread.main(ActivityThread.java:5867)
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:858)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:674)
at dalvik.system.NativeStart.main(Native Method)
Caused by: android.os.BadParcelableException: ClassNotFoundException when unmarshalling: com.facebook.events.common.ActionSource
at android.os.Parcel.readParcelableCreator(Parcel.java:2156)
at android.os.Parcel.readParcelable(Parcel.java:2106)
at android.os.Parcel.readValue(Parcel.java:2022)
at android.os.Parcel.readArrayMapInternal(Parcel.java:2323)
at android.os.Bundle.unparcel(Bundle.java:249)
at android.os.Bundle.getString(Bundle.java:1118)
at android.content.Intent.getStringExtra(Intent.java:4979)
at org.mozilla.gecko.BrowserApp.onCreate(BrowserApp.java:468)
at android.app.Activity.performCreate(Activity.java:5312)
at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1111)
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2541)

Comment 1

[Richard Newman [:rnewman]]

Mmm, malformed.

Comment 2

[Richard Newman [:rnewman]]

Attachment: Be paranoid when parsing external intent extras. v1

This should do the trick.

(Fixed a few things along the way, too.)

Comment 3

[Richard Newman [:rnewman]]

Oh, and should there be any doubt: yeah, this is horrible, and I hate that we have to do this.

Comment 4

[Richard Newman [:rnewman]]

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

Comment 5

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

Just adding this link to the Chrome issue for posterity:
https://code.google.com/p/chromium/issues/detail?id=412527

Comment 6

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

https://hg.mozilla.org/mozilla-central/rev/339a9e5c9303

Comment 7

[Richard Newman [:rnewman]]

We should uplift this and any fix we add for Bug 1079876.

Comment 8

[Richard Newman [:rnewman]]

Comment on attachment 8499899 [details] [diff] [review]
Be paranoid when parsing external intent extras. v1

Approval Request Comment
[Feature/regressing bug #]:
  Facebook broke us. It's not our fault.

[User impact if declined]:
  Crashes when opening links from affected Facebook versions.

[Describe test coverage new/current, TBPL]:
  No test coverage.

[Risks and why]: 
  Very low risk: this just lifts a direct call to something that can throw into a static method with try..catch. We obviously can't parse data out of the intent, but at least we don't crash.
 
[String/UUID change made/needed]:
  None.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

It would have been nice to have the uplift request 5 days ago. We already built fennec 33.0.

Mark, do you think this deserves a respin? The impact to user is a bit unclear to me.

Comment 10

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

I don't think this deserves a respin. The crash numbers from Google Play are noticeable, but not too high. We should keep an eye on the trends though.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8499899 [details] [diff] [review]
Be paranoid when parsing external intent extras. v1

Thanks. Please ping us if the situation changes!

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for Aurora uplift.

Comment 13

[Aaron Train [:aaronmt]]

Taking this on 33.0 (build #2)?

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8499899 [details] [diff] [review]
Be paranoid when parsing external intent extras. v1

We are going to make a build #2 for fennec & desktop, we can take them.

Comment 15

[Richard Newman [:rnewman]]

https://hg.mozilla.org/mozilla-aurora/rev/c89f8a0cf6ee

Comment 16

[Richard Newman [:rnewman]]

https://hg.mozilla.org/releases/mozilla-release/rev/628f8f6c6f72

Comment 17

[u279076]

How do I check Google Play crashes to confirm this has gone away?

Comment 18

[Aaron Train [:aaronmt]]

One would need dashboard access, but I think this was categorized under the signature on the dashboard:

java.lang.ClassNotFoundException
in dalvik.system.BaseDexClassLoader.findClass

Which has been further captured in bug 1090361

Comment 19

[u279076]

(In reply to Aaron Train [:aaronmt] from comment #18)
> One would need dashboard access

Given that I'm the QA lead for Fennec 35 I should probably have access, no? Who can I ask about this?

Comment 20

[Robert Kaiser]

Are there any bugs filed and/or investigations done for catching those issues in our own crash reporter so we do not have to rely on the few people that can access that dashboard?

Comment 21

[Richard Newman [:rnewman]]

Bug 1043457, amongst other things.