From c0d723ee00aae31f466df9c1aef3ac17d3542085 Mon Sep 17 00:00:00 2001 From: Jonas Lochmann Date: Mon, 13 Feb 2023 01:00:00 +0100 Subject: [PATCH] Use a more reliable fragment id generation --- .../java/io/timelimit/android/Application.kt | 12 +++++++++++- .../io/timelimit/android/ui/MainActivity.kt | 17 ++++++----------- .../io/timelimit/android/ui/model/MainModel.kt | 14 +++++++++++--- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/io/timelimit/android/Application.kt b/app/src/main/java/io/timelimit/android/Application.kt index f64ff79..b784db2 100644 --- a/app/src/main/java/io/timelimit/android/Application.kt +++ b/app/src/main/java/io/timelimit/android/Application.kt @@ -1,5 +1,5 @@ /* - * TimeLimit Copyright 2019 - 2021 Jonas Lochmann + * TimeLimit Copyright 2019 - 2023 Jonas Lochmann * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,9 +16,19 @@ package io.timelimit.android import android.app.Application +import android.view.View import com.jakewharton.threetenabp.AndroidThreeTen class Application : Application() { + // two legacy screens use small id numbers as they want; by running generateViewId() often enough, + // all ids that are harcoded this way are not returned from generateViewId + init { (0..1024).forEach { _ -> View.generateViewId() } } + + // allocate some view ids for Fragments that are not used for anything else + // by running this in the Application class, there is a high chance that these + // are always the same ids so that there is no trouble when restoring state + val viewIdPool = (0..4).map { View.generateViewId() } + override fun onCreate() { super.onCreate() diff --git a/app/src/main/java/io/timelimit/android/ui/MainActivity.kt b/app/src/main/java/io/timelimit/android/ui/MainActivity.kt index f3d9b75..51fc86f 100644 --- a/app/src/main/java/io/timelimit/android/ui/MainActivity.kt +++ b/app/src/main/java/io/timelimit/android/ui/MainActivity.kt @@ -79,7 +79,6 @@ class MainActivity : AppCompatActivity(), ActivityViewModelHolder, U2fManager.De private const val EXTRA_AUTH_HANDOVER = "authHandover" private const val MAIN_MODEL_STATE = "mainModelState" private const val FRAGMENT_IDS_STATE = "fragmentIds" - private const val NEXT_FRAGMENT_ID = "nextFragmentId" private var authHandover: Triple? = null @@ -114,7 +113,6 @@ class MainActivity : AppCompatActivity(), ActivityViewModelHolder, U2fManager.De } private val mainModel by viewModels() - private var fragmentIds = mutableSetOf() private val syncModel: SyncStatusModel by lazy { ViewModelProviders.of(this).get(SyncStatusModel::class.java) } @@ -134,8 +132,7 @@ class MainActivity : AppCompatActivity(), ActivityViewModelHolder, U2fManager.De if (savedInstanceState != null) { mainModel.state.value = savedInstanceState.getSerializable(MAIN_MODEL_STATE) as State - fragmentIds.addAll(savedInstanceState.getIntegerArrayList(FRAGMENT_IDS_STATE) ?: emptyList()) - mainModel.nextFragmentId = savedInstanceState.getInt(NEXT_FRAGMENT_ID) + mainModel.fragmentIds.addAll(savedInstanceState.getIntegerArrayList(FRAGMENT_IDS_STATE) ?: emptyList()) } lifecycleScope.launch { @@ -279,7 +276,7 @@ class MainActivity : AppCompatActivity(), ActivityViewModelHolder, U2fManager.De screen = screen, executeCommand = ::execute, fragmentManager = supportFragmentManager, - fragmentIds = fragmentIds, + fragmentIds = mainModel.fragmentIds, modifier = Modifier .fillMaxSize() .padding(paddingValues) @@ -300,8 +297,7 @@ class MainActivity : AppCompatActivity(), ActivityViewModelHolder, U2fManager.De super.onSaveInstanceState(outState) outState.putSerializable(MAIN_MODEL_STATE, mainModel.state.value) - outState.putIntegerArrayList(FRAGMENT_IDS_STATE, ArrayList(fragmentIds)) - outState.putInt(NEXT_FRAGMENT_ID, mainModel.nextFragmentId) + outState.putIntegerArrayList(FRAGMENT_IDS_STATE, ArrayList(mainModel.fragmentIds)) } override fun onStart() { @@ -373,7 +369,7 @@ class MainActivity : AppCompatActivity(), ActivityViewModelHolder, U2fManager.De } private fun cleanupFragments() { - fragmentIds + mainModel.fragmentIds .filter { fragmentId -> var v = mainModel.state.value as State? @@ -385,8 +381,7 @@ class MainActivity : AppCompatActivity(), ActivityViewModelHolder, U2fManager.De true } - .map { supportFragmentManager.findFragmentById(it) } - .filterNotNull() + .mapNotNull { supportFragmentManager.findFragmentById(it) } .filter { it.isDetached } .forEach { if (BuildConfig.DEBUG) { @@ -399,7 +394,7 @@ class MainActivity : AppCompatActivity(), ActivityViewModelHolder, U2fManager.De .remove(it) .commitAllowingStateLoss() - fragmentIds.remove(id) + mainModel.fragmentIds.remove(id) } } diff --git a/app/src/main/java/io/timelimit/android/ui/model/MainModel.kt b/app/src/main/java/io/timelimit/android/ui/model/MainModel.kt index 92eb312..02c0a9e 100644 --- a/app/src/main/java/io/timelimit/android/ui/model/MainModel.kt +++ b/app/src/main/java/io/timelimit/android/ui/model/MainModel.kt @@ -81,7 +81,7 @@ class MainModel(application: Application): AndroidViewModel(application) { val activityCommand: ReceiveChannel = activityCommandInternal val state = MutableStateFlow(State.LaunchState as State) - var nextFragmentId = 1 + var fragmentIds = mutableSetOf() val screen: Flow = flow { while (true) { @@ -93,9 +93,17 @@ class MainModel(application: Application): AndroidViewModel(application) { is State.DiagnoseScreen.DeviceOwner -> emitAll(DeviceOwnerHandling.processState(logic, scope, authenticationModelApi, state)) is FragmentState -> emitAll(state.transformWhile { if (it is FragmentState && it !is State.Overview) { - if (it.containerId == null) it.containerId = nextFragmentId++ + val containerId = it.containerId ?: run { + ((application as io.timelimit.android.Application).viewIdPool - fragmentIds).firstOrNull()?.also { id -> + it.containerId = id + } + } - emit(Screen.FragmentScreen(it, it.toolbarIcons, it.toolbarOptions, it, it.containerId!!)) + if (containerId != null) { + fragmentIds.add(containerId) + + emit(Screen.FragmentScreen(it, it.toolbarIcons, it.toolbarOptions, it, containerId)) + } true } else false