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

Commit 5a542d32 authored by Willie Koomson's avatar Willie Koomson
Browse files

Always scan all packages on user unlock 

Modifies ShortcutService.checkPackageChanges so that it will always scan
all packages for the user on unlock.

Currently, checkPackageChanges will only rescan packages that have a
PackageInfo.lastUpdateTime > ShortcutUser.lastAppScanTime.

This causes an issue if the service was unable to successfully write all
the scanned shortcut info, as it will not rescan these packages when
the user is unlocked the next time.

Also, since the last user unlock, aconfig flags may have been flipped
that cause new shortcuts to be available, even if the
PackageInfo.lastUpdateTime hasn't changed.

This change also re-enables some unit tests that exercise
checkPackageChanges, as well as ShortcutService saving and loading.

Bug: 417712196
Test: ShortcutManagerTest1
Flag: EXEMPT bugfix
Change-Id: I1944f5996fd3da9cea41c5ed0943f7cdd0cf076f
parent 99fcf858

services/core/java/com/android/server/pm/ShortcutService.java

+24 −13
Original line number Diff line number Diff line
@@ -99,6 +99,7 @@ import android.os.UserHandle; 
import android.text.TextUtils;

import android.text.format.TimeMigrationUtils;

import android.util.ArraySet;

import android.util.IntArray;

import android.util.KeyValueListParser;

import android.util.Log;

import android.util.Slog;
@@ -393,7 +394,7 @@ public class ShortcutService extends IShortcutService.Stub { 
    final SparseLongArray mUidLastForegroundElapsedTime = new SparseLongArray();



    @GuardedBy("mServiceLock")

    private List<Integer> mDirtyUserIds = new ArrayList<>();

    private IntArray mDirtyUserIds = new IntArray();



    private final AtomicBoolean mBootCompleted = new AtomicBoolean();

    private final AtomicBoolean mShutdown = new AtomicBoolean();
@@ -1235,14 +1236,29 @@ public class ShortcutService extends IShortcutService.Stub { 
        if (DEBUG || DEBUG_REBOOT) {

            Slog.d(TAG, "Scheduling to save for userId=" + userId);

        }

        addDirtyUserId(userId);

        // If already scheduled, remove that and re-schedule in N seconds.

        mHandler.removeCallbacks(mSaveDirtyInfoRunner);

        mHandler.postDelayed(mSaveDirtyInfoRunner, mSaveDelayMillis);

    }



    @VisibleForTesting

    void addDirtyUserId(int userId) {

        synchronized (mServiceLock) {

            if (!mDirtyUserIds.contains(userId)) {

                mDirtyUserIds.add(userId);

            }

        }

        // If already scheduled, remove that and re-schedule in N seconds.

        mHandler.removeCallbacks(mSaveDirtyInfoRunner);

        mHandler.postDelayed(mSaveDirtyInfoRunner, mSaveDelayMillis);

    }



    private IntArray getDirtyUserIds() {

        IntArray dirtyUserIds = new IntArray();

        synchronized (mServiceLock) {

            IntArray tmp = mDirtyUserIds;

            mDirtyUserIds = dirtyUserIds;

            dirtyUserIds = tmp;

        }

        return dirtyUserIds;

    }



    @VisibleForTesting
@@ -1255,12 +1271,7 @@ public class ShortcutService extends IShortcutService.Stub { 
        }

        try {

            Trace.traceBegin(Trace.TRACE_TAG_SYSTEM_SERVER, "shortcutSaveDirtyInfo");

            List<Integer> dirtyUserIds = new ArrayList<>();

            synchronized (mServiceLock) {

                List<Integer> tmp = mDirtyUserIds;

                mDirtyUserIds = dirtyUserIds;

                dirtyUserIds = tmp;

            }

            IntArray dirtyUserIds = getDirtyUserIds();

            for (int i = dirtyUserIds.size() - 1; i >= 0; i--) {

                final int userId = dirtyUserIds.get(i);

                if (userId == UserHandle.USER_NULL) { // USER_NULL for base state.
@@ -3706,8 +3717,7 @@ public class ShortcutService extends IShortcutService.Stub { 
    /**

     * Called when a user is unlocked.

     * - Check all known packages still exist, and otherwise perform cleanup.

     * - If a package still exists, check the version code.  If it's been updated, may need to

     * update timestamps of its shortcuts.

     * - Rescan installed packages for manifest shortcuts.

     */

    @VisibleForTesting

    void checkPackageChanges(@UserIdInt int ownerUserId) {
@@ -3748,7 +3758,8 @@ public class ShortcutService extends IShortcutService.Stub { 
                    }

                }



                rescanUpdatedPackagesLocked(ownerUserId, user.getLastAppScanTime());

                // Rescan all packages

                rescanUpdatedPackagesLocked(ownerUserId, /* lastScanTime= */ 0);

            }

        } finally {

            logDurationStat(Stats.CHECK_PACKAGE_CHANGES, start);

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

+2 −55
Original line number Diff line number Diff line
@@ -23,13 +23,13 @@ import static com.android.server.pm.shortcutmanagertest.ShortcutManagerTestUtils 
import static com.android.server.pm.shortcutmanagertest.ShortcutManagerTestUtils.makeBundle;

import static com.android.server.pm.shortcutmanagertest.ShortcutManagerTestUtils.set;



import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.ArgumentMatchers.isNull;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.anyLong;

import static org.mockito.ArgumentMatchers.anyString;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.ArgumentMatchers.isNull;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.reset;
@@ -110,7 +110,6 @@ import org.mockito.invocation.InvocationOnMock; 
import org.mockito.stubbing.Answer;



import java.io.BufferedReader;

import java.io.ByteArrayInputStream;

import java.io.ByteArrayOutputStream;

import java.io.File;

import java.io.FileOutputStream;
@@ -572,58 +571,6 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { 
            // During tests, WTF is fatal.

            fail(message + "  exception: " + th + "\n" + Log.getStackTraceString(th));

        }



        @Override

        void injectSaveBaseState() {

            ByteArrayOutputStream baos = new ByteArrayOutputStream();

            try {

                saveBaseStateAsXml(baos);

            } catch (Exception e) {

                throw new RuntimeException(e);

            }

            mBaseState = baos.toByteArray();

        }



        @Override

        protected void injectLoadBaseState() {

            if (mBaseState == null) {

                return;

            }

            ByteArrayInputStream bais = new ByteArrayInputStream(mBaseState);

            try {

                loadBaseStateAsXml(bais);

            } catch (Exception e) {

                throw new RuntimeException(e);

            }

        }



        @Override

        protected void injectSaveUser(@UserIdInt int userId) {

            synchronized (mServiceLock) {

                ByteArrayOutputStream baos = new ByteArrayOutputStream();

                try {

                    saveUserInternalLocked(userId, baos, /* forBackup= */ false);

                    cleanupDanglingBitmapDirectoriesLocked(userId);

                } catch (Exception e) {

                    throw new RuntimeException(e);

                }

                mUserStates.put(userId, baos.toByteArray());

            }

        }



        @Override

        protected ShortcutUser injectLoadUserLocked(@UserIdInt int userId) {

            final byte[] userState = mUserStates.get(userId);

            if (userState == null) {

                return null;

            }

            ByteArrayInputStream bais = new ByteArrayInputStream(userState);

            try {

                return loadUserInternal(userId, bais, /* forBackup= */ false);

            } catch (Exception e) {

                throw new RuntimeException(e);

            }

        }

    }



    /** ShortcutManager with injection override methods. */

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

+19 −16
Original line number Diff line number Diff line
@@ -196,7 +196,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 
    /**

     * Test for the restoration from saved file.

     */

    public void disabled_testInitializeFromSavedFile() {

    public void testInitializeFromSavedFile() {



        mInjectedCurrentTimeMillis = START_TIME + 4 * INTERVAL + 50;

        assertResetTimes(START_TIME + 4 * INTERVAL, START_TIME + 5 * INTERVAL);
@@ -205,6 +205,9 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 


        dumpBaseStateFile();



        // We need to add the dirtyUserId manually since we have not done anything to modify

        // user state.

        mService.addDirtyUserId(USER_10);

        mService.saveDirtyInfo();



        // Restore.
@@ -822,7 +825,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 
        assertEquals(1, mManager.getRemainingCallCount());

    }



    public void disabled_testIcons() throws IOException {

    public void testIcons() throws IOException {

        final Icon res32x32 = Icon.createWithResource(getTestContext(), R.drawable.black_32x32);

        final Icon res64x64 = Icon.createWithResource(getTestContext(), R.drawable.black_64x64);

        final Icon res512x512 = Icon.createWithResource(getTestContext(), R.drawable.black_512x512);
@@ -3919,7 +3922,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 
    /**

     * Try save and load, also stop/start the user.

     */

    public void disabled_testSaveAndLoadUser() {

    public void testSaveAndLoadUser() {

        // First, create some shortcuts and save.

        runWithCaller(CALLING_PACKAGE_1, USER_10, () -> {

            final Icon icon1 = Icon.createWithResource(getTestContext(), R.drawable.black_64x16);
@@ -4575,7 +4578,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 
        });

    }



    public void disabled_testHandleGonePackage_crossProfile() {

    public void testHandleGonePackage_crossProfile() {

        // Create some shortcuts.

        runWithCaller(CALLING_PACKAGE_1, USER_10, () -> {

            assertTrue(mManager.setDynamicShortcuts(list(
@@ -4910,7 +4913,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 
        checkCanRestoreTo(DISABLED_REASON_BACKUP_NOT_SUPPORTED, spi3, true, 10, true, "sig1");

    }



    public void disabled_testHandlePackageDelete() {

    public void testHandlePackageDelete() {

        checkHandlePackageDeleteInner((userId, packageName) -> {

            uninstallPackage(userId, packageName);

            mService.mPackageMonitor.onReceive(getTestContext(),
@@ -4918,7 +4921,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 
        });

    }



    public void disabled_testHandlePackageDisable() {

    public void testHandlePackageDisable() {

        checkHandlePackageDeleteInner((userId, packageName) -> {

            disablePackage(userId, packageName);

            mService.mPackageMonitor.onReceive(getTestContext(),
@@ -5168,7 +5171,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 
        });

    }



    public void disabled_testHandlePackageUpdate() throws Throwable {

    public void testHandlePackageUpdate() throws Throwable {

        // Set up shortcuts and launchers.



        final Icon res32x32 = Icon.createWithResource(getTestContext(), R.drawable.black_32x32);
@@ -5417,7 +5420,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 
        });

    }



    public void disabled_testHandlePackageUpdate_systemAppUpdate() {

    public void testHandlePackageUpdate_systemAppUpdate() {



        // Package1 is a system app.  Package 2 is not a system app, so it's not scanned

        // in this test at all.
@@ -5444,7 +5447,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 


        // Next.

        // Update the packages -- now they have 1 manifest shortcut.

        // But checkPackageChanges() don't notice it, since their version code / timestamp haven't

        // checkPackageChanges() should notice it, even if their version code / timestamp haven't

        // changed.

        addManifestShortcutResource(

                new ComponentName(CALLING_PACKAGE_1, ShortcutActivity.class.getName()),
@@ -5457,15 +5460,15 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 


        runWithCaller(CALLING_PACKAGE_1, USER_10, () -> {

            assertWith(getCallerShortcuts())

                    .isEmpty();

                .haveIds("ms1");

        });

        runWithCaller(CALLING_PACKAGE_2, USER_10, () -> {

            assertWith(getCallerShortcuts())

                    .isEmpty();

                .haveIds("ms1");

        });



        // Next.

        // Update the build finger print.  All apps will be scanned now.

        // Update the build finger print.  All apps will be scanned now, no change in shortcuts.

        mInjectedBuildFingerprint = "update1";

        mInjectedCurrentTimeMillis += 1000;

        mService.checkPackageChanges(USER_10);
@@ -5490,14 +5493,14 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 
        mInjectedCurrentTimeMillis += 1000;

        mService.checkPackageChanges(USER_10);



        // Fingerprint hasn't changed, so there packages weren't scanned.

        // Fingerprint hasn't changed, but packages should still be scanned

        runWithCaller(CALLING_PACKAGE_1, USER_10, () -> {

            assertWith(getCallerShortcuts())

                    .haveIds("ms1");

                    .haveIds("ms1", "ms2");

        });

        runWithCaller(CALLING_PACKAGE_2, USER_10, () -> {

            assertWith(getCallerShortcuts())

                    .haveIds("ms1");

                    .haveIds("ms1", "ms2");

        });



        // Update the fingerprint.  CALLING_PACKAGE_1's version code hasn't changed, but we scan
@@ -6920,7 +6923,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { 
    }



    public void disabled_testDumpsys_withIcons() throws IOException {

        disabled_testIcons();

        testIcons();

        // Dump after having some icons.

        dumpsysOnLogcat("test1", /* force= */ true);

    }

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

+7 −2
Original line number Diff line number Diff line
@@ -54,6 +54,7 @@ import androidx.test.filters.SmallTest; 
import com.android.frameworks.servicestests.R;



import java.io.File;

import java.io.FileOutputStream;

import java.io.IOException;

import java.util.Locale;
@@ -2356,9 +2357,13 @@ public class ShortcutManagerTest2 extends BaseShortcutManagerTest { 
     * can still be read.

     */

    public void testLoadLegacySavedFile() throws Exception {

        final String legacyFile = readTestAsset("shortcut/shortcut_legacy_file.xml");

        mUserStates.put(USER_10, legacyFile.getBytes());

        initService();

        final String legacyFile = readTestAsset("shortcut/shortcut_legacy_file.xml");

        try (ResilientAtomicFile userFile = mService.getUserFile(USER_10)) {

            FileOutputStream stream = userFile.startWrite();

            stream.write(legacyFile.getBytes());

            userFile.finishWrite(stream);

        }



        mService.handleUnlockUser(USER_10);