Skip to content

Commit

Permalink
Update storage access in main demo app
Browse files Browse the repository at this point in the history
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: #6045
PiperOrigin-RevId: 549252474
  • Loading branch information
tonihei authored and icbaker committed Jul 20, 2023
1 parent 553b627 commit 804dfe2
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 63 deletions.
3 changes: 0 additions & 3 deletions constants.gradle
Expand Up @@ -17,9 +17,6 @@ project.ext {
releaseVersionCode = 2_019_000
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
Expand Down
4 changes: 1 addition & 3 deletions demos/main/build.gradle
Expand Up @@ -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
}

Expand Down
4 changes: 3 additions & 1 deletion demos/main/src/main/AndroidManifest.xml
Expand Up @@ -21,6 +21,9 @@
<uses-permission android:name="android.permission.INTERNET"/>
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.READ_MEDIA_AUDIO"/>
<uses-permission android:name="android.permission.READ_MEDIA_IMAGES"/>
<uses-permission android:name="android.permission.READ_MEDIA_VIDEO"/>
<uses-permission android:name="android.permission.RECEIVE_BOOT_COMPLETED"/>
<uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_DATA_SYNC"/>
Expand All @@ -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">
Expand Down
Expand Up @@ -357,7 +357,7 @@ private List<MediaItem> createMediaItems(Intent intent) {
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();
}
Expand Down
Expand Up @@ -87,7 +87,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;
Expand Down Expand Up @@ -185,14 +184,6 @@ public void onDownloadsChanged() {
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(
Expand All @@ -207,30 +198,8 @@ private void handlePostNotificationPermissionGrantResults(int[] grantResults) {
}
}

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);
}
Expand Down Expand Up @@ -285,8 +254,7 @@ && checkSelfPermission(Api33.getPostNotificationPermissionString())
!= 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));
}
Expand Down
Expand Up @@ -73,6 +73,7 @@
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;
Expand Down Expand Up @@ -328,36 +329,37 @@ public static void setForegroundServiceNotification(
}

/**
* 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;
}
}
return false;
}

/**
* 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;
Expand All @@ -366,24 +368,64 @@ public static boolean maybeRequestReadExternalStoragePermission(
if (mediaItem.localConfiguration == null) {
continue;
}
if (maybeRequestReadExternalStoragePermission(activity, mediaItem.localConfiguration.uri)) {
if (maybeRequestReadStoragePermission(activity, mediaItem.localConfiguration.uri)) {
return true;
}
List<MediaItem.SubtitleConfiguration> 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;
}
}
}
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) {
Expand Down Expand Up @@ -3102,6 +3144,24 @@ private static boolean requestExternalStoragePermission(Activity activity) {
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())
Expand Down

0 comments on commit 804dfe2

Please sign in to comment.