consp1racy / android-support-preference

Android Preferences according to Material design specs
Apache License 2.0
331 stars 49 forks source link

java.lang.IllegalArgumentException: Wrong state class -- expecting Preference State #101

Closed dirkam closed 6 years ago

dirkam commented 6 years ago

This might not be related to the library at all, but maybe you might have a clue on what's going on here.

Recently updated to support-preference 2.1.2 and support library 27.1.1 and started to get these crashes.

Caused by java.lang.IllegalArgumentException: Wrong state class -- expecting Preference State
       at android.support.v7.preference.Preference.onRestoreInstanceState(Preference.java:2014)
       at android.support.v7.preference.PreferenceGroup.onRestoreInstanceState(PreferenceGroup.java:457)
       at android.support.v7.preference.Preference.dispatchRestoreInstanceState(Preference.java:1992)
       at android.support.v7.preference.PreferenceGroup.dispatchRestoreInstanceState(PreferenceGroup.java:434)
       at android.support.v7.preference.PreferenceGroup.dispatchRestoreInstanceState(PreferenceGroup.java:439)
       at android.support.v7.preference.PreferenceGroup.dispatchRestoreInstanceState(PreferenceGroup.java:439)
       at android.support.v7.preference.PreferenceFragmentCompat.android.support.v7.preference.Preference.restoreHierarchyState(PreferenceFragmentCompat.java:7974)
       at android.support.v4.app.Fragment.performActivityCreated(Fragment.java:2355)
       at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1451)
       at android.support.v4.app.FragmentManagerImpl.moveFragmentToExpectedState(FragmentManager.java:1759)
       at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1827)
       at android.support.v4.app.FragmentManagerImpl.dispatchStateChange(FragmentManager.java:3244)
       at android.support.v4.app.FragmentManagerImpl.dispatchActivityCreated(FragmentManager.java:3200)
       at android.support.v4.app.FragmentActivity.android.support.v4.app.FragmentController.dispatchActivityCreated(FragmentActivity.java:11195)
       at android.support.v7.app.AppCompatActivity.onStart(AppCompatActivity.java:177)
       at android.app.Instrumentation.callActivityOnStart(Instrumentation.java:1266)
       at android.app.Activity.performStart(Activity.java:6923)
       at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3216)
       at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3349)
       at android.app.ActivityThread.handleRelaunchActivity(ActivityThread.java:5386)
       at android.app.ActivityThread.access$1200(ActivityThread.java:223)
       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1800)
       at android.os.Handler.dispatchMessage(Handler.java:102)
       at android.os.Looper.loop(Looper.java:148)
       at android.app.ActivityThread.main(ActivityThread.java:7223)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1230)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1120)

image

Your help is highly appreciated.

consp1racy commented 6 years ago

Do you have custom preferences that save state?

1)

If you overridePreference#onSaveInstanceState you have to override Preference#onRestoreInstanceState.

2)

You have to maintain the hierarchy of saved state. Your custom saved state must extend Preference.BaseSavedState.

Example from TwoStatePreference:

@Override
protected Parcelable onSaveInstanceState() {
    final Parcelable superState = super.onSaveInstanceState();
    if (isPersistent()) {
        // No need to save instance state since it's persistent
        return superState;
    }

    final TwoStatePreference.SavedState myState = new TwoStatePreference.SavedState(superState);
    myState.checked = isChecked();
    return myState;
}

@Override
protected void onRestoreInstanceState(Parcelable state) {
    if (state == null || !state.getClass().equals(TwoStatePreference.SavedState.class)) {
        // Didn't save state for us in onSaveInstanceState
        super.onRestoreInstanceState(state);
        return;
    }

    SavedState myState = (TwoStatePreference.SavedState) state;
    super.onRestoreInstanceState(myState.getSuperState());
    setChecked(myState.checked);
}

static class SavedState extends BaseSavedState {
    boolean checked;

    public SavedState(Parcel source) {
        super(source);
        checked = source.readInt() == 1;
    }

    @Override
    public void writeToParcel(@NonNull Parcel dest, int flags) {
        super.writeToParcel(dest, flags);
        dest.writeInt(checked ? 1 : 0);
    }

    public SavedState(Parcelable superState) {
        super(superState);
    }

    public static final Parcelable.Creator<SavedState> CREATOR =
        new Parcelable.Creator<SavedState>() {
            public SavedState createFromParcel(Parcel in) {
                return new SavedState(in);
            }

            public SavedState[] newArray(int size) {
                return new SavedState[size];
            }
        };
}

Here's what happens in Preference, the base class for all preferences:

protected Parcelable onSaveInstanceState() {
    mBaseMethodCalled = true;
    return BaseSavedState.EMPTY_STATE;
}

protected void onRestoreInstanceState(Parcelable state) {
    mBaseMethodCalled = true;
    if (state != BaseSavedState.EMPTY_STATE && state != null) {
        throw new IllegalArgumentException("Wrong state class -- expecting Preference State");
    }
}

If you send wrong SavedState to super.onRestoreInstanceState it will crash. Each child class has its own saved state and you need to pass correct state to super class.

3)

Upgrade to Android Gradle Plugin 3.1.x or disable D8. The version of D8 bundled with AGP before 3.1.0 final is known to produce ~broken~ incompatible bytecode with some devices. It may or may not affect this case.

consp1racy commented 6 years ago

MultiSelectListPreference is missing onRestoreInstanceState. I'll fix that.

dirkam commented 6 years ago

I was just typing an answer to these, thanks a lot for taking a look into this.

I'm wondering though how come that it only crashes with the latest versions of these libs? I've been using MultiSelectListPreferences for quite some time.

consp1racy commented 6 years ago

I'm wondering though how come that it only crashes with the latest versions of these libs?

You and me both, the code exempt from Preference class looks exactly the same in 27.1.1 as in 27.1.0.

dirkam commented 6 years ago

Yes, I went back to check even earlier versions before creating the issue, and it's the same code.

consp1racy commented 6 years ago

I see that the original support library MultiSelectListPreference doesn't have onRestoreInstanceState.

dirkam commented 6 years ago

What does that mean? Can it be that the crash is caused by something else?

consp1racy commented 6 years ago

It just means that it would crash even if you weren't using my library. Still doesn't explain why it started crashing now.

consp1racy commented 6 years ago

Fix released in 2.2.0.

dirkam commented 6 years ago

You are awesome, thanks!

consp1racy commented 6 years ago

Finally figured out when this happens. The crash only occurs when you try to save & restore a MultiSelectListPreference with android:persistent="false".

But as mentioned, it's fixed now. Thanks for the report!

dirkam commented 6 years ago

Unfortunately, it still crashes with 2.2.0 (support lib 27.1.1).

consp1racy commented 6 years ago

I wasn't able to pinpoint when it happens, at least now I know who's responsible.

There's a new class in preference-v7 called CollapsiblePreferenceGroupController. In PreferenceGroups it hooks into instance save/restoration mechanism. The PreferenceGroup from error log saves state using this controller but it doesn't use it to restore, because it's null at the time. Then it falls through to Preference which expects an empty state and it crashes.

Until I figure out what's exactly wrong, I'll ask you to use preference-v7 version 27.0.2. No other support libraries depend on preference-v7 so changing version only on that library should be enough. If it doesn't work add this at the end of your module build file:

        configurations.all {
            resolutionStrategy.eachDependency { DependencyResolveDetails details ->
                if (details.requested.group == 'com.android.support' && details.requested.name == 'preference-v7') {
                    details.useVersion "27.0.2"
                }
            }
        }
consp1racy commented 6 years ago

Are you dynamically adding/removing Preferences outside of onCreatePreferences?

Outside of onCreatePreferences you can only use

And root PreferenceScreen can only be changed by above mentioned methods 🤔

CollapsiblePreferenceGroupController only ever affects root PreferenceScreen that has a key.

State restoration happens in onActivityCreated. That happens after adapter creation in onViewCreated. I don't see how this could happen.

Are you doing anything highly custom? Can you share your Settings fragment class?

dirkam commented 6 years ago

I don't think I'm doing anything highly custom, but I do add/remove preferences runtime.

I do remove Preferences in onCreatePreferences2. E.g. if we are on a certain platform version where a certain thing is not available I remove the relevant preferences.

I do use addPreference runtime as well. I have a search function where the user can search for settings and I add the result (preferences and preferencescreens/categories) to a certain preferencescreen (which by default is empty and I hide when settings screen is initially loaded). This happens when user click on the search icon in the toolbar, so it is outside of onCreatePreferences2. This search result preferencescreen is inside the root preferencescreen in a preferencecategory.

The root preferencescreen has a key.

I think that it is not related (or not directly) to the search thing. It seems that it crashes somewhere during onCreatePreferences2. In the logs along with the crash report, I only see some basic stuff that is logged when the settings screen is opened.

consp1racy commented 6 years ago

Honestly, it would be better if you somehow managed to put together a reproducible sample; this is 99% a bug in preference-v7. Describe the issue as best you can here https://issuetracker.google.com/issues and post a link to the issue report here. Thank you

While I don't have a fix adding logging to your settings fragment lifecycle methods may help me figuring out what's wrong. Log names of these methods when they're entered:

Next time a crash occurs, post the logged messages here. I'm looking for setting PreferenceScreen after saving state or any other discrepancy.

dirkam commented 6 years ago

Will do that, at least the logging part and see what it shows. I'll update this thread if there is any new information on this.

dirkam commented 6 years ago

Still trying to get more info on this, but the crash happens in onActivityCreated

dirkam commented 6 years ago

Got this randomly when working on the app. Still can't reproduce it.

Caused by: java.lang.IllegalArgumentException: Wrong state class -- expecting Preference State
        at android.support.v7.preference.Preference.onRestoreInstanceState(Preference.java:2014)
        at android.support.v7.preference.PreferenceGroup.onRestoreInstanceState(PreferenceGroup.java:457)
        at android.support.v7.preference.Preference.dispatchRestoreInstanceState(Preference.java:1992)
        at android.support.v7.preference.PreferenceGroup.dispatchRestoreInstanceState(PreferenceGroup.java:434)
        at android.support.v7.preference.PreferenceGroup.dispatchRestoreInstanceState(PreferenceGroup.java:439)
        at android.support.v7.preference.PreferenceGroup.dispatchRestoreInstanceState(PreferenceGroup.java:439)
        at android.support.v7.preference.Preference.restoreHierarchyState(Preference.java:1974)
        at android.support.v7.preference.PreferenceFragmentCompat.onActivityCreated(PreferenceFragmentCompat.java:347)
        at com.my.app.SettingsFragment.onActivityCreated(SettingsFragment.java:)
        at android.support.v4.app.Fragment.performActivityCreated(Fragment.java:2355)
        at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1451)
        at android.support.v4.app.FragmentManagerImpl.moveFragmentToExpectedState(FragmentManager.java:1759)
        at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1827)
        at android.support.v4.app.FragmentManagerImpl.dispatchStateChange(FragmentManager.java:3244)
        at android.support.v4.app.FragmentManagerImpl.dispatchActivityCreated(FragmentManager.java:3200)
        at android.support.v4.app.FragmentController.dispatchActivityCreated(FragmentController.java:195)
        at android.support.v4.app.FragmentActivity.onStart(FragmentActivity.java:597)
        at android.support.v7.app.AppCompatActivity.onStart(AppCompatActivity.java:177)
        at android.app.Instrumentation.callActivityOnStart(Instrumentation.java:1334)
        at android.app.Activity.performStart(Activity.java:7019)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2741)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2856) 
        at android.app.ActivityThread.-wrap11(Unknown Source:0) 
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1589) 
        at android.os.Handler.dispatchMessage(Handler.java:106) 
        at android.os.Looper.loop(Looper.java:164) 
        at android.app.ActivityThread.main(ActivityThread.java:6494) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:438) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:807) 

Would it be a workaround to remove android:preferences from savedInstanceState in onActivityCreated? Not sure what this could break.

dirkam commented 6 years ago

In the lib RingtonePreference is missing onRestoreInstanceState as well, can it be related?

dirkam commented 6 years ago

Managed to catch another crash (happens rarely and seemingly randomly). Printed the whole android:preferences Bundle from savedInstanceState. Diff found: w/o crash: myPreferenceScreenKey in android:preferences: android.view.AbsSavedState$1@a02544f w/ crash: myPreferenceScreenKey in android:preferences: android.support.v7.preference.CollapsiblePreferenceGroupController$SavedState@f9562f3

myPreferenceScreenKey is not the root.

consp1racy commented 6 years ago

In the lib RingtonePreference is missing onRestoreInstanceState as well, can it be related?

There's no onSaveInstanceState there as well, that's ok.

Nice catch! If I understand this correctly, you're taking a PreferenceScreen that used to be root and attaching it somewhere else later?

dirkam commented 6 years ago

No, I don't do anything with it. I was just stating it because the root always appears in the bundle like this: root_key in android:preferences: android.support.v7.preference.CollapsiblePreferenceGroupController$SavedState@cf9d8

This is how I reproduce this:

  1. Open SettingsActivity. Don't do anything.
  2. Rotate the device many times
  3. Randomly it crashes (it can take a loooong time to get a crash)
dirkam commented 6 years ago

Do you happen to have any workaround in mind (besides reverting to older versions of the support lib)? What if I just remove it if it's CollapsiblePreferenceGroupController$SavedState? Seemingly it's ok, but not sure what this could break.

consp1racy commented 6 years ago

What's myPerferenceScreenKey?

It can only get CollapsiblePreferenceGroupController$SavedState if at some point it has been the root preference screen. Then it can only crash if at the point of restoration it's no longer the root.

Remember the pairing is done by key so if you happen to have multiple preferences with the same key you'll run into problems.


You don't have to revert the whole support library, just preference-v7 should be fine. Use the snippet I mentioned earlier. The only thing you'll lose are collapsible preference groups which is an undocumented feature introduced in 27.1.0. I didn't even know it was there until your bug report. So you're probably not using it anyway.

What if I just remove it if it's CollapsiblePreferenceGroupController$SavedState? Seemingly it's ok, but not sure what this could break.

I'm not sure either. I think using older preference-v7 is a safer bet, at least for now.

dirkam commented 6 years ago

Finally figured it out, your comments helped a lot.

I was still using PreferenceScreenNavigationStrategy.ReplaceRoot and navigating between preference screens caused the issue.

Opening Settings screen - navigating forward to a subscreen - rotating the device: this worked fine. Opening Settings screen - navigating forward to a subscreen - navigating backward to the parent (using up or back button) - rotating the device: crash.

(I'd like to note that my implementation of ReplaceRoot navigation strategy might not have been correct, though it's been working fine for years)

For now, I'll revert back to older version of the support lib for a quick fix and I'll migrate to ReplaceFragment navigation strategy.

I'll close this issue.

consp1racy commented 6 years ago

I'm glad you solved it.

Yes, please use ReplaceFragment, I actually removed ReplaceRoot from the library a week ago — it will be gone in next release.

The bundled implementation of ReplaceRoot was incorrect, it didn't account for the case described in my last comment.

dirkam commented 6 years ago

One question though, if you don't mind. For the search functionality I used a PreferenceScreen (child of the root in preferences xml), which was hidden by default. When the user clicked on the search icon in the toolbar I showed it and called mPreferenceScreenNavigation.onPreferenceScreenClick(preferenceScreenSearch) and was adding the results to this preference screen. This way I could get to the search screen no matter where and how deep I was in the preferences tree.

How could I achieve this with ReplaceFragment?

michaeldiener commented 6 years ago

So it seems the bug is in version 27.1.0 and 27.1.1 of the Android support library. You guys seem to have quite some info about it - may I suggest you report this bug with Google? https://issuetracker.google.com/issues/new?component=192731&template=842428

Regarding reproduce-ability, it helps to enable "Do not keep activities" in the device's developer settings.

consp1racy commented 6 years ago

@michaeldiener Thanks for the tip. Absolutely, now that @dirkam was able to reproduce it, I'll put together a sample project and report it.

consp1racy commented 6 years ago

@dirkam Can you upload a video of what you currently have so I can better understand how it works now? Thanks!

dirkam commented 6 years ago

Basically, I want to add a search functionality similar to the one in the stock Android Settings app. However, I managed to find a way to do this with ReplaceFragment navigation strategy as well, so everything seems to be fine on my end.

consp1racy commented 6 years ago

You can follow the issue here: https://issuetracker.google.com/issues/78921544