androidx.navigation:navigation-*:2.4.0-alpha01 crashes with ConcurrentModificationException [188860458]

Fixed

Bug

Status Update

No update yet.

Description

co...@protonmail.com

created issue #1

May 21, 2021 04:22AM

Component used: Navigation Version used: androidx.navigation:navigation-*:2.4.0-alpha01 Devices/Android versions reproduced on:

I don't have a repro project, but I do have a stacktrace:

2021-05-21 00:19:31.604 12663-12663/com.rollertoaster.app E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.rollertoaster.app, PID: 12663
    java.util.ConcurrentModificationException
        at java.util.ArrayList$Itr.next(ArrayList.java:860)
        at androidx.navigation.NavController.dispatchOnDestinationChanged(NavController.kt:703)
        at androidx.navigation.NavController.navigate(NavController.kt:1421)
        at androidx.navigation.NavController.navigate(NavController.kt:1200)
        at androidx.navigation.NavController.navigate(NavController.kt:1128)
        at androidx.navigation.ui.NavigationUI.onNavDestinationSelected(NavigationUI.kt:90)
        at androidx.navigation.ui.NavigationUI$setupWithNavController$3.onNavigationItemSelected(NavigationUI.kt:392)
        at com.google.android.material.navigation.NavigationView$1.onMenuItemSelected(NavigationView.java:217)
        at androidx.appcompat.view.menu.MenuBuilder.dispatchMenuItemSelected(MenuBuilder.java:834)
        at androidx.appcompat.view.menu.MenuItemImpl.invoke(MenuItemImpl.java:158)
        at androidx.appcompat.view.menu.MenuBuilder.performItemAction(MenuBuilder.java:985)
        at com.google.android.material.internal.NavigationMenuPresenter$1.onClick(NavigationMenuPresenter.java:416)
        at android.view.View.performClick(View.java:7448)
        at android.view.View.performClickInternal(View.java:7425)
        at android.view.View.access$3600(View.java:810)
        at android.view.View$PerformClick.run(View.java:28305)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:223)
        at android.app.ActivityThread.main(ActivityThread.java:7656)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

Comments

jb...@google.com <jb...@google.com> #2May 21, 2021 03:32PM

I'm not sure what you mean by onSizeChanged. Which size? Children of the scroller?

In this case, how do you expect onSizeChanged modifier behave differently from onPositioned other than performance?

il...@google.com <il...@google.com> #3May 21, 2021 04:25PM

Assigned to jb...@google.com.

Sorry, the scroller thing was just an example of when a onPositioned would be call a lot without the size changing.

In this case, how do you expect onSizeChanged modifier behave differently from onPositioned other than performance?

I imagine it would act similarly, but only for the size, and not position.

zo...@gmail.com <zo...@gmail.com> #4May 21, 2021 08:44PM

Oh, I see, thanks for explaining.

I imagine notification only about size change can have some performance wins against position + size notification..

Cc-ing Andrey and reassigning to George, they both have some insights I'm sure. It would be handy to have `Modifier.onSizeChanged`, especially it it will be faster

ap...@google.com <ap...@google.com> #5May 24, 2021 08:29PM

You can do this yourself with this:

fun Modifier.onSizeChanged(block: (size: IntPxSize) -> Unit): Modifier = object : LayoutModifier2 {
   private var previousSize: IntPxSize = PxSize.Zero

   override Density.modifySize(
     constraints: Constraints,
     layoutDirection: LayoutDirection,
     childSize: IntPxSize
   ) {
     if (childSize != previousSize) {
       previousSize = childSize
       block(childSize)
     }
     return childSize
   }
}

Would you like something that only cares about the content size or is this what you would like added to the library?

jb...@google.com <jb...@google.com> #6May 26, 2021 09:03PM

Marked as fixed.

Yep, that's exactly what I'd like in the library.

I can also see the benefit of having another modifier to know the content size within the parent too.

Issue 188860458