From 3c6627d894c7d6fdcc229a594c5f8f857a6aba0c Mon Sep 17 00:00:00 2001 From: Daniel Nishi Date: Wed, 28 Mar 2018 15:31:17 -0700 Subject: [PATCH] Better public volumes handling on secondary users. Public volumes are only mounted for a single user at a time, so only show notifications and launch Intents for the relevant user. Test: RunSettingsRobotest Bug: 73642796 Change-Id: Ic386ec03598ab8968b75320be7844b6ac7f1387b --- .../settings/deviceinfo/StorageSettings.java | 125 ++++++++++-------- .../deviceinfo/StorageSettingsTest.java | 23 +++- 2 files changed, 87 insertions(+), 61 deletions(-) diff --git a/src/com/android/settings/deviceinfo/StorageSettings.java b/src/com/android/settings/deviceinfo/StorageSettings.java index aaa75e343fc..cf9d34b3bca 100644 --- a/src/com/android/settings/deviceinfo/StorageSettings.java +++ b/src/com/android/settings/deviceinfo/StorageSettings.java @@ -36,6 +36,7 @@ import android.os.storage.StorageManager; import android.os.storage.VolumeInfo; import android.os.storage.VolumeRecord; import android.support.annotation.NonNull; +import android.support.annotation.VisibleForTesting; import android.support.v7.preference.Preference; import android.support.v7.preference.PreferenceCategory; import android.text.TextUtils; @@ -71,10 +72,11 @@ public class StorageSettings extends SettingsPreferenceFragment implements Index private static final String TAG_VOLUME_UNMOUNTED = "volume_unmounted"; private static final String TAG_DISK_INIT = "disk_init"; + private static final int METRICS_CATEGORY = MetricsEvent.DEVICEINFO_STORAGE; static final int COLOR_PUBLIC = Color.parseColor("#ff9e9e9e"); - static final int[] COLOR_PRIVATE = new int[] { + static final int[] COLOR_PRIVATE = new int[]{ Color.parseColor("#ff26a69a"), Color.parseColor("#ffab47bc"), Color.parseColor("#fff2a600"), @@ -94,7 +96,7 @@ public class StorageSettings extends SettingsPreferenceFragment implements Index @Override public int getMetricsCategory() { - return MetricsEvent.DEVICEINFO_STORAGE; + return METRICS_CATEGORY; } @Override @@ -139,7 +141,7 @@ public class StorageSettings extends SettingsPreferenceFragment implements Index }; private static boolean isInteresting(VolumeInfo vol) { - switch(vol.getType()) { + switch (vol.getType()) { case VolumeInfo.TYPE_PRIVATE: case VolumeInfo.TYPE_PUBLIC: return true; @@ -301,20 +303,7 @@ public class StorageSettings extends SettingsPreferenceFragment implements Index return true; } else if (vol.getType() == VolumeInfo.TYPE_PUBLIC) { - if (vol.isMountedReadable()) { - startActivity(vol.buildBrowseIntent()); - return true; - } else { - final Bundle args = new Bundle(); - args.putString(VolumeInfo.EXTRA_VOLUME_ID, vol.getId()); - new SubSettingLauncher(getContext()) - .setDestination(PublicVolumeSettings.class.getCanonicalName()) - .setTitle(-1) - .setSourceMetricsCategory(getMetricsCategory()) - .setArguments(args) - .launch(); - return true; - } + return handlePublicVolumeClick(getContext(), vol); } } else if (key.startsWith("disk:")) { @@ -328,7 +317,7 @@ public class StorageSettings extends SettingsPreferenceFragment implements Index args.putString(VolumeRecord.EXTRA_FS_UUID, key); new SubSettingLauncher(getContext()) .setDestination(PrivateVolumeForget.class.getCanonicalName()) - .setTitle(R.string.storage_menu_forget) + .setTitle(R.string.storage_menu_forget) .setSourceMetricsCategory(getMetricsCategory()) .setArguments(args) .launch(); @@ -338,6 +327,25 @@ public class StorageSettings extends SettingsPreferenceFragment implements Index return false; } + @VisibleForTesting + static boolean handlePublicVolumeClick(Context context, VolumeInfo vol) { + final Intent intent = vol.buildBrowseIntent(); + if (vol.isMountedReadable() && intent != null) { + context.startActivity(intent); + return true; + } else { + final Bundle args = new Bundle(); + args.putString(VolumeInfo.EXTRA_VOLUME_ID, vol.getId()); + new SubSettingLauncher(context) + .setDestination(PublicVolumeSettings.class.getCanonicalName()) + .setTitle(-1) + .setSourceMetricsCategory(METRICS_CATEGORY) + .setArguments(args) + .launch(); + return true; + } + } + public static class MountTask extends AsyncTask { private final Context mContext; private final StorageManager mStorageManager; @@ -440,40 +448,45 @@ public class StorageSettings extends SettingsPreferenceFragment implements Index builder.setPositiveButton(R.string.storage_menu_mount, new DialogInterface.OnClickListener() { - /** - * Check if an {@link RestrictedLockUtils#sendShowAdminSupportDetailsIntent admin - * details intent} should be shown for the restriction and show it. - * - * @param restriction The restriction to check - * @return {@code true} iff a intent was shown. - */ - private boolean wasAdminSupportIntentShown(@NonNull String restriction) { - EnforcedAdmin admin = RestrictedLockUtils.checkIfRestrictionEnforced( - getActivity(), restriction, UserHandle.myUserId()); - boolean hasBaseUserRestriction = RestrictedLockUtils.hasBaseUserRestriction( - getActivity(), restriction, UserHandle.myUserId()); - if (admin != null && !hasBaseUserRestriction) { - RestrictedLockUtils.sendShowAdminSupportDetailsIntent(getActivity(), admin); - return true; - } + /** + * Check if an {@link + * RestrictedLockUtils#sendShowAdminSupportDetailsIntent admin + * details intent} should be shown for the restriction and show it. + * + * @param restriction The restriction to check + * @return {@code true} iff a intent was shown. + */ + private boolean wasAdminSupportIntentShown(@NonNull String restriction) { + EnforcedAdmin admin = RestrictedLockUtils.checkIfRestrictionEnforced( + getActivity(), restriction, UserHandle.myUserId()); + boolean hasBaseUserRestriction = + RestrictedLockUtils.hasBaseUserRestriction( + getActivity(), restriction, UserHandle.myUserId()); + if (admin != null && !hasBaseUserRestriction) { + RestrictedLockUtils.sendShowAdminSupportDetailsIntent(getActivity(), + admin); + return true; + } - return false; - } + return false; + } - @Override - public void onClick(DialogInterface dialog, int which) { - if (wasAdminSupportIntentShown(UserManager.DISALLOW_MOUNT_PHYSICAL_MEDIA)) { - return; - } + @Override + public void onClick(DialogInterface dialog, int which) { + if (wasAdminSupportIntentShown( + UserManager.DISALLOW_MOUNT_PHYSICAL_MEDIA)) { + return; + } - if (vol.disk != null && vol.disk.isUsb() && - wasAdminSupportIntentShown(UserManager.DISALLOW_USB_FILE_TRANSFER)) { - return; - } + if (vol.disk != null && vol.disk.isUsb() && + wasAdminSupportIntentShown( + UserManager.DISALLOW_USB_FILE_TRANSFER)) { + return; + } - new MountTask(context, vol).execute(); - } - }); + new MountTask(context, vol).execute(); + } + }); builder.setNegativeButton(R.string.cancel, null); return builder.create(); @@ -511,13 +524,13 @@ public class StorageSettings extends SettingsPreferenceFragment implements Index builder.setPositiveButton(R.string.storage_menu_set_up, new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - final Intent intent = new Intent(context, StorageWizardInit.class); - intent.putExtra(DiskInfo.EXTRA_DISK_ID, diskId); - startActivity(intent); - } - }); + @Override + public void onClick(DialogInterface dialog, int which) { + final Intent intent = new Intent(context, StorageWizardInit.class); + intent.putExtra(DiskInfo.EXTRA_DISK_ID, diskId); + startActivity(intent); + } + }); builder.setNegativeButton(R.string.cancel, null); return builder.create(); @@ -586,7 +599,7 @@ public class StorageSettings extends SettingsPreferenceFragment implements Index for (VolumeInfo vol : vols) { if (isInteresting(vol)) { data.title = storage.getBestVolumeDescription(vol); - data.key = "storage_settings_volume_" +vol.id; + data.key = "storage_settings_volume_" + vol.id; data.screenTitle = context.getString(R.string.storage_settings); result.add(data); } diff --git a/tests/robotests/src/com/android/settings/deviceinfo/StorageSettingsTest.java b/tests/robotests/src/com/android/settings/deviceinfo/StorageSettingsTest.java index 0c9f313e6bc..943bd9d2630 100644 --- a/tests/robotests/src/com/android/settings/deviceinfo/StorageSettingsTest.java +++ b/tests/robotests/src/com/android/settings/deviceinfo/StorageSettingsTest.java @@ -16,14 +16,17 @@ package com.android.settings.deviceinfo; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.Activity; import android.app.usage.StorageStatsManager; +import android.content.Intent; import android.icu.text.NumberFormat; import android.os.storage.VolumeInfo; import android.text.format.Formatter; @@ -71,14 +74,14 @@ public class StorageSettingsTest { when(volumeInfo.isMountedReadable()).thenReturn(true); when(volumeInfo.getType()).thenReturn(VolumeInfo.TYPE_PRIVATE); when(mStorageManagerVolumeProvider - .getTotalBytes(nullable(StorageStatsManager.class), nullable(VolumeInfo.class))) - .thenReturn(500L); + .getTotalBytes(nullable(StorageStatsManager.class), nullable(VolumeInfo.class))) + .thenReturn(500L); when(mStorageManagerVolumeProvider - .getFreeBytes(nullable(StorageStatsManager.class), nullable(VolumeInfo.class))) - .thenReturn(0L); + .getFreeBytes(nullable(StorageStatsManager.class), nullable(VolumeInfo.class))) + .thenReturn(0L); ReflectionHelpers - .setField(provider, "mStorageManagerVolumeProvider", mStorageManagerVolumeProvider); + .setField(provider, "mStorageManagerVolumeProvider", mStorageManagerVolumeProvider); ReflectionHelpers.setField(provider, "mContext", RuntimeEnvironment.application); provider.setListening(true); @@ -89,4 +92,14 @@ public class StorageSettingsTest { RuntimeEnvironment.application.getString( R.string.storage_summary, percentage, freeSpace)); } + + @Test + public void handlePublicVolumeClick_startsANonNullActivityWhenVolumeHasNoBrowse() { + VolumeInfo volumeInfo = mock(VolumeInfo.class, RETURNS_DEEP_STUBS); + when(volumeInfo.isMountedReadable()).thenReturn(true); + StorageSettings.handlePublicVolumeClick(mActivity, volumeInfo); + + verify(mActivity, never()).startActivity(null); + verify(mActivity).startActivity(any(Intent.class)); + } }