Skip to content

Commit 131815c

Browse files
committed
Limit the max number of opened rooms in the backstack
This should help with the `TransactionTooLargeExceptions` we were seeing, since every one of these nodes and their sub-nodes would be saved to the instance state. Also, make sure we use `LoggedInFlowNode.attachRoom` as much as possible to ensure this check is used
1 parent b8865d5 commit 131815c

File tree

1 file changed

+65
-13
lines changed

1 file changed

+65
-13
lines changed

appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt

Lines changed: 65 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -323,12 +323,13 @@ class LoggedInFlowNode(
323323
NavTarget.Home -> {
324324
val callback = object : HomeEntryPoint.Callback {
325325
override fun navigateToRoom(roomId: RoomId, joinedRoom: JoinedRoom?) {
326-
backstack.push(
327-
NavTarget.Room(
326+
sessionCoroutineScope.launch {
327+
attachRoom(
328328
roomIdOrAlias = roomId.toRoomIdOrAlias(),
329-
initialElement = RoomNavigationTarget.Root(joinedRoom = joinedRoom)
329+
joinedRoom = joinedRoom,
330+
clearBackstack = false,
330331
)
331-
)
332+
}
332333
}
333334

334335
override fun navigateToSettings() {
@@ -352,7 +353,9 @@ class LoggedInFlowNode(
352353
}
353354

354355
override fun navigateToRoomSettings(roomId: RoomId) {
355-
backstack.push(NavTarget.Room(roomId.toRoomIdOrAlias(), initialElement = RoomNavigationTarget.Details))
356+
sessionCoroutineScope.launch {
357+
backstack.push(NavTarget.Room(roomId.toRoomIdOrAlias(), initialElement = RoomNavigationTarget.Details))
358+
}
356359
}
357360

358361
override fun navigateToBugReport() {
@@ -368,7 +371,9 @@ class LoggedInFlowNode(
368371
is NavTarget.Room -> {
369372
val joinedRoomCallback = object : JoinedRoomLoadedFlowNode.Callback {
370373
override fun navigateToRoom(roomId: RoomId, serverNames: List<String>) {
371-
backstack.push(NavTarget.Room(roomId.toRoomIdOrAlias(), serverNames))
374+
sessionCoroutineScope.launch {
375+
attachRoom(roomIdOrAlias = roomId.toRoomIdOrAlias(), serverNames = serverNames, clearBackstack = false)
376+
}
372377
}
373378

374379
override fun handlePermalinkClick(data: PermalinkData, pushToBackstack: Boolean) {
@@ -385,7 +390,15 @@ class LoggedInFlowNode(
385390
initialElement = RoomNavigationTarget.Root(data.eventId),
386391
)
387392
if (pushToBackstack) {
388-
backstack.push(target)
393+
sessionCoroutineScope.launch {
394+
attachRoom(
395+
roomIdOrAlias = data.roomIdOrAlias,
396+
serverNames = data.viaParameters,
397+
trigger = JoinedRoomAnalyticsEvent.Trigger.Timeline,
398+
eventId = data.eventId,
399+
clearBackstack = false
400+
)
401+
}
389402
} else {
390403
backstack.replace(target)
391404
}
@@ -413,7 +426,9 @@ class LoggedInFlowNode(
413426
is NavTarget.UserProfile -> {
414427
val callback = object : UserProfileEntryPoint.Callback {
415428
override fun navigateToRoom(roomId: RoomId) {
416-
backstack.push(NavTarget.Room(roomId.toRoomIdOrAlias()))
429+
sessionCoroutineScope.launch {
430+
attachRoom(roomIdOrAlias = roomId.toRoomIdOrAlias(), clearBackstack = false)
431+
}
417432
}
418433
}
419434
userProfileEntryPoint.createNode(
@@ -446,7 +461,9 @@ class LoggedInFlowNode(
446461
}
447462

448463
override fun navigateToEvent(roomId: RoomId, eventId: EventId) {
449-
backstack.push(NavTarget.Room(roomId.toRoomIdOrAlias(), initialElement = RoomNavigationTarget.Root(eventId)))
464+
sessionCoroutineScope.launch {
465+
attachRoom(roomIdOrAlias = roomId.toRoomIdOrAlias(), eventId = eventId, clearBackstack = false)
466+
}
450467
}
451468
}
452469
val inputs = PreferencesEntryPoint.Params(navTarget.initialElement)
@@ -566,6 +583,7 @@ class LoggedInFlowNode(
566583

567584
suspend fun attachRoom(
568585
roomIdOrAlias: RoomIdOrAlias,
586+
joinedRoom: JoinedRoom? = null,
569587
serverNames: List<String> = emptyList(),
570588
trigger: JoinedRoomAnalyticsEvent.Trigger? = null,
571589
eventId: EventId? = null,
@@ -579,7 +597,7 @@ class LoggedInFlowNode(
579597
roomIdOrAlias = roomIdOrAlias,
580598
serverNames = serverNames,
581599
trigger = trigger,
582-
initialElement = RoomNavigationTarget.Root(eventId = eventId)
600+
initialElement = RoomNavigationTarget.Root(eventId = eventId, joinedRoom = joinedRoom)
583601
)
584602
backstack.accept(AttachRoomOperation(roomNavTarget, clearBackstack))
585603
}
@@ -673,16 +691,50 @@ private class AttachRoomOperation(
673691
val roomNavTarget = it.key.navTarget as? LoggedInFlowNode.NavTarget.Room
674692
roomNavTarget?.roomIdOrAlias == roomTarget.roomIdOrAlias
675693
}
694+
695+
// Make sure the backstack of rooms can't grow indefinitely when opening permalinks.
696+
// Having 5 rooms in the backstack seems reasonable and shouldn't grow the saved state size too much.
697+
val maxRoomCount = 5
698+
val roomElementCount = elements.count { it.key.navTarget is LoggedInFlowNode.NavTarget.Room }
699+
700+
Timber.d("Current room nodes: $roomElementCount/$maxRoomCount")
701+
val currentElements = if (roomElementCount + 1 > maxRoomCount) {
702+
var keptRooms = 0
703+
// Reverse the order so we keep the latest room nodes
704+
elements.asReversed().mapNotNull { element ->
705+
if (element.key.navTarget is LoggedInFlowNode.NavTarget.Room) {
706+
keptRooms++
707+
if (keptRooms > maxRoomCount) {
708+
Timber.w("Discarding old room node, already reached the max count: $maxRoomCount")
709+
return@mapNotNull null
710+
}
711+
}
712+
element
713+
}
714+
// Then reverse the order again after removing the extra room nodes
715+
.asReversed()
716+
} else {
717+
elements
718+
}
719+
720+
// If the room already existed, remove it from the stack and add a new node at the end
676721
if (existingRoomElement != null) {
677-
elements.mapNotNull { element ->
722+
currentElements.mapNotNull { element ->
678723
if (element == existingRoomElement) {
679724
null
680725
} else {
681726
element.transitionTo(STASHED, this)
682727
}
683-
} + existingRoomElement.transitionTo(ACTIVE, this)
728+
} + // Always create a new element, otherwise we wouldn't be navigating to the target event id or child node
729+
BackStackElement(
730+
key = NavKey(roomTarget),
731+
fromState = CREATED,
732+
targetState = ACTIVE,
733+
operation = this
734+
)
684735
} else {
685-
Push<LoggedInFlowNode.NavTarget>(roomTarget).invoke(elements)
736+
// Otherwise, just push the new node to the end of the backstack
737+
Push<LoggedInFlowNode.NavTarget>(roomTarget).invoke(currentElements)
686738
}
687739
}
688740
}

0 commit comments

Comments
 (0)