fix(deck-picker): do not block the main thread on startup during sync

[criticalAY]

Note

Assisted-by: Claude Opus 4.8 (some diagnosis and a few parts of code)

Purpose / Description

Fixes an ANR when the DeckPicker is recreated while a sync is stuck on an unresponsive server (for example AnkiWeb being down).

Fixes

• Fixes If ankiweb does not respond, the UI does not respond #21305

Approach

See commit

How Has This Been Tested?

Unit test

Learning (optional, can help others)

NA

Checklist

Please, go through these checks before submitting the PR.

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[david-allison]

Could you provide reproduction instructions (probably a patch which adds a delay) so we can reproduce it on a physical device.

The startup path has historically been nasty. This initially looks good (and the ViewModel makes things easier), but I'd like to run it through a debugger to confirm.

[criticalAY]

Subject: [PATCH] repro: sync anr
---
Index: AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt b/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt	(revision 25da95364f62624be98404df64afae4d699f134c)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt	(date 1783413600156)
@@ -159,6 +159,8 @@
             manualCancelButton = R.string.dialog_cancel,
         ) {
             withCol {
+                Timber.w("#21305 repro: holding the collection queue for 60s")
+                Thread.sleep(60_000)
                 syncCollection(auth2, syncMedia = false) // media is synced by SyncMediaWorker
             }
         }
Index: AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt	(revision 25da95364f62624be98404df64afae4d699f134c)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt	(date 1783413571943)
@@ -522,17 +522,11 @@
         }
 
         Timber.d("handleStartup: Continuing after permission granted")
-        viewModelScope.launch {
-            // opening the collection waits on the collection queue, which a sync stuck on an
-            // unresponsive server can hold for a long time. Waiting on the main thread here
-            // froze the DeckPicker when it was recreated
-            val failure =
-                withContext(Dispatchers.IO) {
-                    InitialActivity.getStartupFailureType(environment::initializeAnkiDroidFolder)
-                }
+        run {
+            val failure = InitialActivity.getStartupFailureType(environment::initializeAnkiDroidFolder)
             if (failure != null) {
                 flowOfStartupResponse.value = StartupResponse.FatalError(failure)
-                return@launch
+                return
             }
 
             // successful startup

[david-allison]

One suspected blocker, a few nits on the test

[david-allison]

LGTM, cheers!

[criticalAY]

Ok let me resolve the conflicts/issues and lets get it in

[criticalAY]

Thanks for the test, the first deck refresh could run before the async startup opened the collection, so Success now refreshes the deck list and rebuilds the menu (the menu had the same gap with an empty collection).

Umm also fixed theCI failure, a test racing the async startup, by making the shared start helper wait for DeckPicker's startup; the trade-off is a second deck load on each DeckPicker creation, which I can avoid if you'd prefer.

[david-allison]

Let's do it right, now the cost of doing it right is so much less

First impressions matter

done, re-reviewing

[david-allison]

Another 'double load' issue

Index: AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerStartupTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerStartupTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerStartupTest.kt
--- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerStartupTest.kt	(revision 50333731d56f14084506cce1c7f7b11678ba5c3e)
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerStartupTest.kt	(date 1790713798545)
@@ -10,12 +10,15 @@
 import com.ichi2.anki.deckpicker.DeckPickerViewModel.AnkiDroidEnvironment
 import com.ichi2.anki.testutils.SingleViewModelFactory
 import com.ichi2.testutils.BackupManagerTestUtilities
+import kotlinx.coroutines.Job
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.kotlin.any
 import org.mockito.kotlin.doAnswer
 import org.mockito.kotlin.doCallRealMethod
 import org.mockito.kotlin.spy
+import org.mockito.kotlin.times
+import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 import org.robolectric.Robolectric
 import org.robolectric.annotation.Config
@@ -30,6 +33,39 @@
 class DeckPickerStartupTest : RobolectricTest() {
     override fun getCollectionStorageMode() = CollectionStorageMode.ON_DISK
 
+    @Test
+    fun `startup finishing before the first resume loads the deck list once`() {
+        setIntroductionSlidesShown(true)
+        InitialActivity.setUpgradedToLatestVersion(getPreferences())
+        getPreferences().edit { putBoolean("backupPromptDisabled", true) }
+        BackupManagerTestUtilities.setupSpaceForBackup(targetContext)
+        addBasicNote("startup", "ordering")
+
+        val viewModel = spy(DeckPickerViewModel())
+        var resumeRefresh: Job? = null
+        doAnswer { invocation ->
+            (invocation.callRealMethod() as Job).also { resumeRefresh = it }
+        }.whenever(viewModel).updateDeckList()
+
+        val controller = Robolectric.buildActivity(DeckPicker::class.java)
+        saveControllerForCleanup(controller)
+        ViewModelProvider(controller.get(), SingleViewModelFactory.create(viewModel))[DeckPickerViewModel::class.java]
+        try {
+            // Let startup finish while STARTED, before the first onResume requests a refresh.
+            controller.create().start()
+            advanceRobolectricLooperUntil {
+                viewModel.startupJob?.isCompleted == true && viewModel.loadDeckCounts?.isCompleted == true
+            }
+            verify(viewModel, times(1)).reloadDeckCounts()
+
+            controller.resume().visible()
+            advanceRobolectricLooperUntil { resumeRefresh?.isCompleted == true }
+            verify(viewModel, times(1)).reloadDeckCounts()
+        } finally {
+            BackupManagerTestUtilities.reset()
+        }
+    }
+
     @Test
     fun `decks load when startup opens the collection after the initial refresh`() =
         runTest {