Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -368,10 +368,10 @@ The message is a lambda, so build strings inline without guarding on build type
logcat { "Plain info-level message" } // defaults to INFO
logcat(LogPriority.DEBUG) { "GraphQL ${operation.name()} START" } // explicit priority
logcat(LogPriority.ERROR, throwable) { "Failed to load X: ${throwable.message}" }
logcat(LogPriority.INFO, tag = DEEP_LINK_STACK_DEBUG_TAG) { "…" } // greppable custom tag
logcat(LogPriority.INFO, tag = SOMETHING_DEBUG_TAG) { "…" } // greppable custom tag
```

Tag is optional — for a one-off log, just omit it (the caller's class name is used). When you want a greppable trace that spans several call sites or files, a shared `const val SOMETHING_DEBUG_TAG = "…"` passed as `tag` everywhere is a handy tool (see `DEEP_LINK_STACK_DEBUG_TAG` usage across `MainActivity`/`BackstackController`). It's a convenience, not a requirement; don't introduce a const for a single isolated log.
Tag is optional — for a one-off log, just omit it (the caller's class name is used). When you want a greppable trace that spans several call sites or files, a shared `const val SOMETHING_DEBUG_TAG = "…"` passed as `tag` everywhere is a handy tool. It's a convenience, not a requirement; don't introduce a const for a single isolated log.

There is also an Apollo overload, `logcat(priority, operationError: ApolloOperationError, tag, message)`, which auto-downgrades to at most `WARN` for unauthenticated errors. Use it when logging a failed `safeExecute`/`safeFlow` result.

Expand Down
66 changes: 64 additions & 2 deletions app/app/src/main/kotlin/com/hedvig/android/app/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,16 @@ class MainActivity : AppCompatActivity() {
* top of [onCreate]; the controller, session reconciler and merged ViewModel factory are read off it.
*/
private val navRetainedViewModel: NavRetainedViewModel by lazy {
// Seed isOwnTask with isTaskRoot at construction so the controller never starts from a guessed
// default; it is refreshed authoritatively on every onResume (see refreshIsOwnTask). Captured as a
// value here because the initializer lambda's receiver is not this Activity. Only used on the
// genuine first creation — a config change reuses the retained instance and skips the initializer.
val isOwnTaskAtCreation = isTaskRoot
ViewModelProvider(
this,
viewModelFactory { initializer { NavRetainedViewModel((application as HedvigApplication).appGraph) } },
viewModelFactory {
initializer { NavRetainedViewModel((application as HedvigApplication).appGraph, isOwnTaskAtCreation) }
},
)[NavRetainedViewModel::class.java]
}

Expand Down Expand Up @@ -197,6 +204,18 @@ class MainActivity : AppCompatActivity() {
}
}
}
val launchedFromHistory = intent.flags and Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY != 0
logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) {
"MainActivity.onCreate launch context: " +
"isColdStart(savedInstanceState==null)=${savedInstanceState == null} | " +
"isTaskRoot=$isTaskRoot | " +
"intent.action=${intent.action} | " +
"intent.categories=${intent.categories} | " +
"intent.data=${intent.data} | " +
"launchedFromHistory=$launchedFromHistory | " +
"intent.flags=0x${Integer.toHexString(intent.flags)} | " +
"isHostedInForeignTask=${isHostedInForeignTask(isTaskRoot, intent.flags)}"
}
if (savedInstanceState == null) {
handleDeepLinkIntent(intent)
}
Expand Down Expand Up @@ -267,8 +286,25 @@ class MainActivity : AppCompatActivity() {
* in onCreate — there is no shared, process-wide controller that a backgrounded Activity could
* steal, which is why no resume/top-resume re-attachment is needed.
*/
override fun onResume() {
super.onResume()
refreshIsOwnTask()
}

/**
* Pushes the current own-task value into the controller (see [isHostedInForeignTask] for why this is
* not just [isTaskRoot]). Unlike the one-time hooks in [attachBackstackTaskHooks], this is a *value*
* that can change over the Activity's life (e.g. an Activity below us finishes, or [isTaskRoot] reads
* more reliably once resumed than at onCreate), so it is refreshed on every onResume to keep the
* snapshot-backed value honest — otherwise the lone-deep-link chrome can latch a stale reading. See
* [BackstackController.isOwnTask].
*/
private fun refreshIsOwnTask() {
backstackController.isOwnTask = !isHostedInForeignTask(isTaskRoot, intent.flags)
}

private fun attachBackstackTaskHooks() {
backstackController.isOwnTask = { isTaskRoot }
refreshIsOwnTask()
backstackController.escapeToOwnTask = { parentStack ->
NavigationStateBridge.escapeToOwnTask(this@MainActivity, parentStack, serializersModules)
}
Expand All @@ -282,6 +318,32 @@ class MainActivity : AppCompatActivity() {
}
}

/**
* Logcat tag for the per-launch navigation breadcrumb emitted in [MainActivity.onCreate]. Grep this to
* see how each launch was classified (action/flags/isTaskRoot -> isHostedInForeignTask) when diagnosing
* task/back-stack oddities like the deep-link-stack-on-Home report.
*/
internal const val DEEP_LINK_STACK_DEBUG_TAG = "DeepLinkStackDebug"

/**
* Whether this Activity was launched into another app's task (genuinely foreign-hosted), given the
* task-root position and the [launchFlags] of the intent that started it. This is the one case that
* wants the lone-deep-link Up/escape affordance; everything else is our own task.
*
* Both conditions must hold to be foreign-hosted:
* - **not the task root** (`!isTaskRoot`): some other activity sits below us in the task; and
* - **launched without [Intent.FLAG_ACTIVITY_NEW_TASK]**: Android's default for that is to place us in
* the *caller's* task rather than one of our own, so we joined whoever started us.
*
* `NEW_TASK` being set means the system gave us our own task (a fresh one, or an existing one brought
* forward), so we are NOT foreign-hosted even when not its root. That is why a launcher relaunch or a
* notification tap that fronted our task (both carry `NEW_TASK`, both can be non-root) is correctly
* treated as own-task and does not render the Up bar on a normal Home. Only a real foreign deep link
* (e.g. an https link tapped in another app, launched with no flags) is foreign-hosted.
*/
internal fun isHostedInForeignTask(isTaskRoot: Boolean, launchFlags: Int): Boolean =
!isTaskRoot && (launchFlags and Intent.FLAG_ACTIVITY_NEW_TASK) == 0

/**
* Applies the theme in two ways:
* 1. Uses UiModeManager to persist the last theme selected so that on new launches the splash screen matches the theme
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.hedvig.android.navigation.common.HedvigNavKey
import com.hedvig.android.navigation.common.StashedSession
import com.hedvig.android.navigation.common.TopLevelTab
import com.hedvig.android.navigation.compose.merge
import kotlin.time.Duration.Companion.minutes
import kotlinx.serialization.Polymorphic
import kotlinx.serialization.PolymorphicSerializer
import kotlinx.serialization.Serializable
Expand All @@ -33,6 +34,14 @@ internal object NavigationStateBridge {
private const val EXTRA_RESTORE_STACK = "com.hedvig.android.app.RESTORE_STACK"
private const val NAV_STATE_REGISTRY_KEY = "com.hedvig.android.app.NAV_STATE"

/**
* How long a [BackstackController.pendingDeepLink] may survive a process death and still be landed at
* the next login. Covers a real "killed mid-login" window (switching to BankID, OTP, etc., is seconds
* to a few minutes) while discarding a link stashed long ago, which would otherwise bleed into an
* unrelated later login and strand the member on a lone deep-link stack. See [restoreAndPersist].
*/
private val MAX_PENDING_DEEP_LINK_AGE = 30.minutes

private val handoffSerializer = ListSerializer(PolymorphicSerializer(HedvigNavKey::class))

/**
Expand Down Expand Up @@ -65,16 +74,23 @@ internal object NavigationStateBridge {
if (!handoff.isNullOrEmpty()) {
backstackController.reseed(handoff)
} else {
savedStateRegistry.consumeRestoredStateForKey(NAV_STATE_REGISTRY_KEY)
val snapshot = savedStateRegistry.consumeRestoredStateForKey(NAV_STATE_REGISTRY_KEY)
?.let { decodeFromSavedState(NavStateSnapshot.serializer(), it, savedStateConfiguration) }
?.let { snapshot ->
backstackController.restoreFromSavedState(
entries = snapshot.entries,
parkedRuns = snapshot.parkedRuns,
pendingDeepLink = snapshot.pendingDeepLink,
stashedSession = snapshot.stashedSession,
)
}
if (snapshot != null) {
// Discard a pending deep link that is too old to still belong to an in-flight login, so it
// can't bleed into an unrelated later login as a lone deep-link stack.
val now = System.currentTimeMillis()
val stashedAt = snapshot.pendingDeepLinkStashedAtEpochMs
val pendingStillValid = snapshot.pendingDeepLink != null &&
isPendingDeepLinkStashTimeFresh(stashedAt, now)
backstackController.restoreFromSavedState(
entries = snapshot.entries,
parkedRuns = snapshot.parkedRuns,
pendingDeepLink = snapshot.pendingDeepLink.takeIf { pendingStillValid },
pendingDeepLinkStashedAtEpochMs = stashedAt.takeIf { pendingStillValid },
stashedSession = snapshot.stashedSession,
)
}
backstackController.seedIfEmpty(listOf(LoginKey))
}

Expand All @@ -87,6 +103,7 @@ internal object NavigationStateBridge {
entries = backstackController.entries.toList(),
parkedRuns = backstackController.parkedRuns.toMap(),
pendingDeepLink = backstackController.pendingDeepLink,
pendingDeepLinkStashedAtEpochMs = backstackController.pendingDeepLinkStashedAtEpochMs,
stashedSession = backstackController.stashedSession,
),
savedStateConfiguration,
Expand Down Expand Up @@ -124,17 +141,34 @@ internal object NavigationStateBridge {
private fun json(serializersModules: Set<SerializersModule>): Json = Json {
serializersModule = serializersModules.merge()
}

/**
* Whether a pending deep link stashed at [stashedAtEpochMs] is recent enough (as of [nowEpochMs]) to
* still be landed after a process-death restore. `null` (no timestamp — e.g. a pre-upgrade snapshot)
* and a negative age (clock moved backwards) both count as not-fresh, so the link is dropped rather
* than risk landing a stale one. Pure and clock-injected so it is unit-testable without Android.
*/
internal fun isPendingDeepLinkStashTimeFresh(stashedAtEpochMs: Long?, nowEpochMs: Long): Boolean {
if (stashedAtEpochMs == null) return false
val ageMs = nowEpochMs - stashedAtEpochMs
return ageMs in 0..MAX_PENDING_DEEP_LINK_AGE.inWholeMilliseconds
}
}

/**
* The full hoisted navigation state, serialized into the Activity's SavedStateRegistry so the
* in-memory [BackstackController] singleton can be re-hydrated after process death. Mirrors the four
* in-memory [BackstackController] singleton can be re-hydrated after process death. Mirrors the
* holders the controller owns.
*
* [pendingDeepLinkStashedAtEpochMs] defaults to `null` so snapshots written before this field existed
* still decode; a missing timestamp means an age can't be computed, so such a restored pending link is
* treated as stale and dropped (see [NavigationStateBridge.restoreAndPersist]).
*/
@Serializable
private data class NavStateSnapshot(
val entries: List<@Polymorphic HedvigNavKey>,
val parkedRuns: Map<TopLevelTab, List<@Polymorphic HedvigNavKey>>,
val pendingDeepLink: (@Polymorphic HedvigNavKey)?,
val stashedSession: StashedSession?,
val pendingDeepLinkStashedAtEpochMs: Long? = null,
)
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.hedvig.android.app.navigation
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.Stable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.runtime.snapshots.Snapshot
import androidx.compose.runtime.snapshots.SnapshotStateList
Expand Down Expand Up @@ -39,18 +40,10 @@ internal class BackstackController(
internal val parkedRuns: SnapshotStateMap<TopLevelTab, List<HedvigNavKey>>,
pendingDeepLinkState: MutableState<HedvigNavKey?>,
stashedSessionState: MutableState<StashedSession?>,
/**
* Whether this activity is the root of its own task. `false` means we were launched into the
* caller's task by an external deep link, so an Up press must escape into our own task rather than
* rebuilding the ancestry in place (which would leave our screens hosted under the foreign app).
*
* Attached by `MainActivity` in `onCreate` (see `attachBackstackTaskHooks`), not at construction: the
* controller is owned by this Activity's retained `ViewModel` and is 1:1 with the Activity, so there
* is no shared, process-wide controller a backgrounded Activity could steal — the hook is wired once
* at creation, with no resume re-attachment needed. Defaults to `true` so unit tests and any
* pre-attach use stay fully in-process.
*/
var isOwnTask: () -> Boolean = { true },
// Defaulted (and placed after the required states) so the many positional test constructions need no
// change; the real construction in NavRetainedViewModel passes it explicitly by name.
pendingDeepLinkStashedAtState: MutableState<Long?> = mutableStateOf(null),
initialIsOwnTask: Boolean = true,
/**
* Re-roots the app in its own task seeded with the given stack (see [navigateUp]). The Activity
* owns the mechanics (relaunch with NEW_TASK|CLEAR_TASK + finish); the controller only supplies
Expand All @@ -65,12 +58,50 @@ internal class BackstackController(
*/
var finishApp: () -> Unit = {},
) : Backstack {
/**
* Whether this Activity is the root of its own task. `false` means we were launched into the
* caller's task by an external deep link, so an Up press must escape into our own task rather than
* rebuilding the ancestry in place (which would leave our screens hosted under the foreign app).
*
* Snapshot-backed on purpose, rather than a live lambda: [loneDeepLinkChrome] derives the whole
* back-arrow-vs-nav-bar decision partly from this, so it must (a) be observable so Compose recomposes
* the chrome when it changes, and (b) never sample a stale value. `MainActivity` pushes the computed
* own-task value here (see `isHostedInForeignTask`, which is *not* just `isTaskRoot`) in `onCreate` (see
* `attachBackstackTaskHooks`) AND on every `onResume` — onResume because the value can settle or
* change after creation (an Activity below us finishing, or an early-launch read returning a
* not-yet-settled value). Defaults to `true` so unit tests and any pre-attach use stay fully
* in-process.
*/
var isOwnTask: Boolean by mutableStateOf(initialIsOwnTask)

/**
* A deep link resolved while logged out, held until [setLoggedIn] consumes it (so it can land
* alone). Persisted across rotation / process death (e.g. mid-OTP) — see the class KDoc.
* alone). Persisted across rotation / process death (e.g. mid-OTP) — see the class KDoc. Always set
* via [stashPendingDeepLink] so its companion timestamp [pendingDeepLinkStashedAtEpochMs] stays in
* sync; cleared (with the timestamp) when consumed by [setLoggedIn] or wiped by [reseed].
*/
internal var pendingDeepLink: HedvigNavKey? by pendingDeepLinkState

/**
* Wall-clock time ([System.currentTimeMillis]) when [pendingDeepLink] was last stashed, or `null`
* when there is none. Persisted alongside [pendingDeepLink] so a process-death restore can discard a
* link that is too old to still belong to the in-flight login (see `NavigationStateBridge`), which
* stops a long-abandoned pending link from bleeding into an unrelated later login.
*/
internal var pendingDeepLinkStashedAtEpochMs: Long? by pendingDeepLinkStashedAtState

/** Stashes [key] as the [pendingDeepLink], stamping [pendingDeepLinkStashedAtEpochMs] with now. */
private fun stashPendingDeepLink(key: HedvigNavKey) {
pendingDeepLink = key
pendingDeepLinkStashedAtEpochMs = System.currentTimeMillis()
}

/** Clears [pendingDeepLink] and its companion timestamp together. */
private fun clearPendingDeepLink() {
pendingDeepLink = null
pendingDeepLinkStashedAtEpochMs = null
}

/**
* The previous logged-in session, held between logout and the next login. Excluded from
* [allLiveContentKeys] on purpose — see [StashedSession]. Persisted across process death.
Expand Down Expand Up @@ -139,12 +170,11 @@ internal class BackstackController(
val loneDeepLinkChrome: LoneDeepLinkChrome
get() {
val first = entries.firstOrNull()
val insideRunsModel = isOwnTask() && (first is HomeKey || first is LoginKey)
if (insideRunsModel) return LoneDeepLinkChrome.ShowSuite
return if (entries.lastOrNull()?.topLevelTabOrNull() != null) {
LoneDeepLinkChrome.ShowUpBar
} else {
LoneDeepLinkChrome.ShowNothing
val insideRunsModel = isOwnTask && (first is HomeKey || first is LoginKey)
return when {
insideRunsModel -> LoneDeepLinkChrome.ShowSuite
entries.lastOrNull()?.topLevelTabOrNull() != null -> LoneDeepLinkChrome.ShowUpBar
else -> LoneDeepLinkChrome.ShowNothing
}
}

Expand Down Expand Up @@ -181,7 +211,7 @@ internal class BackstackController(
*/
fun navigateToInAppLink(key: HedvigNavKey) {
if (!isLoggedIn) {
pendingDeepLink = key
stashPendingDeepLink(key)
return
}
Snapshot.withMutableSnapshot {
Expand All @@ -199,7 +229,7 @@ internal class BackstackController(
*/
fun navigateToExternalDeepLink(key: HedvigNavKey) {
if (!isLoggedIn) {
pendingDeepLink = key
stashPendingDeepLink(key)
return
}
reseed(listOf(key))
Expand All @@ -222,11 +252,11 @@ internal class BackstackController(
val parentStack = synthetic.dropLast(1)
when {
parentStack.isNotEmpty() -> {
if (isOwnTask()) entries.replaceWith(parentStack) else escapeToOwnTask(parentStack)
if (isOwnTask) entries.replaceWith(parentStack) else escapeToOwnTask(parentStack)
return true
}

!isOwnTask() -> {
!isOwnTask -> {
escapeToOwnTask(synthetic)
return true
}
Expand Down Expand Up @@ -265,7 +295,7 @@ internal class BackstackController(
fun setLoggedIn(memberId: String?) {
Snapshot.withMutableSnapshot {
val pending = pendingDeepLink
pendingDeepLink = null
clearPendingDeepLink()
val stash = stashedSession?.takeIf { memberId != null && it.memberId == memberId }
stashedSession = null
parkedRuns.clear()
Expand Down Expand Up @@ -328,7 +358,7 @@ internal class BackstackController(
Snapshot.withMutableSnapshot {
entries.replaceWith(stack)
parkedRuns.clear()
pendingDeepLink = null
clearPendingDeepLink()
stashedSession = null
}
}
Expand All @@ -342,13 +372,15 @@ internal class BackstackController(
entries: List<HedvigNavKey>,
parkedRuns: Map<TopLevelTab, List<HedvigNavKey>>,
pendingDeepLink: HedvigNavKey?,
pendingDeepLinkStashedAtEpochMs: Long?,
stashedSession: StashedSession?,
) {
if (this.entries.isNotEmpty()) return
Snapshot.withMutableSnapshot {
this.entries.addAll(entries)
this.parkedRuns.putAll(parkedRuns)
this.pendingDeepLink = pendingDeepLink
this.pendingDeepLinkStashedAtEpochMs = pendingDeepLinkStashedAtEpochMs
this.stashedSession = stashedSession
}
}
Expand Down
Loading
Loading