From 19b0d942ddf1fce0e5327d1765c783492b38e33f Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 4 May 2026 13:38:47 -0600 Subject: [PATCH 1/4] chore(android): enable StrictMode penaltyLog in the demo app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Configure StrictMode in the demo's Application.onCreate() with detectAll() + penaltyLog() on both ThreadPolicy and VmPolicy, gated on BuildConfig.DEBUG. Phase 1 of #488 — log-only so existing library-side violations surface during local dev without crashing manual testing. Phase 2 (penaltyDeath) is a separate PR and depends on the library-side violations catalogued in docs/code/strictmode.md being addressed. --- .../gutenbergkit/GutenbergKitApplication.kt | 19 +++ docs/code/README.md | 1 + docs/code/strictmode.md | 161 ++++++++++++++++++ 3 files changed, 181 insertions(+) create mode 100644 docs/code/strictmode.md diff --git a/android/app/src/main/java/com/example/gutenbergkit/GutenbergKitApplication.kt b/android/app/src/main/java/com/example/gutenbergkit/GutenbergKitApplication.kt index 51df489e2..557900e6c 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/GutenbergKitApplication.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/GutenbergKitApplication.kt @@ -3,6 +3,7 @@ package com.example.gutenbergkit import android.app.Application import android.net.ConnectivityManager import android.net.NetworkCapabilities +import android.os.StrictMode import rs.wordpress.api.android.KeystorePasswordTransformer import rs.wordpress.api.kotlin.NetworkAvailabilityProvider import rs.wordpress.api.kotlin.WpApiClient @@ -26,6 +27,9 @@ class GutenbergKitApplication : Application() { override fun onCreate() { super.onCreate() + if (BuildConfig.DEBUG) { + enableStrictMode() + } accountRepository = AccountRepository( rootPath = filesDir.resolve("accounts").absolutePath, passwordTransformer = KeystorePasswordTransformer("GutenbergKit") @@ -37,6 +41,21 @@ class GutenbergKitApplication : Application() { } } + private fun enableStrictMode() { + StrictMode.setThreadPolicy( + StrictMode.ThreadPolicy.Builder() + .detectAll() + .penaltyLog() + .build() + ) + StrictMode.setVmPolicy( + StrictMode.VmPolicy.Builder() + .detectAll() + .penaltyLog() + .build() + ) + } + /** * Constructs a [WpApiClient] for the given [account]. Used by the demo app's posts * list and Save button to fetch/persist posts via the WordPress REST API. diff --git a/docs/code/README.md b/docs/code/README.md index fb13097dc..0841a8409 100644 --- a/docs/code/README.md +++ b/docs/code/README.md @@ -18,6 +18,7 @@ This guide is for developers who want to contribute code to GutenbergKit. - [Local WordPress](./local-wordpress.md) - Local WordPress environment for testing - [Physical Device Setup](./physical-device-setup.md) - Running on physical devices - [WordPress.com OAuth](./wpcom-oauth.md) - Connecting demo apps to WordPress.com sites +- [StrictMode (Android demo)](./strictmode.md) - StrictMode configuration for the Android demo app ## Get Involved diff --git a/docs/code/strictmode.md b/docs/code/strictmode.md new file mode 100644 index 000000000..d2e047675 --- /dev/null +++ b/docs/code/strictmode.md @@ -0,0 +1,161 @@ +# StrictMode in the Demo App + +The Android demo app at `android/app/` configures Android `StrictMode` so that disk +I/O on the main thread, leaked closables, registration leaks, and similar +issues introduced inside the GutenbergKit library surface during local development. +This page documents the configuration, the violations the configured demo +currently surfaces, and the path to Phase 2 (`penaltyDeath`). + +## Configuration + +`GutenbergKitApplication.onCreate()` enables `StrictMode` in debug builds only +(`BuildConfig.DEBUG`). Release builds are unaffected. + +```kotlin +StrictMode.setThreadPolicy( + StrictMode.ThreadPolicy.Builder() + .detectAll() + .penaltyLog() + .build() +) +StrictMode.setVmPolicy( + StrictMode.VmPolicy.Builder() + .detectAll() + .penaltyLog() + .build() +) +``` + +Configuration notes: + +- **`detectAll()` is intentional.** It's a superset that includes `detectDiskReads`, + `detectDiskWrites`, `detectNetwork`, `detectCustomSlowCalls`, + `detectResourceMismatches`, `detectUnbufferedIo`, and (on `VmPolicy`) + `detectLeakedClosableObjects`, `detectLeakedRegistrationObjects`, + `detectActivityLeaks`, `detectFileUriExposure`, `detectCleartextNetwork`, + `detectContentUriWithoutPermission`, etc. +- **`Application.onCreate()`** is the right place — earlier is better so the policy + covers everything that follows. +- **`penaltyDialog()` is deliberately not enabled.** It interrupts manual testing + without adding signal. +- **No `StrictMode.allowThreadDiskReads()` permits inside the library.** The + demo-app-only configuration ensures host apps see the same violations the + demo does. + +## Reading violations + +Filter logcat on tag `StrictMode`: + +```bash +adb logcat -s StrictMode +``` + +Each violation prints a stack trace; the topmost app/library frame is the actual +call site. + +## Currently catalogued violations + +The catalogue below was captured by installing the debug build on an Android 14 +emulator (Pixel API 34), launching the app, and tapping **Standalone editor** +(which routes through `SitePreparationActivity`). + +### App launch — `Application.onCreate()` + +10 `DiskReadViolation` lines. All originate from a single pair of statements in +`GutenbergKitApplication.onCreate()`: + +1. **`filesDir.resolve("accounts")`** — 2 violations from + `Context.getFilesDir()` checking and creating the app's data directory on + first access. +2. **`AccountRepository(...)`** constructor — 8 violations from + `com.sun.jna.Native.` running `getTempDir()` / + `removeTemporaryFiles()` while loading the JNA native dispatch library. + Triggered transitively via `uniffi.wp_mobile.UniffiRustCallStatus.` on + first call. + +**Status:** intentional-permit-because-startup. These are all one-time, occur +during process init before any user-visible UI is drawn, and originate from the +JVM/JNA loader rather than GutenbergKit code. Fixing means moving +`AccountRepository` initialization off the main thread, which is a demo-app +question rather than a library one. + +### Site preparation — `SitePreparationViewModel.startLoading()` + +Tapping any editor entry point lands on `SitePreparationActivity`, which kicks +off `SitePreparationViewModel.startLoading()`. This launches a coroutine on +`viewModelScope` (which defaults to `Dispatchers.Main.immediate`) that +synchronously constructs an `EditorService` and reads asset bundle counts. 10 +violations: + +1. **`EditorHTTPClient.` (`EditorHTTPClient.kt:140`)** — 1 + `CustomViolation: newSSLContext`. SSLContext initialization on the main + thread. +2. **`Paths.defaultCacheRoot` (`Paths.kt:58`)** — 2 `DiskReadViolation`s. + `context.cacheDir` checks/creates the cache directory on first access. +3. **`Paths.defaultStorageRoot` (`Paths.kt:20`)** — 1 `DiskReadViolation`. + `context.filesDir` (same pattern as above, on a different lazily-resolved + directory). +4. **`Paths.defaultTempStorageRoot` (`Paths.kt:46`)** — 1 `DiskReadViolation`. + Same pattern via `context.cacheDir`. +5. **`EditorURLCache.` (`EditorURLCache.kt:37`)** — 1 `DiskReadViolation`. + Cache directory existence check during construction. +6. **`EditorAssetsLibrary.` (`EditorAssetsLibrary.kt:46`)** — 1 + `DiskReadViolation`. Asset bundle directory existence check during + construction. +7. **`EditorService.create` (`EditorService.kt:117`)** — 2 `DiskReadViolation`s. + Disk I/O during service construction. +8. **`EditorAssetsLibrary.readAssetBundles` + (`EditorAssetsLibrary.kt:98`)** — 1 `DiskReadViolation`. Synchronous read of + bundle directory contents. + +**Status:** library bug, tracked for fix. `EditorService.create` and the +`Paths`/cache/asset library it constructs perform synchronous disk I/O. The +demo's `SitePreparationViewModel.countAssetBundles` runs them on +`viewModelScope` without an explicit dispatcher, so they execute on +`Dispatchers.Main.immediate`. Either: + +- the demo should wrap the call in `withContext(Dispatchers.IO) { ... }`, or +- `EditorService.create` should accept a `CoroutineDispatcher` and do the + disk-touching work on it itself (preferred — host apps shouldn't have to + know). + +These are real library-side issues that Phase 2 (`penaltyDeath`) cannot ship +until they're resolved. Out of scope for this PR per the issue (#488). + +### Editor flow + +The full editor flow (loading the editor, opening the inserter sheet, picking +photos, etc.) requires network-loaded asset bundles and was not exercised when +this catalogue was written. The catalogue should be expanded as those flows are +exercised manually — see "Adding to this catalogue" below. + +## Adding to this catalogue + +When introducing a feature that touches disk, network, or shared resources: + +1. Run the demo in debug mode on a connected device or emulator. +2. Filter logcat on `StrictMode`. +3. Walk through every code path the feature touches. +4. For each new violation: + - If it's a library bug, file a follow-up issue and link it here. + - If it's an intentional, sub-millisecond, one-time-per-process cost + (e.g. a `SharedPreferences` warm-up), document it here as + *intentional-permit-because-X*. + - **Never** suppress with `StrictMode.allowThreadDiskReads()` inside the + library — that hides the violation from host apps. + +## Phase 2 — `penaltyDeath()` + +Once the library-side violations above have been fixed, the demo will switch +its thread policy to: + +```kotlin +.penaltyDeath() +``` + +Phase 2 is a separate PR. It crashes the demo on any new violation, ensuring +future library changes can't quietly regress. + +`VmPolicy` will remain on `penaltyLog()` even after Phase 2 — VM violations +like `LeakedClosableObject` happen during GC and crash the app at unpredictable +times unrelated to the offending code path, which is unhelpful as a signal. From 0bb7877a86147d9db6a0ea19d5b66c7577b8c80c Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 4 May 2026 13:54:21 -0600 Subject: [PATCH 2/4] chore(android): use penaltyDeath, move startup I/O off the main thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch the demo's StrictMode ThreadPolicy from penaltyLog to penaltyDeath, so any new main-thread disk/network I/O introduced in the library crashes the demo immediately rather than logging quietly. VmPolicy stays on penaltyLog — VM violations fire during GC and would crash at unpredictable times unrelated to the offending code path. To make penaltyDeath viable, the demo's startup and editor flows are restructured to keep the main thread off disk: - AccountRepository init moves to Dispatchers.IO behind a Deferred. Callers use a new withAccountRepository helper that awaits the deferred and then hops to IO for the actual repo call. - All AccountRepository reads/writes (.all(), .store(), .remove()) across MainActivity, AuthenticationManager, EditorActivity, PostsListActivity, and SitePreparationViewModel are wrapped via withAccountRepository. - SitePreparationViewModel.createEditorService wraps EditorService.create in withContext(IO). EditorService.purge / fetchAssetBundleCount also. - SitePreparationActivity.launchEditor moves EditorDependenciesSerializer.writeToDisk into a coroutine. - EditorActivity.onCreate reads dependencies into Compose state on IO and defers setContent until the read completes. NetworkAvailabilityProvider stays eager on the main thread (its constructor only stores a SAM lambda). Verified on a Pixel 9 (Android 16) and an emulator: cold launch + Standalone Editor + Prepare Editor + Start + typing produces zero ThreadPolicy violations. Closes #488. --- .../gutenbergkit/AuthenticationManager.kt | 116 +++++----- .../example/gutenbergkit/EditorActivity.kt | 24 +- .../gutenbergkit/GutenbergKitApplication.kt | 58 ++++- .../com/example/gutenbergkit/MainActivity.kt | 20 +- .../example/gutenbergkit/PostsListActivity.kt | 5 +- .../gutenbergkit/SitePreparationActivity.kt | 24 +- .../gutenbergkit/SitePreparationViewModel.kt | 65 +++--- docs/code/strictmode.md | 219 ++++++++---------- 8 files changed, 298 insertions(+), 233 deletions(-) diff --git a/android/app/src/main/java/com/example/gutenbergkit/AuthenticationManager.kt b/android/app/src/main/java/com/example/gutenbergkit/AuthenticationManager.kt index 6dd8d8a6e..8ae61fd77 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/AuthenticationManager.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/AuthenticationManager.kt @@ -25,12 +25,11 @@ import uniffi.wp_api.WpComSiteIdentifier import uniffi.wp_api.applicationPasswordsUrl import uniffi.wp_api.wordpressComOauth2Configuration import uniffi.wp_mobile.Account -import uniffi.wp_mobile.AccountRepository import uniffi.wp_mobile.wordpressComSiteApiRoot class AuthenticationManager( private val context: Context, - private val accountRepository: AccountRepository, + private val app: GutenbergKitApplication, private val networkAvailabilityProvider: NetworkAvailabilityProvider, private val scope: CoroutineScope ) { @@ -153,32 +152,39 @@ class AuthenticationManager( data: Uri, callback: AuthenticationCallback ) { - try { - val siteUrl = data.getQueryParameter("site_url") - ?: throw IllegalStateException("site_url is missing from authentication") - val username = data.getQueryParameter("user_login") - ?: throw IllegalStateException("username is missing from authentication") - val password = data.getQueryParameter("password") - ?: throw IllegalStateException("password is missing from authentication") - - val discoverySuccess = currentDiscoverySuccess - ?: throw IllegalStateException("API discovery result is not available") - val siteApiRoot = discoverySuccess.apiRootUrl.toURL().toString() - - val account = Account.SelfHostedSite( - id = 0u, - domain = siteUrl, - username = username, - password = password, - siteApiRoot = siteApiRoot - ) - accountRepository.store(account) - - val stored = accountRepository.all().last() - currentDiscoverySuccess = null - callback.onAuthenticationSuccess(stored) - } catch (e: Exception) { - callback.onAuthenticationFailure("Authentication error: ${e.message}") + scope.launch { + try { + val siteUrl = data.getQueryParameter("site_url") + ?: throw IllegalStateException("site_url is missing from authentication") + val username = data.getQueryParameter("user_login") + ?: throw IllegalStateException("username is missing from authentication") + val password = data.getQueryParameter("password") + ?: throw IllegalStateException("password is missing from authentication") + + val discoverySuccess = currentDiscoverySuccess + ?: throw IllegalStateException("API discovery result is not available") + val siteApiRoot = discoverySuccess.apiRootUrl.toURL().toString() + + val account = Account.SelfHostedSite( + id = 0u, + domain = siteUrl, + username = username, + password = password, + siteApiRoot = siteApiRoot + ) + val stored = app.withAccountRepository { repo -> + repo.store(account) + repo.all().last() + } + currentDiscoverySuccess = null + withContext(Dispatchers.Main) { + callback.onAuthenticationSuccess(stored) + } + } catch (e: Exception) { + withContext(Dispatchers.Main) { + callback.onAuthenticationFailure("Authentication error: ${e.message}") + } + } } } @@ -209,35 +215,39 @@ class AuthenticationManager( client.oauth2().requestToken(tokenParams) } - withContext(Dispatchers.Main) { - when (tokenResult) { - is WpRequestResult.Success -> { - val tokenResponse = tokenResult.response.data - val blogId = tokenResponse.blogId - ?: throw OAuthException.MissingBlogId() - val discoverySuccess = currentDiscoverySuccess - val siteHost = discoverySuccess?.parsedSiteUrl?.toURL()?.toURI()?.host - ?: throw OAuthException.MissingSiteHost() - - val siteApiRoot = wordpressComSiteApiRoot(blogId) - - val account = Account.WpCom( - id = 0u, - username = siteHost, - token = tokenResponse.accessToken, - siteApiRoot = siteApiRoot - ) - - accountRepository.store(account) - val stored = accountRepository.all().last() + when (tokenResult) { + is WpRequestResult.Success -> { + val tokenResponse = tokenResult.response.data + val blogId = tokenResponse.blogId + ?: throw OAuthException.MissingBlogId() + val discoverySuccess = currentDiscoverySuccess + val siteHost = discoverySuccess?.parsedSiteUrl?.toURL()?.toURI()?.host + ?: throw OAuthException.MissingSiteHost() + + val siteApiRoot = wordpressComSiteApiRoot(blogId) + + val account = Account.WpCom( + id = 0u, + username = siteHost, + token = tokenResponse.accessToken, + siteApiRoot = siteApiRoot + ) + + val stored = app.withAccountRepository { repo -> + repo.store(account) + repo.all().last() + } - currentOAuthConfig = null - currentOAuthState = null - currentDiscoverySuccess = null + currentOAuthConfig = null + currentOAuthState = null + currentDiscoverySuccess = null + withContext(Dispatchers.Main) { callback.onAuthenticationSuccess(stored) } - else -> { + } + else -> { + withContext(Dispatchers.Main) { callback.onAuthenticationFailure("Token exchange failed") } } diff --git a/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt b/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt index 213b75ab7..bdb239b16 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt @@ -47,8 +47,10 @@ import androidx.compose.ui.viewinterop.AndroidView import androidx.lifecycle.lifecycleScope import com.example.gutenbergkit.ui.theme.AppTheme import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.suspendCancellableCoroutine +import kotlinx.coroutines.withContext import org.wordpress.gutenberg.model.EditorConfiguration import org.wordpress.gutenberg.GutenbergView import org.wordpress.gutenberg.RecordedNetworkRequest @@ -97,18 +99,29 @@ class EditorActivity : ComponentActivity() { intent.getParcelableExtra(MainActivity.EXTRA_CONFIGURATION) } ?: EditorConfiguration.bundled() - // Read dependencies from disk if a file path was provided + // The dependencies blob is on disk; deserialize it off the main thread + // and feed the editor once it's ready. val dependenciesPath = intent.getStringExtra(EXTRA_DEPENDENCIES_PATH) - val dependencies = dependenciesPath?.let { EditorDependenciesSerializer.readFromDisk(it) } + val dependenciesState = mutableStateOf(null) + val dependenciesReady = mutableStateOf(dependenciesPath == null) + if (dependenciesPath != null) { + lifecycleScope.launch { + dependenciesState.value = withContext(Dispatchers.IO) { + EditorDependenciesSerializer.readFromDisk(dependenciesPath) + } + dependenciesReady.value = true + } + } // Optional account ID for REST API persistence (set when launched from PostsListActivity) val accountId = intent.getLongExtra(EXTRA_ACCOUNT_ID, -1L).takeIf { it >= 0 }?.toULong() setContent { AppTheme { + if (!dependenciesReady.value) return@AppTheme EditorScreen( configuration = configuration, - dependencies = dependencies, + dependencies = dependenciesState.value, accountId = accountId, coroutineScope = this.lifecycleScope, onClose = { finish() }, @@ -365,8 +378,9 @@ private suspend fun persistPost( } val app = context.applicationContext as GutenbergKitApplication - val account = app.accountRepository.all().firstOrNull { it.id() == accountId } - ?: error("Account not found") + val account = app.withAccountRepository { repo -> + repo.all().firstOrNull { it.id() == accountId } + } ?: error("Account not found") val client = app.createApiClient(account) val endpointType = when (configuration.postType.postType) { diff --git a/android/app/src/main/java/com/example/gutenbergkit/GutenbergKitApplication.kt b/android/app/src/main/java/com/example/gutenbergkit/GutenbergKitApplication.kt index 557900e6c..153135d71 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/GutenbergKitApplication.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/GutenbergKitApplication.kt @@ -4,6 +4,13 @@ import android.app.Application import android.net.ConnectivityManager import android.net.NetworkCapabilities import android.os.StrictMode +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.async +import kotlinx.coroutines.withContext import rs.wordpress.api.android.KeystorePasswordTransformer import rs.wordpress.api.kotlin.NetworkAvailabilityProvider import rs.wordpress.api.kotlin.WpApiClient @@ -19,9 +26,28 @@ import uniffi.wp_mobile.Account import uniffi.wp_mobile.AccountRepository class GutenbergKitApplication : Application() { - lateinit var accountRepository: AccountRepository - private set + /** + * The [AccountRepository] is initialized off the main thread because its + * constructor touches `filesDir` (a `getDataDir()` disk access) and loads + * JNA's native dispatch library, which itself does several disk reads + * during ``. Both would trip [StrictMode] with `penaltyDeath`. + * + * Access via [accountRepository] from a coroutine. + */ + private val applicationScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + private val accountRepositoryAsync: Deferred = + applicationScope.async(start = CoroutineStart.LAZY) { + AccountRepository( + rootPath = filesDir.resolve("accounts").absolutePath, + passwordTransformer = KeystorePasswordTransformer("GutenbergKit") + ) + } + + /** + * The [NetworkAvailabilityProvider] constructor only stores a SAM lambda, + * so it is safe to initialize eagerly on the main thread. + */ lateinit var networkAvailabilityProvider: NetworkAvailabilityProvider private set @@ -30,22 +56,42 @@ class GutenbergKitApplication : Application() { if (BuildConfig.DEBUG) { enableStrictMode() } - accountRepository = AccountRepository( - rootPath = filesDir.resolve("accounts").absolutePath, - passwordTransformer = KeystorePasswordTransformer("GutenbergKit") - ) networkAvailabilityProvider = NetworkAvailabilityProvider { val cm = getSystemService(ConnectivityManager::class.java) val capabilities = cm.getNetworkCapabilities(cm.activeNetwork) capabilities?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true } + // Kick off background init so the first caller doesn't pay the cost. + accountRepositoryAsync.start() + } + + /** + * Suspends until the [AccountRepository] is ready. Subsequent calls are + * cheap. Callers that then read or write the repository must still hop to + * [Dispatchers.IO] for the actual call. + */ + suspend fun accountRepository(): AccountRepository = accountRepositoryAsync.await() + + /** + * Awaits the repository, then runs [block] on [Dispatchers.IO]. This is + * the standard pattern for repository access from the demo UI. + */ + suspend fun withAccountRepository(block: suspend (AccountRepository) -> T): T { + val repo = accountRepository() + return withContext(Dispatchers.IO) { block(repo) } } private fun enableStrictMode() { + // ThreadPolicy uses penaltyDeath so any new main-thread disk/network + // I/O introduced in the library crashes the demo immediately. VmPolicy + // stays on penaltyLog — VM violations like LeakedClosableObject fire + // during GC and would crash at unpredictable times unrelated to the + // offending code path. StrictMode.setThreadPolicy( StrictMode.ThreadPolicy.Builder() .detectAll() .penaltyLog() + .penaltyDeath() .build() ) StrictMode.setVmPolicy( diff --git a/android/app/src/main/java/com/example/gutenbergkit/MainActivity.kt b/android/app/src/main/java/com/example/gutenbergkit/MainActivity.kt index 27ce7170f..0854d1bca 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/MainActivity.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/MainActivity.kt @@ -6,6 +6,7 @@ import androidx.activity.ComponentActivity import androidx.activity.compose.setContent import androidx.activity.enableEdgeToEdge import androidx.lifecycle.lifecycleScope +import kotlinx.coroutines.launch import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Arrangement @@ -56,7 +57,6 @@ class MainActivity : ComponentActivity(), AuthenticationManager.AuthenticationCa private val isLoadingCapabilities = mutableStateOf(false) private val authError = mutableStateOf(null) private val gutenbergKitApp by lazy { application as GutenbergKitApplication } - private val accountRepository by lazy { gutenbergKitApp.accountRepository } private val networkAvailabilityProvider by lazy { gutenbergKitApp.networkAvailabilityProvider } private lateinit var authenticationManager: AuthenticationManager private val siteCapabilitiesDiscovery = SiteCapabilitiesDiscovery() @@ -69,7 +69,7 @@ class MainActivity : ComponentActivity(), AuthenticationManager.AuthenticationCa super.onCreate(savedInstanceState) enableEdgeToEdge() - authenticationManager = AuthenticationManager(this, accountRepository, networkAvailabilityProvider, lifecycleScope) + authenticationManager = AuthenticationManager(this, gutenbergKitApp, networkAvailabilityProvider, lifecycleScope) // Add default bundled editor configuration configurations.add(ConfigurationItem.BundledEditor) @@ -77,10 +77,14 @@ class MainActivity : ComponentActivity(), AuthenticationManager.AuthenticationCa // Add local WordPress option configurations.add(ConfigurationItem.LocalWordPress) - // Load saved accounts - configurations.addAll( - accountRepository.all().map { ConfigurationItem.ConfiguredEditor.fromAccount(it) } - ) + // Load saved accounts off the main thread; the encrypted store is on + // disk and reads decrypt every account via Keystore. + lifecycleScope.launch { + val accounts = gutenbergKitApp.withAccountRepository { it.all() } + configurations.addAll( + accounts.map { ConfigurationItem.ConfiguredEditor.fromAccount(it) } + ) + } setContent { AppTheme { @@ -106,7 +110,9 @@ class MainActivity : ComponentActivity(), AuthenticationManager.AuthenticationCa }, onDeleteConfiguration = { config -> if (config is ConfigurationItem.ConfiguredEditor) { - accountRepository.remove(config.accountId) + lifecycleScope.launch { + gutenbergKitApp.withAccountRepository { it.remove(config.accountId) } + } } configurations.remove(config) }, diff --git a/android/app/src/main/java/com/example/gutenbergkit/PostsListActivity.kt b/android/app/src/main/java/com/example/gutenbergkit/PostsListActivity.kt index 933c07c04..c775c0af5 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/PostsListActivity.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/PostsListActivity.kt @@ -171,8 +171,9 @@ class PostsListViewModel( try { val app = application as GutenbergKitApplication - val account = app.accountRepository.all().firstOrNull { it.id() == accountId } - ?: error("Account not found") + val account = app.withAccountRepository { repo -> + repo.all().firstOrNull { it.id() == accountId } + } ?: error("Account not found") val client = app.createApiClient(account) val endpointType = when (postType.postType) { diff --git a/android/app/src/main/java/com/example/gutenbergkit/SitePreparationActivity.kt b/android/app/src/main/java/com/example/gutenbergkit/SitePreparationActivity.kt index a66f9326a..c71057f73 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/SitePreparationActivity.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/SitePreparationActivity.kt @@ -3,6 +3,10 @@ package com.example.gutenbergkit import android.content.Context import android.content.Intent import android.os.Bundle +import androidx.lifecycle.lifecycleScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import org.json.JSONObject import androidx.activity.ComponentActivity import androidx.activity.compose.setContent @@ -168,16 +172,22 @@ class SitePreparationActivity : ComponentActivity() { configuration: EditorConfiguration, dependencies: EditorDependencies? ) { - val intent = Intent(this, EditorActivity::class.java).apply { - putExtra(MainActivity.EXTRA_CONFIGURATION, configuration) + lifecycleScope.launch { + val filePath = if (dependencies != null) { + withContext(Dispatchers.IO) { + EditorDependenciesSerializer.writeToDisk( + this@SitePreparationActivity, + dependencies + ) + } + } else null - // Serialize dependencies to disk and pass the file path - if (dependencies != null) { - val filePath = EditorDependenciesSerializer.writeToDisk(this@SitePreparationActivity, dependencies) - putExtra(EditorActivity.EXTRA_DEPENDENCIES_PATH, filePath) + val intent = Intent(this@SitePreparationActivity, EditorActivity::class.java).apply { + putExtra(MainActivity.EXTRA_CONFIGURATION, configuration) + filePath?.let { putExtra(EditorActivity.EXTRA_DEPENDENCIES_PATH, it) } } + startActivity(intent) } - startActivity(intent) } private fun launchPostsList( diff --git a/android/app/src/main/java/com/example/gutenbergkit/SitePreparationViewModel.kt b/android/app/src/main/java/com/example/gutenbergkit/SitePreparationViewModel.kt index ea5c558f4..cc1993df7 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/SitePreparationViewModel.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/SitePreparationViewModel.kt @@ -5,11 +5,13 @@ import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewModelScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import org.wordpress.gutenberg.model.EditorCachePolicy import org.wordpress.gutenberg.model.EditorConfiguration import org.wordpress.gutenberg.model.EditorDependencies @@ -93,29 +95,41 @@ class SitePreparationViewModel( _uiState.update { it.copy(selectedPostType = postType) } } - fun prepareEditor() { - val configuration = _uiState.value.editorConfiguration ?: return - - val cacheIntervalSeconds = 86_400L // Cache for one day - val editorService = EditorService.create( + /** + * Constructs the [EditorService] off the main thread. The constructor + * touches `context.filesDir` / `context.cacheDir`, allocates a + * synchronous SSL context, and reads the asset bundles directory. + */ + private suspend fun createEditorService( + configuration: EditorConfiguration, + cachePolicy: EditorCachePolicy = EditorCachePolicy.Always + ): EditorService = withContext(Dispatchers.IO) { + EditorService.create( context = getApplication(), configuration = configuration, - cachePolicy = EditorCachePolicy.MaxAge(cacheIntervalSeconds), + cachePolicy = cachePolicy, coroutineScope = viewModelScope ) - prepareEditor(editorService) } - fun prepareEditorFromScratch() { + fun prepareEditor() { val configuration = _uiState.value.editorConfiguration ?: return + viewModelScope.launch { + val cacheIntervalSeconds = 86_400L // Cache for one day + val editorService = createEditorService( + configuration, + EditorCachePolicy.MaxAge(cacheIntervalSeconds) + ) + prepareEditor(editorService) + } + } - val editorService = EditorService.create( - context = getApplication(), - configuration = configuration, - cachePolicy = EditorCachePolicy.Ignore, - coroutineScope = viewModelScope - ) - prepareEditor(editorService) + fun prepareEditorFromScratch() { + val configuration = _uiState.value.editorConfiguration ?: return + viewModelScope.launch { + val editorService = createEditorService(configuration, EditorCachePolicy.Ignore) + prepareEditor(editorService) + } } private fun prepareEditor(editorService: EditorService) { @@ -157,12 +171,8 @@ class SitePreparationViewModel( try { _uiState.update { it.copy(editorDependencies = null) } - val editorService = EditorService.create( - context = getApplication(), - configuration = configuration, - coroutineScope = viewModelScope - ) - editorService.purge() + val editorService = createEditorService(configuration) + withContext(Dispatchers.IO) { editorService.purge() } countAssetBundles() @@ -186,12 +196,8 @@ class SitePreparationViewModel( return@launch } - val editorService = EditorService.create( - context = getApplication(), - configuration = configuration, - coroutineScope = viewModelScope - ) - val count = editorService.fetchAssetBundleCount() + val editorService = createEditorService(configuration) + val count = withContext(Dispatchers.IO) { editorService.fetchAssetBundleCount() } _uiState.update { it.copy(cacheBundleCount = count) } } catch (e: Exception) { @@ -292,8 +298,9 @@ class SitePreparationViewModel( config: ConfigurationItem.ConfiguredEditor ): List? { val app = getApplication() - val account = app.accountRepository.all().firstOrNull { it.id() == config.accountId } - ?: return null + val account = app.withAccountRepository { repo -> + repo.all().firstOrNull { it.id() == config.accountId } + } ?: return null val client = app.createApiClient(account) val result = client.request { builder -> diff --git a/docs/code/strictmode.md b/docs/code/strictmode.md index d2e047675..6bc3901db 100644 --- a/docs/code/strictmode.md +++ b/docs/code/strictmode.md @@ -1,10 +1,10 @@ # StrictMode in the Demo App -The Android demo app at `android/app/` configures Android `StrictMode` so that disk -I/O on the main thread, leaked closables, registration leaks, and similar -issues introduced inside the GutenbergKit library surface during local development. -This page documents the configuration, the violations the configured demo -currently surfaces, and the path to Phase 2 (`penaltyDeath`). +The Android demo app at `android/app/` configures Android `StrictMode` so that +disk I/O on the main thread, leaked closables, registration leaks, and similar +issues introduced inside the GutenbergKit library surface during local +development. The demo crashes on any new `ThreadPolicy` violation, so library +regressions can't quietly slip in. ## Configuration @@ -16,6 +16,7 @@ StrictMode.setThreadPolicy( StrictMode.ThreadPolicy.Builder() .detectAll() .penaltyLog() + .penaltyDeath() .build() ) StrictMode.setVmPolicy( @@ -28,19 +29,67 @@ StrictMode.setVmPolicy( Configuration notes: -- **`detectAll()` is intentional.** It's a superset that includes `detectDiskReads`, - `detectDiskWrites`, `detectNetwork`, `detectCustomSlowCalls`, - `detectResourceMismatches`, `detectUnbufferedIo`, and (on `VmPolicy`) - `detectLeakedClosableObjects`, `detectLeakedRegistrationObjects`, - `detectActivityLeaks`, `detectFileUriExposure`, `detectCleartextNetwork`, - `detectContentUriWithoutPermission`, etc. -- **`Application.onCreate()`** is the right place — earlier is better so the policy - covers everything that follows. -- **`penaltyDialog()` is deliberately not enabled.** It interrupts manual testing - without adding signal. -- **No `StrictMode.allowThreadDiskReads()` permits inside the library.** The - demo-app-only configuration ensures host apps see the same violations the - demo does. +- **`detectAll()` is intentional.** It's a superset that includes + `detectDiskReads`, `detectDiskWrites`, `detectNetwork`, + `detectCustomSlowCalls`, `detectResourceMismatches`, `detectUnbufferedIo`, + and (on `VmPolicy`) `detectLeakedClosableObjects`, + `detectLeakedRegistrationObjects`, `detectActivityLeaks`, + `detectFileUriExposure`, `detectCleartextNetwork`, etc. +- **`Application.onCreate()`** is the right place — earlier is better so the + policy covers everything that follows. +- **`penaltyDeath()` on `ThreadPolicy`, but `penaltyLog()` on `VmPolicy`.** VM + violations like `LeakedClosableObject` fire during GC and would crash at + unpredictable times unrelated to the offending code path, which is + unhelpful as a signal. +- **`penaltyDialog()` is deliberately not enabled.** It interrupts manual + testing without adding signal. +- **No `StrictMode.allowThreadDiskReads()` permits inside the library.** That + would hide the violation from host apps that also run StrictMode. + +## Demo-side initialization shape + +Because `penaltyDeath` crashes on any violation, the demo had to be +restructured so that nothing on the main thread touches disk: + +- **`AccountRepository`** is initialized lazily on `Dispatchers.IO` — its + constructor reads `filesDir` and triggers `com.sun.jna.Native.`, + which itself runs ~8 disk reads while loading the dispatch library. See + `GutenbergKitApplication.accountRepository()` and the `withAccountRepository` + helper. +- **All `AccountRepository` reads/writes** (`.all()`, `.store()`, `.remove()`) + are wrapped in `withAccountRepository { ... }`, which awaits the deferred + init and then dispatches the call to `Dispatchers.IO`. The Keystore-backed + password transformer does both disk and crypto work per call. +- **`EditorService.create`** touches `context.filesDir` / `context.cacheDir`, + allocates a synchronous SSL context, and reads the asset bundles + directory. `SitePreparationViewModel.createEditorService` wraps the call + in `withContext(Dispatchers.IO)`. +- **`EditorService.purge` / `EditorService.fetchAssetBundleCount`** also + touch disk. Wrapped in `withContext(Dispatchers.IO)` at the call site. +- **`EditorDependenciesSerializer.writeToDisk` / `readFromDisk`** are + wrapped at the demo entry points: `SitePreparationActivity.launchEditor` + (write before `startActivity`) and `EditorActivity.onCreate` (read into + Compose state, render once ready). + +`NetworkAvailabilityProvider` stays initialized eagerly on the main thread — +its constructor only stores a SAM lambda, no I/O. + +## What's still in the library + +Several violations originate inside `:Gutenberg`. The demo wraps the call sites +in `withContext(Dispatchers.IO)` rather than fixing them upstream — per +[#488](https://github.com/wordpress-mobile/GutenbergKit/issues/488), library +fixes are tracked separately and don't ship with the StrictMode work. Each is +a candidate for a follow-up that moves the I/O off the caller's thread or +exposes a `suspend` API: + +- `EditorService.create` (sync `EditorHTTPClient` + `Paths.cacheRoot` + + `Paths.storageRoot` + `Paths.defaultTempStorageRoot` + + `EditorURLCache.` + `EditorAssetsLibrary.`). +- `EditorService.fetchAssetBundleCount` / + `EditorAssetsLibrary.readAssetBundles`. +- `EditorDependenciesSerializer.writeToDisk` / + `EditorDependenciesSerializer.readFromDisk`. ## Reading violations @@ -50,112 +99,34 @@ Filter logcat on tag `StrictMode`: adb logcat -s StrictMode ``` -Each violation prints a stack trace; the topmost app/library frame is the actual -call site. - -## Currently catalogued violations - -The catalogue below was captured by installing the debug build on an Android 14 -emulator (Pixel API 34), launching the app, and tapping **Standalone editor** -(which routes through `SitePreparationActivity`). - -### App launch — `Application.onCreate()` - -10 `DiskReadViolation` lines. All originate from a single pair of statements in -`GutenbergKitApplication.onCreate()`: - -1. **`filesDir.resolve("accounts")`** — 2 violations from - `Context.getFilesDir()` checking and creating the app's data directory on - first access. -2. **`AccountRepository(...)`** constructor — 8 violations from - `com.sun.jna.Native.` running `getTempDir()` / - `removeTemporaryFiles()` while loading the JNA native dispatch library. - Triggered transitively via `uniffi.wp_mobile.UniffiRustCallStatus.` on - first call. - -**Status:** intentional-permit-because-startup. These are all one-time, occur -during process init before any user-visible UI is drawn, and originate from the -JVM/JNA loader rather than GutenbergKit code. Fixing means moving -`AccountRepository` initialization off the main thread, which is a demo-app -question rather than a library one. - -### Site preparation — `SitePreparationViewModel.startLoading()` - -Tapping any editor entry point lands on `SitePreparationActivity`, which kicks -off `SitePreparationViewModel.startLoading()`. This launches a coroutine on -`viewModelScope` (which defaults to `Dispatchers.Main.immediate`) that -synchronously constructs an `EditorService` and reads asset bundle counts. 10 -violations: - -1. **`EditorHTTPClient.` (`EditorHTTPClient.kt:140`)** — 1 - `CustomViolation: newSSLContext`. SSLContext initialization on the main - thread. -2. **`Paths.defaultCacheRoot` (`Paths.kt:58`)** — 2 `DiskReadViolation`s. - `context.cacheDir` checks/creates the cache directory on first access. -3. **`Paths.defaultStorageRoot` (`Paths.kt:20`)** — 1 `DiskReadViolation`. - `context.filesDir` (same pattern as above, on a different lazily-resolved - directory). -4. **`Paths.defaultTempStorageRoot` (`Paths.kt:46`)** — 1 `DiskReadViolation`. - Same pattern via `context.cacheDir`. -5. **`EditorURLCache.` (`EditorURLCache.kt:37`)** — 1 `DiskReadViolation`. - Cache directory existence check during construction. -6. **`EditorAssetsLibrary.` (`EditorAssetsLibrary.kt:46`)** — 1 - `DiskReadViolation`. Asset bundle directory existence check during - construction. -7. **`EditorService.create` (`EditorService.kt:117`)** — 2 `DiskReadViolation`s. - Disk I/O during service construction. -8. **`EditorAssetsLibrary.readAssetBundles` - (`EditorAssetsLibrary.kt:98`)** — 1 `DiskReadViolation`. Synchronous read of - bundle directory contents. - -**Status:** library bug, tracked for fix. `EditorService.create` and the -`Paths`/cache/asset library it constructs perform synchronous disk I/O. The -demo's `SitePreparationViewModel.countAssetBundles` runs them on -`viewModelScope` without an explicit dispatcher, so they execute on -`Dispatchers.Main.immediate`. Either: - -- the demo should wrap the call in `withContext(Dispatchers.IO) { ... }`, or -- `EditorService.create` should accept a `CoroutineDispatcher` and do the - disk-touching work on it itself (preferred — host apps shouldn't have to - know). - -These are real library-side issues that Phase 2 (`penaltyDeath`) cannot ship -until they're resolved. Out of scope for this PR per the issue (#488). - -### Editor flow - -The full editor flow (loading the editor, opening the inserter sheet, picking -photos, etc.) requires network-loaded asset bundles and was not exercised when -this catalogue was written. The catalogue should be expanded as those flows are -exercised manually — see "Adding to this catalogue" below. - -## Adding to this catalogue - -When introducing a feature that touches disk, network, or shared resources: - -1. Run the demo in debug mode on a connected device or emulator. -2. Filter logcat on `StrictMode`. -3. Walk through every code path the feature touches. -4. For each new violation: - - If it's a library bug, file a follow-up issue and link it here. - - If it's an intentional, sub-millisecond, one-time-per-process cost - (e.g. a `SharedPreferences` warm-up), document it here as - *intentional-permit-because-X*. - - **Never** suppress with `StrictMode.allowThreadDiskReads()` inside the - library — that hides the violation from host apps. - -## Phase 2 — `penaltyDeath()` - -Once the library-side violations above have been fixed, the demo will switch -its thread policy to: +Each violation prints a stack trace; the topmost app/library frame is the +actual call site. With `penaltyDeath` enabled, the violation also raises +`FATAL EXCEPTION: main` — `adb logcat -s AndroidRuntime` will show the crash. -```kotlin -.penaltyDeath() -``` +## Verified flows + +The following flows were exercised on a Pixel 9 (Android 16) and an Android +14 emulator with **zero** `ThreadPolicy` violations: + +- App cold launch. +- Tap **Standalone editor** → **Prepare Editor** → **Start**. +- Type into the editor. + +The full demo flow listed in [#488](https://github.com/wordpress-mobile/GutenbergKit/issues/488) +(connected sites, native inserter sheet, photo permission paths, search, +overflow menu, backgrounding) wasn't exercised end-to-end. As new flows are +walked through and StrictMode flags new call sites, fix or wrap them at the +demo-app level rather than adding library-side permits. + +## Adding new code -Phase 2 is a separate PR. It crashes the demo on any new violation, ensuring -future library changes can't quietly regress. +When introducing demo-app code that touches disk, network, or shared +resources: -`VmPolicy` will remain on `penaltyLog()` even after Phase 2 — VM violations -like `LeakedClosableObject` happen during GC and crash the app at unpredictable -times unrelated to the offending code path, which is unhelpful as a signal. +1. Run the demo in debug mode. +2. Walk through the new code path end-to-end. +3. If the app crashes with `RuntimeException: StrictMode ThreadPolicy + violation`, the offending call is in the stack trace — wrap it in + `withContext(Dispatchers.IO)` (or move it into a coroutine). +4. **Never** suppress with `StrictMode.allowThreadDiskReads()` inside the + library — that hides the violation from host apps. From ec873601e380844fc05dab80b65436e7c2af3599 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 4 May 2026 14:05:10 -0600 Subject: [PATCH 3/4] fix(android): show loading/error states in EditorActivity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The async deserialize of EditorDependencies left the activity rendering nothing during the read and would have either hung blank or crashed the activity coroutine on a deserialization failure. Replace the boolean ready flag with a sealed Loading/Failed/Ready state. While loading, show a CircularProgressIndicator. On failure, catch the exception, surface its message, and offer a Close button. The Ready path is unchanged. This also makes the activity safe across process-death restore — a stale EXTRA_DEPENDENCIES_PATH whose temp file has since been cleaned up now lands on the error screen instead of crashing. --- .../example/gutenbergkit/EditorActivity.kt | 79 +++++++++++++++---- 1 file changed, 62 insertions(+), 17 deletions(-) diff --git a/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt b/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt index bdb239b16..04ed02594 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt @@ -16,10 +16,14 @@ import androidx.activity.compose.setContent import androidx.activity.enableEdgeToEdge import androidx.activity.result.ActivityResultLauncher import androidx.activity.result.contract.ActivityResultContracts +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.imePadding import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.automirrored.filled.Redo @@ -40,9 +44,11 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView import androidx.lifecycle.lifecycleScope import com.example.gutenbergkit.ui.theme.AppTheme @@ -102,14 +108,19 @@ class EditorActivity : ComponentActivity() { // The dependencies blob is on disk; deserialize it off the main thread // and feed the editor once it's ready. val dependenciesPath = intent.getStringExtra(EXTRA_DEPENDENCIES_PATH) - val dependenciesState = mutableStateOf(null) - val dependenciesReady = mutableStateOf(dependenciesPath == null) + val dependenciesLoad = mutableStateOf( + if (dependenciesPath == null) DependenciesLoad.Ready(null) else DependenciesLoad.Loading + ) if (dependenciesPath != null) { lifecycleScope.launch { - dependenciesState.value = withContext(Dispatchers.IO) { - EditorDependenciesSerializer.readFromDisk(dependenciesPath) + dependenciesLoad.value = try { + val deps = withContext(Dispatchers.IO) { + EditorDependenciesSerializer.readFromDisk(dependenciesPath) + } + DependenciesLoad.Ready(deps) + } catch (e: Exception) { + DependenciesLoad.Failed(e.message ?: e::class.java.simpleName) } - dependenciesReady.value = true } } @@ -118,22 +129,34 @@ class EditorActivity : ComponentActivity() { setContent { AppTheme { - if (!dependenciesReady.value) return@AppTheme - EditorScreen( - configuration = configuration, - dependencies = dependenciesState.value, - accountId = accountId, - coroutineScope = this.lifecycleScope, - onClose = { finish() }, - onGutenbergViewCreated = { view -> - gutenbergView = view - setupFileChooserListener(view) - } - ) + when (val load = dependenciesLoad.value) { + is DependenciesLoad.Loading -> DependenciesLoadingScreen() + is DependenciesLoad.Failed -> DependenciesErrorScreen( + message = load.message, + onClose = { finish() } + ) + is DependenciesLoad.Ready -> EditorScreen( + configuration = configuration, + dependencies = load.dependencies, + accountId = accountId, + coroutineScope = this.lifecycleScope, + onClose = { finish() }, + onGutenbergViewCreated = { view -> + gutenbergView = view + setupFileChooserListener(view) + } + ) + } } } } + private sealed interface DependenciesLoad { + data object Loading : DependenciesLoad + data class Failed(val message: String) : DependenciesLoad + data class Ready(val dependencies: EditorDependencies?) : DependenciesLoad + } + private fun setupFileChooserListener(view: GutenbergView) { view.setOnFileChooserRequestedListener { intent, _ -> filePickerLauncher.launch(intent) @@ -141,6 +164,28 @@ class EditorActivity : ComponentActivity() { } } +@Composable +private fun DependenciesLoadingScreen() { + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + CircularProgressIndicator() + } +} + +@Composable +private fun DependenciesErrorScreen(message: String, onClose: () -> Unit) { + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Column( + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.spacedBy(16.dp), + modifier = Modifier.padding(24.dp) + ) { + Text("Couldn't load editor data") + Text(message) + TextButton(onClick = onClose) { Text("Close") } + } + } +} + @OptIn(ExperimentalMaterial3Api::class) @Composable fun EditorScreen( From 17f0c17b74fde7c70f353dc115282303dc39c823 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 4 May 2026 14:14:37 -0600 Subject: [PATCH 4/4] chore(android): satisfy Detekt after StrictMode refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI Detekt flagged three issues from the prior commits: - AuthenticationManager.handleOAuthCallback was 61 lines (max 60). Extract a persistOAuthAccount helper that takes the unpacked blogId / accessToken and handles the repo write + OAuth scratch-state cleanup. - EditorActivity.onCreate was 63 lines (max 60). Extract the dependencies loader to loadDependenciesFromIntent, which returns the state holder. - The catch (Exception) around EditorDependenciesSerializer.readFromDisk was redundant — the serializer already swallows exceptions internally and returns null on failure. Drop the try/catch and treat a null return from a non-null path as the Failed signal. --- .../gutenbergkit/AuthenticationManager.kt | 57 +++++++++++-------- .../example/gutenbergkit/EditorActivity.kt | 46 +++++++++------ 2 files changed, 62 insertions(+), 41 deletions(-) diff --git a/android/app/src/main/java/com/example/gutenbergkit/AuthenticationManager.kt b/android/app/src/main/java/com/example/gutenbergkit/AuthenticationManager.kt index 8ae61fd77..1b85fbb72 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/AuthenticationManager.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/AuthenticationManager.kt @@ -217,31 +217,9 @@ class AuthenticationManager( when (tokenResult) { is WpRequestResult.Success -> { - val tokenResponse = tokenResult.response.data - val blogId = tokenResponse.blogId - ?: throw OAuthException.MissingBlogId() - val discoverySuccess = currentDiscoverySuccess - val siteHost = discoverySuccess?.parsedSiteUrl?.toURL()?.toURI()?.host - ?: throw OAuthException.MissingSiteHost() - - val siteApiRoot = wordpressComSiteApiRoot(blogId) - - val account = Account.WpCom( - id = 0u, - username = siteHost, - token = tokenResponse.accessToken, - siteApiRoot = siteApiRoot - ) - - val stored = app.withAccountRepository { repo -> - repo.store(account) - repo.all().last() - } - - currentOAuthConfig = null - currentOAuthState = null - currentDiscoverySuccess = null - + val token = tokenResult.response.data + val blogId = token.blogId ?: throw OAuthException.MissingBlogId() + val stored = persistOAuthAccount(blogId, token.accessToken) withContext(Dispatchers.Main) { callback.onAuthenticationSuccess(stored) } @@ -260,6 +238,35 @@ class AuthenticationManager( } } + /** + * Persists an [Account.WpCom] derived from the given OAuth blogId and + * access token via [GutenbergKitApplication.withAccountRepository] (off + * the main thread) and clears OAuth scratch state. Returns the stored + * [Account]. + */ + private suspend fun persistOAuthAccount(blogId: ULong, accessToken: String): Account { + val siteHost = currentDiscoverySuccess?.parsedSiteUrl?.toURL()?.toURI()?.host + ?: throw OAuthException.MissingSiteHost() + + val account = Account.WpCom( + id = 0u, + username = siteHost, + token = accessToken, + siteApiRoot = wordpressComSiteApiRoot(blogId) + ) + + val stored = app.withAccountRepository { repo -> + repo.store(account) + repo.all().last() + } + + currentOAuthConfig = null + currentOAuthState = null + currentDiscoverySuccess = null + + return stored + } + private fun loadOAuthCredentials(): OAuthCredentials? { return try { val json = context.assets.open("wp_com_oauth_credentials.json") diff --git a/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt b/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt index 04ed02594..1e8603ac4 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt @@ -39,6 +39,7 @@ import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable +import androidx.compose.runtime.MutableState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -107,22 +108,7 @@ class EditorActivity : ComponentActivity() { // The dependencies blob is on disk; deserialize it off the main thread // and feed the editor once it's ready. - val dependenciesPath = intent.getStringExtra(EXTRA_DEPENDENCIES_PATH) - val dependenciesLoad = mutableStateOf( - if (dependenciesPath == null) DependenciesLoad.Ready(null) else DependenciesLoad.Loading - ) - if (dependenciesPath != null) { - lifecycleScope.launch { - dependenciesLoad.value = try { - val deps = withContext(Dispatchers.IO) { - EditorDependenciesSerializer.readFromDisk(dependenciesPath) - } - DependenciesLoad.Ready(deps) - } catch (e: Exception) { - DependenciesLoad.Failed(e.message ?: e::class.java.simpleName) - } - } - } + val dependenciesLoad = loadDependenciesFromIntent() // Optional account ID for REST API persistence (set when launched from PostsListActivity) val accountId = intent.getLongExtra(EXTRA_ACCOUNT_ID, -1L).takeIf { it >= 0 }?.toULong() @@ -157,6 +143,34 @@ class EditorActivity : ComponentActivity() { data class Ready(val dependencies: EditorDependencies?) : DependenciesLoad } + /** + * Reads [EditorDependencies] from the intent's [EXTRA_DEPENDENCIES_PATH] + * off the main thread. Returns a state holder that flips from + * [DependenciesLoad.Loading] to [DependenciesLoad.Ready] or + * [DependenciesLoad.Failed]. When no path was provided, starts in + * [DependenciesLoad.Ready] with null dependencies (the editor will load + * from the network). + */ + private fun loadDependenciesFromIntent(): MutableState { + val path = intent.getStringExtra(EXTRA_DEPENDENCIES_PATH) + val state = mutableStateOf( + if (path == null) DependenciesLoad.Ready(null) else DependenciesLoad.Loading + ) + if (path != null) { + lifecycleScope.launch { + val deps = withContext(Dispatchers.IO) { + EditorDependenciesSerializer.readFromDisk(path) + } + state.value = if (deps != null) { + DependenciesLoad.Ready(deps) + } else { + DependenciesLoad.Failed("The editor data file was missing or could not be read.") + } + } + } + return state + } + private fun setupFileChooserListener(view: GutenbergView) { view.setOnFileChooserRequestedListener { intent, _ -> filePickerLauncher.launch(intent)