Request different permission on 33+ and some more feedback

Author: lfkellogg

firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/TesterApiHttpClientTest.java

@@ -252,7 +252,6 @@ public void makePostRequest_whenConnectionFails_throwsError() throws Exception {
   public void makeUploadRequest_writesRequestBodyAndSetsCorrectHeaders() throws Exception {
     String responseJson = readTestFile("testSimpleResponse.json");
 
-    // Setup test streams. They should be closed but close them anyway just to be safe.
     try (InputStream postBodyInputStream =
             new ByteArrayInputStream("Test post body".getBytes(UTF_8));
         ByteArrayOutputStream requestBodyOutputStream = new ByteArrayOutputStream();

firebase-appdistribution/test-app/src/main/AndroidManifest.xml

@@ -1,6 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
     package="com.googletest.firebase.appdistribution.testapp">
+  <uses-permission android:name="android.permission.READ_MEDIA_IMAGES" />
 
   <application
       android:allowBackup="true"

firebase-appdistribution/test-app/src/main/java/com/googletest/firebase/appdistribution/testapp/MainActivity.kt

@@ -70,7 +70,7 @@ class MainActivity : AppCompatActivity() {
         screenshotTrigger =
             ScreenshotDetectionFeedbackTrigger(
                 this,
-                "Here's some terms and conditions",
+                R.string.terms_and_conditions,
                 Handler(screenshotTriggerThread.looper)
             )
     }

firebase-appdistribution/test-app/src/main/java/com/googletest/firebase/appdistribution/testapp/ScreenshotDetectionFeedbackTrigger.java

@@ -14,11 +14,11 @@
 
 package com.googletest.firebase.appdistribution.testapp;
 
-import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE;
+import static android.Manifest.permission.READ_EXTERNAL_STORAGE;
+import static android.Manifest.permission.READ_MEDIA_IMAGES;
 import static android.content.pm.PackageManager.PERMISSION_GRANTED;
 
 import android.app.AlertDialog;
-import android.content.Context;
 import android.database.ContentObserver;
 import android.database.Cursor;
 import android.net.Uri;
@@ -27,17 +27,22 @@
 import android.provider.MediaStore;
 import android.provider.MediaStore.Images.Media;
 import android.util.Log;
-import androidx.activity.ComponentActivity;
 import androidx.activity.result.ActivityResultLauncher;
 import androidx.activity.result.contract.ActivityResultContracts;
+import androidx.appcompat.app.AppCompatActivity;
 import androidx.core.content.ContextCompat;
 import com.google.firebase.appdistribution.FirebaseAppDistribution;
 import java.util.HashSet;
 import java.util.Set;
 
 class ScreenshotDetectionFeedbackTrigger extends ContentObserver {
-  private static final String TAG = "FeedbackTriggers";
-  private static final boolean SHOULD_CHECK_IF_PENDING = Build.VERSION.SDK_INT == 29;
+  private static final String TAG = "ScreenshotDetectionFeedbackTrigger";
+
+  private static final String PERMISSION_TO_REQUEST =
+      Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU
+          ? READ_MEDIA_IMAGES
+          : READ_EXTERNAL_STORAGE;
+  private static final boolean SHOULD_CHECK_IF_PENDING = Build.VERSION.SDK_INT >= 29;
   private static final String[] PROJECTION =
       SHOULD_CHECK_IF_PENDING
           ? new String[] {MediaStore.Images.Media.DATA}
@@ -46,8 +51,8 @@ class ScreenshotDetectionFeedbackTrigger extends ContentObserver {
           };
   private final Set<Uri> seenImages = new HashSet<>();
 
-  private final Context context;
-  private final CharSequence infoText;
+  private final AppCompatActivity activity;
+  private final int infoTextResourceId;
 
   private final ActivityResultLauncher<String> requestPermissionLauncher;
 
@@ -62,10 +67,10 @@ class ScreenshotDetectionFeedbackTrigger extends ContentObserver {
    * @param handler The handler to run {@link #onChange} on, or null if none.
    */
   public ScreenshotDetectionFeedbackTrigger(
-      ComponentActivity activity, CharSequence infoText, Handler handler) {
+          AppCompatActivity activity, int infoTextResourceId, Handler handler) {
     super(handler);
-    this.context = activity;
-    this.infoText = infoText;
+    this.activity = activity;
+    this.infoTextResourceId = infoTextResourceId;
 
     // Register the permissions launcher
     requestPermissionLauncher =
@@ -75,28 +80,23 @@ public ScreenshotDetectionFeedbackTrigger(
               if (isGranted) {
                 maybeStartFeedbackForScreenshot(currentUri);
               } else {
-                new AlertDialog.Builder(context)
-                    .setMessage(
-                        "Without storage access, screenshots taken while in the app will not be able to automatically start the feedback flow. To enable this feature, grant the app storage access in Settings.")
-                    .setPositiveButton(
-                        "OK",
-                        (a, b) -> {
-                          /* do nothing */
-                        })
-                    .show();
+                Log.i(TAG, "Permission not granted");
+                // TODO: Ideally we would show a message indicating the impact of not enabling the
+                // permission, but there's no way to know if they've permanently denied the
+                // permission, and we don't want to show them a message on every screenshot.
               }
             });
   }
 
   void registerScreenshotObserver() {
-    context
+    activity
         .getContentResolver()
         .registerContentObserver(
             Media.EXTERNAL_CONTENT_URI, /* notifyForDescendants= */ true, this);
   }
 
   void unRegisterScreenshotObserver() {
-    context.getContentResolver().unregisterContentObserver(this);
+    activity.getContentResolver().unregisterContentObserver(this);
   }
 
   @Override
@@ -106,28 +106,39 @@ public void onChange(boolean selfChange, Uri uri) {
       return;
     }
 
-    if (ContextCompat.checkSelfPermission(context, WRITE_EXTERNAL_STORAGE) == PERMISSION_GRANTED) {
+    if (ContextCompat.checkSelfPermission(activity, PERMISSION_TO_REQUEST) == PERMISSION_GRANTED) {
       maybeStartFeedbackForScreenshot(uri);
+    } else if (activity.shouldShowRequestPermissionRationale(PERMISSION_TO_REQUEST)) {
+      Log.i(TAG, "Showing customer rationale for requesting permission.");
+      new AlertDialog.Builder(activity)
+              .setMessage(
+                      "Taking a screenshot of the app can initiate feedback to the developer. To enable this feature, allow the app access to device storage.")
+              .setPositiveButton(
+                      "OK",
+                      (a, b) -> {
+                        Log.i(TAG, "Launching request for permission.");
+                        currentUri = uri;
+                        requestPermissionLauncher.launch(PERMISSION_TO_REQUEST);
+                      })
+              .show();
     } else {
-      // Ideally we'd give the user some context here, but we have no way of knowing if they
-      // have previously "Deny & don't ask again" in which case we wouldn't want to show them
-      // anything here.
+      Log.i(TAG, "Does not have permission. Launching request.");
       currentUri = uri;
-      requestPermissionLauncher.launch(WRITE_EXTERNAL_STORAGE);
+      requestPermissionLauncher.launch(PERMISSION_TO_REQUEST);
     }
   }
 
   private void maybeStartFeedbackForScreenshot(Uri uri) {
     Cursor cursor = null;
     try {
-      cursor = context.getContentResolver().query(uri, PROJECTION, null, null, null);
+      cursor = activity.getContentResolver().query(uri, PROJECTION, null, null, null);
       if (cursor != null && cursor.moveToFirst()) {
         // TODO: check if it's pending
         // (http://google3/lens/screenshots/demo/java/com/google/android/lensonscreenshots/ScreenshotDetector.java?l=184)
         String path = cursor.getString(cursor.getColumnIndexOrThrow(MediaStore.Images.Media.DATA));
         Log.i(TAG, "Path: " + path);
         if (path.toLowerCase().contains("screenshot")) {
-          FirebaseAppDistribution.getInstance().startFeedback(infoText, uri);
+          FirebaseAppDistribution.getInstance().startFeedback(infoTextResourceId, uri);
         }
       }
     } catch (Exception e) {