diff options
| -rw-r--r-- | photopicker/res/values/feature_search_strings.xml | 3 | ||||
| -rw-r--r-- | photopicker/src/com/android/photopicker/features/search/Search.kt | 44 | ||||
| -rw-r--r-- | src/com/android/providers/media/LocalCallingIdentity.java | 12 | ||||
| -rw-r--r-- | src/com/android/providers/media/util/PermissionUtils.java | 35 | ||||
| -rw-r--r-- | tests/Android.bp | 20 | ||||
| -rw-r--r-- | tests/AndroidManifest.xml | 1 | ||||
| -rw-r--r-- | tests/AndroidTest.xml | 1 | ||||
| -rw-r--r-- | tests/src/com/android/providers/media/util/PermissionUtilsTest.java | 49 | ||||
| -rw-r--r-- | tests/test_app/LegacyTestAppWithTargetSdk33.xml | 40 |
9 files changed, 195 insertions, 10 deletions
diff --git a/photopicker/res/values/feature_search_strings.xml b/photopicker/res/values/feature_search_strings.xml index 78af2a470..dacf372f7 100644 --- a/photopicker/res/values/feature_search_strings.xml +++ b/photopicker/res/values/feature_search_strings.xml @@ -24,6 +24,9 @@ <!-- Search view placeholder text when videos MIME type filter is applied--> <string name="photopicker_search_videos_placeholder_text" translation_description="Place holder text shown in Search Bar for videos MIME type filter">Search your videos</string> + <!-- Search Bar trailing icon description text--> + <string name="photopicker_search_clear_text" translation_description="Description for trailing icon to clear search text in Search Bar">Clear search text</string> + <!-- Empty state title when the search has no results --> <string name="photopicker_search_result_empty_state_title" translation_description="Title of the message shown to the user when there are no search results to show">No results found</string> diff --git a/photopicker/src/com/android/photopicker/features/search/Search.kt b/photopicker/src/com/android/photopicker/features/search/Search.kt index 0f2c619be..9cd927e09 100644 --- a/photopicker/src/com/android/photopicker/features/search/Search.kt +++ b/photopicker/src/com/android/photopicker/features/search/Search.kt @@ -39,6 +39,7 @@ import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material.icons.filled.Close import androidx.compose.material.icons.filled.Today import androidx.compose.material.icons.outlined.HideImage import androidx.compose.material.icons.outlined.History @@ -118,6 +119,7 @@ import com.android.photopicker.core.theme.LocalWindowSizeClass import com.android.photopicker.extensions.navigateToPreviewMedia import com.android.photopicker.extensions.transferScrollableTouchesToHostInEmbedded import com.android.photopicker.features.preview.PreviewFeature +import com.android.photopicker.features.search.SearchViewModel.Companion.ZERO_STATE_SEARCH_QUERY import com.android.photopicker.features.search.model.SearchSuggestion import com.android.photopicker.features.search.model.SearchSuggestionType import com.android.photopicker.features.search.model.UserSearchState @@ -428,16 +430,12 @@ fun SearchInputContent( * @param searchQuery The current text entered in search bar input field. * @param focused A boolean value indicating whether the search input field is currently focused. * @param onSearchQueryChanged A callback function that is invoked when the search query text - * changes. - * * This function receives the updated search query as a parameter. - * + * changes. This function receives the updated search query as a parameter. * @param onFocused A callback function that is invoked when the focus state of the search field - * changes. - * * This function receives a boolean value indicating the new focus state. - * + * changes. This function receives a boolean value indicating the new focus state. * @param onSearch A callback function to be invoked when a text is searched. * @param modifier A Modifier that can be applied to the SearchInput composable to customize its - * * appearance and behavior. + * appearance and behavior. */ @Composable @OptIn(ExperimentalMaterial3Api::class) @@ -473,11 +471,43 @@ private fun SearchInput( expanded = focused, onExpandedChange = onFocused, leadingIcon = { SearchBarIcon(focused, onFocused, onSearchQueryChanged) }, + trailingIcon = { + SearchBarTrailingIcon( + focused && !searchQuery.equals(ZERO_STATE_SEARCH_QUERY), + onSearchQueryChanged, + ) + }, modifier = modifier.focusRequester(focusRequester), ) RequestFocusOnResume(focusRequester = focusRequester, focused) } +/** + * A composable function that displays the trailing icon in a SearchBar. The icon is shown when + * query is typed clicking on which clears the typed text. + * + * @param showClearIcon A boolean value indicating whether clear icon is to be shown + * @param onSearchQueryChanged A callback function that is invoked when the search query text + * changes. This function receives the updated search query as a parameter. + * @param viewModel The `SearchViewModel` providing the search logic and state. + */ +@Composable +private fun SearchBarTrailingIcon( + showClearIcon: Boolean, + onSearchQueryChanged: (String) -> Unit, + viewModel: SearchViewModel = obtainViewModel(), +) { + val searchState by viewModel.searchState.collectAsStateWithLifecycle() + if (showClearIcon && searchState is SearchState.Inactive) { + IconButton(onClick = { onSearchQueryChanged("") }) { + Icon( + Icons.Filled.Close, + contentDescription = stringResource(R.string.photopicker_search_clear_text), + ) + } + } +} + @Composable private fun RequestFocusOnResume( focusRequester: FocusRequester, diff --git a/src/com/android/providers/media/LocalCallingIdentity.java b/src/com/android/providers/media/LocalCallingIdentity.java index 8074656bd..2a5e452ce 100644 --- a/src/com/android/providers/media/LocalCallingIdentity.java +++ b/src/com/android/providers/media/LocalCallingIdentity.java @@ -29,8 +29,8 @@ import static com.android.providers.media.util.PermissionUtils.checkPermissionIn import static com.android.providers.media.util.PermissionUtils.checkPermissionManager; import static com.android.providers.media.util.PermissionUtils.checkPermissionQueryAllPackages; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadAudio; +import static com.android.providers.media.util.PermissionUtils.checkPermissionReadForLegacyStorage; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadImages; -import static com.android.providers.media.util.PermissionUtils.checkPermissionReadStorage; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadVideo; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadVisualUserSelected; import static com.android.providers.media.util.PermissionUtils.checkPermissionSelf; @@ -576,8 +576,14 @@ public class LocalCallingIdentity { } private boolean isLegacyReadInternal() { - return hasPermission(PERMISSION_IS_LEGACY_GRANTED) - && checkPermissionReadStorage(context, pid, uid, getPackageName(), attributionTag); + boolean isLegacyStorageGranted = hasPermission(PERMISSION_IS_LEGACY_GRANTED); + if (!isLegacyStorageGranted) { + return false; + } + + boolean isTargetSdkAtleastT = getTargetSdkVersion() >= Build.VERSION_CODES.TIRAMISU; + return checkPermissionReadForLegacyStorage(context, pid, uid, getPackageName(), + attributionTag, isTargetSdkAtleastT); } /** System internals or callers holding permission have no redaction */ diff --git a/src/com/android/providers/media/util/PermissionUtils.java b/src/com/android/providers/media/util/PermissionUtils.java index 2458785e2..7dbfd23da 100644 --- a/src/com/android/providers/media/util/PermissionUtils.java +++ b/src/com/android/providers/media/util/PermissionUtils.java @@ -149,6 +149,41 @@ public class PermissionUtils { } /** + * Check for read permission when legacy storage is granted. + * There is a bug in AppOpsManager that keeps legacy storage granted even + * when an app updates its targetSdkVersion from value <30 to >=30. + * If an app upgrades from targetSdk 29 to targetSdk 33, legacy storage + * remains granted and in targetSdk 33, app are required to replace R_E_S + * with R_M_*. If an app updates its manifest with R_M_*, permission check + * in MediaProvider will look for R_E_S and will not grant read access as + * the app would be still treated as legacy. Ensure that legacy app either has + * R_E_S or all of R_M_* to get read permission. Since this is a fix for legacy + * app op bug, we are avoiding granular permission checks based on media type. + */ + public static boolean checkPermissionReadForLegacyStorage(@NonNull Context context, + int pid, int uid, @NonNull String packageName, @Nullable String attributionTag, + boolean isTargetSdkAtleastT) { + if (isTargetSdkAtleastT) { + return checkPermissionForDataDelivery(context, READ_EXTERNAL_STORAGE, pid, uid, + packageName, attributionTag, + generateAppOpMessage(packageName, sOpDescription.get())) || ( + checkPermissionForDataDelivery(context, READ_MEDIA_IMAGES, pid, uid, + packageName, attributionTag, + generateAppOpMessage(packageName, sOpDescription.get())) + && checkPermissionForDataDelivery(context, READ_MEDIA_VIDEO, pid, uid, + packageName, attributionTag, + generateAppOpMessage(packageName, sOpDescription.get())) + && checkPermissionForDataDelivery(context, READ_MEDIA_AUDIO, pid, uid, + packageName, attributionTag, + generateAppOpMessage(packageName, sOpDescription.get()))); + } else { + return checkPermissionForDataDelivery(context, READ_EXTERNAL_STORAGE, pid, uid, + packageName, attributionTag, + generateAppOpMessage(packageName, sOpDescription.get())); + } + } + + /** * Check if the given package has been granted the * android.Manifest.permission#ACCESS_MEDIA_LOCATION permission. */ diff --git a/tests/Android.bp b/tests/Android.bp index cdb44b814..3de35a077 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -155,6 +155,25 @@ android_test_helper_app { ], } +android_test_helper_app { + name: "LegacyMediaProviderTestAppFor33", + manifest: "test_app/LegacyTestAppWithTargetSdk33.xml", + srcs: [ + "test_app/src/**/*.java", + "src/com/android/providers/media/util/TestUtils.java", + ], + static_libs: [ + "cts-install-lib", + ], + sdk_version: "test_current", + target_sdk_version: "33", + min_sdk_version: "30", + test_suites: [ + "general-tests", + "mts-mediaprovider", + ], +} + // This looks a bit awkward, but we need our tests to run against either // MediaProvider or MediaProviderGoogle, and we don't know which one is // on the device being tested, so we can't sign our tests with a key that @@ -250,6 +269,7 @@ android_test { data: [ ":LegacyMediaProviderTestApp", + ":LegacyMediaProviderTestAppFor33", ":LegacyMediaProviderTestAppFor35", ":MediaProviderTestAppForPermissionActivity", ":MediaProviderTestAppForPermissionActivity33", diff --git a/tests/AndroidManifest.xml b/tests/AndroidManifest.xml index 0917ac388..2e7d9348c 100644 --- a/tests/AndroidManifest.xml +++ b/tests/AndroidManifest.xml @@ -13,6 +13,7 @@ <package android:name="com.android.providers.media.testapp.withuserselectedperms" /> <package android:name="com.android.providers.media.testapp.legacy" /> <package android:name="com.android.providers.media.testapp.legacywithtargetsdk35" /> + <package android:name="com.android.providers.media.testapp.legacywithtargetsdk33" /> </queries> <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" /> diff --git a/tests/AndroidTest.xml b/tests/AndroidTest.xml index 31d9e1535..44fb27930 100644 --- a/tests/AndroidTest.xml +++ b/tests/AndroidTest.xml @@ -30,6 +30,7 @@ <option name="test-file-name" value="MediaProviderTestAppWithUserSelectedPerms.apk" /> <option name="test-file-name" value="MediaProviderTestAppWithoutPerms.apk" /> <option name="test-file-name" value="LegacyMediaProviderTestApp.apk" /> + <option name="test-file-name" value="LegacyMediaProviderTestAppFor33.apk" /> <option name="test-file-name" value="LegacyMediaProviderTestAppFor35.apk" /> <option name="install-arg" value="-g" /> </target_preparer> diff --git a/tests/src/com/android/providers/media/util/PermissionUtilsTest.java b/tests/src/com/android/providers/media/util/PermissionUtilsTest.java index f58f1bbf6..7eb6c667b 100644 --- a/tests/src/com/android/providers/media/util/PermissionUtilsTest.java +++ b/tests/src/com/android/providers/media/util/PermissionUtilsTest.java @@ -21,6 +21,7 @@ import static android.Manifest.permission.MANAGE_EXTERNAL_STORAGE; import static android.Manifest.permission.MANAGE_MEDIA; import static android.Manifest.permission.UPDATE_APP_OPS_STATS; import static android.app.AppOpsManager.OPSTR_ACCESS_MEDIA_LOCATION; +import static android.app.AppOpsManager.OPSTR_LEGACY_STORAGE; import static android.app.AppOpsManager.OPSTR_NO_ISOLATED_STORAGE; import static android.app.AppOpsManager.OPSTR_READ_MEDIA_AUDIO; import static android.app.AppOpsManager.OPSTR_READ_MEDIA_IMAGES; @@ -44,6 +45,7 @@ import static com.android.providers.media.util.PermissionUtils.checkPermissionIn import static com.android.providers.media.util.PermissionUtils.checkPermissionManageMedia; import static com.android.providers.media.util.PermissionUtils.checkPermissionManager; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadAudio; +import static com.android.providers.media.util.PermissionUtils.checkPermissionReadForLegacyStorage; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadImages; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadStorage; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadVideo; @@ -105,6 +107,10 @@ public class PermissionUtilsTest { "com.android.providers.media.testapp.legacy", 1, false, "LegacyMediaProviderTestApp.apk"); + private static final TestApp LEGACY_TEST_APP_33 = new TestApp("LegacyTestAppWithTargetSdk33", + "com.android.providers.media.testapp.legacywithtargetsdk33", 1, false, + "LegacyMediaProviderTestAppFor33.apk"); + private static final TestApp LEGACY_TEST_APP_35 = new TestApp("LegacyTestAppWithTargetSdk35", "com.android.providers.media.testapp.legacywithtargetsdk35", 1, false, "LegacyMediaProviderTestAppFor35.apk"); @@ -251,6 +257,44 @@ public class PermissionUtilsTest { getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); assertThat(checkPermissionReadStorage( getContext(), TEST_APP_PID, testAppUid, packageName, null)).isTrue(); + assertThat(checkPermissionReadForLegacyStorage( + getContext(), TEST_APP_PID, testAppUid, packageName, + null, /* isTargetSdkAtLeastT */ true)).isTrue(); + } finally { + dropShellPermission(); + } + } + + @Test + @SdkSuppress(minSdkVersion = Build.VERSION_CODES.TIRAMISU, codeName = "Tiramisu") + public void testDefaultPermissionsOnLegacyTestAppWithTargetSdk33() throws Exception { + String packageName = LEGACY_TEST_APP_33.getPackageName(); + int testAppUid = getContext().getPackageManager().getPackageUid(packageName, 0); + adoptShellPermission(UPDATE_APP_OPS_STATS, MANAGE_APP_OPS_MODES); + + try { + assertThat(checkPermissionSelf(getContext(), TEST_APP_PID, testAppUid)).isFalse(); + assertThat(checkPermissionShell(testAppUid)).isFalse(); + assertThat(checkPermissionInstallPackages( + getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); + assertThat(checkPermissionAccessMtp( + getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); + assertThat(checkPermissionWriteStorage( + getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); + + modifyAppOp(testAppUid, OPSTR_READ_MEDIA_IMAGES, AppOpsManager.MODE_ALLOWED); + modifyAppOp(testAppUid, OPSTR_READ_MEDIA_VIDEO, AppOpsManager.MODE_ALLOWED); + modifyAppOp(testAppUid, OPSTR_READ_MEDIA_AUDIO, AppOpsManager.MODE_ALLOWED); + modifyAppOp(testAppUid, OPSTR_LEGACY_STORAGE, AppOpsManager.MODE_ALLOWED); + + assertThat(checkIsLegacyStorageGranted(getContext(), testAppUid, + packageName, /* isTargetSdkAtLeastV */ false)).isTrue(); + // Since R_E_S is not granted, this is should return false + assertThat(checkPermissionReadStorage( + getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); + assertThat(checkPermissionReadForLegacyStorage( + getContext(), TEST_APP_PID, testAppUid, packageName, + null, /* isTargetSdkAtLeastT */ true)).isTrue(); } finally { dropShellPermission(); } @@ -330,6 +374,11 @@ public class PermissionUtilsTest { checkIsLegacyStorageGranted(getContext(), testAppUid, packageName, /* isTargetSdkAtLeastS */ false)).isTrue(); assertThat( + checkPermissionReadForLegacyStorage(getContext(), TEST_APP_PID, + testAppUid, packageName, + null, /* isTargetSdkAtLeastT */ false)).isTrue(); + + assertThat( checkPermissionInstallPackages(getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); assertThat( diff --git a/tests/test_app/LegacyTestAppWithTargetSdk33.xml b/tests/test_app/LegacyTestAppWithTargetSdk33.xml new file mode 100644 index 000000000..541e5abcf --- /dev/null +++ b/tests/test_app/LegacyTestAppWithTargetSdk33.xml @@ -0,0 +1,40 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- + ~ Copyright (C) 2025 The Android Open Source Project + ~ + ~ Licensed under the Apache License, Version 2.0 (the "License"); + ~ you may not use this file except in compliance with the License. + ~ You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> + +<manifest xmlns:android="http://schemas.android.com/apk/res/android" + package="com.android.providers.media.testapp.legacywithtargetsdk33" + android:versionCode="1" + android:versionName="1.0"> + + <uses-sdk android:minSdkVersion="30" android:targetSdkVersion="33" /> + + <uses-permission android:name="android.permission.READ_MEDIA_IMAGES" /> + <uses-permission android:name="android.permission.READ_MEDIA_VIDEO" /> + <uses-permission android:name="android.permission.READ_MEDIA_AUDIO" /> + + <application android:label="LegacyTestAppWithTargetSdk33" + android:requestLegacyExternalStorage="true"> + <activity android:name="com.android.providers.media.util.TestAppActivity" + android:exported="true"> + <intent-filter> + <action android:name="android.intent.action.MAIN"/> + <category android:name="android.intent.category.DEFAULT"/> + <category android:name="android.intent.category.LAUNCHER"/> + </intent-filter> + </activity> + </application> +</manifest> |