From 01841e9faf68d9bae28194abb7a97e0730da4463 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Thu, 2 Jul 2026 17:19:42 -0500 Subject: [PATCH] feat(onboarding): deep-link battery step nearer the per-app background-activity screen (best-effort) Spiked #150 against the AOSP Settings source (not just the reference docs): no public, non-hidden Settings action opens the "Unrestricted/Optimized/Restricted" screen directly for a specific package. ACTION_VIEW_ADVANCED_POWER_USAGE_DETAIL would, but it's @hide/non-SDK; the other public battery action, ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS, isn't package-scoped and is a worse landing for one known app. So ACTION_APPLICATION_DETAILS_SETTINGS (one tap from the target via "Battery" on stock/Pixel/AOSP) stays the primary target. BatteryOptimizationManager.settingsIntent() is restructured into a verified, never-dead-end fallback chain: try app-details, and if it doesn't resolve on some device, fall back to the battery-optimization list rather than nothing. The ordering/selection logic is extracted into a small Android-free helper so it's directly unit-testable; a new instrumented test checks the real candidate intents/order against a real PackageManager. Co-Authored-By: Claude Opus 4.8 --- .../BatteryOptimizationManagerIntentTest.kt | 48 ++++++++++++++ .../push/BatteryOptimizationManager.kt | 64 ++++++++++++++++--- .../onboarding/BatteryOptimizationScreen.kt | 8 ++- .../push/BatteryOptimizationManagerTest.kt | 49 ++++++++++++++ 4 files changed, 157 insertions(+), 12 deletions(-) create mode 100644 app/src/androidTest/kotlin/org/libremail/push/BatteryOptimizationManagerIntentTest.kt create mode 100644 app/src/test/kotlin/org/libremail/push/BatteryOptimizationManagerTest.kt diff --git a/app/src/androidTest/kotlin/org/libremail/push/BatteryOptimizationManagerIntentTest.kt b/app/src/androidTest/kotlin/org/libremail/push/BatteryOptimizationManagerIntentTest.kt new file mode 100644 index 0000000..8989de7 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/push/BatteryOptimizationManagerIntentTest.kt @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.push + +import android.content.Context +import android.net.Uri +import android.provider.Settings +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Exercises the real [BatteryOptimizationManager.candidateIntents]/[BatteryOptimizationManager.settingsIntent] + * against a real `Context`/`PackageManager` (#150): `Intent`/`Uri`/`PackageManager.resolveActivity` are + * unmocked SDK stubs off-device, so this on-device coverage is the only place the actual candidate + * actions, order, and package scoping can be checked directly. The fallback-selection logic itself + * (which candidate wins, and that there's always a last resort) is covered Android-free by + * `BatteryOptimizationManagerTest` in the `test` source set; end-to-end launch-from-the-onboarding- + * button coverage lives in `BatteryOptimizationStepTest`. + */ +@RunWith(AndroidJUnit4::class) +class BatteryOptimizationManagerIntentTest { + + private val context = ApplicationProvider.getApplicationContext() + private val manager = BatteryOptimizationManager(context) + + @Test + fun candidateIntents_tryAppDetailsFirst_thenTheBatteryOptimizationList() { + val candidates = manager.candidateIntents() + + assertEquals(2, candidates.size) + assertEquals(Settings.ACTION_APPLICATION_DETAILS_SETTINGS, candidates[0].action) + assertEquals(Uri.fromParts("package", context.packageName, null), candidates[0].data) + assertEquals(Settings.ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS, candidates[1].action) + } + + @Test + fun settingsIntent_landsOnAppDetails_becauseItAlwaysResolvesOnARealDevice() { + // Every real/emulated Android device ships a Settings app that resolves app-details for any + // installed package, so the primary (most direct) candidate always wins here. The fallback + // exists for devices this test environment can't represent (see the class KDoc). + val intent = manager.settingsIntent() + + assertEquals(Settings.ACTION_APPLICATION_DETAILS_SETTINGS, intent.action) + assertEquals(Uri.fromParts("package", context.packageName, null), intent.data) + } +} diff --git a/app/src/main/kotlin/org/libremail/push/BatteryOptimizationManager.kt b/app/src/main/kotlin/org/libremail/push/BatteryOptimizationManager.kt index a694b7c..ff67541 100644 --- a/app/src/main/kotlin/org/libremail/push/BatteryOptimizationManager.kt +++ b/app/src/main/kotlin/org/libremail/push/BatteryOptimizationManager.kt @@ -18,8 +18,32 @@ import javax.inject.Singleton * * We deliberately do NOT use the restricted `REQUEST_IGNORE_BATTERY_OPTIMIZATIONS` permission or its * one-tap dialog: Google Play limits that permission to an approved set of use cases (rejection risk, - * see #17). Sending the user to the system screen instead needs no extra permission and is safe on - * both Play and F-Droid (#16). + * see #17). Sending the user to a system screen instead needs no extra permission and is safe on both + * Play and F-Droid (#16). + * + * ### Which screen, and why (#150) + * The onboarding goal is to land as close as possible to the per-app "Unrestricted / Optimized / + * Restricted" screen. Of the `Settings` actions that are both public (part of the SDK, not `@hide`) + * and don't require the restricted permission above, none opens that exact screen for a specific + * package — confirmed against the AOSP `Settings` source, not just the reference docs: + * - [Settings.ACTION_APPLICATION_DETAILS_SETTINGS], scoped to our package, is the closest safe option: + * on stock Android/Pixel/AOSP it lands one tap ("Battery") away from the target screen. This is + * [settingsIntent]'s primary candidate, unchanged from before this investigation. + * - [Settings.ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS] is public and needs no permission, but is + * *not* package-scoped: it opens the system-wide "Battery Optimization" app list (defaulting to a + * filter that hides already-optimized apps, so the user must switch it to "All apps" to even find + * LibreMail), and tapping an app there shows a legacy two-state Optimize/"Don't optimize" dialog that + * predates the "Restricted" option. That is a worse landing than app-details for one known app, so + * it is used only as a fallback for the rare case app-details itself doesn't resolve. + * - The one action that *would* jump straight to the target, `ACTION_VIEW_ADVANCED_POWER_USAGE_DETAIL`, + * is `@hide` in the platform source — not part of the public SDK. Using its literal string would + * mean depending on a private, unversioned implementation detail, exactly the fragility this class + * already avoids for the restricted-permission route, so it is out for the same reason. + * + * OEM skins (Samsung, Xiaomi, etc.) may rename, relocate, or move this control into a manufacturer + * settings app entirely. There is no public intent for those short of hardcoding fragile, + * version-specific OEM component names, so devices without a "Battery" entry on the app-details page + * simply keep the app-details landing rather than this class reaching for one. * * Note [isIgnoringBatteryOptimizations] reflects the Doze allowlist: it is `true` only for the * "Unrestricted" setting and `false` for *both* "Optimized" and "Restricted", so it cannot single out @@ -41,13 +65,35 @@ class BatteryOptimizationManager @Inject constructor(@ApplicationContext private } /** - * Intent to this app's system details screen, where the user can open **Battery** and choose - * **Unrestricted**. App-details is targeted (rather than the flat battery-optimization list) - * because it is the only route to the Unrestricted/Optimized/Restricted setting and lands - * directly on LibreMail. Always resolvable since API 9. + * Best-effort deep link toward the per-app battery-optimization screen (see the class KDoc for the + * rationale and OEM caveats). Returns the first of [candidateIntents] that resolves an activity on + * this device, falling back to the last candidate — today, app-details, resolvable since API 9 — + * so the caller always gets an intent that lands somewhere useful instead of a dead end. */ - fun settingsIntent(): Intent = Intent( - Settings.ACTION_APPLICATION_DETAILS_SETTINGS, - Uri.fromParts("package", context.packageName, null), + fun settingsIntent(): Intent = + firstResolvableOrElseLast(candidateIntents()) { it.resolveActivity(context.packageManager) != null } + + /** + * Candidate settings screens, most specific/direct first. `internal` (rather than private) so the + * candidate list itself — actions, order, and the package scoping — is directly checkable from a + * test. See the class KDoc for why each candidate is here, in this order. + */ + internal fun candidateIntents(): List = listOf( + Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS, Uri.fromParts("package", context.packageName, null)), + Intent(Settings.ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS), ) } + +/** + * Returns the first element of [candidates] for which [resolves] holds, or the last element if none do + * — so a caller building a best-effort fallback chain always gets something usable rather than `null`. + * Top-level and `internal` (rather than folded into [BatteryOptimizationManager.settingsIntent]) so + * this selection/fallback logic is exhaustively unit-testable without touching real Android intent + * resolution, the same reasoning as [BatteryPromptDecision]. + * + * @param candidates must be non-empty, ordered most-preferred first. + */ +internal fun firstResolvableOrElseLast(candidates: List, resolves: (T) -> Boolean): T { + require(candidates.isNotEmpty()) { "candidates must not be empty" } + return candidates.firstOrNull(resolves) ?: candidates.last() +} diff --git a/app/src/main/kotlin/org/libremail/ui/onboarding/BatteryOptimizationScreen.kt b/app/src/main/kotlin/org/libremail/ui/onboarding/BatteryOptimizationScreen.kt index 0314c5a..9ad77bc 100644 --- a/app/src/main/kotlin/org/libremail/ui/onboarding/BatteryOptimizationScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/onboarding/BatteryOptimizationScreen.kt @@ -35,9 +35,11 @@ import org.libremail.R /** * Final onboarding step (shown only when needed, see [OnboardingViewModel.batteryPromptNeeded]): * invites the user to allow unrestricted background/battery usage so push and periodic sync aren't - * throttled by Doze. **Take me there** deep-links to the system screen (no restricted permission); - * **Not now** skips. Either way [onFinish] proceeds to the inbox. On returning from Settings the - * status is re-read and, if the app is now unrestricted, the screen reflects that with a "done" state. + * throttled by Doze. **Take me there** deep-links as directly as possible toward the per-app battery + * screen (see [org.libremail.push.BatteryOptimizationManager] for the best-effort fallback chain; no + * restricted permission is ever used); **Not now** skips. Either way [onFinish] proceeds to the inbox. + * On returning from Settings the status is re-read and, if the app is now unrestricted, the screen + * reflects that with a "done" state. * * @param viewModel the graph-scoped onboarding view model (holds live battery status + the flag). * @param onFinish leaves onboarding for the inbox; the caller also marks the prompt handled. diff --git a/app/src/test/kotlin/org/libremail/push/BatteryOptimizationManagerTest.kt b/app/src/test/kotlin/org/libremail/push/BatteryOptimizationManagerTest.kt new file mode 100644 index 0000000..54b2f2b --- /dev/null +++ b/app/src/test/kotlin/org/libremail/push/BatteryOptimizationManagerTest.kt @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.push + +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +/** + * Unit tests for the `firstResolvableOrElseLast` fallback-selection helper that backs + * [BatteryOptimizationManager.settingsIntent] (#150). Deliberately exercised with plain `String` + * candidates rather than real `Intent`s: `Intent`/`Uri`/`PackageManager` are unmocked Android SDK stubs + * in a JVM unit test and throw on use, so the selection/ordering logic is kept generic and Android-free + * specifically so it's testable here (see its KDoc). The real candidate intents and end-to-end + * resolution are covered by the instrumented `BatteryOptimizationStepTest`. + */ +class BatteryOptimizationManagerTest { + + @Test + fun `returns the first candidate that resolves`() { + assertEquals("b", firstResolvableOrElseLast(listOf("a", "b", "c")) { it == "b" }) + } + + @Test + fun `returns the only candidate when it is the first checked, even if it would not resolve`() { + assertEquals("only", firstResolvableOrElseLast(listOf("only")) { it == "only" }) + } + + @Test + fun `falls back to the last candidate when none resolve, rather than dead-ending`() { + assertEquals("c", firstResolvableOrElseLast(listOf("a", "b", "c")) { false }) + } + + @Test + fun `falls back to the last candidate even when it is the only one and it does not resolve`() { + assertEquals("only", firstResolvableOrElseLast(listOf("only")) { false }) + } + + @Test + fun `prefers an earlier resolving candidate over a later one, even if both resolve`() { + assertEquals("first", firstResolvableOrElseLast(listOf("first", "second")) { true }) + } + + @Test + fun `rejects an empty candidate list rather than silently returning nothing`() { + assertFailsWith { + firstResolvableOrElseLast(emptyList()) { true } + } + } +}