Merge fad/next into master to prepare for next release (#4784)

* Some App Distribution javadoc updates (#4740)

* Support JPGs (in addition to PNGs) as feedback screenshots and set correct content-type/filename (#4783)

* Support PNGs as feedback screenshots

* Add tests and inject ContentResolver

* Address feedback

* Flip order of conditional

* Update changelog and version for M129

Author: lfkellogg

firebase-appdistribution-api/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java

@@ -43,7 +43,7 @@ public interface FirebaseAppDistribution {
    * following actions:
    *
    * <ol>
-   *   <li>If tester is not signed in, presents the tester with a Google Sign-in UI.
+   *   <li>If the tester is not signed in, presents the tester with a Google Sign-in UI.
    *   <li>Checks if a newer release is available. If so, presents the tester with a confirmation
    *       dialog to begin the download.
    *   <li>If the newest release is an APK, downloads the binary and starts an installation. If the
@@ -119,9 +119,9 @@ public interface FirebaseAppDistribution {
    * <p>Performs the following actions:
    *
    * <ol>
-   *   <li>Takes a screenshot of the current activity
-   *   <li>If tester is not signed in, presents the tester with a Google Sign-in UI
-   *   <li>Starts a full screen activity for the tester to compose and submit the feedback
+   *   <li>Takes a screenshot of the current activity.
+   *   <li>If the tester is not signed in, presents the tester with a Google Sign-in UI.
+   *   <li>Starts a full screen activity for the tester to compose and submit the feedback.
    * </ol>
    *
    * @param additionalFormText string resource ID of text that will be shown to the tester before
@@ -137,9 +137,9 @@ public interface FirebaseAppDistribution {
    * <p>Performs the following actions:
    *
    * <ol>
-   *   <li>Takes a screenshot of the current activity
-   *   <li>If tester is not signed in, presents the tester with a Google Sign-in UI
-   *   <li>Starts a full screen activity for the tester to compose and submit the feedback
+   *   <li>Takes a screenshot of the current activity.
+   *   <li>If the tester is not signed in, presents the tester with a Google Sign-in UI.
+   *   <li>Starts a full screen activity for the tester to compose and submit the feedback.
    * </ol>
    *
    * @param additionalFormText text that will be shown to the tester before they submit feedback. If
@@ -155,8 +155,8 @@ public interface FirebaseAppDistribution {
    * <p>Performs the following actions:
    *
    * <ol>
-   *   <li>If tester is not signed in, presents the tester with a Google Sign-in UI
-   *   <li>Starts a full screen activity for the tester to compose and submit the feedback
+   *   <li>If the tester is not signed in, presents the tester with a Google Sign-in UI.
+   *   <li>Starts a full screen activity for the tester to compose and submit the feedback.
    * </ol>
    *
    * @param additionalFormText string resource ID of text that will be shown to the tester before
@@ -175,8 +175,8 @@ public interface FirebaseAppDistribution {
    * <p>Performs the following actions:
    *
    * <ol>
-   *   <li>If tester is not signed in, presents the tester with a Google Sign-in UI
-   *   <li>Starts a full screen activity for the tester to compose and submit the feedback
+   *   <li>If the tester is not signed in, presents the tester with a Google Sign-in UI.
+   *   <li>Starts a full screen activity for the tester to compose and submit the feedback.
    * </ol>
    *
    * @param additionalFormText text that will be shown to the tester before they submit feedback. If
@@ -191,28 +191,29 @@ public interface FirebaseAppDistribution {
    * Displays a notification that, when tapped, will take a screenshot of the current activity, then
    * start a new activity to collect and submit feedback from the tester along with the screenshot.
    *
-   * <p>On Android 13 and above, this method requires the <a
-   * href="https://developer.android.com/develop/ui/views/notifications/notification-permission">runtime
-   * permission for sending notifications</a>: {@code POST_NOTIFICATIONS}. If your app targets
-   * Android 13 (API level 33) or above, you should <a
+   * <p>On Android 13 and above, this method requires the runtime permission for sending
+   * notifications: <a
+   * href="https://developer.android.com/develop/ui/views/notifications/notification-permission">{@code
+   * POST_NOTIFICATIONS}</a>. If your app targets Android 13 (API level 33) or above, you should <a
    * href="https://developer.android.com/training/permissions/requesting">request the
    * permission</a>.
    *
    * <p>When the notification is tapped:
    *
    * <ol>
-   *   <li>If the app is open, take a screenshot of the current activity
-   *   <li>If tester is not signed in, presents the tester with a Google Sign-in UI
-   *   <li>Starts a full screen activity for the tester to compose and submit the feedback
+   *   <li>If the app is open, takes a screenshot of the current activity.
+   *   <li>If the tester is not signed in, presents the tester with a Google Sign-in UI.
+   *   <li>Starts a full screen activity for the tester to compose and submit the feedback.
    * </ol>
    *
    * @param additionalFormText string resource ID of text that will be shown to the tester before
    *     they submit feedback. If you’re a customer who would like to provide notice to your testers
    *     about collection and processing of their feedback data, you can use this text to provide
    *     such notice.
    * @param interruptionLevel the level of interruption for the feedback notification. On platforms
-   *     below Android 8, this corresponds to a notification channel importance and once set cannot
-   *     be changed except by the user.
+   *     below Android 8, this corresponds to a <a
+   *     href="https://developer.android.com/develop/ui/views/notifications/channels#importance">notification
+   *     channel importance</a> and once set cannot be changed except by the user.
    */
   void showFeedbackNotification(
       @StringRes int additionalFormText, @NonNull InterruptionLevel interruptionLevel);
@@ -221,32 +222,36 @@ void showFeedbackNotification(
    * Displays a notification that, when tapped, will take a screenshot of the current activity, then
    * start a new activity to collect and submit feedback from the tester along with the screenshot.
    *
-   * <p>On Android 13 and above, this method requires the <a
-   * href="https://developer.android.com/develop/ui/views/notifications/notification-permission">runtime
-   * permission for sending notifications</a>: {@code POST_NOTIFICATIONS}. If your app targets
-   * Android 13 (API level 33) or above, you should <a
+   * <p>On Android 13 and above, this method requires the runtime permission for sending
+   * notifications: <a
+   * href="https://developer.android.com/develop/ui/views/notifications/notification-permission">{@code
+   * POST_NOTIFICATIONS}</a>. If your app targets Android 13 (API level 33) or above, you should <a
    * href="https://developer.android.com/training/permissions/requesting">request the
    * permission</a>.
    *
    * <p>When the notification is tapped:
    *
    * <ol>
-   *   <li>If the app is open, take a screenshot of the current activity
-   *   <li>If tester is not signed in, presents the tester with a Google Sign-in UI
-   *   <li>Starts a full screen activity for the tester to compose and submit the feedback
+   *   <li>If the app is open, takes a screenshot of the current activity.
+   *   <li>If the tester is not signed in, presents the tester with a Google Sign-in UI.
+   *   <li>Starts a full screen activity for the tester to compose and submit the feedback.
    * </ol>
    *
    * @param additionalFormText text that will be shown to the tester before they submit feedback. If
    *     you’re a customer who would like to provide notice to your testers about collection and
    *     processing of their feedback data, you can use this text to provide such notice.
    * @param interruptionLevel the level of interruption for the feedback notification. On platforms
-   *     below Android 8, this corresponds to a notification channel importance and once set cannot
-   *     be changed except by the user.
+   *     below Android 8, this corresponds to a <a
+   *     href="https://developer.android.com/develop/ui/views/notifications/channels#importance">notification
+   *     channel importance</a> and once set cannot be changed except by the user.
    */
   void showFeedbackNotification(
       @NonNull CharSequence additionalFormText, @NonNull InterruptionLevel interruptionLevel);
 
-  /** Hides the notification shown with {@link #showFeedbackNotification}. */
+  /**
+   * Hides the notification shown with {@link #showFeedbackNotification(int, InterruptionLevel)} or
+   * {@link #showFeedbackNotification(CharSequence, InterruptionLevel)}.
+   */
   void cancelFeedbackNotification();
 
   /** Gets the singleton {@link FirebaseAppDistribution} instance. */

firebase-appdistribution/CHANGELOG.md

@@ -1,4 +1,7 @@
 # Unreleased
+* [feature] Adds support for attaching JPEG screenshots to tester feedback.
+
+# 16.0.0-beta06
 * [feature] Adds support for in-app tester feedback. To learn more, see
   [Collect feedback from testers](/docs/app-distribution/collect-feedback-from-testers).
 * [fixed] Fixed a bug where only the last listener added to an `UpdateTask` using

firebase-appdistribution/gradle.properties

@@ -12,5 +12,5 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-version=16.0.0-beta06
-latestReleasedVersion=16.0.0-beta05
+version=16.0.0-beta07
+latestReleasedVersion=16.0.0-beta06

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

@@ -14,6 +14,7 @@
 
 package com.google.firebase.appdistribution.impl;
 
+import android.content.ContentResolver;
 import android.content.Context;
 import com.google.firebase.FirebaseApp;
 import com.google.firebase.FirebaseOptions;
@@ -28,6 +29,7 @@
 import dagger.BindsInstance;
 import dagger.Component;
 import dagger.Module;
+import dagger.Provides;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ScheduledExecutorService;
@@ -85,32 +87,39 @@ interface Builder {
   void inject(TakeScreenshotAndStartFeedbackActivity activity);
 
   @Module
-  interface MainModule {
+  abstract class MainModule {
     @Binds
-    FirebaseAppDistribution bindAppDistro(FirebaseAppDistributionImpl impl);
+    abstract FirebaseAppDistribution bindAppDistro(FirebaseAppDistributionImpl impl);
 
     @Binds
     @Background
-    ExecutorService bindBackgroundExecutorService(@Background ScheduledExecutorService ses);
+    abstract ExecutorService bindBackgroundExecutorService(
+        @Background ScheduledExecutorService ses);
 
     @Binds
     @Background
-    Executor bindBackgroundExecutor(@Background ExecutorService es);
+    abstract Executor bindBackgroundExecutor(@Background ExecutorService es);
 
     @Binds
     @Lightweight
-    ExecutorService bindLightweightExecutorService(@Lightweight ScheduledExecutorService ses);
+    abstract ExecutorService bindLightweightExecutorService(
+        @Lightweight ScheduledExecutorService ses);
 
     @Binds
     @Lightweight
-    Executor bindLightweightExecutor(@Lightweight ExecutorService es);
+    abstract Executor bindLightweightExecutor(@Lightweight ExecutorService es);
 
     @Binds
     @Blocking
-    ExecutorService bindBlockingExecutorService(@Blocking ScheduledExecutorService ses);
+    abstract ExecutorService bindBlockingExecutorService(@Blocking ScheduledExecutorService ses);
 
     @Binds
     @Blocking
-    Executor bindBlockingExecutor(@Blocking ExecutorService es);
+    abstract Executor bindBlockingExecutor(@Blocking ExecutorService es);
+
+    @Provides
+    static ContentResolver bindContentResolver(Context applicationContext) {
+      return applicationContext.getContentResolver();
+    }
   }
 }

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

@@ -126,7 +126,8 @@ private void setupView() {
             v -> {
               Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT);
               intent.addCategory(Intent.CATEGORY_OPENABLE);
-              intent.setType("image/png");
+              intent.setType("*/*");
+              intent.putExtra(Intent.EXTRA_MIME_TYPES, new String[] {"image/png", "image/jpeg"});
               chooseScreenshotLauncher.launch(intent);
             });
 

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

@@ -14,23 +14,40 @@
 
 package com.google.firebase.appdistribution.impl;
 
+import android.content.ContentResolver;
+import android.database.Cursor;
 import android.net.Uri;
+import android.provider.OpenableColumns;
 import androidx.annotation.Nullable;
 import com.google.android.gms.tasks.Task;
 import com.google.android.gms.tasks.Tasks;
 import com.google.firebase.annotations.concurrent.Lightweight;
+import com.google.firebase.appdistribution.FirebaseAppDistributionException;
+import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
 import java.util.concurrent.Executor;
 import javax.inject.Inject;
 
 /** Sends tester feedback to the Tester API. */
 class FeedbackSender {
+  static final String CONTENT_TYPE_JPEG = "image/jpeg";
+  static final String CONTENT_TYPE_PNG = "image/png";
+
+  private static final String TAG = "FeedbackSender";
+  private static final String FILE_EXTENSION_JPG = ".jpg";
+  private static final String FILE_EXTENSION_JPEG = ".jpeg";
+  private static final String FILE_EXTENSION_PNG = ".png";
+  private static final String DEFAULT_FILENAME = "screenshot.png";
+
+  private final ContentResolver contentResolver;
   private final FirebaseAppDistributionTesterApiClient testerApiClient;
   @Lightweight private final Executor lightweightExecutor;
 
   @Inject
   FeedbackSender(
+      ContentResolver contentResolver,
       FirebaseAppDistributionTesterApiClient testerApiClient,
       @Lightweight Executor lightweightExecutor) {
+    this.contentResolver = contentResolver;
     this.testerApiClient = testerApiClient;
     this.lightweightExecutor = lightweightExecutor;
   }
@@ -54,6 +71,96 @@ private Task<String> attachScreenshot(String feedbackName, @Nullable Uri screens
     if (screenshotUri == null) {
       return Tasks.forResult(feedbackName);
     }
-    return testerApiClient.attachScreenshot(feedbackName, screenshotUri);
+    if (!screenshotUri.getScheme().equals("file") && !screenshotUri.getScheme().equals("content")) {
+      return Tasks.forException(
+          new FirebaseAppDistributionException(
+              String.format(
+                  "Screenshot URI '%s' was not a content or file URI. Not starting feedback.",
+                  screenshotUri),
+              Status.UNKNOWN));
+    }
+    String contentType = getContentType(screenshotUri);
+    return testerApiClient.attachScreenshot(
+        feedbackName,
+        screenshotUri,
+        getScreenshotFilename(screenshotUri, contentType),
+        contentType);
+  }
+
+  private String getContentType(Uri screenshotUri) {
+    String contentType = null;
+    if (screenshotUri.getScheme().equals("file")) {
+      contentType = getContentTypeForFilename(screenshotUri.getLastPathSegment());
+    } else if (screenshotUri.getScheme().equals("content")) {
+      contentType = contentResolver.getType(screenshotUri);
+    }
+
+    if (contentType != null) {
+      return contentType;
+    }
+
+    LogWrapper.w(
+        TAG, String.format("Could not get content type for URI %s. Assuming PNG.", screenshotUri));
+    return CONTENT_TYPE_PNG;
+  }
+
+  private String getScreenshotFilename(Uri screenshotUri, String contentType) {
+    if (screenshotUri.getScheme().equals("file")) {
+      return screenshotUri.getLastPathSegment();
+    } else {
+      return getContentFilename(screenshotUri, contentType);
+    }
+  }
+
+  private String getContentFilename(Uri contentUri, String contentType) {
+    try (Cursor returnCursor =
+        contentResolver.query(
+            contentUri,
+            new String[] {OpenableColumns.DISPLAY_NAME},
+            /* selection= */ null,
+            /* projectionArgs= */ null,
+            /* sortOrder= */ null)) {
+      if (returnCursor == null) {
+        LogWrapper.w(
+            TAG,
+            String.format("Unable to get filename from URI '%s', using content type", contentUri));
+        return getDefaultFilename(contentType);
+      }
+      int nameIndex = returnCursor.getColumnIndex(OpenableColumns.DISPLAY_NAME);
+      returnCursor.moveToFirst();
+      String name = returnCursor.getString(nameIndex);
+      return name;
+    } catch (Exception e) {
+      LogWrapper.e(
+          TAG,
+          String.format("Error getting filename from URI '%s', using content type", contentUri),
+          e);
+      return getDefaultFilename(contentType);
+    }
+  }
+
+  private String getDefaultFilename(String contentType) {
+    if (contentType.equals(CONTENT_TYPE_JPEG)) {
+      return "screenshot.jpg";
+    } else if (contentType.equals(CONTENT_TYPE_PNG)) {
+      return "screenshot.png";
+    } else {
+      LogWrapper.w(
+          TAG,
+          String.format(
+              "Unexpected content type '%s', using filename without extension", contentType));
+      return "screenshot";
+    }
+  }
+
+  @Nullable
+  private static String getContentTypeForFilename(String filename) {
+    if (filename.endsWith(FILE_EXTENSION_JPG) || filename.endsWith(FILE_EXTENSION_JPEG)) {
+      return CONTENT_TYPE_JPEG;
+    } else if (filename.endsWith(FILE_EXTENSION_PNG)) {
+      return CONTENT_TYPE_PNG;
+    } else {
+      return null;
+    }
   }
 }

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

@@ -381,6 +381,14 @@ private void startFeedback(
       @NonNull CharSequence additionalFormText,
       @Nullable Uri screenshotUri,
       FeedbackTrigger trigger) {
+    if (!screenshotUri.getScheme().equals("content") && !screenshotUri.getScheme().equals("file")) {
+      LogWrapper.e(
+          TAG,
+          String.format(
+              "Screenshot URI %s was not a content or file URI. Not starting feedback.",
+              screenshotUri));
+      return;
+    }
     if (!feedbackInProgress.compareAndSet(/* expect= */ false, /* update= */ true)) {
       LogWrapper.i(TAG, "Ignoring startFeedback() call because feedback is already in progress");
       return;