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

Commit 21038d5b authored by Akhil Gangu's avatar Akhil Gangu
Browse files

Attempt to deprecate redundant usesPermission data structures. 

Considering a map of permision name to the ParsedUsesPermission object
is already stored, we no longer need to keep track of a set of
permission names. This can just be derived from the keyset of the map.
The getRequestedPermissions() method is kept to avoid several changes
throughout the codebase; it also helps provide a nice abstraction to get
the permission names without accessing the map and calling for the
keyset explicitly.

Similarly, the list to store the ParsesUsesPermission objects can be
deprecated in favor of deriving the information through the map's
values. Some use cases had to be adjusted to not use the get() method
since the .values() of the map returns a Collection not a List.

Bug: 419394776
Test: presubmit; verified a few sys health tests via ABTD
Flag: EXEMPT refactor
Change-Id: Ic3abefb5f8c1b2b40620969d4038a4c555fe9cc0
parent dda2de47

core/java/com/android/internal/pm/parsing/PackageInfoCommonUtils.java

+11 −8
Original line number Diff line number Diff line
@@ -67,6 +67,7 @@ import com.android.internal.util.ArrayUtils; 
import com.android.internal.util.CollectionUtils;

import com.android.server.pm.pkg.AndroidPackage;



import java.util.Collection;

import java.util.List;



/**
@@ -149,26 +150,28 @@ public class PackageInfoCommonUtils { 
                    info.permissions[i] = permissionInfo;

                }

            }

            final List<ParsedUsesPermission> usesPermissions = pkg.getUsesPermissions();

            final Collection<ParsedUsesPermission> usesPermissions =

                    pkg.getUsesPermissionMapping().values();

            size = usesPermissions.size();

            if (size > 0) {

                info.requestedPermissions = new String[size];

                info.requestedPermissionsFlags = new int[size];

                for (int i = 0; i < size; i++) {

                    final ParsedUsesPermission usesPermission = usesPermissions.get(i);

                    info.requestedPermissions[i] = usesPermission.getName();

                int index = 0;

                for (ParsedUsesPermission usesPermission : usesPermissions) {

                    info.requestedPermissions[index] = usesPermission.getName();

                    // The notion of required permissions is deprecated but for compatibility.

                    info.requestedPermissionsFlags[i] |=

                    info.requestedPermissionsFlags[index] |=

                            PackageInfo.REQUESTED_PERMISSION_REQUIRED;

                    if ((usesPermission.getUsesPermissionFlags()

                            & ParsedUsesPermission.FLAG_NEVER_FOR_LOCATION) != 0) {

                        info.requestedPermissionsFlags[i] |=

                        info.requestedPermissionsFlags[index] |=

                                PackageInfo.REQUESTED_PERMISSION_NEVER_FOR_LOCATION;

                    }

                    if (pkg.getImplicitPermissions().contains(info.requestedPermissions[i])) {

                        info.requestedPermissionsFlags[i] |=

                    if (pkg.getImplicitPermissions().contains(info.requestedPermissions[index])) {

                        info.requestedPermissionsFlags[index] |=

                                PackageInfo.REQUESTED_PERMISSION_IMPLICIT;

                    }

                    index++;

                }

            }

        }

core/java/com/android/internal/pm/parsing/pkg/PackageImpl.java

+5 −45
Original line number Diff line number Diff line
@@ -41,7 +41,6 @@ import android.os.Parcel; 
import android.os.Parcelable;

import android.os.UserHandle;

import android.os.storage.StorageManager;

import android.permission.flags.Flags;

import android.text.TextUtils;

import android.util.ArrayMap;

import android.util.ArraySet;
@@ -156,14 +155,6 @@ public class PackageImpl implements ParsedPackage, AndroidPackageInternal, 
    @NonNull

    @DataClass.ParcelWith(Parcelling.BuiltIn.ForInternedStringList.class)

    protected List<String> adoptPermissions = emptyList();

    /**

     * @deprecated consider migrating to {@link #getUsesPermissions} which has

     *             more parsed details, such as flags

     */

    @NonNull

    @Deprecated

    @DataClass.ParcelWith(Parcelling.BuiltIn.ForInternedStringSet.class)

    protected Set<String> requestedPermissions = emptySet();

    @NonNull

    @DataClass.ParcelWith(Parcelling.BuiltIn.ForInternedStringList.class)

    protected List<String> protectedBroadcasts = emptyList();
@@ -282,8 +273,6 @@ public class PackageImpl implements ParsedPackage, AndroidPackageInternal, 
    @Nullable

    private byte[] restrictUpdateHash;

    @NonNull

    private List<ParsedUsesPermission> usesPermissions = emptyList();

    @NonNull

    private Map<String, ParsedUsesPermission> usesPermissionMapping = emptyMap();

    @NonNull

    @DataClass.ParcelWith(Parcelling.BuiltIn.ForInternedStringSet.class)
@@ -727,23 +716,12 @@ public class PackageImpl implements ParsedPackage, AndroidPackageInternal, 
        return this;

    }



    // TODO(419394776) - Use single source of truth for storing uses permission metadata.

    @Override

    public PackageImpl addUsesPermission(ParsedUsesPermission permission) {

        this.usesPermissions = CollectionUtils.add(this.usesPermissions, permission);



        // Continue populating legacy data structures to avoid performance

        // issues until all that code can be migrated

        this.requestedPermissions =

                CollectionUtils.add(this.requestedPermissions, permission.getName());



        if (Flags.purposeDeclarationEnabled()) {

            // During manifest parsing, we ignore duplicate permission requests. Therefore, it's

            // safe to directly add to the mapping.

        // During manifest parsing, we ignore duplicate permission requests. Therefore, it's safe

        // to directly add to the mapping.

        this.usesPermissionMapping =

                    CollectionUtils.add(

                            this.usesPermissionMapping, permission.getName(), permission);

        }

                CollectionUtils.add(this.usesPermissionMapping, permission.getName(), permission);



        return this;

    }
@@ -1284,15 +1262,10 @@ public class PackageImpl implements ParsedPackage, AndroidPackageInternal, 
        return reqFeatures;

    }



    /**

     * @deprecated consider migrating to {@link #getUsesPermissions} which has

     *             more parsed details, such as flags

     */

    @NonNull

    @Override

    @Deprecated

    public Set<String> getRequestedPermissions() {

        return requestedPermissions;

        return usesPermissionMapping.keySet();

    }



    @Nullable
@@ -1499,12 +1472,6 @@ public class PackageImpl implements ParsedPackage, AndroidPackageInternal, 
        return usesOptionalNativeLibraries;

    }



    @NonNull

    @Override

    public List<ParsedUsesPermission> getUsesPermissions() {

        return usesPermissions;

    }



    @NonNull

    @Override

    public Map<String, ParsedUsesPermission> getUsesPermissionMapping() {
@@ -2859,7 +2826,6 @@ public class PackageImpl implements ParsedPackage, AndroidPackageInternal, 
        usesOptionalNativeLibraries = Collections.unmodifiableList(usesOptionalNativeLibraries);

        originalPackages = Collections.unmodifiableList(originalPackages);

        adoptPermissions = Collections.unmodifiableList(adoptPermissions);

        requestedPermissions = Collections.unmodifiableSet(requestedPermissions);

        protectedBroadcasts = Collections.unmodifiableList(protectedBroadcasts);

        apexSystemServices = Collections.unmodifiableList(apexSystemServices);
@@ -2878,7 +2844,6 @@ public class PackageImpl implements ParsedPackage, AndroidPackageInternal, 
        configPreferences = Collections.unmodifiableList(configPreferences);

        reqFeatures = Collections.unmodifiableList(reqFeatures);

        featureGroups = Collections.unmodifiableList(featureGroups);

        usesPermissions = Collections.unmodifiableList(usesPermissions);

        usesPermissionMapping = Collections.unmodifiableMap(usesPermissionMapping);

        usesSdkLibraries = Collections.unmodifiableList(usesSdkLibraries);

        implicitPermissions = Collections.unmodifiableSet(implicitPermissions);
@@ -3258,8 +3223,6 @@ public class PackageImpl implements ParsedPackage, AndroidPackageInternal, 
        dest.writeByteArray(this.restrictUpdateHash);

        dest.writeStringList(this.originalPackages);

        sForInternedStringList.parcel(this.adoptPermissions, dest, flags);

        sForInternedStringSet.parcel(this.requestedPermissions, dest, flags);

        ParsingUtils.writeParcelableList(dest, this.usesPermissions);

        writeUsesPermissionMapping(dest);

        sForInternedStringSet.parcel(this.implicitPermissions, dest, flags);

        sForStringSet.parcel(this.upgradeKeySets, dest, flags);
@@ -3456,9 +3419,6 @@ public class PackageImpl implements ParsedPackage, AndroidPackageInternal, 
        this.restrictUpdateHash = in.createByteArray();

        this.originalPackages = in.createStringArrayList();

        this.adoptPermissions = sForInternedStringList.unparcel(in);

        this.requestedPermissions = sForInternedStringSet.unparcel(in);

        this.usesPermissions = ParsingUtils.createTypedInterfaceList(in,

                ParsedUsesPermissionImpl.CREATOR);

        readUsesPermissionMapping(in);

        this.implicitPermissions = sForInternedStringSet.unparcel(in);

        this.upgradeKeySets = sForStringSet.unparcel(in);

core/java/com/android/internal/pm/pkg/parsing/ParsingPackage.java

+3 −3
Original line number Diff line number Diff line
@@ -489,6 +489,9 @@ public interface ParsingPackage { 
    @NonNull

    Set<String> getRequestedPermissions();



    @NonNull

    Map<String, ParsedUsesPermission> getUsesPermissionMapping();



    @Nullable

    Boolean getResizeableActivity();
@@ -523,9 +526,6 @@ public interface ParsingPackage { 
    @NonNull

    List<String> getUsesNativeLibraries();



    @NonNull

    List<ParsedUsesPermission> getUsesPermissions();



    @NonNull

    List<String> getUsesSdkLibraries();

core/java/com/android/internal/pm/pkg/parsing/ParsingPackageUtils.java

+4 −4
Original line number Diff line number Diff line
@@ -139,6 +139,7 @@ import java.lang.annotation.Retention; 
import java.lang.annotation.RetentionPolicy;

import java.security.PublicKey;

import java.util.ArrayList;

import java.util.Collection;

import java.util.List;

import java.util.Map;

import java.util.Objects;
@@ -1506,10 +1507,9 @@ public class ParsingPackageUtils { 
            // Quietly ignore duplicate permission requests, but fail loudly if

            // the two requests have conflicting flags or purposes.

            boolean found = false;

            final List<ParsedUsesPermission> usesPermissions = pkg.getUsesPermissions();

            final int size = usesPermissions.size();

            for (int i = 0; i < size; i++) {

                final ParsedUsesPermission usesPermission = usesPermissions.get(i);

            final Collection<ParsedUsesPermission> usesPermissions =

                    pkg.getUsesPermissionMapping().values();

            for (ParsedUsesPermission usesPermission : usesPermissions) {

                if (Objects.equals(usesPermission.getName(), name)) {

                    if (usesPermission.getUsesPermissionFlags() != usesPermissionFlags) {

                        return input.error("Conflicting uses-permissions flags: "

core/java/com/android/server/pm/pkg/AndroidPackage.java

+0 −5
Original line number Diff line number Diff line
@@ -1348,11 +1348,6 @@ public interface AndroidPackage { 
    @NonNull

    List<String> getUsesOptionalNativeLibraries();



    /** @hide */

    @Immutable.Ignore

    @NonNull

    List<ParsedUsesPermission> getUsesPermissions();



    /**

     * A mapping of the requested permission name to its {@link ParsedUsesPermission} object.

     *