ReviewOS

also looking at this

craft-native/craft

fix(mobile): preserve native editing, navigation, and durable media requests

#369
Merged chris wants to merge codex/native-vertical-scroll-tab-swipes into main
36 files +864 -185

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

packages/android/templates/CraftNativeShell.kt.templatemodified+36-12
Changes to packages/android/templates/CraftNativeShell.kt.template
@@ -1248,6 +1248,7 @@ internal object CraftNativeSymbols {
12481248 * transfer finishes on its own; the next keep answers from the copy.
12491249 */
12501250internal object CraftNativeMedia {
1251 const val ATTEMPT_SETTLED = "craft.media.attemptSettled"
12511252 private const val TAG = "craft-media"
12521253 private const val PREFS = "craft.media"
12531254 private val lock = Any()
@@ -1315,7 +1316,7 @@ internal object CraftNativeMedia {
13151316 try {
13161317 if (generation(app) == started) {
13171318 val pending = work.getWorkInfosForUniqueWork(name).get().any {
1318 "$TAG:generation:$started" in it.tags &&
1319 "$TAG:generation:$started" in it.tags && !it.progress.getBoolean(ATTEMPT_SETTLED, false) &&
13191320 (it.state == androidx.work.WorkInfo.State.RUNNING ||
13201321 (it.state == androidx.work.WorkInfo.State.ENQUEUED && it.runAttemptCount == 0))
13211322 }
@@ -1363,12 +1364,22 @@ internal object CraftNativeMedia {
13631364 * file where it stopped, moves it into place whole, and answers whoever
13641365 * waits. RETRY leaves the part file for the next attempt.
13651366 */
1366 fun transfer(context: Context, address: String, headers: JSONObject, started: Int, attempt: Int, stopped: () -> Boolean): Outcome {
1367 fun transfer(context: Context, address: String, headers: JSONObject, started: Int, attempt: Int, beforeAnswer: () -> Unit = {}, stopped: () -> Boolean): Outcome {
1368 fun answer(result: CraftNativeCapabilityResult) {
1369 // The callback can immediately call keep again. Publish completion
1370 // before that callback, while WorkManager may still say RUNNING.
1371 synchronized(lock) {
1372 // A keep registered after this marker belongs to the next
1373 // attempt; it must not enter the old callback batch.
1374 beforeAnswer()
1375 deliver(context, address, started, result)
1376 }
1377 }
13671378 // clear() already answered the old generation's waiters. Never answer
13681379 // a new athlete's waiters from an obsolete worker for the same URL.
13691380 if (generation(context) != started) return Outcome.DONE
13701381 local(context, address)?.let {
1371 deliver(context, address, started, CraftNativeCapabilityResult(data = describe(address, it)))
1382 answer(CraftNativeCapabilityResult(data = describe(address, it)))
13721383 return Outcome.DONE
13731384 }
13741385 val writer = writers.getOrPut(address) { Any() }
@@ -1378,7 +1389,7 @@ internal object CraftNativeMedia {
13781389 if (generation(context) != started) return Outcome.DONE
13791390 if (stopped()) return Outcome.RETRY
13801391 local(context, address)?.let {
1381 deliver(context, address, started, CraftNativeCapabilityResult(data = describe(address, it)))
1392 answer(CraftNativeCapabilityResult(data = describe(address, it)))
13821393 return Outcome.DONE
13831394 }
13841395 val target = file(context, address)
@@ -1397,22 +1408,22 @@ internal object CraftNativeMedia {
13971408 }
13981409 if (full) {
13991410 staging.delete()
1400 deliver(context, address, started, CraftNativeCapabilityResult(code = "NO_SPACE", message = error.message ?: "The phone is out of space"))
1411 answer(CraftNativeCapabilityResult(code = "NO_SPACE", message = error.message ?: "The phone is out of space"))
14011412 return Outcome.DONE
14021413 }
14031414 if (stopped()) return Outcome.RETRY
14041415 // Lost the connection part way: the part waits for the next attempt.
1405 deliver(context, address, started, if (!online(context)) queued(address, "offline") else CraftNativeCapabilityResult(code = if (error is java.net.SocketTimeoutException) "TIMEOUT" else "NETWORK_ERROR", message = error.message ?: "media.keep failed"))
1416 answer(if (!online(context)) queued(address, "offline") else CraftNativeCapabilityResult(code = if (error is java.net.SocketTimeoutException) "TIMEOUT" else "NETWORK_ERROR", message = error.message ?: "media.keep failed"))
14061417 return if (attempt < 8) Outcome.RETRY else Outcome.DONE.also { staging.delete() }
14071418 }
14081419 when (fetched) {
14091420 is CraftResumableDownload.Fetched.Stopped, is CraftResumableDownload.Fetched.Restart -> return Outcome.RETRY
14101421 is CraftResumableDownload.Fetched.Refused -> {
1411 deliver(context, address, started, CraftNativeCapabilityResult(data = JSONObject().put("ok", false).put("url", address).put("status", fetched.status)))
1422 answer(CraftNativeCapabilityResult(data = JSONObject().put("ok", false).put("url", address).put("status", fetched.status)))
14121423 return Outcome.DONE
14131424 }
14141425 is CraftResumableDownload.Fetched.Incomplete -> {
1415 deliver(context, address, started, CraftNativeCapabilityResult(code = "NETWORK_ERROR", message = "media.keep received an incomplete file"))
1426 answer(CraftNativeCapabilityResult(code = "NETWORK_ERROR", message = "media.keep received an incomplete file"))
14161427 return if (attempt < 8) Outcome.RETRY else Outcome.DONE.also { staging.delete() }
14171428 }
14181429 is CraftResumableDownload.Fetched.Complete -> Unit
@@ -1424,11 +1435,11 @@ internal object CraftNativeMedia {
14241435 }
14251436 target.delete()
14261437 if (!staging.renameTo(target)) {
1427 deliver(context, address, started, CraftNativeCapabilityResult(code = "IO_ERROR", message = "media.keep could not store the file"))
1438 answer(CraftNativeCapabilityResult(code = "IO_ERROR", message = "media.keep could not store the file"))
14281439 return Outcome.DONE
14291440 }
14301441 }
1431 deliver(context, address, started, CraftNativeCapabilityResult(data = describe(address, target)))
1442 answer(CraftNativeCapabilityResult(data = describe(address, target)))
14321443 return Outcome.DONE
14331444 }
14341445 }
@@ -1584,8 +1595,21 @@ internal class CraftNativeMediaWorker(context: Context, parameters: androidx.wor
15841595 override fun doWork(): Result {
15851596 val address = inputData.getString("address") ?: return Result.failure()
15861597 val headers = runCatching { JSONObject(inputData.getString("headers") ?: "{}") }.getOrDefault(JSONObject())
1587 val outcome = CraftNativeMedia.transfer(applicationContext, address, headers, inputData.getInt("generation", 0), runAttemptCount) { isStopped }
1588 return if (outcome == CraftNativeMedia.Outcome.RETRY) Result.retry() else Result.success()
1598 val started = inputData.getInt("generation", 0)
1599 return try {
1600 setProgressAsync(androidx.work.workDataOf(CraftNativeMedia.ATTEMPT_SETTLED to false)).get()
1601 val outcome = CraftNativeMedia.transfer(applicationContext, address, headers, started, runAttemptCount,
1602 beforeAnswer = { setProgressAsync(androidx.work.workDataOf(CraftNativeMedia.ATTEMPT_SETTLED to true)).get() }
1603 ) { isStopped }
1604 if (outcome == CraftNativeMedia.Outcome.RETRY) Result.retry() else Result.success()
1605 } catch (error: Exception) {
1606 if (error is InterruptedException) Thread.currentThread().interrupt()
1607 // A replacement owns the address's new waiters. A real progress
1608 // storage failure still needs an answer instead of a lost callback.
1609 if (!isStopped) CraftNativeMedia.deliver(applicationContext, address, started,
1610 CraftNativeCapabilityResult(code = "IO_ERROR", message = error.message ?: "media.keep could not store worker progress"))
1611 if (isStopped) Result.retry() else Result.failure()
1612 }
15891613 }
15901614}
15911615
packages/typescript/fixtures/media-cache/media-cache.ktmodified+28-1
Changes to packages/typescript/fixtures/media-cache/media-cache.kt
@@ -1,3 +1,14 @@
1/** Holds a completed attempt in RUNNING, before WorkManager commits its result. */
2class SettledMediaAttemptWorker(context: Context, parameters: androidx.work.WorkerParameters) : androidx.work.Worker(context, parameters) {
3 private val released = java.util.concurrent.CountDownLatch(1)
4 override fun doWork(): Result {
5 setProgressAsync(androidx.work.workDataOf("craft.media.attemptSettled" to true)).get()
6 released.await(30, java.util.concurrent.TimeUnit.SECONDS)
7 return Result.retry()
8 }
9 override fun onStopped() { released.countDown() }
10}
11
112class DownloadProbe : android.app.Instrumentation() {
213 override fun onCreate(arguments: android.os.Bundle?) {
314 super.onCreate(arguments)
@@ -113,8 +124,24 @@ class DownloadProbe : android.app.Instrumentation() {
113124 check(!keep(retry), "a separate interrupted transfer reports its failure")
114125 check(keep(retry), "an explicit retry runs immediately, without waiting for backoff")
115126 check(root.listFiles()?.none { it.name.endsWith(".part") } != false, "finished retries leave no staging files")
116 val queued = "$base/queued.mp4"
117127 val work = androidx.work.WorkManager.getInstance(targetContext)
128 val settled = "$base/settled.mp4"
129 val settledName = "craft-media:${CraftNativeMedia.digest(settled)}:any"
130 val settledRequest = androidx.work.OneTimeWorkRequestBuilder<SettledMediaAttemptWorker>()
131 .addTag("craft-media").addTag("craft-media:${CraftNativeMedia.digest(settled)}")
132 .addTag("craft-media:generation:${CraftNativeMedia.generation(targetContext)}").build()
133 work.enqueueUniqueWork(settledName, androidx.work.ExistingWorkPolicy.KEEP, settledRequest).result.get()
134 val settledDeadline = System.currentTimeMillis() + 10000
135 var settledInfo = work.getWorkInfoById(settledRequest.id).get()!!
136 while ((settledInfo.state != androidx.work.WorkInfo.State.RUNNING || !settledInfo.progress.getBoolean("craft.media.attemptSettled", false)) && System.currentTimeMillis() < settledDeadline) {
137 Thread.sleep(25)
138 settledInfo = work.getWorkInfoById(settledRequest.id).get()!!
139 }
140 check(settledInfo.state == androidx.work.WorkInfo.State.RUNNING && settledInfo.progress.getBoolean("craft.media.attemptSettled", false), "the settled attempt still has WorkManager's running identity")
141 check(keep(settled), "an explicit keep replaces a settled running attempt immediately")
142 check(work.getWorkInfoById(settledRequest.id).get()!!.state == androidx.work.WorkInfo.State.CANCELLED, "the settled work identity was replaced, not joined")
143
144 val queued = "$base/queued.mp4"
118145 val queuedName = "craft-media:${CraftNativeMedia.digest(queued)}:any"
119146 val queuedRequest = androidx.work.OneTimeWorkRequestBuilder<CraftNativeMediaWorker>()
120147 .setInitialDelay(10, java.util.concurrent.TimeUnit.SECONDS)
packages/typescript/src/mobile-media-testing.tsmodified+1-0
Changes to packages/typescript/src/mobile-media-testing.ts
@@ -236,6 +236,7 @@ dependencies { implementation("androidx.work:work-runtime-ktx:2.9.1") }`)
236236 if (counts.get('/android/restart.mp4') !== 1) throw new Error('Android process did not start interrupted download')
237237 await probe('recover')
238238 if (counts.get('/android/retry.mp4') !== 2) throw new Error('Android explicit retry did not fetch once more immediately')
239 if (counts.get('/android/settled.mp4') !== 1) throw new Error('Android explicit keep did not replace the settled running attempt exactly once')
239240 if (!resumed.has('/android/restart.mp4')) throw new Error('Android did not resume the interrupted download from its part file')
240241 if (counts.get('/android/slow.mp4') !== 1) throw new Error('Android duplicate fetch: keep did not coalesce')
241242 if (counts.get('/android/queued.mp4') !== 1) throw new Error('Android queued work was replaced instead of joined')