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

Commit 66e9f33c authored by Winson's avatar Winson Committed by Winson Chiu
Browse files

Fix up discrepancies between v1 and v2 package parsing 

For Activity aliases, it's possible some values are already
set, which means they cannot be assumed to be 0, and can't be
overwritten if a attribute in the alias is undefined. For the
parsing v2 refactor, this was cleaned up to avoid
redundant != 0 checks, but those checks are indeed necessary.
This copies over the old logic and uses it exactly.

In some future cleanup, there should be a more structured way
of doing this, since it's not immediately obvious which values
are overridden or not. For example, description is always
overwritten even if no new value is provided in the alias.

This also fixes up the comparison tests and other bugs that
popped up because of them. The core issue was that when
auto-generating the dumpToString methods, the Alt+Insert
macro default selects all the fields in the current class,
but not all the parent classes, so some shared fields like
name/icon were not considered.

A notable case that was found when running the comparison tests
is that persistableMode is now "fixed" with v2. Previously,
a bug in PackageParser caused this value to be dropped if
the ActivityInfo object ever had to be copied. This is a change
from Q behavior, but there's no good way to reconcile this, and
it's better to be correct and consistent than broken, so this
fix was left in and excluded from the comparison tests.

Bug: 150106908

Test: manual run through steps in bug
Test: atest com.android.server.pm.parsing

Change-Id: I1301e28540314d0e643b73af7146c1a366eca6b5
parent 8c5b1c79

core/java/android/content/pm/parsing/component/ParsedActivityUtils.java

+8 −1
Original line number Diff line number Diff line
@@ -302,7 +302,14 @@ public class ParsedActivityUtils { 
        }



        String permission = array.getNonConfigurationString(permissionAttr, 0);

        if (isAlias) {

            // An alias will override permissions to allow referencing an Activity through its alias

            // without needing the original permission. If an alias needs the same permission,

            // it must be re-declared.

            activity.setPermission(permission);

        } else {

            activity.setPermission(permission != null ? permission : pkg.getPermission());

        }



        final boolean setExported = array.hasValue(exportedAttr);

        if (setExported) {

core/java/android/content/pm/parsing/component/ParsedComponentUtils.java

+20 −9
Original line number Diff line number Diff line
@@ -20,7 +20,10 @@ import android.annotation.NonNull; 
import android.annotation.Nullable;

import android.content.pm.PackageManager;

import android.content.pm.parsing.ParsingPackage;

import android.content.pm.parsing.ParsingPackageUtils;

import android.content.pm.parsing.ParsingUtils;

import android.content.pm.parsing.result.ParseInput;

import android.content.pm.parsing.result.ParseResult;

import android.content.res.Resources;

import android.content.res.TypedArray;

import android.content.res.XmlResourceParser;
@@ -29,9 +32,6 @@ import android.text.TextUtils; 
import android.util.TypedValue;



import com.android.internal.annotations.VisibleForTesting;

import android.content.pm.parsing.ParsingPackageUtils;

import android.content.pm.parsing.result.ParseInput;

import android.content.pm.parsing.result.ParseResult;



/** @hide */

class ParsedComponentUtils {
@@ -60,16 +60,27 @@ class ParsedComponentUtils { 
        component.setName(className);

        component.setPackageName(packageName);



        if (useRoundIcon) {

            component.icon = array.getResourceId(roundIconAttr, 0);

        int roundIconVal = useRoundIcon ? array.getResourceId(roundIconAttr, 0) : 0;

        if (roundIconVal != 0) {

            component.icon = roundIconVal;

            component.nonLocalizedLabel = null;

        } else {

            int iconVal = array.getResourceId(iconAttr, 0);

            if (iconVal != 0) {

                component.icon = iconVal;

                component.nonLocalizedLabel = null;

            }

        }



        if (component.icon == 0) {

            component.icon = array.getResourceId(iconAttr, 0);

        int logoVal = array.getResourceId(logoAttr, 0);

        if (logoVal != 0) {

            component.logo = logoVal;

        }



        component.logo = array.getResourceId(logoAttr, 0);

        component.banner = array.getResourceId(bannerAttr, 0);

        int bannerVal = array.getResourceId(bannerAttr, 0);

        if (bannerVal != 0) {

            component.banner = bannerVal;

        }



        if (descriptionAttr != null) {

            component.descriptionRes = array.getResourceId(descriptionAttr, 0);

services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingEquivalenceTest.kt

+79 −10
Original line number Diff line number Diff line
@@ -18,9 +18,9 @@ package com.android.server.pm.parsing 


import android.content.pm.PackageManager

import android.platform.test.annotations.Presubmit

import androidx.test.filters.LargeTest

import com.google.common.truth.Expect

import com.google.common.truth.Truth.assertWithMessage

import org.junit.Ignore



import org.junit.Rule

import org.junit.Test
@@ -35,7 +35,6 @@ class AndroidPackageParsingEquivalenceTest : AndroidPackageParsingTestBase() { 
    val expect = Expect.create()



    @Test

    @Ignore("b/155935153")

    fun applicationInfoEquality() {

        val flags = PackageManager.GET_META_DATA or PackageManager.GET_SHARED_LIBRARY_FILES

        val oldAppInfo = oldPackages.asSequence().map { oldAppInfo(it, flags) }
@@ -54,8 +53,8 @@ class AndroidPackageParsingEquivalenceTest : AndroidPackageParsingTestBase() { 
        }

    }



    @LargeTest

    @Test

    @Ignore("b/155935153")

    fun packageInfoEquality() {

        val flags = PackageManager.GET_ACTIVITIES or

                PackageManager.GET_CONFIGURATIONS or
@@ -68,7 +67,9 @@ class AndroidPackageParsingEquivalenceTest : AndroidPackageParsingTestBase() { 
                PackageManager.GET_SERVICES or

                PackageManager.GET_SHARED_LIBRARY_FILES or

                PackageManager.GET_SIGNATURES or

                PackageManager.GET_SIGNING_CERTIFICATES

                PackageManager.GET_SIGNING_CERTIFICATES or

                PackageManager.MATCH_DIRECT_BOOT_UNAWARE or

                PackageManager.MATCH_DIRECT_BOOT_AWARE

        val oldPackageInfo = oldPackages.asSequence().map { oldPackageInfo(it, flags) }

        val newPackageInfo = newPackages.asSequence().map { newPackageInfo(it, flags) }
@@ -80,11 +81,79 @@ class AndroidPackageParsingEquivalenceTest : AndroidPackageParsingTestBase() { 
            } else {

                "$firstName | $secondName"

            }

            expect.withMessage("${it.first?.applicationInfo?.sourceDir} $packageName")

                    .that(it.first?.dumpToString())

                    .isEqualTo(it.second?.dumpToString())



            // Main components are asserted independently to separate the failures. Otherwise the

            // comparison would include every component in one massive string.



            val prefix = "${it.first?.applicationInfo?.sourceDir} $packageName"



            expect.withMessage("$prefix PackageInfo")

                    .that(it.second?.dumpToString())

                    .isEqualTo(it.first?.dumpToString())



            expect.withMessage("$prefix ApplicationInfo")

                    .that(it.second?.applicationInfo?.dumpToString())

                    .isEqualTo(it.first?.applicationInfo?.dumpToString())



            val firstActivityNames = it.first?.activities?.map { it.name } ?: emptyList()

            val secondActivityNames = it.second?.activities?.map { it.name } ?: emptyList()

            expect.withMessage("$prefix activities")

                    .that(secondActivityNames)

                    .containsExactlyElementsIn(firstActivityNames)

                    .inOrder()



            if (!it.first?.activities.isNullOrEmpty() && !it.second?.activities.isNullOrEmpty()) {

                it.first?.activities?.zip(it.second?.activities!!)?.forEach {

                    expect.withMessage("$prefix ${it.first.name}")

                            .that(it.second.dumpToString())

                            .isEqualTo(it.first.dumpToString())

                }

            }



            val firstReceiverNames = it.first?.receivers?.map { it.name } ?: emptyList()

            val secondReceiverNames = it.second?.receivers?.map { it.name } ?: emptyList()

            expect.withMessage("$prefix receivers")

                    .that(secondReceiverNames)

                    .containsExactlyElementsIn(firstReceiverNames)

                    .inOrder()



            if (!it.first?.receivers.isNullOrEmpty() && !it.second?.receivers.isNullOrEmpty()) {

                it.first?.receivers?.zip(it.second?.receivers!!)?.forEach {

                    expect.withMessage("$prefix ${it.first.name}")

                            .that(it.second.dumpToString())

                            .isEqualTo(it.first.dumpToString())

                }

            }



            val firstProviderNames = it.first?.providers?.map { it.name } ?: emptyList()

            val secondProviderNames = it.second?.providers?.map { it.name } ?: emptyList()

            expect.withMessage("$prefix providers")

                    .that(secondProviderNames)

                    .containsExactlyElementsIn(firstProviderNames)

                    .inOrder()



            if (!it.first?.providers.isNullOrEmpty() && !it.second?.providers.isNullOrEmpty()) {

                it.first?.providers?.zip(it.second?.providers!!)?.forEach {

                    expect.withMessage("$prefix ${it.first.name}")

                            .that(it.second.dumpToString())

                            .isEqualTo(it.first.dumpToString())

                }

            }



            val firstServiceNames = it.first?.services?.map { it.name } ?: emptyList()

            val secondServiceNames = it.second?.services?.map { it.name } ?: emptyList()

            expect.withMessage("$prefix services")

                    .that(secondServiceNames)

                    .containsExactlyElementsIn(firstServiceNames)

                    .inOrder()



            if (!it.first?.services.isNullOrEmpty() && !it.second?.services.isNullOrEmpty()) {

                it.first?.services?.zip(it.second?.services!!)?.forEach {

                    expect.withMessage("$prefix ${it.first.name}")

                            .that(it.second.dumpToString())

                            .isEqualTo(it.first.dumpToString())

                }

            }

        }

    }

}

services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt

+137 −37

File changed.

Preview size limit exceeded, changes collapsed.