Ask the scope table, not the uid map, whether an install matters

A scope row names a package; the cache derived from it is keyed by uid.
Uninstalling a target and installing it again crosses that gap: the app
returns under a new uid, the entry under the old one went when
ACTION_UID_REMOVED rebuilt the cache, and the install handler — which
recognised a target by looking its uid up in that same cache — matched
nothing and asked for no rebuild. The row it is still configured under
sat there unread.

Which left the configuration correct and inert. The manager reads the
table, so it went on showing the target ticked, truthfully; the draft
and the saved set agreed, so no apply bar appeared and there was no
difference to write. Nothing on that screen could put it right. It took
a scope edit on some other module, or the next boot, to rebuild the map
and bring the app back into a scope it had never left.

Removal now asks for its own rebuild for the same reason it should
always have: the entry was cleaned up only incidentally, by the
ACTION_UID_REMOVED that follows a uid actually being retired — which a
package sharing one never is, and which a daemon that was not running
never hears. What is left behind is keyed by a uid that no longer names
the app, and Android hands a freed app id to the next installer to ask.

The uid test stays behind the new one for the scope rows no table holds:
a module in its own scope, and the self-scope derived for a legacy one.

Author: JingMatrix

daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt

@@ -312,6 +312,16 @@ object VectorService : IVectorDaemon.Stub() {
           if (isRemovedForAllUsers && ModuleDatabase.removeModule(moduleName)) {
             // If it was in our DB and we successfully removed it, we treat it as an Xposed module.
             isXposedModule = true
+          } else if (ModuleDatabase.isEnabledScopeTarget(moduleName)) {
+            // A target's scope rows outlive it deliberately — they are what puts the module back
+            // when the app returns — but the entry the cache derived from them must not, and this
+            // branch asked for nothing. What cleaned it up was ACTION_UID_REMOVED, further down,
+            // and only ever incidentally: it fires when the *uid* is retired, which is neither
+            // this event nor guaranteed to follow it. A package that shares a uid with another
+            // retires none, and a daemon not running to hear the one that is sent never learns of
+            // it. The entry left behind is keyed by a uid that no longer names this app, and
+            // Android does hand a freed app id to the next installer that asks.
+            ConfigCache.requestCacheUpdate()
           }
         }
       }
@@ -323,9 +333,23 @@ object VectorService : IVectorDaemon.Stub() {
               ModuleDatabase.updateModuleApkPath(
                   moduleName, ConfigCache.getModuleApkPath(appInfo), false)
         } else {
-          if (ConfigCache.state.scopes.keys.any { it.uid == uid }) {
-            // If not a module, but it's an app that was previously a "scope" (target)
-            // for a module, we need to refresh the cache.
+          // If not a module, but it's an app some module targets, the cache has to be rebuilt so
+          // that the target is resolved again.
+          //
+          // The configuration is asked first, and by name, because the cache alone gets the one
+          // case that matters most wrong. Its scopes are keyed by uid: an app that is uninstalled
+          // and installed again comes back under a *new* uid, and the entry under the old one went
+          // when ACTION_UID_REMOVED rebuilt the cache — so no key matched, no rebuild was asked
+          // for, and the app was left out of the scope map it is still configured to be in. The
+          // row was never deleted, so the manager went on showing the target ticked, correctly,
+          // beside an app nothing was being loaded into; there was no difference to apply and
+          // therefore no way to put it right from the manager at all. It stayed that way until
+          // something unrelated rebuilt the cache — any scope edit, or the next boot.
+          //
+          // The uid test is kept behind it for the rows no scope table holds: a module in its own
+          // scope, and the self-scope a legacy module gets derived rather than stored.
+          if ((moduleName != null && ModuleDatabase.isEnabledScopeTarget(moduleName)) ||
+              ConfigCache.state.scopes.keys.any { it.uid == uid }) {
             ConfigCache.requestCacheUpdate()
           }
 

daemon/src/main/kotlin/org/matrix/vector/daemon/data/ModuleDatabase.kt

@@ -143,6 +143,28 @@ object ModuleDatabase {
     return rows
   }
 
+  /**
+   * Whether any enabled module names [appPackage] as a target, in any user.
+   *
+   * Asked of the configuration rather than of [ConfigCache], because the cache cannot answer it.
+   * Its scope map is keyed by uid, and a package that was uninstalled and installed again comes
+   * back under a *new* one — while the row here, keyed by name, has been waiting for it the whole
+   * time. Matching the cache by uid therefore said "not a target" about the one case that most
+   * needs a rebuild, and the app stayed unhooked until something unrelated rebuilt the cache.
+   */
+  fun isEnabledScopeTarget(appPackage: String): Boolean =
+      dbHelper.readableDatabase
+          .query(
+              "scope INNER JOIN modules ON scope.mid = modules.mid",
+              arrayOf("1"),
+              "app_pkg_name = ? AND enabled = 1",
+              arrayOf(appPackage),
+              null,
+              null,
+              null,
+              "1")
+          .use { it.moveToFirst() }
+
   /** Enabled modules scoped to the system framework, with the path last resolved for each. */
   fun systemServerModuleRows(): List<EnabledModuleRow> {
     val rows = mutableListOf<EnabledModuleRow>()