Address Kai's feedback

Author: lfkellogg

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/ApkInstaller.java

@@ -17,6 +17,7 @@
 import android.app.Activity;
 import android.content.Intent;
 import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
 import com.google.android.gms.tasks.Task;
 import com.google.android.gms.tasks.TaskCompletionSource;
 import com.google.firebase.annotations.concurrent.Lightweight;
@@ -32,6 +33,7 @@ class ApkInstaller {
   private final TaskCompletionSourceCache<Void> installTaskCompletionSourceCache;
   private final @Lightweight Executor lightweightExecutor;
 
+  @VisibleForTesting
   ApkInstaller(
       FirebaseAppDistributionLifecycleNotifier lifeCycleNotifier,
       @Lightweight Executor lightweightExecutor) {
@@ -49,7 +51,10 @@ void onActivityDestroyed(@Nullable Activity activity) {
     if (activity instanceof InstallActivity) {
       // Since install activity is destroyed but app is still active, installation has failed /
       // cancelled.
-      this.trySetInstallTaskError();
+      installTaskCompletionSourceCache.setException(
+          new FirebaseAppDistributionException(
+              ErrorMessages.APK_INSTALLATION_FAILED,
+              FirebaseAppDistributionException.Status.INSTALLATION_FAILURE));
     }
   }
 
@@ -67,11 +72,4 @@ private void startInstallActivity(String path, Activity currentActivity) {
     currentActivity.startActivity(intent);
     LogWrapper.v(TAG, "Prompting tester with install activity");
   }
-
-  void trySetInstallTaskError() {
-    installTaskCompletionSourceCache.setException(
-        new FirebaseAppDistributionException(
-            ErrorMessages.APK_INSTALLATION_FAILED,
-            FirebaseAppDistributionException.Status.INSTALLATION_FAILURE));
-  }
 }

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/TaskCompletionSourceCache.java

@@ -43,7 +43,9 @@ interface TaskCompletionSourceProducer<T> {
    * Constructor for a {@link TaskCompletionSourceCache} that controls access using its own
    * sequential executor backed by the given base executor.
    *
-   * @param baseExecutor Executor (typically {@link Lightweight}) to back the sequential executor.
+   * @param baseExecutor Executor to back the sequential executor. This can be a {@link Lightweight}
+   *     executor unless the {@link TaskCompletionSourceProducer} passed to {@link
+   *     #getOrCreateTaskFromCompletionSource} needs to be executed on a different executor.
    */
   TaskCompletionSourceCache(Executor baseExecutor) {
     sequentialExecutor = FirebaseExecutors.newSequentialExecutor(baseExecutor);

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/TaskUtils.java

@@ -147,6 +147,9 @@ static <T> UpdateTask onSuccessUpdateTask(
 
   /** Set a {@link TaskCompletionSource} to be resolved with the result of another {@link Task}. */
   static <T> void shadowTask(TaskCompletionSource<T> taskCompletionSource, Task<T> task) {
+    // Using direct executor here ensures that any handlers that were themselves added using a
+    // direct executor will behave as expected: they'll be executed on the thread that sets the
+    // result.
     task.addOnSuccessListener(FirebaseExecutors.directExecutor(), taskCompletionSource::setResult)
         .addOnFailureListener(
             FirebaseExecutors.directExecutor(), taskCompletionSource::setException);

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java

@@ -70,6 +70,9 @@ void updateProgress(@NonNull UpdateProgress updateProgress) {
    * progress or completing this task with the same changes.
    */
   void shadow(UpdateTask updateTask) {
+    // Using direct executor here ensures that any handlers that were themselves added using a
+    // direct executor will behave as expected: they'll be executed on the thread that sets the
+    // result or updates progress.
     updateTask
         .addOnProgressListener(FirebaseExecutors.directExecutor(), this::updateProgress)
         .addOnSuccessListener(FirebaseExecutors.directExecutor(), unused -> this.setResult())