This bug was filed from the Socorro interface and is report bp-efc6c7a1-7b4e-49ed-a0b3-bdc3e2150210. ============================================================= java.lang.ClassCastException: java.util.HashSet cannot be cast to java.lang.String at android.app.SharedPreferencesImpl.getString(SharedPreferencesImpl.java:205) at org.mozilla.gecko.util.PrefUtils.getFromJSON(PrefUtils.java:48) at org.mozilla.gecko.util.PrefUtils.getStringSet(PrefUtils.java:30) at org.mozilla.gecko.home.RemoteTabsExpandableListState.getStringSet(RemoteTabsExpandableListState.java:71) at org.mozilla.gecko.home.RemoteTabsExpandableListState.<init>(RemoteTabsExpandableListState.java:57) at org.mozilla.gecko.home.RemoteTabsBaseFragment.onActivityCreated(RemoteTabsBaseFragment.java:97) at org.mozilla.gecko.home.RemoteTabsExpandableListFragment.onActivityCreated(RemoteTabsExpandableListFragment.java:134) at android.support.v4.app.Fragment.performActivityCreated(Unknown Source) at android.support.v4.app.FragmentManagerImpl.moveToState(Unknown Source) at android.support.v4.app.FragmentManagerImpl.moveToState(Unknown Source) at android.support.v4.app.BackStackRecord.run(Unknown Source) at android.support.v4.app.FragmentManagerImpl.execPendingActions(Unknown Source) at android.support.v4.app.FragmentManagerImpl$1.run(Unknown Source) at android.os.Handler.handleCallback(Handler.java:605) at android.os.Handler.dispatchMessage(Handler.java:92) at android.os.Looper.loop(Looper.java:137) at android.app.ActivityThread.main(ActivityThread.java:4448) 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:823) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:590) at dalvik.system.NativeStart.main(Native Method)

vivek and I have dug into this, and we conclude that an API 9-10 APK is running on a version 15 device. In general, this is "safe", but here it is not. What appears to have happened is that a StringSet has been written to SharedPrefs (with an API 11+ APK?) and then is read *as a String* (with an API 9-10 APK). That fails. We could work-around this by gracefully handling the error in PrefUtils, but I am loathe to do that because usually this is a legitimate coding error. Alternately, we could prefix or suffix depending on the API range automatically in PrefUtils, which would prevent this particular issue.

So, somehow we have a "downgrade" situation: written using 11+, then installed 9-10 and failed to read. Could the same thing happen in reverse? written using 9-10, then read using 11+? Would the code handle it? Also, Nick points out an issue: AppConstants.Version replaced Build.SDK_INT in our code, but the behavior is not the same. How do we guard against misuse? How do we serve two different behaviors?

(In reply to Mark Finkle (:mfinkle) from comment #2) > So, somehow we have a "downgrade" situation: written using 11+, then > installed 9-10 and failed to read. This should be as simple as the user having manually installed a v9 APK over the top of a pre-split APK, or an update having gone wrong. (That was tested, but who knows?) > Could the same thing happen in reverse? written using 9-10, then read using > 11+? Would the code handle it? It could happen, and legitimately so: OS updates. > Also, Nick points out an issue: AppConstants.Version replaced Build.SDK_INT > in our code, but the behavior is not the same. How do we guard against > misuse? How do we serve two different behaviors? The behavior is strictly the same if you're running the correct build for your OS. That's the idea: it's a build-time optimization based on what we know about the device we're running on. (HotSpot would do the same thing, given the opportunity. dalvik not so much.) The correct approach here is to make sure we have an escape hatch for people who end up in the wrong 'room', not to eliminate those optimizations.

> > Could the same thing happen in reverse? written using 9-10, then read using > > 11+? Would the code handle it? > > It could happen, and legitimately so: OS updates. N.B., this could happen regardless of splitapk, too -- it would have switched code paths at runtime after an OS update.

Nick - Was the code that tickles this added in Fx38 or not?

(In reply to Mark Finkle (:mfinkle) from comment #5) > Nick - Was the code that tickles this added in Fx38 or not? The code that tickles this was added in Fx 35, I believe [1], but the underlying issue -- that PrefUtils handles StringSets differently depending on the build-time value of Versions.preHC -- has existed for longer. (Probably since we added Versions.) [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1064304

(In reply to Nick Alexander :nalexander from comment #6) > (In reply to Mark Finkle (:mfinkle) from comment #5) > > Nick - Was the code that tickles this added in Fx38 or not? > > The code that tickles this was added in Fx 35, I believe [1], but the > underlying issue -- that PrefUtils handles StringSets differently depending > on the build-time value of Versions.preHC -- has existed for longer. > (Probably since we added Versions.) I saw RemoteTabsExpandableListState in the stack and thought it might be tickled by newer code, even though the footgun was lying in the shadows... waiting. Marking "+"

Closing because no crash reported since 12 weeks.

Closing because no crash reported since 12 weeks.