CI-915 Progress Dialog State Loss Fix

.github/workflows/linter.yml

@@ -28,7 +28,11 @@ jobs:
         env:
           # Only lint changed files in PR
           VALIDATE_ALL_CODEBASE: false
-          DEFAULT_BRANCH: master
+          # Diff against the branch the PR actually targets. Super-linter picks its file set with
+          # a two-dot diff against DEFAULT_BRANCH, so hardcoding master made every PR based on a
+          # release branch lint that branch's entire divergence from master rather than the files
+          # the PR touches. This workflow is pull_request-only, so base_ref is always set.
+          DEFAULT_BRANCH: ${{ github.base_ref }}
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
           # File handling

RELEASES.md

@@ -9,6 +9,16 @@ This file is meant as an easy way for us to collate notes and change logs across
 #### Important Bug Fixes
 
 - Fixed an issue where recovering a PersonalID account via backup code could result in the account being stored without a pin, causing authentication to fail after recovery.
+- Backgrounding the app while logging in no longer crashes the app when the login sync finishes.
+- The STOP button on the login sync dialog cancels the login again, instead of hanging on "Cancelling...".
+
+### QA Notes
+
+- The login tests below need a login that takes the remote-sync path: clear the app's data first so the user has no local sandbox on the device. Make sure "Don't keep activities" is OFF in developer options, since it destroys the activity and hides the bug.
+- Log in as a traditional CommCare user (not from a Connect opportunity), background the app while the sync dialog is showing, and stay backgrounded until the restore finishes. The app should not crash. On returning to it, the login should either complete or report a failure you can retry from, and no progress dialog should be left stuck on screen.
+- Repeat the same steps but rotate the device or lock/unlock the screen mid-sync; progress dialogs should reappear correctly with no crash.
+- While the login sync dialog is showing, press STOP. The dialog should close and you should be back on the login screen, and logging in again should work normally.
+- Regression check on progress dialogs generally, since the fix touches the shared activity base class: app update installs, form record loading, multimedia inflation and app verification should all still show and dismiss their progress dialogs correctly, including when backgrounded and resumed mid-task.
 
 ## CommCare 2.63.5
 

app/src/org/commcare/activities/CommCareActivity.java

@@ -124,6 +124,7 @@ public abstract class CommCareActivity<R> extends CommonBaseActivity
      */
     private boolean triedBlockingWhilePaused;
     private int taskIdForPendingDismissal = UNDEFINED_TASK_ID;
+    private int taskIdForPendingShow = UNDEFINED_TASK_ID;
 
     /**
      * Store the id of a task progress dialog so it can be disabled/enabled
@@ -390,6 +391,21 @@ private void syncTaskBlockingWithDialogFragment() {
             triedBlockingWhilePaused = false;
             showNewProgressDialog();
         }
+
+        showPendingProgressDialog();
+    }
+
+    private void showPendingProgressDialog() {
+        int taskId = taskIdForPendingShow;
+        taskIdForPendingShow = UNDEFINED_TASK_ID;
+
+        if (taskId != UNDEFINED_TASK_ID) {
+            CustomProgressDialog current = getCurrentProgressDialog();
+            if (current != null && current.getTaskId() != taskId) {
+                dismissCurrentProgressDialog();
+            }
+            showProgressDialogIfNeeded(taskId);
+        }
     }
 
     @Override
@@ -399,6 +415,8 @@ public void startBlockingForTask(int id) {
         if (areFragmentsPaused) {
             // post-pone dialog transactions until after fragments have fully resumed.
             triedBlockingWhilePaused = true;
+            // This is the newer request, so it supersedes any postponed direct show.
+            taskIdForPendingShow = UNDEFINED_TASK_ID;
         } else {
             showNewProgressDialog();
         }
@@ -642,6 +660,13 @@ private void warnInvalidProgressUpdate(int taskId) {
     @Override
     public void showProgressDialog(int taskId) {
         if (taskId >= 0) {
+            if (areFragmentsPaused) {
+                // post-pone the dialog transaction until after fragments have fully resumed
+                taskIdForPendingShow = taskId;
+                return;
+            }
+
+            taskIdForPendingShow = UNDEFINED_TASK_ID;
             CustomProgressDialog dialog = generateProgressDialog(taskId);
             if (dialog != null) {
                 dialog.showNow(getSupportFragmentManager(), KEY_PROGRESS_DIALOG_FRAG);
@@ -673,6 +698,17 @@ public void dismissCurrentProgressDialog() {
     }
 
     private void dismissProgressDialog(int taskId, boolean dismissAny) {
+        if (dismissAny) {
+            // Nothing pending should outlive a blanket dismissal, but the dialog that is actually
+            // added still has to be dealt with below.
+            taskIdForPendingShow = UNDEFINED_TASK_ID;
+        } else if (taskIdForPendingShow != UNDEFINED_TASK_ID && taskIdForPendingShow == taskId) {
+            // This task's show was postponed and never committed, so dropping it is the whole
+            // dismissal.
+            taskIdForPendingShow = UNDEFINED_TASK_ID;
+            return;
+        }
+
         taskIdForPendingDismissal = UNDEFINED_TASK_ID;
         CustomProgressDialog progressDialog = getCurrentProgressDialog();
         if (progressDialog != null && progressDialog.isAdded() && (progressDialog.getTaskId() == taskId

app/src/org/commcare/activities/LoginActivity.java

@@ -57,6 +57,7 @@
 import org.commcare.suite.model.OfflineUserRestore;
 import org.commcare.tasks.DataPullTask;
 import org.commcare.tasks.InstallStagedUpdateTask;
+import org.commcare.util.LogTypes;
 import org.commcare.utils.ConsumerAppsUtil;
 import org.commcare.utils.Permissions;
 import org.commcare.utils.StringUtils;
@@ -74,6 +75,9 @@
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.Map;
+import java.util.concurrent.CancellationException;
+
+import kotlinx.coroutines.Job;
 
 import static org.commcare.activities.DispatchActivity.REDIRECT_TO_CONNECT_OPPORTUNITY_INFO;
 import static org.commcare.connect.ConnectConstants.PERSONALID_MANAGED_LOGIN;
@@ -133,6 +137,12 @@ public class LoginActivity extends BaseDrawerActivity<LoginActivity>
 
     private LoginPhase currentLoginPhase;
 
+    /**
+     * Job for the in-flight login pipeline, retained so that the progress dialog's STOP button can
+     * cancel it. Null whenever no login is running.
+     */
+    private Job loginJob;
+
     @Override
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -310,7 +320,7 @@ private void runLoginPipeline(
         );
 
         LoginController controller = new LoginController(this);
-        controller.start(this, request, createLoginProgressListener(), this::handleLoginResult);
+        loginJob = controller.start(this, request, createLoginProgressListener(), this::handleLoginResult);
     }
 
     private LoginProgressListener createLoginProgressListener() {
@@ -345,6 +355,25 @@ private int taskIdForPhase(LoginPhase phase) {
         return TASK_KEY_EXCHANGE;
     }
 
+    /**
+     * The login engine runs its tasks on a HeadlessTaskConnector, so they are never registered with
+     * the activity's task connector and the base implementation has nothing to cancel. Cancel the
+     * pipeline's job instead, which tears down the running task through its cancellation handler.
+     */
+    @Override
+    public void cancelCurrentTask() {
+        if (loginJob != null) {
+            Logger.log(LogTypes.TYPE_USER, "Login cancelled by user during "
+                    + currentLoginPhase + " phase");
+            loginJob.cancel(new CancellationException("Login cancelled by user"));
+            loginJob = null;
+            dismissLoginProgressDialog();
+            return;
+        }
+
+        super.cancelCurrentTask();
+    }
+
     private void dismissLoginProgressDialog() {
         if (currentLoginPhase != null) {
             dismissProgressDialogForTask(taskIdForPhase(currentLoginPhase));
@@ -505,6 +534,7 @@ private void setLoginResultAndFinish(
     }
 
     private void handleLoginResult(LoginResult result) {
+        loginJob = null;
         dismissLoginProgressDialog();
 
         if (result instanceof LoginResult.Success) {

app/src/org/commcare/login/KeyRecordOperations.kt

@@ -134,7 +134,10 @@ internal open class KeyRecordOperations(
                     }
                 }
 
-            continuation.invokeOnCancellation { task.cancel(true) }
+            continuation.invokeOnCancellation {
+                task.cancel(true)
+                task.tryAbort()
+            }
             task.connect(HeadlessTaskConnector(receiver))
             task.executeParallel()
         }

app/src/org/commcare/login/SyncOperations.kt

@@ -135,7 +135,10 @@ internal open class SyncOperations(
                     }
                 }
 
-            continuation.invokeOnCancellation { task.cancel(true) }
+            continuation.invokeOnCancellation {
+                task.cancel(true)
+                task.tryAbort()
+            }
             task.connect(HeadlessTaskConnector(receiver))
             task.executeParallel()
         }