Repository navigation
Conversation
upFrom(DESTROYED) is null, so a lifecycle event after destroy throws "no event up from DESTROYED". Returning inside moveUp() would spin the state loop, so moveToLifecycleState returns before that loop.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
I've signed the CLA now. |
kikoso
left a comment
There was a problem hiding this comment.
I've signed the CLA now.
Thanks, @spsaucier ! The CLA check is still failing, but it is not about your signature: the fix commit (b594cc3e) is authored as @saucier-consulting with a different email than the one on your CLA, and the bot checks every commit author. Could you either amend it with the email covered by your CLA (git commit --amend --reset-author, then force-push, which also drops my empty commit), or add that email and account to your CLA and use the rescan link in the check?
The fix itself looks good to me, so LGTM once the CLA is green. Thanks for such a detailed issue, it made this very easy to review!
| verify(exactly = 0) { mapView.onDestroy() } | ||
| } | ||
|
|
||
| private fun setLifecycleState(observer: MapLifecycleEventObserver, state: Lifecycle.State) { |
There was a problem hiding this comment.
nit: setting currentLifecycleState through reflection couples the test to the private field name. We could reach the same states through the public API instead, e.g. onStateChanged(owner, Lifecycle.Event.ON_RESUME) to get to RESUMED and moveToDestroyedState() to get to DESTROYED. Not a blocker though.
There was a problem hiding this comment.
Dropped the reflection in e29f470. Both tests now reach RESUMED with onStateChanged(owner, ON_RESUME) and DESTROYED with moveToDestroyedState(). Events after that still do not add MapView calls.
Reflection on currentLifecycleState coupled the test to a private field. ON_RESUME and moveToDestroyedState land on the same states.
Fixes #1054
MapLifecycleEventObserver.moveToLifecycleStatewalks aMapViewwithLifecycle.Event.upFrom.DESTROYEDis the minimum state, soupFrom(DESTROYED)is null andmoveUp()throwsIllegalStateException: no event up from DESTROYED.MapView.onRelease()on main already detaches the observer beforemoveToDestroyedState(), which covers Compose disposal. The throw remains when the observer is still registered and a lifecycle event arrives after destroy. Released tags through 9.0.1 only remove the observer fromonViewDetachedFromWindow. Returning insidemoveUp()would not fix this:while (currentLifecycleState != targetState)would never finish. The guard returns at the start ofmoveToLifecycleStatewhen the state is alreadyDESTROYED.moveToDestroyedStateonly enters that function while the state is aboveINITIALIZED, so the path down toonDestroyis unchanged.This matches a production crash on maps-compose 6.10.0, whose
moveUpis the same method as current main (symbolicated to theerror(...)line). 29 Android crashes in 30 days on React Native 0.85.3; 22 of them were immediately after the app was backgrounded. Median time since the last interaction was about 3.9s. Sample device: Samsung SM-A166U, Android 16../gradlew spotlessKotlinCheckpassed.:maps-compose:testDebugUnitTest --tests com.google.maps.android.compose.MapLifecycleEventObserverTestpassed (2 tests): a lifecycle event afterDESTROYEDdoes not throw or callMapView, andmoveToDestroyedStatefromRESUMEDstill callsonPause,onStop, andonDestroy.