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

Commit 07ac1442 authored by Narayan Kamath's avatar Narayan Kamath
Browse files

PackageParser: String interning fixes. 

Stop interning string metadata values as well as class names as it's
unlikely there will be much duplication among these.

Also make sure we intern the same set of strings when parsing packages
from their cache entries as we do when parsing them from the package
itself.

This change also improves error reporting for the unit-test and fixes
a failure that was introduced by a previous change (the addition of
static libraries).

Test: PackageParserTest
Bug: 34726698

Change-Id: Ia0d0342b91b3294bd5569756255918d1dc886e05
parent 02737595

core/java/android/content/pm/PackageParser.java

+67 −7
Original line number Diff line number Diff line
@@ -2745,15 +2745,15 @@ public class PackageParser { 
        String cls = clsSeq.toString();

        char c = cls.charAt(0);

        if (c == '.') {

            return (pkg + cls).intern();

            return pkg + cls;

        }

        if (cls.indexOf('.') < 0) {

            StringBuilder b = new StringBuilder(pkg);

            b.append('.');

            b.append(cls);

            return b.toString().intern();

            return b.toString();

        }

        return cls.intern();

        return cls;

    }



    private static String buildCompoundName(String pkg,
@@ -2773,7 +2773,7 @@ public class PackageParser { 
                        + pkg + ": " + nameError;

                return null;

            }

            return (pkg + proc).intern();

            return pkg + proc;

        }

        String nameError = validateName(proc, true, false);

        if (nameError != null && !"system".equals(proc)) {
@@ -2781,7 +2781,7 @@ public class PackageParser { 
                    + pkg + ": " + nameError;

            return null;

        }

        return proc.intern();

        return proc;

    }



    private static String buildProcessName(String pkg, String defProc,
@@ -5083,7 +5083,7 @@ public class PackageParser { 
            if (v != null) {

                if (v.type == TypedValue.TYPE_STRING) {

                    CharSequence cs = v.coerceToString();

                    data.putString(name, cs != null ? cs.toString().intern() : null);

                    data.putString(name, cs != null ? cs.toString() : null);

                } else if (v.type == TypedValue.TYPE_INT_BOOLEAN) {

                    data.putBoolean(name, v.data != 0);

                } else if (v.type >= TypedValue.TYPE_FIRST_INT
@@ -5866,7 +5866,7 @@ public class PackageParser { 
            // We use the boot classloader for all classes that we load.

            final ClassLoader boot = Object.class.getClassLoader();



            packageName = dest.readString();

            packageName = dest.readString().intern();

            manifestPackageName = dest.readString();

            splitNames = dest.readStringArray();

            volumeUuid = dest.readString();
@@ -5879,6 +5879,9 @@ public class PackageParser { 
            splitPrivateFlags = dest.createIntArray();

            baseHardwareAccelerated = (dest.readInt() == 1);

            applicationInfo = dest.readParcelable(boot);

            if (applicationInfo.permission != null) {

                applicationInfo.permission = applicationInfo.permission.intern();

            }



            // We don't serialize the "owner" package and the application info object for each of

            // these components, in order to save space and to avoid circular dependencies while
@@ -5899,7 +5902,10 @@ public class PackageParser { 
            fixupOwner(instrumentation);



            dest.readStringList(requestedPermissions);

            internStringArrayList(requestedPermissions);

            protectedBroadcasts = dest.createStringArrayList();

            internStringArrayList(protectedBroadcasts);



            parentPackage = dest.readParcelable(boot);



            childPackages = new ArrayList<>();
@@ -5909,16 +5915,23 @@ public class PackageParser { 
            }



            staticSharedLibName = dest.readString();

            if (staticSharedLibName != null) {

                staticSharedLibName = staticSharedLibName.intern();

            }

            staticSharedLibVersion = dest.readInt();

            libraryNames = dest.createStringArrayList();

            internStringArrayList(libraryNames);

            usesLibraries = dest.createStringArrayList();

            internStringArrayList(usesLibraries);

            usesOptionalLibraries = dest.createStringArrayList();

            internStringArrayList(usesOptionalLibraries);

            usesLibraryFiles = dest.readStringArray();



            final int libCount = dest.readInt();

            if (libCount > 0) {

                usesStaticLibraries = new ArrayList<>(libCount);

                dest.readStringList(usesStaticLibraries);

                internStringArrayList(usesStaticLibraries);

                usesStaticLibrariesVersions = new int[libCount];

                dest.readIntArray(usesStaticLibrariesVersions);

                usesStaticLibrariesCertDigests = new String[libCount];
@@ -5937,7 +5950,13 @@ public class PackageParser { 
            mAppMetaData = dest.readBundle();

            mVersionCode = dest.readInt();

            mVersionName = dest.readString();

            if (mVersionName != null) {

                mVersionName = mVersionName.intern();

            }

            mSharedUserId = dest.readString();

            if (mSharedUserId != null) {

                mSharedUserId = mSharedUserId.intern();

            }

            mSharedUserLabel = dest.readInt();



            mSignatures = (Signature[]) dest.readParcelableArray(boot, Signature.class);
@@ -5988,6 +6007,15 @@ public class PackageParser { 
            restrictUpdateHash = dest.createByteArray();

        }



        private static void internStringArrayList(List<String> list) {

            if (list != null) {

                final int N = list.size();

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

                    list.set(i, list.get(i).intern());

                }

            }

        }



        /**

         * Sets the package owner and the the {@code applicationInfo} for every component

         * owner by this package.
@@ -6375,6 +6403,10 @@ public class PackageParser { 
            super(in);

            final ClassLoader boot = Object.class.getClassLoader();

            info = in.readParcelable(boot);

            if (info.group != null) {

                info.group = info.group.intern();

            }



            tree = (in.readInt() == 1);

            group = in.readParcelable(boot);

        }
@@ -6651,6 +6683,10 @@ public class PackageParser { 
            for (ActivityIntentInfo aii : intents) {

                aii.activity = this;

            }



            if (info.permission != null) {

                info.permission = info.permission.intern();

            }

        }



        public static final Parcelable.Creator CREATOR = new Parcelable.Creator<Activity>() {
@@ -6735,6 +6771,10 @@ public class PackageParser { 
            for (ServiceIntentInfo aii : intents) {

                aii.service = this;

            }



            if (info.permission != null) {

                info.permission = info.permission.intern();

            }

        }



        public static final Parcelable.Creator CREATOR = new Parcelable.Creator<Service>() {
@@ -6816,6 +6856,18 @@ public class PackageParser { 
            for (ProviderIntentInfo aii : intents) {

                aii.provider = this;

            }



            if (info.readPermission != null) {

                info.readPermission = info.readPermission.intern();

            }



            if (info.writePermission != null) {

                info.writePermission = info.writePermission.intern();

            }



            if (info.authority != null) {

                info.authority = info.authority.intern();

            }

        }



        public static final Parcelable.Creator CREATOR = new Parcelable.Creator<Provider>() {
@@ -6888,6 +6940,14 @@ public class PackageParser { 
        private Instrumentation(Parcel in) {

            super(in);

            info = in.readParcelable(Object.class.getClassLoader());



            if (info.targetPackage != null) {

                info.targetPackage = info.targetPackage.intern();

            }



            if (info.targetProcess != null) {

                info.targetProcess = info.targetProcess.intern();

            }

        }



        public static final Parcelable.Creator CREATOR = new Parcelable.Creator<Instrumentation>() {

services/tests/servicestests/src/com/android/server/pm/PackageParserTest.java

+63 −25
Original line number Diff line number Diff line
@@ -140,6 +140,36 @@ public class PackageParserTest { 
        assertAllFieldsExist(deserialized);

    }



    @Test

    public void test_stringInterning() throws Exception {

        PackageParser.Package pkg = new PackageParser.Package("foo");

        setKnownFields(pkg);



        Parcel p = Parcel.obtain();

        pkg.writeToParcel(p, 0 /* flags */);



        p.setDataPosition(0);

        PackageParser.Package deserialized = new PackageParser.Package(p);



        p.setDataPosition(0);

        PackageParser.Package deserialized2 = new PackageParser.Package(p);



        assertSame(deserialized.packageName, deserialized2.packageName);

        assertSame(deserialized.applicationInfo.permission,

                deserialized2.applicationInfo.permission);

        assertSame(deserialized.requestedPermissions.get(0),

                deserialized2.requestedPermissions.get(0));

        assertSame(deserialized.protectedBroadcasts.get(0),

                deserialized2.protectedBroadcasts.get(0));

        assertSame(deserialized.usesLibraries.get(0),

                deserialized2.usesLibraries.get(0));

        assertSame(deserialized.usesOptionalLibraries.get(0),

                deserialized2.usesOptionalLibraries.get(0));

        assertSame(deserialized.mVersionName, deserialized2.mVersionName);

        assertSame(deserialized.mSharedUserId, deserialized2.mSharedUserId);

    }





    /**

     * A trivial subclass of package parser that only caches the package name, and throws away

     * all other information.
@@ -407,11 +437,11 @@ public class PackageParserTest { 
        pkg.mTrustedOverlay = true;

        pkg.use32bitAbi = true;

        pkg.packageName = "foo";

        pkg.splitNames = new String[] { "foo" };

        pkg.volumeUuid = "foo";

        pkg.codePath = "foo";

        pkg.baseCodePath = "foo";

        pkg.splitCodePaths = new String[] { "foo" };

        pkg.splitNames = new String[] { "foo2" };

        pkg.volumeUuid = "foo3";

        pkg.codePath = "foo4";

        pkg.baseCodePath = "foo5";

        pkg.splitCodePaths = new String[] { "foo6" };

        pkg.splitRevisionCodes = new int[] { 100 };

        pkg.splitFlags = new int[] { 100 };

        pkg.splitPrivateFlags = new int[] { 100 };
@@ -428,48 +458,55 @@ public class PackageParserTest { 
        pkg.providers.add(new PackageParser.Provider(dummy, new ProviderInfo()));

        pkg.services.add(new PackageParser.Service(dummy, new ServiceInfo()));

        pkg.instrumentation.add(new PackageParser.Instrumentation(dummy, new InstrumentationInfo()));

        pkg.requestedPermissions.add("foo");

        pkg.requestedPermissions.add("foo7");



        pkg.protectedBroadcasts = new ArrayList<>();

        pkg.protectedBroadcasts.add("foo");

        pkg.protectedBroadcasts.add("foo8");



        pkg.parentPackage = new PackageParser.Package("foo");

        pkg.parentPackage = new PackageParser.Package("foo9");



        pkg.childPackages = new ArrayList<>();

        pkg.childPackages.add(new PackageParser.Package("bar"));



        pkg.staticSharedLibName = "foo23";

        pkg.staticSharedLibVersion = 100;

        pkg.usesStaticLibraries = new ArrayList<>();

        pkg.usesStaticLibraries.add("foo23");

        pkg.usesStaticLibrariesCertDigests = new String[] { "digest" };

        pkg.usesStaticLibrariesVersions = new int[] { 100 };



        pkg.libraryNames = new ArrayList<>();

        pkg.libraryNames.add("foo");

        pkg.libraryNames.add("foo10");



        pkg.usesLibraries = new ArrayList<>();

        pkg.usesLibraries.add("foo");

        pkg.usesLibraries.add("foo11");



        pkg.usesOptionalLibraries = new ArrayList<>();

        pkg.usesOptionalLibraries.add("foo");

        pkg.usesOptionalLibraries.add("foo12");



        pkg.usesLibraryFiles = new String[] { "foo "};

        pkg.usesLibraryFiles = new String[] { "foo13"};



        pkg.mOriginalPackages = new ArrayList<>();

        pkg.mOriginalPackages.add("foo");

        pkg.mOriginalPackages.add("foo14");



        pkg.mRealPackage = "foo";

        pkg.mRealPackage = "foo15";



        pkg.mAdoptPermissions = new ArrayList<>();

        pkg.mAdoptPermissions.add("foo");

        pkg.mAdoptPermissions.add("foo16");



        pkg.mAppMetaData = new Bundle();

        pkg.mVersionName = "foo";

        pkg.mSharedUserId = "foo";

        pkg.mVersionName = "foo17";

        pkg.mSharedUserId = "foo18";

        pkg.mSignatures = new Signature[] { new Signature(new byte[16]) };

        pkg.mCertificates = new Certificate[][] { new Certificate[] { null }};

        pkg.mExtras = new Bundle();

        pkg.mRestrictedAccountType = "foo";

        pkg.mRequiredAccountType = "foo";

        pkg.mOverlayTarget = "foo";

        pkg.mRestrictedAccountType = "foo19";

        pkg.mRequiredAccountType = "foo20";

        pkg.mOverlayTarget = "foo21";

        pkg.mSigningKeys = new ArraySet<>();

        pkg.mUpgradeKeySets = new ArraySet<>();

        pkg.mKeySetMapping = new ArrayMap<>();

        pkg.cpuAbiOverride = "foo";

        pkg.cpuAbiOverride = "foo22";

        pkg.restrictUpdateHash = new byte[16];



        pkg.preferredActivityFilters = new ArrayList<>();
@@ -504,7 +541,7 @@ public class PackageParserTest { 
                // Sanity check for list fields: Assume they're non-null and contain precisely

                // one element.

                List<?> list = (List<?>) f.get(pkg);

                assertNotNull(list);

                assertNotNull("List was null: " + f, list);

                assertEquals(1, list.size());

            } else if (fieldType.getComponentType() != null) {

                // Sanity check for array fields: Assume they're non-null and contain precisely
@@ -514,15 +551,16 @@ public class PackageParserTest { 
            } else if (fieldType == String.class) {

                // String fields: Check that they're set to "foo".

                String value = (String) f.get(pkg);

                assertEquals("foo", value);



                assertTrue("Bad value for field: " + f, value != null && value.startsWith("foo"));

            } else if (fieldType == int.class) {

                // int fields: Check that they're set to 100.

                int value = (int) f.get(pkg);

                assertEquals(100, value);

                assertEquals("Bad value for field: " + f, 100, value);

            } else {

                // All other fields: Check that they're set.

                Object o = f.get(pkg);

                assertNotNull("Field was null: " + f.getName(), o);

                assertNotNull("Field was null: " + f, o);

            }

        }

    }