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

Commit 764346e2 authored by Mark Punzalan's avatar Mark Punzalan
Browse files

Fix multi-user bug with re-registered FRROs 

On a device with multiple users (e.g., HSUM), when an existing FRRO is
re-registered (i.e., a new FRRO is created with same name/target/owner)
the resources for the target app are not updated with the new FRRO for
all users. In fact, only user 0 (system user on HSUM) gets the new
assets. For user 0, the new idmap is created but for later users, the
idmap is reused, hence OverlayManagerServiceImpl.updateState() returns
false and the target is not considered updated for the later users.

The fix is to also consider the baseCodePath (path to FRRO file) when
determining if a target is updated. The baseCodePath always changes when
there is a new FRRO.

Fixes: 406839614
Flag: EXEMPT bugfix
Test: atest OverlayManagerServiceImplTests
Test: atest FabricatedOverlayTests on HSUM device
Change-Id: I4417676ad165af8c149955efb59c52485771d278
parent 8df8b77e

services/core/java/com/android/server/om/OverlayManagerService.java

+6 −1
Original line number Diff line number Diff line
@@ -1051,8 +1051,13 @@ public final class OverlayManagerService extends SystemService { 
            Set<UserPackage> affectedPackagesToUpdate = null;

            for (Iterator<Request> it = transaction.getRequests(); it.hasNext(); ) {

                Request request = it.next();

                final var affectedPackagesFromRequest = executeRequestLocked(request);

                affectedPackagesToUpdate = CollectionUtils.addAll(affectedPackagesToUpdate,

                        executeRequestLocked(request));

                        affectedPackagesFromRequest);

                if (DEBUG) {

                    Slog.d(TAG, "Executed request=" + request

                            + " affected packages from request=" + affectedPackagesFromRequest);

                }

            }



            // past the point of no return: the entire transaction has been

services/core/java/com/android/server/om/OverlayManagerServiceImpl.java

+12 −3
Original line number Diff line number Diff line
@@ -575,7 +575,9 @@ final class OverlayManagerServiceImpl { 
            }

        }

        try {

            if (mustReinitializeOverlay(info, oi)) {

            final boolean mustReinitializeOverlay = mustReinitializeOverlay(info, oi);

            boolean updated = false;

            if (mustReinitializeOverlay) {

                if (oi != null) {

                    // If the fabricated overlay changes its target package, update the previous

                    // target package so it no longer is overlaid.
@@ -587,10 +589,17 @@ final class OverlayManagerServiceImpl { 
            } else {

                // The only non-critical part of the info that will change is path to the fabricated

                // overlay.

                mSettings.setBaseCodePath(overlayIdentifier, userId, info.path);

                updated |= mSettings.setBaseCodePath(overlayIdentifier, userId, info.path);

            }

            // No constraints should be applied when registering a fabricated overlay.

            if (updateState(oi, userId, 0 /* flags */, Collections.emptyList() /* constraints */)) {

            updated |= updateState(oi, userId, 0 /* flags */,

                    Collections.emptyList() /* constraints */);

            if (DEBUG) {

                Slog.d(TAG, "In registerFabricatedOverlay, OverlayInfo=" + oi

                        + " mustReinitializeOverlay=" + mustReinitializeOverlay

                        + " updated=" + updated);

            }

            if (updated) {

                updatedTargets.add(UserPackage.of(userId, oi.targetPackageName));

            }

        } catch (OverlayManagerSettings.BadKeyException e) {

services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplTests.java

+26 −0
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import android.content.om.OverlayIdentifier; 
import android.content.om.OverlayInfo;

import android.content.pm.UserPackage;

import android.content.res.Flags;

import android.os.FabricatedOverlayInternal;

import android.platform.test.annotations.RequiresFlagsDisabled;

import android.platform.test.annotations.RequiresFlagsEnabled;

import android.platform.test.flag.junit.CheckFlagsRule;
@@ -351,6 +352,31 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes 
                Set.of(UserPackage.of(USER, TARGET)));

    }



    @Test

    public void testRegisterFabricatedOverlayTwice() throws Exception {

        installAndAssert(target(TARGET), USER,

                Set.of(UserPackage.of(USER, TARGET)));

        installAndAssert(target(TARGET), USER2,

                Set.of(UserPackage.of(USER2, TARGET)));

        installAndAssert(overlay(OVERLAY, TARGET), USER,

                Set.of(UserPackage.of(USER, OVERLAY), UserPackage.of(USER, TARGET)));

        installAndAssert(overlay(OVERLAY, TARGET), USER2,

                Set.of(UserPackage.of(USER2, OVERLAY), UserPackage.of(USER2, TARGET)));



        final var overlay = new FabricatedOverlayInternal();

        overlay.packageName = TARGET;

        overlay.overlayName = "Test";

        overlay.targetPackageName = TARGET;

        overlay.targetOverlayable = "";

        overlay.entries = Collections.emptyList();



        final var impl = getImpl();

        final var expected = Set.of(UserPackage.of(USER, TARGET), UserPackage.of(USER2, TARGET));

        assertEquals(expected, impl.registerFabricatedOverlay(overlay));



        assertEquals(expected, impl.registerFabricatedOverlay(overlay));

    }



    private void testSetEnabledAtVariousConditions(final List<OverlayConstraint> constraints)

            throws Exception {

        final OverlayManagerServiceImpl impl = getImpl();

services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplTestsBase.java

+5 −3
Original line number Diff line number Diff line
@@ -44,7 +44,6 @@ import com.android.server.pm.pkg.AndroidPackage; 
import com.android.server.pm.pkg.AndroidPackageSplit;

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



import org.junit.Assert;

import org.junit.Before;

import org.mockito.Mockito;
@@ -255,7 +254,7 @@ class OverlayManagerServiceImplTestsBase { 


        void add(PackageBuilder pkgBuilder, int userId) {

            final Package pkg = pkgBuilder.build();

            final Package previousPkg = select(pkg.packageName, userId);

            final Package previousPkg = mPackages.get(pkg.packageName);

            mPackages.put(pkg.packageName, pkg);



            pkg.installedUserIds.add(userId);
@@ -475,7 +474,10 @@ class OverlayManagerServiceImplTestsBase { 


        private int getCrc(@NonNull final String path) {

            final FakeDeviceState.Package pkg = mState.selectFromPath(path);

            Assert.assertNotNull("path = " + path, pkg);

            if (pkg == null) {

                // This is for fabricated overlays which are not in the fake package/device state.

                return path.hashCode();

            }

            return pkg.versionCode;

        }