material-components / material-components-android

Modular and customizable Material Design UI components for Android
Apache License 2.0
16.38k stars 3.07k forks source link

Received some reports java.lang.IllegalArgumentException: Illegal state argument: 5 #925

Closed CristianMG closed 4 years ago

CristianMG commented 4 years ago

We have received some reports about this stacktrace.

Fatal Exception: java.lang.IllegalArgumentException: Illegal state argument: 5
       at com.google.android.material.bottomsheet.BottomSheetBehavior.startSettlingAnimation + 755(BottomSheetBehavior.java:755)
       at com.google.android.material.bottomsheet.BottomSheetBehavior$1.run + 646(BottomSheetBehavior.java:646)
       at android.os.Handler.handleCallback + 739(Handler.java:739)
       at android.os.Handler.dispatchMessage + 95(Handler.java:95)
       at android.os.Looper.loop + 135(Looper.java:135)
       at android.app.ActivityThread.main + 5376(ActivityThread.java:5376)
       at java.lang.reflect.Method.invoke(Method.java)
       at java.lang.reflect.Method.invoke + 372(Method.java:372)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run + 908(ZygoteInit.java:908)
       at com.android.internal.os.ZygoteInit.main + 703(ZygoteInit.java:703)

I have read and analyze the problem it seems as the view is not hideable, but it isn't true, the view is hideable as you can see in the next code.

This is the function use for hide view.

@Override
    public void hidePlayer() {
        if (bottomSheet.getState() != BottomSheetBehavior.STATE_HIDDEN) {
            mContentFrame.setPadding(0, 0, 0, 0);
            bottomSheetLayout.setVisibility(View.GONE);
            bottomSheetLayout.post(() -> {
                try {
                    bottomSheet.setHideable(true);
                    bottomSheet.setState(BottomSheetBehavior.STATE_HIDDEN);
                    bottomSheet.setHideable(false);
                } catch (Exception e) {
                    bottomSheet.setHideable(false);
                    Timber.e(e, "Workaround fabric random crash");
                }
            });
            userPreferences.setMiniplayerHidden(true);
        }
    }

Devices report by version: Android 9: 33% Android 8: 22% Android 7: 16% android 6: 12%

Devices report by brands: Samsung: 38% Huawei: 25% Xiaomi 22% Motorola: 7%

Version library: com.google.android.material:material:{strictly 1.0.0} -> 1.0.0 (c)

pekingme commented 4 years ago

Hi @CristianMG, thanks for sharing the data with us. Could you clarify what the percentage means? For Android 9 as an example, are there 33% devices will experience this issue for sure, or overall 33% chance to hide the bottom sheet? Could you help me to reproduce this issue with a demo project? Thanks.

CristianMG commented 4 years ago

Hi, this percentage is about all crashes, how many are by each android's platform, the total did not too much.

I found the problem in our platform, The status update in library is in different thread, we don't want the user be able to dismiss the "BottomSheet", but we want to dismiss the "BottomSheet" by means of an icon close. This force us to change "setHideable" to each state change. This cause race condition between threads when the thread go to check if "BottomSheet" is hideable it isn't, then throw the "IllegalStateException".

The solution is easy, block "setHideable false" when received the state change by listener.

           @Override
            public void onStateChanged(@NonNull View bottomSheet, int newState) {
                /**
                 * It's safer to avoid randomly crashes about BottomSheetState
                 */
                if ((newState == BottomSheetBehavior.STATE_EXPANDED
                        || newState == BottomSheetBehavior.STATE_COLLAPSED)
                        || newState == BottomSheetBehavior.STATE_HALF_EXPANDED) {
                    bottomSheetBehavior.setHideable(false);
                }
            }

At the moment we don't receive none crash with this code. I hope this help someone.

Thank you

znakeeye commented 2 years ago

The reported issue could happen if you store a state to be set some time later. E.g. when saving/restoring layout.

Derived BottomSheetBehavior

if (!isHideable() && mPendingState == STATE_HIDDEN) {
   mPendingState = STATE_COLLAPSED;
}

super.setState(mPendingState);

The above fixed the issue for me, but I really think setState() should check hideable and transform STATE_HIDDEN to STATE_COLLAPSED. Kind of makes sense.