fix: #319 install queue gets stuck with multiple concurrent downloads

- Protect InstallCancelled from being overwritten by stale native events - Preserve watchdog timestamps across state transitions to prevent timeout reset - Clear install slot in orphan handler and sync to unblock the queue - Add watchdog coverage for ReadyToInstall (detect silent native failures) - Use Activity context for install dialog to prevent Android task suppression - Re-launch pending install dialog from Kotlin watchdog when app is in foreground - Exclude InstallCancelled from batch progress totals - Auto-clear completed operations after 3s with proper Timer cancellation - Dynamic "updated"/"installed" label in batch completion banner
2026-10-05 12:38:24 +00:00 · 2026-02-12 20:31:59 -03:00
parent 099aecb7ac
commit fa51e09522
5 changed files with 190 additions and 38 deletions
@@ -17,6 +17,8 @@ import androidx.lifecycle.DefaultLifecycleObserver
 import androidx.lifecycle.LifecycleOwner
 import androidx.lifecycle.ProcessLifecycleOwner
 import io.flutter.embedding.engine.plugins.FlutterPlugin
+import io.flutter.embedding.engine.plugins.activity.ActivityAware
+import io.flutter.embedding.engine.plugins.activity.ActivityPluginBinding
 import io.flutter.plugin.common.EventChannel
 import io.flutter.plugin.common.MethodCall
 import io.flutter.plugin.common.MethodChannel
@@ -90,7 +92,7 @@ object ErrorCode {
 * no polling, no probing, no hanging awaits.
 */
 class AndroidPackageManagerPlugin :
-        FlutterPlugin, MethodCallHandler, EventChannel.StreamHandler, DefaultLifecycleObserver {
+        FlutterPlugin, MethodCallHandler, EventChannel.StreamHandler, DefaultLifecycleObserver, ActivityAware {

    private lateinit var methodChannel: MethodChannel
    private lateinit var eventChannel: EventChannel
@@ -138,6 +140,15 @@ class AndroidPackageManagerPlugin :

        private var appContext: Context? = null

+        /**
+         * Weak reference to the current Activity. Used by launchConfirmDialog to start
+         * the PackageInstaller confirmation dialog in the same task as the app, which
+         * ensures the dialog appears on top of the current UI. Using Application context
+         * with FLAG_ACTIVITY_NEW_TASK creates a separate task that Android may place
+         * behind the current app, making the dialog invisible to the user.
+         */
+        private var activityContext: java.lang.ref.WeakReference<android.app.Activity>? = null
+
        /**
         * Called by InstallResultReceiver when a broadcast arrives. Emits the status event to Dart
         * via EventChannel.
@@ -218,7 +229,6 @@ class AndroidPackageManagerPlugin :
        }

        private fun launchConfirmDialog(packageName: String, intent: Intent) {
-            val ctx = appContext ?: return
            val inst = instance

            // Post with delay to ensure the system is ready to show the dialog.
@@ -226,11 +236,31 @@ class AndroidPackageManagerPlugin :
            // immediately after session.commit() while the app is still processing.
            val launcher: () -> Unit = {
                try {
-                    intent.addFlags(
-                            Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_SINGLE_TOP
-                    )
-                    ctx.startActivity(intent)
-                    Log.d(TAG, "Launched confirmation dialog for $packageName")
+                    // CRITICAL: Prefer Activity context over Application context.
+                    // Using Application context with FLAG_ACTIVITY_NEW_TASK creates
+                    // the install dialog in a SEPARATE task. When the user is actively
+                    // interacting with the app, Android may place this new task behind
+                    // the current one, making the dialog invisible.
+                    // Using Activity context starts the dialog in the SAME task,
+                    // guaranteeing it appears on top of the current UI.
+                    val activity = activityContext?.get()
+                    if (activity != null && !activity.isFinishing && !activity.isDestroyed) {
+                        // Remove NEW_TASK flag when launching from Activity context —
+                        // we want the dialog in the same task stack.
+                        intent.flags = intent.flags and Intent.FLAG_ACTIVITY_NEW_TASK.inv()
+                        activity.startActivity(intent)
+                        Log.d(TAG, "Launched confirmation dialog for $packageName (Activity context)")
+                    } else {
+                        // Fallback to Application context (e.g., during config change)
+                        val ctx = appContext
+                        if (ctx != null) {
+                            intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
+                            ctx.startActivity(intent)
+                            Log.d(TAG, "Launched confirmation dialog for $packageName (Application context fallback)")
+                        } else {
+                            Log.w(TAG, "Cannot launch dialog for $packageName: no context available")
+                        }
+                    }
                } catch (e: Exception) {
                    Log.w(TAG, "Failed to launch confirmation dialog for $packageName", e)
                }
@@ -291,6 +321,26 @@ class AndroidPackageManagerPlugin :
        instance = null
    }

+    // ═══════════════════════════════════════════════════════════════════════════════
+    // ACTIVITY AWARE (Provides Activity context for dialog launching)
+    // ═══════════════════════════════════════════════════════════════════════════════
+
+    override fun onAttachedToActivity(binding: ActivityPluginBinding) {
+        activityContext = java.lang.ref.WeakReference(binding.activity)
+    }
+
+    override fun onDetachedFromActivityForConfigChanges() {
+        activityContext = null
+    }
+
+    override fun onReattachedToActivityForConfigChanges(binding: ActivityPluginBinding) {
+        activityContext = java.lang.ref.WeakReference(binding.activity)
+    }
+
+    override fun onDetachedFromActivity() {
+        activityContext = null
+    }
+
    // ═══════════════════════════════════════════════════════════════════════════════
    // LIFECYCLE OBSERVER (Auto foreground detection)
    // ═══════════════════════════════════════════════════════════════════════════════
@@ -504,6 +554,15 @@ class AndroidPackageManagerPlugin :
                    if (now2 < deadline && (hasSession || hasPendingUi || verifyingAlive)) {
                        // Nudge Dart to show the most accurate state (esp. after reconnect).
                        if (hasPendingUi) {
+                            // Re-launch the dialog in case it was suppressed by user interaction
+                            // or Android's activity stack. The initial launchConfirmDialog may
+                            // have failed silently if the user was actively using the app.
+                            if (isAppInForeground) {
+                                pendingUserActionIntents[appId]?.let { intent ->
+                                    Log.d(TAG, "Watchdog: Re-launching install dialog for $appId")
+                                    launchConfirmDialog(appId, intent)
+                                }
+                            }
                            emitInstallStatus(
                                    appId,
                                    InstallStatus.PENDING_USER_ACTION,
@@ -381,23 +381,36 @@ class _UpdatesListBody extends HookConsumerWidget {
    final showAllDone = useState(false);
    final wasInProgress = useRef(false);

-    // Detect transition from in-progress to all-complete
+    // Detect transition from in-progress to all-complete.
+    // FEAT-001 spec: "After 3 seconds with no in-progress operations,
+    // completed operations auto-clear."
+    final autoClearTimer = useRef<Timer?>(null);
    useEffect(() {
      if (progress != null && progress.hasInProgress) {
+        autoClearTimer.value?.cancel();
        wasInProgress.value = true;
        showAllDone.value = false; // Reset if new operations start
      } else if (wasInProgress.value &&
          progress != null &&
          progress.isAllComplete) {
-        // Just finished - show "All done" until dismissed
+        // Just finished - show "All done" banner briefly, then auto-clear
        showAllDone.value = true;
        wasInProgress.value = false;
+        autoClearTimer.value = Timer(const Duration(seconds: 3), () {
+          if (showAllDone.value) {
+            showAllDone.value = false;
+            ref
+                .read(packageManagerProvider.notifier)
+                .clearCompletedOperations();
+          }
+        });
      } else if (progress == null) {
        // Operations cleared externally
+        autoClearTimer.value?.cancel();
        wasInProgress.value = false;
        showAllDone.value = false;
      }
-      return null;
+      return () => autoClearTimer.value?.cancel();
    }, [progress?.hasInProgress, progress?.isAllComplete]);

    // Determine what to show (mutually exclusive states):
@@ -426,7 +439,7 @@ class _UpdatesListBody extends HookConsumerWidget {
                child: _StatusBannerContent(
                  icon: Icons.check_circle_rounded,
                  iconColor: Colors.green.shade400,
-                  text: 'All done (${progress?.completed ?? 0} updated)',
+                  text: 'All done (${progress?.completed ?? 0} ${progress?.completedLabel ?? 'completed'})',
                  failedCount: progress?.failed ?? 0,
                  showDismiss: true,
                  onTap: () {
@@ -164,6 +164,9 @@ final class AndroidPackageManager extends PackageManager {
        debugPrint(
          '[PackageManager] No tracked operation for appId=$appId, aborting orphaned native session',
        );
+        // Release the install slot in case this app was the active install
+        // (e.g., sync cleared the operation before the native event arrived).
+        clearInstallSlot(appId);
        unawaited(abortInstall(appId));
      }
      return;
@@ -183,6 +186,15 @@ final class AndroidPackageManager extends PackageManager {
      return;
    }

+    // INVARIANT: Once the user cancels (or the system dismisses the dialog),
+    // the operation is InstallCancelled. The native side may retry the install
+    // session (sending verifying/started/pendingUserAction), but we must NOT
+    // let those events overwrite InstallCancelled. The only way out of
+    // InstallCancelled is the user tapping "Install (retry)" (FEAT-001 spec).
+    if (existingOp is InstallCancelled) {
+      return;
+    }
+
    final target = existingOp.target;
    final filePath = existingOp.filePath;

@@ -217,7 +229,12 @@ final class AndroidPackageManager extends PackageManager {
      case InstallStatus.pendingUserAction:
        // User action required. Ensure we don't get stuck in Verifying if the
        // STARTED event was missed; show Installing state.
-        if (filePath != null && existingOp is! Installing) {
+        // Skip if already in Installing or SystemProcessing to preserve the
+        // original startedAt timestamp — otherwise the watchdog timeout resets
+        // on every pendingUserAction/systemProcessing bounce and never fires.
+        if (filePath != null &&
+            existingOp is! Installing &&
+            existingOp is! SystemProcessing) {
          final pkg = state.installed[appId];
          final isSilent = pkg?.canInstallSilently ?? false;
          setOperation(
@@ -241,10 +258,20 @@ final class AndroidPackageManager extends PackageManager {
      case InstallStatus.systemProcessing:
        // Install session is committed and taking longer than expected.
        // Cannot be cancelled - system will eventually complete or fail.
-        if (filePath != null) {
+        // Only create a new SystemProcessing if not already in that state,
+        // to preserve the original startedAt timestamp for the watchdog.
+        // CRITICAL: When transitioning from Installing, preserve the original
+        // startedAt so the Dart watchdog doesn't reset its countdown.
+        if (filePath != null && existingOp is! SystemProcessing) {
+          final preservedStart =
+              existingOp is Installing ? existingOp.startedAt : null;
          setOperation(
            appId,
-            SystemProcessing(target: target, filePath: filePath),
+            SystemProcessing(
+              target: target,
+              filePath: filePath,
+              startedAt: preservedStart,
+            ),
          );
        }
        break;
@@ -256,13 +283,16 @@ final class AndroidPackageManager extends PackageManager {

      case InstallStatus.success:
        _deleteFile(filePath);
+        // Check BEFORE updating installed package info — after update the app
+        // will always appear as installed, losing the update/new-install distinction.
+        final wasInstalled = isInstalled(appId);
        // CRITICAL: Update installed package info DIRECTLY from target metadata.
        // We cannot rely on syncInstalledPackages() here because Android's package
        // database may not have committed yet, causing a race condition where
        // we get stale data and show "Update" instead of "Open".
        _updateInstalledPackage(appId, target);
        // Transition to Completed state (stays in map for batch progress tracking)
-        setOperation(appId, Completed(target: target));
+        setOperation(appId, Completed(target: target, isUpdate: wasInstalled));
        // Sync in background to get accurate info (signature hash, etc.)
        // but our state machine doesn't depend on it.
        unawaited(syncInstalledPackages());
@@ -716,12 +746,21 @@ final class AndroidPackageManager extends PackageManager {
            (installedV != null && installedV == targetV);

        // Only clear if we can establish completion reliably.
+        // Transition to Completed (not clearOperation) so batchProgressProvider
+        // counts this app. The Completed op stays until the user dismisses the
+        // banner or clearCompletedOperations runs.
        if (completed) {
          debugPrint(
-            '[PackageManager] Sync: clearing completed operation for $appId '
+            '[PackageManager] Sync: completing operation for $appId '
            '(installedVc=$installedVc, targetVc=$targetVc, installedV=$installedV, targetV=$targetV)',
          );
-          clearOperation(appId);
+          // Sync fallback: state.installed was already overwritten with native
+          // data above (line 723), so we must NOT call _updateInstalledPackage
+          // here — that would replace accurate native info with target metadata.
+          // isUpdate defaults to true: sync mostly catches silent updates where
+          // the app was already installed. The impact of a wrong label is cosmetic.
+          setOperation(appId, Completed(target: op.target, isUpdate: true));
+          clearInstallSlot(appId);
        } else {
          debugPrint(
            '[PackageManager] Sync: keeping operation for $appId '
@@ -123,11 +123,18 @@ class AwaitingPermission extends InstallOperation {
 // INSTALL PHASE (No cancel - Android controls)
 // ═══════════════════════════════════════════════════════════════════════════════

-/// File verified, waiting for install to be triggered
+/// File verified, waiting for install to be triggered.
+/// [triggeredAt] is set when [triggerInstall] is actually called, so the
+/// watchdog can detect native sessions that never respond.
 class ReadyToInstall extends InstallOperation {
  final String filePath;
+  final DateTime? triggeredAt;

-  const ReadyToInstall({required super.target, required this.filePath});
+  const ReadyToInstall({
+    required super.target,
+    required this.filePath,
+    this.triggeredAt,
+  });
 }

 /// Native installation in progress
@@ -182,7 +189,10 @@ class SystemProcessing extends InstallOperation {
 class Completed extends InstallOperation {
  final DateTime completedAt;

-  Completed({required super.target, DateTime? completedAt})
+  /// Whether this was an update (app was already installed) or a new install.
+  final bool isUpdate;
+
+  Completed({required super.target, DateTime? completedAt, this.isUpdate = false})
    : completedAt = completedAt ?? DateTime.now();
 }

@@ -258,7 +268,8 @@ extension InstallOperationX on InstallOperation {
      this is Downloading ||
      this is Verifying ||
      this is Installing ||
-      this is SystemProcessing;
+      this is SystemProcessing ||
+      (this is ReadyToInstall && (this as ReadyToInstall).triggeredAt != null);

  /// Whether this operation is in the verification phase
  bool get isVerifying => this is Verifying;
@@ -277,6 +288,7 @@ extension InstallOperationX on InstallOperation {
    Verifying(:final startedAt) => startedAt,
    Installing(:final startedAt) => startedAt,
    SystemProcessing(:final startedAt) => startedAt,
+    ReadyToInstall(:final triggeredAt) => triggeredAt,
    _ => null,
  };

@@ -255,7 +255,7 @@ abstract class PackageManager extends StateNotifier<PackageManagerState> {
            filePath: op.filePath,
          ),
        );
-        if (op is Installing || op is SystemProcessing) {
+        if (op is Installing || op is SystemProcessing || op is ReadyToInstall) {
          clearInstallSlot(appId);
          needsQueueProcessing = true;
        }
@@ -308,11 +308,15 @@ abstract class PackageManager extends StateNotifier<PackageManagerState> {
    _updateWatchdogTimer();
  }

-  /// Clear all completed and failed operations from the map.
+  /// Clear all terminal operations from the map.
  /// Called after the batch completion display timeout.
+  /// Terminal states: Completed, OperationFailed, InstallCancelled.
  void clearCompletedOperations() {
    final remaining = Map.of(state.operations)
-      ..removeWhere((_, op) => op is Completed || op is OperationFailed);
+      ..removeWhere(
+        (_, op) =>
+            op is Completed || op is OperationFailed || op is InstallCancelled,
+      );
    state = state.copyWith(operations: remaining);
    _updateWatchdogTimer();
  }
@@ -997,7 +1001,16 @@ abstract class PackageManager extends StateNotifier<PackageManagerState> {

        if (op is ReadyToInstall) {
          activeInstall = appId;
-          debugPrint('[PackageManager] Starting install for $appId');
+          // Mark triggeredAt so the watchdog can detect native sessions that
+          // never respond (e.g. Android PackageInstaller silently fails).
+          setOperation(
+            appId,
+            ReadyToInstall(
+              target: op.target,
+              filePath: op.filePath,
+              triggeredAt: DateTime.now(),
+            ),
+          );
          unawaited(triggerInstall(appId));
        }
        // If operation changed, it will be picked up on next process cycle
@@ -1460,28 +1473,31 @@ enum BatchPhase { downloading, verifying, installing, completed, idle }

 /// Batch progress summary - ALL state derived from operations map.
 ///
-/// Key insight: total = operations.length (includes Completed state).
 /// When an operation succeeds, it transitions to Completed instead of being removed.
-/// This allows us to derive completed count without separate tracking.
+/// InstallCancelled operations are excluded from totals (already resolved).
 class BatchProgress {
  const BatchProgress({
    required this.total,
    required this.completed,
+    required this.completedLabel,
    required this.downloading,
    required this.verifying,
    required this.installing,
    required this.queued,
    required this.failed,
-    required this.cancelled,
    required this.phase,
  });

-  /// Total operations (everything in the map, including completed)
+  /// Total operations (excludes InstallCancelled — those are already resolved
+  /// from the batch's perspective and should not inflate the count).
  final int total;

  /// Operations that completed successfully
  final int completed;

+  /// Label for completed operations: "updated", "installed", or "completed" (mixed)
+  final String completedLabel;
+
  /// Operations currently downloading
  final int downloading;

@@ -1497,14 +1513,11 @@ class BatchProgress {
  /// Operations that failed
  final int failed;

-  /// Operations cancelled by user (InstallCancelled - can retry individually)
-  final int cancelled;
-
  /// Current dominant phase
  final BatchPhase phase;

  /// Whether any operations are in progress (not terminal)
-  /// Terminal states: Completed, OperationFailed, InstallCancelled
+  /// Terminal states: Completed, OperationFailed
  bool get hasInProgress =>
      downloading > 0 || verifying > 0 || installing > 0 || queued > 0;

@@ -1529,17 +1542,18 @@ final batchProgressProvider = Provider<BatchProgress?>((ref) {

  // Count operations by type
  int completed = 0,
+      completedUpdates = 0,
      downloading = 0,
      verifying = 0,
      installing = 0,
      queued = 0,
-      failed = 0,
-      cancelled = 0;
+      failed = 0;

  for (final op in ops.values) {
    switch (op) {
      case Completed():
        completed++;
+        if (op.isUpdate) completedUpdates++;
      case DownloadQueued() || ReadyToInstall():
        queued++;
      case Downloading() || DownloadPaused():
@@ -1551,7 +1565,9 @@ final batchProgressProvider = Provider<BatchProgress?>((ref) {
      case OperationFailed():
        failed++;
      case InstallCancelled():
-        cancelled++; // Terminal for batch - user can retry individually
+        // Not counted in batch — already resolved (user can retry individually).
+        // Stays in the operations map for the "Install (retry)" UI button.
+        break;
      case AwaitingPermission():
        queued++; // Waiting for permission
      case Uninstalling():
@@ -1559,7 +1575,12 @@ final batchProgressProvider = Provider<BatchProgress?>((ref) {
    }
  }

-  final total = ops.length;
+  // Total excludes InstallCancelled — those are already resolved from the
+  // batch's perspective and should not inflate the count.
+  final total = completed + downloading + verifying + installing + queued + failed;
+
+  // If only InstallCancelled operations remain, no batch to show
+  if (total == 0) return null;

  // Determine current phase (priority: installing > verifying > downloading > completed)
  final phase = installing > 0
@@ -1572,15 +1593,23 @@ final batchProgressProvider = Provider<BatchProgress?>((ref) {
      ? BatchPhase.completed
      : BatchPhase.idle;

+  // Derive label: "updated" if all updates, "installed" if all new, "completed" if mixed
+  final completedInstalls = completed - completedUpdates;
+  final completedLabel = completedUpdates > 0 && completedInstalls == 0
+      ? 'updated'
+      : completedInstalls > 0 && completedUpdates == 0
+          ? 'installed'
+          : 'completed';
+
  return BatchProgress(
    total: total,
    completed: completed,
+    completedLabel: completedLabel,
    downloading: downloading,
    verifying: verifying,
    installing: installing,
    queued: queued,
    failed: failed,
-    cancelled: cancelled,
    phase: phase,
  );
 });