Fix store installs, update offers and in-app browsing (#825)

Three of these are what borrowing `com.android.shell`'s uid costs the manager.
Below API 33 `ContextCompat.registerReceiver` demands
`<package>.DYNAMIC_RECEIVER_NOT_EXPORTED_PERMISSION`, which under the host is
nobody's, so every store install threw before `commit()`; both installers
register by hand now, under a UUID-named action. `createSessionInternal` skips
`INSTALL_REPLACE_EXISTING` for `SHELL_UID`, so a module already on the device
failed with `ALREADY_EXISTS` -- that branch is unchanged from API 27 to AOSP
main, so a store update has never worked parasitically. And `AwSettings` reads
`checkSelfPermission(INTERNET)` in its constructor, which AOSP's Shell did not
request until android-12, so every in-app page failed as `ERR_CACHE_MISS` on a
device whose networking is fine; the context a WebView is built with now answers
that one question, and only where the platform says no.

The fourth is that an offer is decided by numbers a git tag states, which nothing
obliges to be the ones in the APK's manifest, and no rule over `(code, name)` can
bridge that: a module that never changes its tag code is only ever seen to update
through the name clause, and one that reuses a versionName across several codes
only through the code clause. So the Store stops inferring and records instead --
the release it installed, and the version the device reported once it was on --
read in `StoreEntry.upgradable` so every count, filter and badge agrees. Where an
offer names the version already installed it is worded as a reinstall rather than
as an update from a version to itself, which is true whether the tag disagrees
with the manifest or the release is a genuine rebuild. All eighteen locales are
filled in.

Fixes #823.

Author: JingMatrix

manager/src/main/kotlin/org/matrix/vector/manager/data/model/RepoModels.kt

@@ -137,10 +137,29 @@ data class ReleaseAsset(
  */
 data class RepoVersion(val versionCode: Long, val versionName: String) {
 
+    /** The tag this was read from, which is also how [StoreInstall] writes one back down. */
+    val tag: String
+        get() = "$versionCode-$versionName"
+
     fun upgradableOver(installedCode: Long, installedName: String): Boolean =
         versionCode > installedCode ||
             (versionCode == installedCode && installedName.replace(' ', '_') != versionName)
 
+    /**
+     * Whether installing this would leave the reader on the version they already have, by name.
+     *
+     * Which is all the offer can be worded as when it is true. Two different things reach here — a
+     * rebuild of the same version under a higher code, and a tag whose code is simply not the APK's
+     * — and nothing in either number tells them apart, so the wording has to be true of both. What
+     * is certain in both is where the reader ends up: on this version name again.
+     *
+     * The underscores are the same normalisation [upgradableOver] applies, and for the same reason:
+     * a git tag cannot carry a space, so an author whose versionName has one writes it with an
+     * underscore.
+     */
+    fun sameVersionAs(installed: RepoVersion?): Boolean =
+        installed != null && installed.versionName.replace(' ', '_') == versionName
+
     companion object {
         fun parse(raw: String?): RepoVersion? {
             val text = raw?.takeIf { it.isNotBlank() } ?: return null
@@ -152,6 +171,35 @@ data class RepoVersion(val versionCode: Long, val versionName: String) {
     }
 }
 
+/**
+ * A release this manager installed, and what the device said the module was afterwards.
+ *
+ * Two versions, because they are not the same kind of fact and need not be the same number:
+ * [release] is what a tag claimed, [installed] is what the APK inside it turned out to be.
+ *
+ * That difference is the whole reason this is recorded. The comparison above believes the tag, and
+ * nothing obliges an author to tag a release with the version their manifest actually states. Where
+ * the two disagree the offer cannot be satisfied by taking it: installing leaves the device on a
+ * version the tag still claims to beat, so the row asks again, and again, for ever.
+ *
+ * Nor can it be settled by reading the two numbers harder, because both halves of the comparison
+ * are load-bearing for someone: a module that never changes its tag code is only ever seen to
+ * update through the name clause, and one that reuses a versionName across several codes only
+ * through the code clause. Any rule over `(code, name)` is wrong for one of them.
+ *
+ * So the Store stops inferring and records instead. An offer it has already installed, on a device
+ * still reporting what that install produced, is one the reader has taken.
+ *
+ * [installed] is what makes the record expire on its own: it is checked against what the device
+ * reports now, so a module replaced from anywhere else stops matching and the offer comes back.
+ */
+data class StoreInstall(val release: RepoVersion, val installed: RepoVersion) {
+
+    /** Whether this note says [latest] is already here, as [current]. */
+    fun satisfies(latest: RepoVersion?, current: RepoVersion?): Boolean =
+        release == latest && installed == current
+}
+
 /**
  * One row of the Store: a catalogue entry, plus what this device has to say about it.
  *
@@ -164,10 +212,30 @@ data class StoreEntry(
     val installed: RepoVersion?,
     /** The reader asked not to be told about this one again. */
     val updatesMuted: Boolean = false,
+    /** What this manager last installed here, if this manager is what installed it. */
+    val storeInstall: StoreInstall? = null,
 ) {
+
+    /** The newest release is one we installed, and the device still reports what it left behind. */
+    private val alreadyInstalled: Boolean
+        get() = storeInstall?.satisfies(latest, installed) == true
+
+    /**
+     * The offer would not change which version this device says it has. See [sameVersionAs].
+     *
+     * Read by everything that *words* an offer, because `1.1.1 → 1.1.1` is a sentence the app cannot
+     * mean. [upgradable] deliberately does not consult it: whether to offer at all is a different
+     * question from what to call it, and a rebuild is worth offering.
+     */
+    val sameVersion: Boolean
+        get() = latest?.sameVersionAs(installed) == true
+
     /**
      * There is a newer version *and* the reader wants to hear about it.
      *
+     * A release this manager itself installed is not a newer version, whatever the two numbers say;
+     * see [StoreInstall].
+     *
      * Muting is folded in here rather than at each place that reads this, because every list and
      * count that mentions updates reads it — the Store's header count, its updates filter, its row
      * badge, and the set the Modules screen badges from — and a mute that only some of them
@@ -184,6 +252,7 @@ data class StoreEntry(
             !updatesMuted &&
                 installed != null &&
                 latest != null &&
+                !alreadyInstalled &&
                 latest.upgradableOver(installed.versionCode, installed.versionName)
 }
 

manager/src/main/kotlin/org/matrix/vector/manager/data/repository/ManagerInstaller.kt

@@ -1,27 +1,21 @@
 package org.matrix.vector.manager.data.repository
 
-import android.app.PendingIntent
-import android.content.BroadcastReceiver
 import android.content.Context
-import android.content.Intent
-import android.content.IntentFilter
 import android.content.pm.PackageInstaller
-import android.os.Build
 import android.util.Log
-import androidx.core.content.ContextCompat
-import androidx.core.content.IntentCompat
 import java.io.FileInputStream
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
-import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlinx.coroutines.withContext
 import kotlinx.coroutines.withTimeoutOrNull
 import org.matrix.vector.manager.BuildConfig
 import org.matrix.vector.manager.Constants
 import org.matrix.vector.manager.ipc.DaemonClient
+import org.matrix.vector.manager.ipc.commitForResult
+import org.matrix.vector.manager.ipc.requestReplaceExisting
 
 /** Where installing the manager as an app has got to. */
 sealed interface ManagerInstallStep {
@@ -132,6 +126,9 @@ class ManagerInstaller(private val context: Context, private val daemon: DaemonC
                             // with it. A daemon serving something else cannot install it as Vector.
                             setAppPackageName(BuildConfig.MANAGER_PACKAGE_NAME)
                             if (size > 0) setSize(size)
+                            // Updating an installed manager from the host is a replace, and
+                            // parasitically the platform does not make it one for us.
+                            requestReplaceExisting()
                         }
                 sessionId = packageInstaller.createSession(params)
 
@@ -179,85 +176,25 @@ class ManagerInstaller(private val context: Context, private val daemon: DaemonC
     /**
      * Commits the session and waits for the platform's verdict.
      *
-     * Registered at runtime rather than declared, because parasitically nothing in this app's
-     * manifest exists and a declared receiver would never fire. `STATUS_PENDING_USER_ACTION` is not
-     * terminal — it means the system is asking, and the real status follows the answer. It should
-     * not arise here: the host holds `INSTALL_PACKAGES`, so the commit is silent. It is handled
-     * anyway, because the same code runs from a manager that is already installed and updating
-     * itself, where the prompt is exactly what the platform will do.
+     * `STATUS_PENDING_USER_ACTION` should not arise here — the host holds `INSTALL_PACKAGES`, so
+     * the commit is silent — but it is handled anyway, because the same code runs from a manager
+     * that is already installed and updating itself, where the prompt is exactly what the platform
+     * will do.
+     *
+     * @see commitForResult
      */
     private suspend fun commit(
         session: PackageInstaller.Session,
         sessionId: Int,
-    ): Pair<Int, String?> = suspendCancellableCoroutine { continuation ->
-        val action = "$RESULT_ACTION.$sessionId"
-        val receiver =
-            object : BroadcastReceiver() {
-                override fun onReceive(received: Context, intent: Intent) {
-                    val status =
-                        intent.getIntExtra(
-                            PackageInstaller.EXTRA_STATUS,
-                            PackageInstaller.STATUS_FAILURE,
-                        )
-                    if (status == PackageInstaller.STATUS_PENDING_USER_ACTION) {
-                        IntentCompat.getParcelableExtra(
-                                intent,
-                                Intent.EXTRA_INTENT,
-                                Intent::class.java,
-                            )
-                            ?.let { confirm ->
-                                runCatching {
-                                        context.startActivity(
-                                            confirm.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
-                                        )
-                                    }
-                                    .onFailure { e ->
-                                        Log.e(
-                                            Constants.TAG,
-                                            "actions: manager install prompt could not be started",
-                                            e,
-                                        )
-                                    }
-                            }
-                        return
-                    }
-                    runCatching { context.unregisterReceiver(this) }
-                    if (continuation.isActive) {
-                        continuation.resumeWith(
-                            Result.success(
-                                status to
-                                    intent.getStringExtra(PackageInstaller.EXTRA_STATUS_MESSAGE)
-                            )
-                        )
-                    }
-                }
-            }
-
-        ContextCompat.registerReceiver(
-            context,
-            receiver,
-            IntentFilter(action),
-            ContextCompat.RECEIVER_NOT_EXPORTED,
+    ): Pair<Int, String?> =
+        context.commitForResult(
+            session,
+            sessionId,
+            promptFailure = "actions: manager install prompt could not be started",
         )
-        continuation.invokeOnCancellation { runCatching { context.unregisterReceiver(receiver) } }
-
-        val flags =
-            PendingIntent.FLAG_UPDATE_CURRENT or
-                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) PendingIntent.FLAG_MUTABLE
-                else 0
-        val pending =
-            PendingIntent.getBroadcast(
-                context,
-                sessionId,
-                Intent(action).setPackage(context.packageName),
-                flags,
-            )
-        session.commit(pending.intentSender)
-    }
 
     private companion object {
         const val WRITE_NAME = "manager.apk"
-        const val RESULT_ACTION = "org.matrix.vector.manager.INSTALL_MANAGER_RESULT"
 
         /** What the platform calls it in `EXTRA_STATUS_MESSAGE`; see PackageManagerException. */
         const val SIGNATURE_CONFLICT = "INSTALL_FAILED_UPDATE_INCOMPATIBLE"

manager/src/main/kotlin/org/matrix/vector/manager/data/repository/ModuleInstaller.kt

@@ -1,15 +1,8 @@
 package org.matrix.vector.manager.data.repository
 
-import android.app.PendingIntent
-import android.content.BroadcastReceiver
 import android.content.Context
-import android.content.Intent
-import android.content.IntentFilter
 import android.content.pm.PackageInstaller
-import android.os.Build
 import android.util.Log
-import androidx.core.content.ContextCompat
-import androidx.core.content.IntentCompat
 import java.io.IOException
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.Dispatchers
@@ -18,12 +11,13 @@ import kotlinx.coroutines.ensureActive
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
-import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlinx.coroutines.withContext
 import okhttp3.OkHttpClient
 import okhttp3.Request
 import org.matrix.vector.manager.Constants
 import org.matrix.vector.manager.data.model.ReleaseAsset
+import org.matrix.vector.manager.ipc.commitForResult
+import org.matrix.vector.manager.ipc.requestReplaceExisting
 
 /** Where an install has got to. One at a time, because a user installs one module at a time. */
 sealed interface InstallStep {
@@ -80,6 +74,11 @@ class ModuleInstaller(private val context: Context, private val client: OkHttpCl
      * Returns true only when the platform reports the package installed. There is no resume: a
      * dropped connection costs the whole transfer, which is an acceptable trade for module APKs
      * (tens to a few hundred kilobytes) in exchange for never touching the filesystem.
+     *
+     * What became of it is recorded by the caller rather than here — see RepoRepository.readInstalled
+     * and SettingsRepository.noteStoreInstall — because the version to record has to be read the way
+     * the Store reads it, across every user, and this class talks to the platform rather than to the
+     * daemon.
      */
     suspend fun install(packageName: String, asset: ReleaseAsset): Boolean =
         withContext(Dispatchers.IO) {
@@ -102,6 +101,7 @@ class ModuleInstaller(private val context: Context, private val client: OkHttpCl
                         .apply {
                             setAppPackageName(packageName)
                             if (asset.size > 0) setSize(asset.size)
+                            requestReplaceExisting()
                         }
                 sessionId = packageInstaller.createSession(params)
 
@@ -182,82 +182,24 @@ class ModuleInstaller(private val context: Context, private val client: OkHttpCl
     /**
      * Commits the session and waits for the platform's verdict.
      *
-     * The result arrives as a broadcast, and the receiver is registered at runtime rather than
-     * declared: parasitically nothing in the manifest exists, so a declared receiver would simply
-     * never fire. `STATUS_PENDING_USER_ACTION` is not terminal — it means the system is asking the
-     * user, and the real status follows once they answer.
+     * @see commitForResult
      */
     private suspend fun commit(
         session: PackageInstaller.Session,
         sessionId: Int,
         packageName: String,
-    ): Pair<Int, String?> = suspendCancellableCoroutine { continuation ->
-        val action = "$RESULT_ACTION.$sessionId"
-        val receiver =
-            object : BroadcastReceiver() {
-                override fun onReceive(received: Context, intent: Intent) {
-                    val status =
-                        intent.getIntExtra(
-                            PackageInstaller.EXTRA_STATUS,
-                            PackageInstaller.STATUS_FAILURE,
-                        )
-                    if (status == PackageInstaller.STATUS_PENDING_USER_ACTION) {
-                        _state.value = InstallStep.Confirming(packageName)
-                        IntentCompat.getParcelableExtra(intent, Intent.EXTRA_INTENT, Intent::class.java)
-                            ?.let { confirm ->
-                                runCatching {
-                                        context.startActivity(
-                                            confirm.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
-                                        )
-                                    }
-                                    .onFailure { e ->
-                                        Log.e(
-                                            Constants.TAG,
-                                            "store: install prompt for $packageName could not be started",
-                                            e,
-                                        )
-                                    }
-                            }
-                        return
-                    }
-                    runCatching { context.unregisterReceiver(this) }
-                    if (continuation.isActive) {
-                        continuation.resumeWith(
-                            Result.success(
-                                status to
-                                    intent.getStringExtra(PackageInstaller.EXTRA_STATUS_MESSAGE)
-                            )
-                        )
-                    }
-                }
-            }
-
-        ContextCompat.registerReceiver(
-            context,
-            receiver,
-            IntentFilter(action),
-            ContextCompat.RECEIVER_NOT_EXPORTED,
-        )
-        continuation.invokeOnCancellation { runCatching { context.unregisterReceiver(receiver) } }
-
-        val flags =
-            PendingIntent.FLAG_UPDATE_CURRENT or
-                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) PendingIntent.FLAG_MUTABLE
-                else 0
-        val pending =
-            PendingIntent.getBroadcast(
-                context,
-                sessionId,
-                Intent(action).setPackage(context.packageName),
-                flags,
-            )
-        session.commit(pending.intentSender)
-    }
+    ): Pair<Int, String?> =
+        context.commitForResult(
+            session,
+            sessionId,
+            promptFailure = "store: install prompt for $packageName could not be started",
+        ) {
+            _state.value = InstallStep.Confirming(packageName)
+        }
 
     private companion object {
         const val WRITE_NAME = "module.apk"
         const val CHUNK_BYTES = 64 * 1024
         const val PROGRESS_STEP_BYTES = 256L * 1024
-        const val RESULT_ACTION = "org.matrix.vector.manager.INSTALL_RESULT"
     }
 }