From 5c5e7cd257100dea8946555e51a396a87bc5351f Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 19 Jul 2023 10:24:29 +0100 Subject: [PATCH] Update storage access in main demo app The main demo app was still targeting API 29 to avoid scoped storage restrictions. It is now updated to 34 (like the rest of the demo apps) and handles scoped storage as it should handle it. More specifically: - We need to request READ_MEDIA_... permissions instead of READ_EXTERNAL_STORAGE from API33. - The legacy scoped storage opt-out can be removed - READ_MEDIA_... permissions don't allow arbitrary file access if the file doesn't end in a typical media file extension, so this change adds a remark on the guide page to place samples in the app-specific directory. - We also don't have to request permissions for the app-specific directories. - Custom json files can't be placed in arbitray local locations because they don't end in a media file extension, as there is no way we can request a permission to load them. This means we can remove the storage request logic in SampleChooserActivity. Issue: google/ExoPlayer#6045 PiperOrigin-RevId: 549252474 --- api.txt | 5 +- constants.gradle | 3 - demos/main/build.gradle | 4 +- demos/main/src/main/AndroidManifest.xml | 4 +- .../media3/demo/main/PlayerActivity.java | 2 +- .../demo/main/SampleChooserActivity.java | 34 +----- .../androidx/media3/common/util/Util.java | 104 ++++++++++++++---- 7 files changed, 91 insertions(+), 65 deletions(-) diff --git a/api.txt b/api.txt index 163431944a..0b8e1a3c79 100644 --- a/api.txt +++ b/api.txt @@ -1246,8 +1246,9 @@ package androidx.media3.common.util { method @androidx.media3.common.C.ContentType public static int inferContentType(android.net.Uri); method @androidx.media3.common.C.ContentType public static int inferContentTypeForExtension(String); method @androidx.media3.common.C.ContentType public static int inferContentTypeForUriAndMimeType(android.net.Uri, @Nullable String); - method public static boolean maybeRequestReadExternalStoragePermission(android.app.Activity, android.net.Uri...); - method public static boolean maybeRequestReadExternalStoragePermission(android.app.Activity, androidx.media3.common.MediaItem...); + method @Deprecated public static boolean maybeRequestReadExternalStoragePermission(android.app.Activity, android.net.Uri...); + method @Deprecated public static boolean maybeRequestReadExternalStoragePermission(android.app.Activity, androidx.media3.common.MediaItem...); + method public static boolean maybeRequestReadStoragePermission(android.app.Activity, androidx.media3.common.MediaItem...); method @org.checkerframework.checker.nullness.qual.EnsuresNonNullIf(result=false, expression="#1") public static boolean shouldShowPlayButton(@Nullable androidx.media3.common.Player); } diff --git a/constants.gradle b/constants.gradle index 118ccb2017..0f30434e0f 100644 --- a/constants.gradle +++ b/constants.gradle @@ -16,9 +16,6 @@ project.ext { releaseVersionCode = 1_001_000_3_00 minSdkVersion = 16 appTargetSdkVersion = 34 - // API version before restricting local file access. - // https://developer.android.com/training/data-storage/app-specific - mainDemoAppTargetSdkVersion = 29 // Upgrading this requires [Internal ref: b/193254928] to be fixed, or some // additional robolectric config. targetSdkVersion = 30 diff --git a/demos/main/build.gradle b/demos/main/build.gradle index 8038d49ee7..850a24ac0a 100644 --- a/demos/main/build.gradle +++ b/demos/main/build.gradle @@ -30,9 +30,7 @@ android { versionName project.ext.releaseVersion versionCode project.ext.releaseVersionCode minSdkVersion project.ext.minSdkVersion - // Not using appTargetSDKVersion to allow local file access on API 29 - // and higher [Internal ref: b/191644662] - targetSdkVersion project.ext.mainDemoAppTargetSdkVersion + targetSdkVersion project.ext.appTargetSdkVersion multiDexEnabled true } diff --git a/demos/main/src/main/AndroidManifest.xml b/demos/main/src/main/AndroidManifest.xml index 63266bfbd4..da13a42ca4 100644 --- a/demos/main/src/main/AndroidManifest.xml +++ b/demos/main/src/main/AndroidManifest.xml @@ -21,6 +21,9 @@ + + + @@ -36,7 +39,6 @@ android:banner="@drawable/ic_banner" android:largeHeap="true" android:allowBackup="false" - android:requestLegacyExternalStorage="true" android:supportsRtl="true" android:name="androidx.multidex.MultiDexApplication" tools:targetApi="29"> diff --git a/demos/main/src/main/java/androidx/media3/demo/main/PlayerActivity.java b/demos/main/src/main/java/androidx/media3/demo/main/PlayerActivity.java index d812c2ffbf..ad4b4c0fa7 100644 --- a/demos/main/src/main/java/androidx/media3/demo/main/PlayerActivity.java +++ b/demos/main/src/main/java/androidx/media3/demo/main/PlayerActivity.java @@ -354,7 +354,7 @@ public class PlayerActivity extends AppCompatActivity finish(); return Collections.emptyList(); } - if (Util.maybeRequestReadExternalStoragePermission(/* activity= */ this, mediaItem)) { + if (Util.maybeRequestReadStoragePermission(/* activity= */ this, mediaItem)) { // The player will be reinitialized if the permission is granted. return Collections.emptyList(); } diff --git a/demos/main/src/main/java/androidx/media3/demo/main/SampleChooserActivity.java b/demos/main/src/main/java/androidx/media3/demo/main/SampleChooserActivity.java index 5afa2613e3..7d10a65966 100644 --- a/demos/main/src/main/java/androidx/media3/demo/main/SampleChooserActivity.java +++ b/demos/main/src/main/java/androidx/media3/demo/main/SampleChooserActivity.java @@ -80,7 +80,6 @@ public class SampleChooserActivity extends AppCompatActivity private static final String TAG = "SampleChooserActivity"; private static final String GROUP_POSITION_PREFERENCE_KEY = "sample_chooser_group_position"; private static final String CHILD_POSITION_PREFERENCE_KEY = "sample_chooser_child_position"; - private static final int POST_NOTIFICATION_PERMISSION_REQUEST_CODE = 100; private String[] uris; private boolean useExtensionRenderers; @@ -179,14 +178,6 @@ public class SampleChooserActivity extends AppCompatActivity public void onRequestPermissionsResult( int requestCode, String[] permissions, int[] grantResults) { super.onRequestPermissionsResult(requestCode, permissions, grantResults); - if (requestCode == POST_NOTIFICATION_PERMISSION_REQUEST_CODE) { - handlePostNotificationPermissionGrantResults(grantResults); - } else { - handleExternalStoragePermissionGrantResults(grantResults); - } - } - - private void handlePostNotificationPermissionGrantResults(int[] grantResults) { if (!notificationPermissionToastShown && (grantResults.length == 0 || grantResults[0] != PackageManager.PERMISSION_GRANTED)) { Toast.makeText( @@ -201,30 +192,8 @@ public class SampleChooserActivity extends AppCompatActivity } } - private void handleExternalStoragePermissionGrantResults(int[] grantResults) { - if (grantResults.length == 0) { - // Empty results are triggered if a permission is requested while another request was already - // pending and can be safely ignored in this case. - return; - } else if (grantResults[0] == PackageManager.PERMISSION_GRANTED) { - loadSample(); - } else { - Toast.makeText(getApplicationContext(), R.string.sample_list_load_error, Toast.LENGTH_LONG) - .show(); - finish(); - } - } - private void loadSample() { checkNotNull(uris); - - for (int i = 0; i < uris.length; i++) { - Uri uri = Uri.parse(uris[i]); - if (Util.maybeRequestReadExternalStoragePermission(this, uri)) { - return; - } - } - SampleListLoader loaderTask = new SampleListLoader(); loaderTask.execute(uris); } @@ -279,8 +248,7 @@ public class SampleChooserActivity extends AppCompatActivity != PackageManager.PERMISSION_GRANTED) { downloadMediaItemWaitingForNotificationPermission = playlistHolder.mediaItems.get(0); requestPermissions( - new String[] {Api33.getPostNotificationPermissionString()}, - /* requestCode= */ POST_NOTIFICATION_PERMISSION_REQUEST_CODE); + new String[] {Api33.getPostNotificationPermissionString()}, /* requestCode= */ 0); } else { toggleDownload(playlistHolder.mediaItems.get(0)); } diff --git a/libraries/common/src/main/java/androidx/media3/common/util/Util.java b/libraries/common/src/main/java/androidx/media3/common/util/Util.java index 8e76c0bada..374e70f689 100644 --- a/libraries/common/src/main/java/androidx/media3/common/util/Util.java +++ b/libraries/common/src/main/java/androidx/media3/common/util/Util.java @@ -73,6 +73,7 @@ import android.util.SparseLongArray; import android.view.Display; import android.view.SurfaceView; import android.view.WindowManager; +import androidx.annotation.ChecksSdkIntAtLeast; import androidx.annotation.DoNotInline; import androidx.annotation.DrawableRes; import androidx.annotation.Nullable; @@ -331,19 +332,12 @@ public final class Util { } /** - * Checks whether it's necessary to request the {@link permission#READ_EXTERNAL_STORAGE} - * permission read the specified {@link Uri}s, requesting the permission if necessary. - * - * @param activity The host activity for checking and requesting the permission. - * @param uris {@link Uri}s that may require {@link permission#READ_EXTERNAL_STORAGE} to read. - * @return Whether a permission request was made. + * @deprecated Use {@link #maybeRequestReadStoragePermission(Activity, MediaItem...)} instead. */ + @Deprecated public static boolean maybeRequestReadExternalStoragePermission(Activity activity, Uri... uris) { - if (SDK_INT < 23) { - return false; - } for (Uri uri : uris) { - if (maybeRequestReadExternalStoragePermission(activity, uri)) { + if (maybeRequestReadStoragePermission(activity, uri)) { return true; } } @@ -351,16 +345,24 @@ public final class Util { } /** - * Checks whether it's necessary to request the {@link permission#READ_EXTERNAL_STORAGE} - * permission for the specified {@link MediaItem media items}, requesting the permission if - * necessary. + * @deprecated Use {@link #maybeRequestReadStoragePermission(Activity, MediaItem...)} instead. + */ + @Deprecated + public static boolean maybeRequestReadExternalStoragePermission( + Activity activity, MediaItem... mediaItems) { + return maybeRequestReadStoragePermission(activity, mediaItems); + } + + /** + * Checks whether it's necessary to request storage reading permissions for the specified {@link + * MediaItem media items}, requesting the permissions if necessary. * * @param activity The host activity for checking and requesting the permission. - * @param mediaItems {@link MediaItem Media items}s that may require {@link - * permission#READ_EXTERNAL_STORAGE} to read. + * @param mediaItems {@link MediaItem Media items}s that may require storage reading permissions + * to read. * @return Whether a permission request was made. */ - public static boolean maybeRequestReadExternalStoragePermission( + public static boolean maybeRequestReadStoragePermission( Activity activity, MediaItem... mediaItems) { if (SDK_INT < 23) { return false; @@ -369,13 +371,13 @@ public final class Util { if (mediaItem.localConfiguration == null) { continue; } - if (maybeRequestReadExternalStoragePermission(activity, mediaItem.localConfiguration.uri)) { + if (maybeRequestReadStoragePermission(activity, mediaItem.localConfiguration.uri)) { return true; } List subtitleConfigs = mediaItem.localConfiguration.subtitleConfigurations; for (int i = 0; i < subtitleConfigs.size(); i++) { - if (maybeRequestReadExternalStoragePermission(activity, subtitleConfigs.get(i).uri)) { + if (maybeRequestReadStoragePermission(activity, subtitleConfigs.get(i).uri)) { return true; } } @@ -383,10 +385,50 @@ public final class Util { return false; } - private static boolean maybeRequestReadExternalStoragePermission(Activity activity, Uri uri) { - return SDK_INT >= 23 - && (isLocalFileUri(uri) || isMediaStoreExternalContentUri(uri)) - && requestExternalStoragePermission(activity); + private static boolean maybeRequestReadStoragePermission(Activity activity, Uri uri) { + if (!isReadStoragePermissionRequestNeeded(activity, uri)) { + return false; + } + if (SDK_INT < 33) { + return requestExternalStoragePermission(activity); + } else { + return requestReadMediaPermissions(activity); + } + } + + @ChecksSdkIntAtLeast(api = 23) + private static boolean isReadStoragePermissionRequestNeeded(Activity activity, Uri uri) { + if (SDK_INT < 23) { + // Permission automatically granted via manifest below API 23. + return false; + } + if (isLocalFileUri(uri)) { + return !isAppSpecificStorageFileUri(activity, uri); + } + if (isMediaStoreExternalContentUri(uri)) { + return true; + } + return false; + } + + private static boolean isAppSpecificStorageFileUri(Activity activity, Uri uri) { + try { + @Nullable String uriPath = uri.getPath(); + if (uriPath == null) { + return false; + } + String filePath = new File(uriPath).getCanonicalPath(); + String internalAppDirectoryPath = activity.getFilesDir().getCanonicalPath(); + @Nullable File externalAppDirectory = activity.getExternalFilesDir(/* type= */ null); + @Nullable + String externalAppDirectoryPath = + externalAppDirectory == null ? null : externalAppDirectory.getCanonicalPath(); + return filePath.startsWith(internalAppDirectoryPath) + || (externalAppDirectoryPath != null && filePath.startsWith(externalAppDirectoryPath)); + } catch (IOException e) { + // Error while querying canonical paths. + return false; + } } private static boolean isMediaStoreExternalContentUri(Uri uri) { @@ -3225,6 +3267,24 @@ public final class Util { return false; } + @RequiresApi(api = 33) + private static boolean requestReadMediaPermissions(Activity activity) { + if (activity.checkSelfPermission(permission.READ_MEDIA_AUDIO) + != PackageManager.PERMISSION_GRANTED + || activity.checkSelfPermission(permission.READ_MEDIA_VIDEO) + != PackageManager.PERMISSION_GRANTED + || activity.checkSelfPermission(permission.READ_MEDIA_IMAGES) + != PackageManager.PERMISSION_GRANTED) { + activity.requestPermissions( + new String[] { + permission.READ_MEDIA_AUDIO, permission.READ_MEDIA_IMAGES, permission.READ_MEDIA_VIDEO + }, + /* requestCode= */ 0); + return true; + } + return false; + } + @RequiresApi(api = Build.VERSION_CODES.N) private static boolean isTrafficRestricted(Uri uri) { return "http".equals(uri.getScheme())