Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2b312a0e authored by Nate Myren's avatar Nate Myren
Browse files

Fix several issues with Storage permission UI

Ensure UserPackageInfos live data updates on permission changes, ensure
that UI correctly handles case where app may have special app access,
but not permission, ensure special app access is calculated correctly,
kills app on app op change.

Fixes: 160166191
Test: atest CtsPermissionTestCases CtsPermission3TestCases
Change-Id: Ie194f41d11da3316350e2715049c55d596f45b19
parent 74640e41
Loading
Loading
Loading
Loading
+18 −6
Original line number Original line Diff line number Diff line
@@ -16,10 +16,12 @@


package com.android.permissioncontroller.permission.data
package com.android.permissioncontroller.permission.data


import android.Manifest
import android.Manifest.permission.MANAGE_EXTERNAL_STORAGE
import android.Manifest.permission_group.STORAGE
import android.Manifest.permission_group.STORAGE
import android.app.AppOpsManager
import android.app.AppOpsManager
import android.app.AppOpsManager.MODE_ALLOWED
import android.app.AppOpsManager.MODE_ALLOWED
import android.app.AppOpsManager.MODE_DEFAULT
import android.app.AppOpsManager.MODE_FOREGROUND
import android.app.AppOpsManager.OPSTR_LEGACY_STORAGE
import android.app.AppOpsManager.OPSTR_LEGACY_STORAGE
import android.app.AppOpsManager.OPSTR_MANAGE_EXTERNAL_STORAGE
import android.app.AppOpsManager.OPSTR_MANAGE_EXTERNAL_STORAGE
import android.app.Application
import android.app.Application
@@ -64,7 +66,7 @@ object FullStoragePermissionAppsLiveData :
        for ((user, packageInfoList) in AllPackageInfosLiveData.value ?: emptyMap()) {
        for ((user, packageInfoList) in AllPackageInfosLiveData.value ?: emptyMap()) {
            val userPackages = packageInfoList.filter {
            val userPackages = packageInfoList.filter {
                storagePackages.contains(it.packageName to user) ||
                storagePackages.contains(it.packageName to user) ||
                    it.requestedPermissions.contains(Manifest.permission.MANAGE_EXTERNAL_STORAGE)
                    it.requestedPermissions.contains(MANAGE_EXTERNAL_STORAGE)
            }
            }


            for (packageInfo in userPackages) {
            for (packageInfo in userPackages) {
@@ -80,10 +82,12 @@ object FullStoragePermissionAppsLiveData :
                        isLegacy = true, isGranted = true))
                        isLegacy = true, isGranted = true))
                    continue
                    continue
                }
                }
                if (packageInfo.requestedPermissions.contains(
                if (MANAGE_EXTERNAL_STORAGE in packageInfo.requestedPermissions) {
                        Manifest.permission.MANAGE_EXTERNAL_STORAGE)) {
                    val mode = appOpsManager.unsafeCheckOpNoThrow(OPSTR_MANAGE_EXTERNAL_STORAGE,
                    val granted = appOpsManager.unsafeCheckOpNoThrow(OPSTR_MANAGE_EXTERNAL_STORAGE,
                        packageInfo.uid, packageInfo.packageName)
                            packageInfo.uid, packageInfo.packageName) == MODE_ALLOWED
                    val granted = mode == MODE_ALLOWED || mode == MODE_FOREGROUND ||
                        (mode == MODE_DEFAULT &&
                            MANAGE_EXTERNAL_STORAGE in packageInfo.grantedPermissions)
                    fullStoragePackages.add(FullStoragePackageState(packageInfo.packageName, user,
                    fullStoragePackages.add(FullStoragePackageState(packageInfo.packageName, user,
                        isLegacy = false, isGranted = granted))
                        isLegacy = false, isGranted = granted))
                }
                }
@@ -97,4 +101,12 @@ object FullStoragePermissionAppsLiveData :
        super.onActive()
        super.onActive()
        updateAsync()
        updateAsync()
    }
    }

    /**
     * Recalculate the LiveData
     * TODO ntmyren: Make livedata properly observe app ops
     */
    fun recalculate() {
        updateAsync()
    }
}
}
 No newline at end of file
+2 −1
Original line number Original line Diff line number Diff line
@@ -118,7 +118,8 @@ class LightPackageInfoLiveData private constructor(
            registeredUid = uid
            registeredUid = uid
            PermissionListenerMultiplexer.addCallback(it, this)
            PermissionListenerMultiplexer.addCallback(it, this)
        }
        }
        if (userPackagesLiveData.hasActiveObservers() && !watchingUserPackagesLiveData) {
        if (userPackagesLiveData.hasActiveObservers() && !watchingUserPackagesLiveData &&
            !userPackagesLiveData.permChangeStale) {
            watchingUserPackagesLiveData = true
            watchingUserPackagesLiveData = true
            addSource(userPackagesLiveData, userPackageInfosObserver)
            addSource(userPackagesLiveData, userPackageInfosObserver)
        } else {
        } else {
+38 −1
Original line number Original line Diff line number Diff line
@@ -35,12 +35,39 @@ class UserPackageInfosLiveData private constructor(
    private val app: Application,
    private val app: Application,
    private val user: UserHandle
    private val user: UserHandle
) : SmartAsyncMediatorLiveData<@kotlin.jvm.JvmSuppressWildcards List<LightPackageInfo>>(),
) : SmartAsyncMediatorLiveData<@kotlin.jvm.JvmSuppressWildcards List<LightPackageInfo>>(),
    PackageBroadcastReceiver.PackageBroadcastListener {
    PackageBroadcastReceiver.PackageBroadcastListener,
    PermissionListenerMultiplexer.PermissionChangeCallback {

    /**
     * Whether or not the permissions in this liveData are out of date
     */
    var permChangeStale = false


    override fun onPackageUpdate(packageName: String) {
    override fun onPackageUpdate(packageName: String) {
        updateAsync()
        updateAsync()
    }
    }


    // TODO ntmyren: replace with correctly updating
    override fun onPermissionChange() {
        permChangeStale = true
        for (packageInfo in value ?: emptyList()) {
            PermissionListenerMultiplexer.removeCallback(packageInfo.uid, this)
        }
    }

    override fun setValue(newValue: List<LightPackageInfo>?) {
        if (newValue != value) {
            for (packageInfo in value ?: emptyList()) {
                PermissionListenerMultiplexer.removeCallback(packageInfo.uid, this)
            }
            for (packageInfo in newValue ?: emptyList()) {
                PermissionListenerMultiplexer.addCallback(packageInfo.uid, this)
            }
        }
        super.setValue(newValue)
        permChangeStale = false
    }

    /**
    /**
     * Get all of the packages in the system, organized by user.
     * Get all of the packages in the system, organized by user.
     */
     */
@@ -53,6 +80,7 @@ class UserPackageInfosLiveData private constructor(
            "${user.identifier}")
            "${user.identifier}")
        val packageInfos = app.applicationContext.packageManager
        val packageInfos = app.applicationContext.packageManager
            .getInstalledPackagesAsUser(GET_PERMISSIONS or MATCH_ALL, user.identifier)
            .getInstalledPackagesAsUser(GET_PERMISSIONS or MATCH_ALL, user.identifier)

        postValue(packageInfos.map { packageInfo -> LightPackageInfo(packageInfo) })
        postValue(packageInfos.map { packageInfo -> LightPackageInfo(packageInfo) })
    }
    }


@@ -60,12 +88,21 @@ class UserPackageInfosLiveData private constructor(
        super.onActive()
        super.onActive()


        PackageBroadcastReceiver.addAllCallback(this)
        PackageBroadcastReceiver.addAllCallback(this)

        for (packageInfo in value ?: emptyList()) {
            PermissionListenerMultiplexer.addCallback(packageInfo.uid, this)
        }

        updateAsync()
        updateAsync()
    }
    }


    override fun onInactive() {
    override fun onInactive() {
        super.onInactive()
        super.onInactive()


        for (packageInfo in value ?: emptyList()) {
            PermissionListenerMultiplexer.removeCallback(packageInfo.uid, this)
        }

        PackageBroadcastReceiver.removeAllCallback(this)
        PackageBroadcastReceiver.removeAllCallback(this)
    }
    }


+10 −7
Original line number Original line Diff line number Diff line
@@ -359,6 +359,11 @@ public class AppPermissionFragment extends SettingsWithLargeHeader
            setResult(DENIED);
            setResult(DENIED);
        });
        });
        mDenyButton.setOnClickListener((v) -> {
        mDenyButton.setOnClickListener((v) -> {

            if (mViewModel.getFullStorageStateLiveData().getValue() != null
                    && !mViewModel.getFullStorageStateLiveData().getValue().isLegacy()) {
                mViewModel.setAllFilesAccess(false);
            }
            mViewModel.requestChange(false, this, this, ChangeRequest.REVOKE_BOTH,
            mViewModel.requestChange(false, this, this, ChangeRequest.REVOKE_BOTH,
                    APP_PERMISSION_FRAGMENT_ACTION_REPORTED__BUTTON_PRESSED__DENY);
                    APP_PERMISSION_FRAGMENT_ACTION_REPORTED__BUTTON_PRESSED__DENY);
            setResult(DENIED_DO_NOT_ASK_AGAIN);
            setResult(DENIED_DO_NOT_ASK_AGAIN);
@@ -421,12 +426,8 @@ public class AppPermissionFragment extends SettingsWithLargeHeader
            return;
            return;
        }
        }


        if (storageState.isGranted()) {
        textView.setText(R.string.app_permission_footer_special_file_access);
        textView.setText(R.string.app_permission_footer_special_file_access);
        textView.setVisibility(View.VISIBLE);
        textView.setVisibility(View.VISIBLE);
        } else {
            textView.setVisibility(View.GONE);
        }
    }
    }


    private void setResult(@GrantPermissionsViewHandler.Result int result) {
    private void setResult(@GrantPermissionsViewHandler.Result int result) {
@@ -532,7 +533,7 @@ public class AppPermissionFragment extends SettingsWithLargeHeader
        private static final String KEY = ConfirmDialog.class.getName() + ".arg.key";
        private static final String KEY = ConfirmDialog.class.getName() + ".arg.key";
        private static final String BUTTON = ConfirmDialog.class.getName() + ".arg.button";
        private static final String BUTTON = ConfirmDialog.class.getName() + ".arg.button";
        private static final String ONE_TIME = ConfirmDialog.class.getName() + ".arg.onetime";
        private static final String ONE_TIME = ConfirmDialog.class.getName() + ".arg.onetime";

        private static int sCode =  APP_PERMISSION_FRAGMENT_ACTION_REPORTED__BUTTON_PRESSED__ALLOW;
        @Override
        @Override
        public Dialog onCreateDialog(Bundle savedInstanceState) {
        public Dialog onCreateDialog(Bundle savedInstanceState) {
            AppPermissionFragment fragment = (AppPermissionFragment) getParentFragment();
            AppPermissionFragment fragment = (AppPermissionFragment) getParentFragment();
@@ -550,6 +551,8 @@ public class AppPermissionFragment extends SettingsWithLargeHeader
                            (DialogInterface dialog, int which) -> {
                            (DialogInterface dialog, int which) -> {
                                if (isGrantFileAccess) {
                                if (isGrantFileAccess) {
                                    fragment.mViewModel.setAllFilesAccess(true);
                                    fragment.mViewModel.setAllFilesAccess(true);
                                    fragment.mViewModel.requestChange(false, fragment,
                                            fragment, ChangeRequest.GRANT_BOTH, sCode);
                                } else {
                                } else {
                                    fragment.mViewModel.onDenyAnyWay((ChangeRequest)
                                    fragment.mViewModel.onDenyAnyWay((ChangeRequest)
                                                    getArguments().getSerializable(CHANGE_REQUEST),
                                                    getArguments().getSerializable(CHANGE_REQUEST),
+0 −2
Original line number Original line Diff line number Diff line
@@ -131,8 +131,6 @@ public final class PermissionAppsFragment extends SettingsWithLargeHeader {
                    setLoading(true /* loading */, false /* animate */);
                    setLoading(true /* loading */, false /* animate */);
                }
                }
            }, SHOW_LOAD_DELAY_MS);
            }, SHOW_LOAD_DELAY_MS);
        } else if (mViewModel.getCategorizedAppsLiveData().getValue() != null) {
            onPackagesLoaded(mViewModel.getCategorizedAppsLiveData().getValue());
        }
        }


        setHasOptionsMenu(true);
        setHasOptionsMenu(true);
Loading