Migrate executors in fad/in-app-feedback

Author: lfkellogg

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

@@ -65,7 +65,7 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution {
   private final SignInStorage signInStorage;
   private final ReleaseIdentifier releaseIdentifier;
   private final ScreenshotTaker screenshotTaker;
-  private final Executor taskExecutor;
+  private final Executor blockingExecutor;
   private final FirebaseAppDistributionNotificationsManager notificationsManager;
 
   private final Object cachedNewReleaseLock = new Object();
@@ -95,7 +95,7 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution {
       @NonNull FirebaseAppDistributionLifecycleNotifier lifecycleNotifier,
       @NonNull ReleaseIdentifier releaseIdentifier,
       @NonNull ScreenshotTaker screenshotTaker,
-      @NonNull Executor taskExecutor) {
+      @NonNull Executor blockingExecutor) {
     this.firebaseApp = firebaseApp;
     this.testerSignInManager = testerSignInManager;
     this.newReleaseFetcher = newReleaseFetcher;
@@ -105,7 +105,7 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution {
     this.releaseIdentifier = releaseIdentifier;
     this.lifecycleNotifier = lifecycleNotifier;
     this.screenshotTaker = screenshotTaker;
-    this.taskExecutor = taskExecutor;
+    this.blockingExecutor = blockingExecutor;
     this.notificationsManager =
         new FirebaseAppDistributionNotificationsManager(firebaseApp.getApplicationContext());
     lifecycleNotifier.addOnActivityDestroyedListener(this::onActivityDestroyed);
@@ -324,13 +324,13 @@ public void startFeedback(@NonNull CharSequence infoText) {
     screenshotTaker
         .takeScreenshot()
         .addOnFailureListener(
-            taskExecutor,
+            blockingExecutor,
             e -> {
               LogWrapper.getInstance().w("Failed to take screenshot for feedback", e);
               doStartFeedback(infoText, null);
             })
         .addOnSuccessListener(
-            taskExecutor, screenshotUri -> doStartFeedback(infoText, screenshotUri));
+            blockingExecutor, screenshotUri -> doStartFeedback(infoText, screenshotUri));
   }
 
   @Override
@@ -369,18 +369,19 @@ private void doStartFeedback(CharSequence infoText, @Nullable Uri screenshotUri)
     testerSignInManager
         .signInTester()
         .addOnFailureListener(
-            taskExecutor,
+            blockingExecutor,
             e -> {
               feedbackInProgress.set(false);
               LogWrapper.getInstance()
                   .e("Failed to sign in tester. Could not collect feedback.", e);
             })
         .onSuccessTask(
-            taskExecutor,
+            blockingExecutor,
             unused ->
                 releaseIdentifier
                     .identifyRelease()
                     .addOnFailureListener(
+                        blockingExecutor,
                         e -> {
                           feedbackInProgress.set(false);
                           LogWrapper.getInstance().e("Failed to identify release", e);
@@ -391,11 +392,12 @@ private void doStartFeedback(CharSequence infoText, @Nullable Uri screenshotUri)
                               .show();
                         })
                     .onSuccessTask(
-                        taskExecutor,
+                        blockingExecutor,
                         releaseName ->
                             // in development-mode the releaseName might be null
                             launchFeedbackActivity(releaseName, infoText, screenshotUri)
                                 .addOnFailureListener(
+                                    blockingExecutor,
                                     e -> {
                                       feedbackInProgress.set(false);
                                       LogWrapper.getInstance()

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

@@ -20,6 +20,7 @@
 import androidx.annotation.Keep;
 import androidx.annotation.NonNull;
 import com.google.firebase.FirebaseApp;
+import com.google.firebase.annotations.concurrent.Background;
 import com.google.firebase.annotations.concurrent.Blocking;
 import com.google.firebase.appdistribution.FirebaseAppDistribution;
 import com.google.firebase.components.Component;
@@ -33,10 +34,9 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.Executor;
-import java.util.concurrent.Executors;
 
 /**
- * Registers FirebaseAppDistribution.
+ * Registers FirebaseAppDistribution and related components.
  *
  * @hide
  */
@@ -48,21 +48,27 @@ public class FirebaseAppDistributionRegistrar implements ComponentRegistrar {
 
   @Override
   public @NonNull List<Component<?>> getComponents() {
+    Qualified<Executor> backgroundExecutor = Qualified.qualified(Background.class, Executor.class);
     Qualified<Executor> blockingExecutor = Qualified.qualified(Blocking.class, Executor.class);
     return Arrays.asList(
         Component.builder(FirebaseAppDistribution.class)
             .name(LIBRARY_NAME)
             .add(Dependency.required(FirebaseApp.class))
             .add(Dependency.requiredProvider(FirebaseInstallationsApi.class))
+            .add(Dependency.required(backgroundExecutor))
             .add(Dependency.required(blockingExecutor))
-            .factory(c -> buildFirebaseAppDistribution(c, c.get(blockingExecutor)))
+            .factory(
+                c ->
+                    buildFirebaseAppDistribution(
+                        c, c.get(backgroundExecutor), c.get(blockingExecutor)))
             // construct FirebaseAppDistribution instance on startup so we can register for
             // activity lifecycle callbacks before the API is called
             .alwaysEager()
             .build(),
         Component.builder(FeedbackSender.class)
             .add(Dependency.required(FirebaseApp.class))
             .add(Dependency.requiredProvider(FirebaseInstallationsApi.class))
+            .add(Dependency.required(backgroundExecutor))
             .add(Dependency.required(blockingExecutor))
             .factory(c -> buildFeedbackSender(c, c.get(blockingExecutor)))
             .build(),
@@ -86,7 +92,7 @@ private FeedbackSender buildFeedbackSender(
   // TODO(b/258264924): Migrate to go/firebase-android-executors
   @SuppressLint("ThreadPoolCreation")
   private FirebaseAppDistribution buildFirebaseAppDistribution(
-      ComponentContainer container, Executor blockingExecutor) {
+      ComponentContainer container, Executor backgroundExecutor, Executor blockingExecutor) {
     FirebaseApp firebaseApp = container.get(FirebaseApp.class);
     Context context = firebaseApp.getApplicationContext();
     Provider<FirebaseInstallationsApi> firebaseInstallationsApiProvider =
@@ -112,8 +118,8 @@ private FirebaseAppDistribution buildFirebaseAppDistribution(
             signInStorage,
             lifecycleNotifier,
             releaseIdentifier,
-            new ScreenshotTaker(firebaseApp, lifecycleNotifier),
-            Executors.newSingleThreadExecutor());
+            new ScreenshotTaker(firebaseApp, lifecycleNotifier, backgroundExecutor),
+            blockingExecutor);
 
     if (context instanceof Application) {
       Application firebaseApplication = (Application) context;

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

@@ -14,7 +14,6 @@
 
 package com.google.firebase.appdistribution.impl;
 
-import android.annotation.SuppressLint;
 import android.annotation.TargetApi;
 import android.app.Activity;
 import android.content.Context;
@@ -37,7 +36,6 @@
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.concurrent.Executor;
-import java.util.concurrent.Executors;
 
 /** A class that takes screenshots of the host app. */
 class ScreenshotTaker {
@@ -47,23 +45,15 @@ class ScreenshotTaker {
 
   private final FirebaseApp firebaseApp;
   private final FirebaseAppDistributionLifecycleNotifier lifecycleNotifier;
-  private final Executor taskExecutor;
+  private final Executor backgroundExecutor;
 
-  // TODO(b/258264924): Migrate to go/firebase-android-executors
-  @SuppressLint("ThreadPoolCreation")
-  ScreenshotTaker(
-      FirebaseApp firebaseApp, FirebaseAppDistributionLifecycleNotifier lifecycleNotifier) {
-    this(firebaseApp, lifecycleNotifier, Executors.newSingleThreadExecutor());
-  }
-
-  @VisibleForTesting
   ScreenshotTaker(
       FirebaseApp firebaseApp,
       FirebaseAppDistributionLifecycleNotifier lifecycleNotifier,
-      Executor taskExecutor) {
+      Executor backgroundExecutor) {
     this.firebaseApp = firebaseApp;
     this.lifecycleNotifier = lifecycleNotifier;
-    this.taskExecutor = taskExecutor;
+    this.backgroundExecutor = backgroundExecutor;
   }
 
   /**
@@ -81,7 +71,7 @@ Task<Uri> takeScreenshot() {
   /** Deletes any previously taken screenshot. */
   Task<Void> deleteScreenshot() {
     return TaskUtils.runAsyncInTask(
-        taskExecutor,
+        backgroundExecutor,
         () -> {
           firebaseApp.getApplicationContext().deleteFile(SCREENSHOT_FILE_NAME);
           return null;
@@ -157,7 +147,7 @@ private Task<Uri> writeToFile(@Nullable Bitmap bitmap) {
       return Tasks.forResult(null);
     }
     return TaskUtils.runAsyncInTask(
-        taskExecutor,
+        backgroundExecutor,
         () -> {
           try (FileOutputStream outputStream = openFileOutputStream()) {
             // PNG is a lossless format, the compression factor (100) is ignored

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

@@ -30,6 +30,9 @@ class SignInStorage {
     this.signInSharedPreferences =
         new Lazy(
             () ->
+                // TODO(lkellogg): This constructs a SharedPreferences object, which touches disk
+                //   and therefore should be run on a @Background executor. This could ideally be
+                //   done by a new shared component.
                 applicationContext.getSharedPreferences(
                     SIGNIN_PREFERENCES_NAME, Context.MODE_PRIVATE));
   }