OSDN Git Service

Handle locale change and pacakge change in different way
authorMakoto Onuki <omakoto@google.com>
Wed, 13 Jul 2016 23:14:01 +0000 (16:14 -0700)
committerMakoto Onuki <omakoto@google.com>
Thu, 14 Jul 2016 00:37:06 +0000 (17:37 -0700)
- Stop using a custom callback from AM to detect locale changes
and use the LOCALE_CHANGED broadcast instead.

- This would open up a chance where a publisher app fetches
its won manifest shortcuts after a locale change but
ShortcutManager hasn't updated string resources.

- So instead, at every entry point from ShortcutManager, check
if the locale has changed, and if so, update all resources
(and reset throttling).

- Do the same for package change events too.  At every entry point
from ShortcutManager, check if the caller package has been updated,
or any target activities have been disabled.  If so, rescan the
caller package.

- We do *not* do the same check at the LauncherApps entry points,
because the launcher should use the callback to listen to
shortcut changes.

- Also stopped using PackageMonitor for now because we want to
set a higher priority and changing PackageMonitor at this point
seems too much for DR.

Bug 29895275
Bug 30123329

Change-Id: Ib4a2f626a936c7328e2cc032324f5c3d1c3b9122

core/java/android/content/pm/ShortcutServiceInternal.java
services/core/java/com/android/server/am/ActivityManagerService.java
services/core/java/com/android/server/pm/ShortcutPackage.java
services/core/java/com/android/server/pm/ShortcutPackageItem.java
services/core/java/com/android/server/pm/ShortcutService.java
services/core/java/com/android/server/pm/ShortcutUser.java
services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java
services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java
services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java
services/tests/shortcutmanagerutils/src/com/android/server/pm/shortcutmanagertest/ShortcutManagerTestUtils.java

index 3f8bad1..f994d7e 100644 (file)
@@ -67,10 +67,4 @@ public abstract class ShortcutServiceInternal {
 
     public abstract boolean hasShortcutHostPermission(int launcherUserId,
             @NonNull String callingPackage);
-
-    /**
-     * Called by AM when the system locale changes *within the AM lock*.  ABSOLUTELY do not take
-     * any locks in this method.
-     */
-    public abstract void onSystemLocaleChangedNoLock();
 }
index 4762a67..1c86d7c 100644 (file)
@@ -140,7 +140,6 @@ import android.content.pm.PermissionInfo;
 import android.content.pm.ProviderInfo;
 import android.content.pm.ResolveInfo;
 import android.content.pm.ServiceInfo;
-import android.content.pm.ShortcutServiceInternal;
 import android.content.pm.UserInfo;
 import android.content.res.CompatibilityInfo;
 import android.content.res.Configuration;
@@ -18862,15 +18861,6 @@ public final class ActivityManagerService extends ActivityManagerNative
                         null, AppOpsManager.OP_NONE, null, false, false,
                         MY_PID, Process.SYSTEM_UID, UserHandle.USER_ALL);
                 if ((changes&ActivityInfo.CONFIG_LOCALE) != 0) {
-                    // Tell the shortcut manager that the system locale changed.  It needs to know
-                    // it before any other apps receive ACTION_LOCALE_CHANGED, which is why
-                    // we "push" from here, rather than having the service listen to the broadcast.
-                    final ShortcutServiceInternal shortcutService =
-                            LocalServices.getService(ShortcutServiceInternal.class);
-                    if (shortcutService != null) {
-                        shortcutService.onSystemLocaleChangedNoLock();
-                    }
-
                     intent = new Intent(Intent.ACTION_LOCALE_CHANGED);
                     intent.addFlags(Intent.FLAG_RECEIVER_FOREGROUND);
                     if (!mProcessesReady) {
index b94d0f0..3c18198 100644 (file)
@@ -34,6 +34,7 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.Preconditions;
 import com.android.internal.util.XmlUtils;
 import com.android.server.pm.ShortcutService.ShortcutOperation;
+import com.android.server.pm.ShortcutService.Stats;
 
 import org.xmlpull.v1.XmlPullParser;
 import org.xmlpull.v1.XmlPullParserException;
@@ -437,8 +438,6 @@ class ShortcutPackage extends ShortcutPackageItem {
      * locale changes.
      */
     public int getApiCallCount() {
-        mShortcutUser.resetThrottlingIfNeeded();
-
         final ShortcutService s = mShortcutUser.mService;
 
         // Reset the counter if:
@@ -598,7 +597,37 @@ class ShortcutPackage extends ShortcutPackageItem {
     }
 
     /**
-     * Called when the package is updated or added.
+     * @return false if any of the target activities are no longer enabled.
+     */
+    private boolean areAllActivitiesStillEnabled() {
+        if (mShortcuts.size() == 0) {
+            return true;
+        }
+        final ShortcutService s = mShortcutUser.mService;
+
+        // Normally the number of target activities is 1 or so, so no need to use a complex
+        // structure like a set.
+        final ArrayList<ComponentName> checked = new ArrayList<>(4);
+
+        for (int i = mShortcuts.size() - 1; i >= 0; i--) {
+            final ShortcutInfo si = mShortcuts.valueAt(i);
+            final ComponentName activity = si.getActivity();
+
+            if (checked.contains(activity)) {
+                continue; // Already checked.
+            }
+            checked.add(activity);
+
+            if (!s.injectIsActivityEnabledAndExported(activity, getOwnerUserId())) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Called when the package may be added or updated, or its activities may be disabled, and
+     * if so, rescan the package and do the necessary stuff.
      *
      * Add case:
      * - Publish manifest shortcuts.
@@ -606,23 +635,35 @@ class ShortcutPackage extends ShortcutPackageItem {
      * Update case:
      * - Re-publish manifest shortcuts.
      * - If there are shortcuts with resources (icons or strings), update their timestamps.
+     * - Disable shortcuts whose target activities are disabled.
      *
      * @return TRUE if any shortcuts have been changed.
      */
-    public boolean handlePackageAddedOrUpdated(boolean isNewApp, boolean forceRescan) {
-        final PackageInfo pi = mShortcutUser.mService.getPackageInfo(
-                getPackageName(), getPackageUserId());
-        if (pi == null) {
-            return false; // Shouldn't happen.
-        }
+    public boolean rescanPackageIfNeeded(boolean isNewApp, boolean forceRescan) {
+        final ShortcutService s = mShortcutUser.mService;
+        final long start = s.injectElapsedRealtime();
 
-        if (!isNewApp && !forceRescan) {
-            // Make sure the version code or last update time has changed.
-            // Otherwise, nothing to do.
-            if (getPackageInfo().getVersionCode() >= pi.versionCode
-                    && getPackageInfo().getLastUpdateTime() >= pi.lastUpdateTime) {
-                return false;
+        final PackageInfo pi;
+        try {
+            pi = mShortcutUser.mService.getPackageInfo(
+                    getPackageName(), getPackageUserId());
+            if (pi == null) {
+                return false; // Shouldn't happen.
+            }
+
+            if (!isNewApp && !forceRescan) {
+                // Return if the package hasn't changed, ie:
+                // - version code hasn't change
+                // - lastUpdateTime hasn't change
+                // - all target activities are still enabled.
+                if ((getPackageInfo().getVersionCode() >= pi.versionCode)
+                        && (getPackageInfo().getLastUpdateTime() >= pi.lastUpdateTime)
+                        && areAllActivitiesStillEnabled()) {
+                    return false;
+                }
             }
+        } finally {
+            s.logDurationStat(Stats.PACKAGE_UPDATE_CHECK, start);
         }
 
         // Now prepare to publish manifest shortcuts.
@@ -654,8 +695,6 @@ class ShortcutPackage extends ShortcutPackageItem {
 
         getPackageInfo().updateVersionInfo(pi);
 
-        final ShortcutService s = mShortcutUser.mService;
-
         boolean changed = false;
 
         // For existing shortcuts, update timestamps if they have any resources.
@@ -1001,7 +1040,7 @@ class ShortcutPackage extends ShortcutPackageItem {
             }
         }
         if (changed) {
-            s.scheduleSaveUser(getPackageUserId());
+            s.packageShortcutsChanged(getPackageName(), getPackageUserId());
         }
     }
 
index 757dd19..26b52e9 100644 (file)
@@ -48,6 +48,10 @@ abstract class ShortcutPackageItem {
         mPackageInfo = Preconditions.checkNotNull(packageInfo);
     }
 
+    public ShortcutUser getUser() {
+        return mShortcutUser;
+    }
+
     /**
      * ID of the user who actually has this package running on.  For {@link ShortcutPackage},
      * this is the same thing as {@link #getOwnerUserId}, but if it's a {@link ShortcutLauncher} and
index 5f8cbbf..e2f656e 100644 (file)
@@ -24,9 +24,11 @@ import android.app.ActivityManagerNative;
 import android.app.AppGlobals;
 import android.app.IUidObserver;
 import android.app.usage.UsageStatsManagerInternal;
+import android.content.BroadcastReceiver;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
+import android.content.IntentFilter;
 import android.content.pm.ActivityInfo;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.IPackageManager;
@@ -49,10 +51,12 @@ import android.graphics.Bitmap.CompressFormat;
 import android.graphics.Canvas;
 import android.graphics.RectF;
 import android.graphics.drawable.Icon;
+import android.net.Uri;
 import android.os.Binder;
 import android.os.Environment;
 import android.os.FileUtils;
 import android.os.Handler;
+import android.os.LocaleList;
 import android.os.Looper;
 import android.os.ParcelFileDescriptor;
 import android.os.PersistableBundle;
@@ -116,15 +120,11 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 
 /**
  * TODO:
- * - Deal with the async nature of PACKAGE_ADD.  Basically when a publisher does anything after
- *   it's upgraded, the manager should make sure the upgrade process has been executed.
- *
  * - getIconMaxWidth()/getIconMaxHeight() should use xdpi and ydpi.
  *   -> But TypedValue.applyDimension() doesn't differentiate x and y..?
  *
@@ -179,7 +179,6 @@ public class ShortcutService extends IShortcutService.Stub {
 
     private static final String TAG_ROOT = "root";
     private static final String TAG_LAST_RESET_TIME = "last_reset_time";
-    private static final String TAG_LOCALE_CHANGE_SEQUENCE_NUMBER = "locale_seq_no";
 
     private static final String ATTR_VALUE = "value";
 
@@ -292,16 +291,6 @@ public class ShortcutService extends IShortcutService.Stub {
     @GuardedBy("mLock")
     private List<Integer> mDirtyUserIds = new ArrayList<>();
 
-    /**
-     * A counter that increments every time the system locale changes.  We keep track of it to
-     * reset
-     * throttling counters on the first call from each package after the last locale change.
-     *
-     * We need this mechanism because we can't do much in the locale change callback, which is
-     * {@link ShortcutServiceInternal#onSystemLocaleChangedNoLock()}.
-     */
-    private final AtomicLong mLocaleChangeSequenceNumber = new AtomicLong();
-
     private final AtomicBoolean mBootCompleted = new AtomicBoolean();
 
     private static final int PACKAGE_MATCH_FLAGS =
@@ -326,8 +315,9 @@ public class ShortcutService extends IShortcutService.Stub {
         int GET_LAUNCHER_ACTIVITY = 11;
         int CHECK_LAUNCHER_ACTIVITY = 12;
         int IS_ACTIVITY_ENABLED = 13;
+        int PACKAGE_UPDATE_CHECK = 14;
 
-        int COUNT = IS_ACTIVITY_ENABLED + 1;
+        int COUNT = PACKAGE_UPDATE_CHECK + 1;
     }
 
     final Object mStatLock = new Object();
@@ -381,7 +371,25 @@ public class ShortcutService extends IShortcutService.Stub {
             return; // Don't do anything further.  For unit tests only.
         }
 
-        mPackageMonitor.register(context, looper, UserHandle.ALL, /* externalStorage= */ false);
+        // Register receivers.
+
+        // We need to set a priority, so let's just not use PackageMonitor for now.
+        // TODO Refactor PackageMonitor to support priorities.
+        final IntentFilter packageFilter = new IntentFilter();
+        packageFilter.addAction(Intent.ACTION_PACKAGE_ADDED);
+        packageFilter.addAction(Intent.ACTION_PACKAGE_REMOVED);
+        packageFilter.addAction(Intent.ACTION_PACKAGE_CHANGED);
+        packageFilter.addAction(Intent.ACTION_PACKAGE_DATA_CLEARED);
+        packageFilter.addDataScheme("package");
+        packageFilter.setPriority(IntentFilter.SYSTEM_HIGH_PRIORITY);
+        mContext.registerReceiverAsUser(mPackageMonitor, UserHandle.ALL,
+                packageFilter, null, mHandler);
+
+        final IntentFilter localeFilter = new IntentFilter();
+        localeFilter.addAction(Intent.ACTION_LOCALE_CHANGED);
+        localeFilter.setPriority(IntentFilter.SYSTEM_HIGH_PRIORITY);
+        mContext.registerReceiverAsUser(mReceiver, UserHandle.ALL,
+                localeFilter, null, mHandler);
 
         injectRegisterUidObserver(mUidObserver, ActivityManager.UID_OBSERVER_PROCSTATE
                 | ActivityManager.UID_OBSERVER_GONE);
@@ -394,8 +402,9 @@ public class ShortcutService extends IShortcutService.Stub {
         }
     }
 
-    public long getLocaleChangeSequenceNumber() {
-        return mLocaleChangeSequenceNumber.get();
+    public String injectGetLocaleTagsForUser(@UserIdInt int userId) {
+        // TODO This should get the per-user locale.  b/30123329 b/30119489
+        return LocaleList.getDefault().toLanguageTags();
     }
 
     final private IUidObserver mUidObserver = new IUidObserver.Stub() {
@@ -504,8 +513,11 @@ public class ShortcutService extends IShortcutService.Stub {
             Slog.d(TAG, "handleUnlockUser: user=" + userId);
         }
         synchronized (mLock) {
-            // Preload
-            getUserShortcutsLocked(userId);
+            // Preload the user's shortcuts.
+            // Also see if the locale has changed.
+            // Note as of nyc, the locale is per-user, so the locale shouldn't change
+            // when the user is locked.  However due to b/30119489 it still happens.
+            getUserShortcutsLocked(userId).detectLocaleChange();
 
             checkPackageChanges(userId);
         }
@@ -751,8 +763,6 @@ public class ShortcutService extends IShortcutService.Stub {
 
             // Body.
             writeTagValue(out, TAG_LAST_RESET_TIME, mRawLastResetTime);
-            writeTagValue(out, TAG_LOCALE_CHANGE_SEQUENCE_NUMBER,
-                    mLocaleChangeSequenceNumber.get());
 
             // Epilogue.
             out.endTag(null, TAG_ROOT);
@@ -797,9 +807,6 @@ public class ShortcutService extends IShortcutService.Stub {
                     case TAG_LAST_RESET_TIME:
                         mRawLastResetTime = parseLongAttribute(parser, ATTR_VALUE);
                         break;
-                    case TAG_LOCALE_CHANGE_SEQUENCE_NUMBER:
-                        mLocaleChangeSequenceNumber.set(parseLongAttribute(parser, ATTR_VALUE));
-                        break;
                     default:
                         Slog.e(TAG, "Invalid tag: " + tag);
                         break;
@@ -1497,6 +1504,7 @@ public class ShortcutService extends IShortcutService.Stub {
 
         synchronized (mLock) {
             final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+            ps.getUser().onCalledByPublisher(packageName);
 
             ps.ensureImmutableShortcutsNotIncluded(newShortcuts);
 
@@ -1546,6 +1554,7 @@ public class ShortcutService extends IShortcutService.Stub {
 
         synchronized (mLock) {
             final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+            ps.getUser().onCalledByPublisher(packageName);
 
             ps.ensureImmutableShortcutsNotIncluded(newShortcuts);
 
@@ -1624,6 +1633,7 @@ public class ShortcutService extends IShortcutService.Stub {
 
         synchronized (mLock) {
             final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+            ps.getUser().onCalledByPublisher(packageName);
 
             ps.ensureImmutableShortcutsNotIncluded(newShortcuts);
 
@@ -1671,6 +1681,7 @@ public class ShortcutService extends IShortcutService.Stub {
 
         synchronized (mLock) {
             final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+            ps.getUser().onCalledByPublisher(packageName);
 
             ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds);
 
@@ -1698,6 +1709,7 @@ public class ShortcutService extends IShortcutService.Stub {
 
         synchronized (mLock) {
             final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+            ps.getUser().onCalledByPublisher(packageName);
 
             ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds);
 
@@ -1718,6 +1730,7 @@ public class ShortcutService extends IShortcutService.Stub {
 
         synchronized (mLock) {
             final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+            ps.getUser().onCalledByPublisher(packageName);
 
             ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds);
 
@@ -1739,7 +1752,9 @@ public class ShortcutService extends IShortcutService.Stub {
         verifyCaller(packageName, userId);
 
         synchronized (mLock) {
-            getPackageShortcutsLocked(packageName, userId).deleteAllDynamicShortcuts();
+            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+            ps.getUser().onCalledByPublisher(packageName);
+            ps.deleteAllDynamicShortcuts();
         }
         packageShortcutsChanged(packageName, userId);
 
@@ -1784,7 +1799,9 @@ public class ShortcutService extends IShortcutService.Stub {
 
         final ArrayList<ShortcutInfo> ret = new ArrayList<>();
 
-        getPackageShortcutsLocked(packageName, userId).findAll(ret, query, cloneFlags);
+        final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+        ps.getUser().onCalledByPublisher(packageName);
+        ps.findAll(ret, query, cloneFlags);
 
         return new ParceledListSlice<>(ret);
     }
@@ -1802,8 +1819,9 @@ public class ShortcutService extends IShortcutService.Stub {
         verifyCaller(packageName, userId);
 
         synchronized (mLock) {
-            return mMaxUpdatesPerInterval
-                    - getPackageShortcutsLocked(packageName, userId).getApiCallCount();
+            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+            ps.getUser().onCalledByPublisher(packageName);
+            return mMaxUpdatesPerInterval - ps.getApiCallCount();
         }
     }
 
@@ -1838,6 +1856,8 @@ public class ShortcutService extends IShortcutService.Stub {
 
         synchronized (mLock) {
             final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+            ps.getUser().onCalledByPublisher(packageName);
+
             if (ps.findShortcutById(shortcutId) == null) {
                 Log.w(TAG, String.format("reportShortcutUsed: package %s doesn't have shortcut %s",
                         packageName, shortcutId));
@@ -2035,7 +2055,7 @@ public class ShortcutService extends IShortcutService.Stub {
         if (appStillExists && (packageUserId == owningUserId)) {
             // This will do the notification and save when needed, so do it after the above
             // notifyListeners.
-            user.handlePackageAddedOrUpdated(packageName, /* forceRescan=*/ true);
+            user.rescanPackageIfNeeded(packageName, /* forceRescan=*/ true);
         }
 
         if (!wasUserLoaded) {
@@ -2278,36 +2298,19 @@ public class ShortcutService extends IShortcutService.Stub {
                 @NonNull String callingPackage) {
             return ShortcutService.this.hasShortcutHostPermission(callingPackage, launcherUserId);
         }
+    }
 
-        /**
-         * Called by AM when the system locale changes *within the AM lock.  ABSOLUTELY do not take
-         * any locks in this method.
-         */
+    final BroadcastReceiver mReceiver = new BroadcastReceiver() {
         @Override
-        public void onSystemLocaleChangedNoLock() {
-            // DO NOT HOLD ANY LOCKS HERE.
-
-            // We want to reset throttling for all packages for all users.  But we can't just do so
-            // here because:
-            // - We can't load/save users that are locked.
-            // - Even for loaded users, resetting the counters would require us to hold mLock.
-            //
-            // So we use a "pull" model instead.  In here, we just increment the "locale change
-            // sequence number".  Each ShortcutUser has the "last known locale change sequence".
-            //
-            // This allows ShortcutUser's to detect the system locale change, so they can reset
-            // counters.
-
-            // Ignore all callback during system boot.
-            if (mBootCompleted.get()) {
-                mLocaleChangeSequenceNumber.incrementAndGet();
-                if (DEBUG) {
-                    Slog.d(TAG, "onSystemLocaleChangedNoLock: " + mLocaleChangeSequenceNumber.get());
-                }
-                injectPostToHandler(() -> handleLocaleChanged());
+        public void onReceive(Context context, Intent intent) {
+            if (!mBootCompleted.get()) {
+                return; // Boot not completed, ignore the broadcast.
+            }
+            if (Intent.ACTION_LOCALE_CHANGED.equals(intent.getAction())) {
+                handleLocaleChanged();
             }
         }
-    }
+    };
 
     void handleLocaleChanged() {
         if (DEBUG) {
@@ -2317,7 +2320,7 @@ public class ShortcutService extends IShortcutService.Stub {
 
         final long token = injectClearCallingIdentity();
         try {
-            forEachLoadedUserLocked(u -> u.forAllPackages(p -> p.resolveResourceStrings()));
+            forEachLoadedUserLocked(user -> user.detectLocaleChange());
         } finally {
             injectRestoreCallingIdentity(token);
         }
@@ -2327,52 +2330,64 @@ public class ShortcutService extends IShortcutService.Stub {
      * Package event callbacks.
      */
     @VisibleForTesting
-    final PackageMonitor mPackageMonitor = new PackageMonitor() {
-
-        private boolean isUserUnlocked() {
-            return mUserManager.isUserUnlocked(getChangingUserId());
-        }
-
+    final BroadcastReceiver mPackageMonitor = new BroadcastReceiver() {
         @Override
         public void onReceive(Context context, Intent intent) {
-            // clearCallingIdentity is not needed normally, but need to do it for the unit test.
+            final int userId  = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, UserHandle.USER_NULL);
+            if (userId == UserHandle.USER_NULL) {
+                Slog.w(TAG, "Intent broadcast does not contain user handle: " + intent);
+                return;
+            }
+
+            final String action = intent.getAction();
+
+            // This is normally called on Handler, so clearCallingIdentity() isn't needed,
+            // but we still check it in unit tests.
             final long token = injectClearCallingIdentity();
             try {
-                super.onReceive(context, intent);
-            } finally {
-                injectRestoreCallingIdentity(token);
-            }
-        }
 
-        @Override
-        public void onPackageAdded(String packageName, int uid) {
-            if (!isUserUnlocked()) return;
-            handlePackageAdded(packageName, getChangingUserId());
-        }
+                if (!mUserManager.isUserUnlocked(userId)) {
+                    if (DEBUG) {
+                        Slog.d(TAG, "Ignoring package broadcast " + action
+                                + " for locked/stopped user " + userId);
+                    }
+                    return;
+                }
 
-        @Override
-        public void onPackageUpdateFinished(String packageName, int uid) {
-            if (!isUserUnlocked()) return;
-            handlePackageUpdateFinished(packageName, getChangingUserId());
-        }
+                final Uri intentUri = intent.getData();
+                final String packageName = (intentUri != null) ? intentUri.getSchemeSpecificPart()
+                        : null;
+                if (packageName == null) {
+                    Slog.w(TAG, "Intent broadcast does not contain package name: " + intent);
+                    return;
+                }
 
-        @Override
-        public void onPackageRemoved(String packageName, int uid) {
-            if (!isUserUnlocked()) return;
-            handlePackageRemoved(packageName, getChangingUserId());
-        }
+                final boolean replacing = intent.getBooleanExtra(Intent.EXTRA_REPLACING, false);
 
-        @Override
-        public void onPackageDataCleared(String packageName, int uid) {
-            if (!isUserUnlocked()) return;
-            handlePackageDataCleared(packageName, getChangingUserId());
-        }
+                switch (action) {
+                    case Intent.ACTION_PACKAGE_ADDED:
+                        if (replacing) {
+                            handlePackageUpdateFinished(packageName, userId);
+                        } else {
+                            handlePackageAdded(packageName, userId);
+                        }
+                        break;
+                    case Intent.ACTION_PACKAGE_REMOVED:
+                        if (!replacing) {
+                            handlePackageRemoved(packageName, userId);
+                        }
+                        break;
+                    case Intent.ACTION_PACKAGE_CHANGED:
+                        handlePackageChanged(packageName, userId);
 
-        @Override
-        public boolean onPackageChanged(String packageName, int uid, String[] components) {
-            if (!isUserUnlocked()) return false;
-            handlePackageChanged(packageName, getChangingUserId());
-            return false; // We don't need to receive onSomePackagesChanged(), so just false.
+                        break;
+                    case Intent.ACTION_PACKAGE_DATA_CLEARED:
+                        handlePackageDataCleared(packageName, userId);
+                        break;
+                }
+            } finally {
+                injectRestoreCallingIdentity(token);
+            }
         }
     };
 
@@ -2423,7 +2438,7 @@ public class ShortcutService extends IShortcutService.Stub {
 
                 // Then for each installed app, publish manifest shortcuts when needed.
                 forUpdatedPackages(ownerUserId, user.getLastAppScanTime(), ai -> {
-                    user.handlePackageAddedOrUpdated(ai.packageName, /* forceRescan=*/ false);
+                    user.rescanPackageIfNeeded(ai.packageName, /* forceRescan=*/ false);
                 });
 
                 // Write the time just before the scan, because there may be apps that have just
@@ -2444,7 +2459,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             final ShortcutUser user = getUserShortcutsLocked(userId);
             user.attemptToRestoreIfNeededAndSave(this, packageName, userId);
-            user.handlePackageAddedOrUpdated(packageName, /* forceRescan=*/ false);
+            user.rescanPackageIfNeeded(packageName, /* forceRescan=*/ false);
         }
         verifyStates();
     }
@@ -2459,7 +2474,7 @@ public class ShortcutService extends IShortcutService.Stub {
             user.attemptToRestoreIfNeededAndSave(this, packageName, userId);
 
             if (isPackageInstalled(packageName, userId)) {
-                user.handlePackageAddedOrUpdated(packageName, /* forceRescan=*/ false);
+                user.rescanPackageIfNeeded(packageName, /* forceRescan=*/ false);
             }
         }
         verifyStates();
@@ -2495,7 +2510,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             final ShortcutUser user = getUserShortcutsLocked(packageUserId);
 
-            user.handlePackageAddedOrUpdated(packageName, /* forceRescan=*/ true);
+            user.rescanPackageIfNeeded(packageName, /* forceRescan=*/ true);
         }
 
         verifyStates();
@@ -2971,10 +2986,6 @@ public class ShortcutService extends IShortcutService.Stub {
             pw.print("] ");
             pw.print(formatTime(next));
 
-            pw.print("  Locale change seq#: ");
-            pw.print(mLocaleChangeSequenceNumber.get());
-            pw.println();
-
             pw.print("  Config:");
             pw.print("    Max icon dim: ");
             pw.println(mMaxIconDimension);
@@ -3010,6 +3021,7 @@ public class ShortcutService extends IShortcutService.Stub {
                 dumpStatLS(pw, p, Stats.GET_LAUNCHER_ACTIVITY, "getLauncherActivity");
                 dumpStatLS(pw, p, Stats.CHECK_LAUNCHER_ACTIVITY, "checkLauncherActivity");
                 dumpStatLS(pw, p, Stats.IS_ACTIVITY_ENABLED, "isActivityEnabled");
+                dumpStatLS(pw, p, Stats.PACKAGE_UPDATE_CHECK, "packageUpdateCheck");
             }
 
             pw.println();
index 7ea89c9..9649641 100644 (file)
@@ -19,6 +19,8 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.UserIdInt;
 import android.content.ComponentName;
+import android.content.pm.ShortcutManager;
+import android.text.TextUtils;
 import android.text.format.Formatter;
 import android.util.ArrayMap;
 import android.util.Slog;
@@ -51,7 +53,7 @@ class ShortcutUser {
     private static final String TAG_LAUNCHER = "launcher";
 
     private static final String ATTR_VALUE = "value";
-    private static final String ATTR_KNOWN_LOCALE_CHANGE_SEQUENCE_NUMBER = "locale-seq-no";
+    private static final String ATTR_KNOWN_LOCALES = "locales";
     private static final String ATTR_LAST_APP_SCAN_TIME = "last-app-scan-time";
 
     static final class PackageWithUser {
@@ -104,7 +106,7 @@ class ShortcutUser {
     /** Default launcher that can access the launcher apps APIs. */
     private ComponentName mDefaultLauncherComponent;
 
-    private long mKnownLocaleChangeSequenceNumber;
+    private String mKnownLocales;
 
     private long mLastAppScanTime;
 
@@ -225,29 +227,62 @@ class ShortcutUser {
     }
 
     /**
-     * Reset all throttling counters for all packages, if there has been a system locale change.
+     * Must be called at any entry points on {@link ShortcutManager} APIs to make sure the
+     * information on the package is up-to-date.
+     *
+     * We use broadcasts to handle locale changes and package changes, but because broadcasts
+     * are asynchronous, there's a chance a publisher calls getXxxShortcuts() after a certain event
+     * (e.g. system locale change) but shortcut manager hasn't finished processing the broadcast.
+     *
+     * So we call this method at all entry points from publishers to make sure we update all
+     * relevant information.
+     *
+     * Similar inconsistencies can happen when the launcher fetches shortcut information, but
+     * that's a less of an issue because for the launcher we report shortcut changes with
+     * callbacks.
      */
-    public void resetThrottlingIfNeeded() {
-        final long currentNo = mService.getLocaleChangeSequenceNumber();
-        if (mKnownLocaleChangeSequenceNumber < currentNo) {
-            if (ShortcutService.DEBUG) {
-                Slog.d(TAG, "LocaleChange detected for user " + mUserId);
-            }
-
-            mKnownLocaleChangeSequenceNumber = currentNo;
-
-            forAllPackages(p -> p.resetRateLimiting());
+    public void onCalledByPublisher(@NonNull String packageName) {
+        detectLocaleChange();
+        rescanPackageIfNeeded(packageName, /*forceRescan=*/ false);
+    }
 
+    private String getKnownLocales() {
+        if (TextUtils.isEmpty(mKnownLocales)) {
+            mKnownLocales = mService.injectGetLocaleTagsForUser(mUserId);
             mService.scheduleSaveUser(mUserId);
         }
+        return mKnownLocales;
+    }
+
+    /**
+     * Check to see if the system locale has changed, and if so, reset throttling
+     * and update resource strings.
+     */
+    public void detectLocaleChange() {
+        final String currentLocales = mService.injectGetLocaleTagsForUser(mUserId);
+        if (getKnownLocales().equals(currentLocales)) {
+            return;
+        }
+        if (ShortcutService.DEBUG) {
+            Slog.d(TAG, "Locale changed from " + currentLocales + " to " + mKnownLocales
+                    + " for user " + mUserId);
+        }
+        mKnownLocales = currentLocales;
+
+        forAllPackages(pkg -> {
+            pkg.resetRateLimiting();
+            pkg.resolveResourceStrings();
+        });
+
+        mService.scheduleSaveUser(mUserId);
     }
 
-    public void handlePackageAddedOrUpdated(@NonNull String packageName, boolean forceRescan) {
+    public void rescanPackageIfNeeded(@NonNull String packageName, boolean forceRescan) {
         final boolean isNewApp = !mPackages.containsKey(packageName);
 
         final ShortcutPackage shortcutPackage = getPackageShortcuts(packageName);
 
-        if (!shortcutPackage.handlePackageAddedOrUpdated(isNewApp, forceRescan)) {
+        if (!shortcutPackage.rescanPackageIfNeeded(isNewApp, forceRescan)) {
             if (isNewApp) {
                 mPackages.remove(packageName);
             }
@@ -265,8 +300,7 @@ class ShortcutUser {
             throws IOException, XmlPullParserException {
         out.startTag(null, TAG_ROOT);
 
-        ShortcutService.writeAttr(out, ATTR_KNOWN_LOCALE_CHANGE_SEQUENCE_NUMBER,
-                mKnownLocaleChangeSequenceNumber);
+        ShortcutService.writeAttr(out, ATTR_KNOWN_LOCALES, mKnownLocales);
         ShortcutService.writeAttr(out, ATTR_LAST_APP_SCAN_TIME,
                 mLastAppScanTime);
 
@@ -307,8 +341,8 @@ class ShortcutUser {
             boolean fromBackup) throws IOException, XmlPullParserException {
         final ShortcutUser ret = new ShortcutUser(s, userId);
 
-        ret.mKnownLocaleChangeSequenceNumber = ShortcutService.parseLongAttribute(parser,
-                ATTR_KNOWN_LOCALE_CHANGE_SEQUENCE_NUMBER);
+        ret.mKnownLocales = ShortcutService.parseStringAttribute(parser,
+                ATTR_KNOWN_LOCALES);
 
         // If lastAppScanTime is in the future, that means the clock went backwards.
         // Just scan all apps again.
@@ -377,8 +411,8 @@ class ShortcutUser {
         pw.print(prefix);
         pw.print("User: ");
         pw.print(mUserId);
-        pw.print("  Known locale seq#: ");
-        pw.print(mKnownLocaleChangeSequenceNumber);
+        pw.print("  Known locales: ");
+        pw.print(mKnownLocales);
         pw.print("  Last app scan: [");
         pw.print(mLastAppScanTime);
         pw.print("] ");
index 1be57bc..2652b8f 100644 (file)
@@ -110,13 +110,14 @@ import java.util.function.Function;
 public abstract class BaseShortcutManagerTest extends InstrumentationTestCase {
     protected static final String TAG = "ShortcutManagerTest";
 
+    protected static final boolean DUMP_IN_TEARDOWN = false; // DO NOT SUBMIT WITH true
+
     /**
      * Whether to enable dump or not.  Should be only true when debugging to avoid bugs where
      * dump affecting the behavior.
      */
-    protected static final boolean ENABLE_DUMP = false; // DO NOT SUBMIT WITH true
-
-    protected static final boolean DUMP_IN_TEARDOWN = false; // DO NOT SUBMIT WITH true
+    protected static final boolean ENABLE_DUMP = false // DO NOT SUBMIT WITH true
+            || DUMP_IN_TEARDOWN || ShortcutService.DEBUG;
 
     protected static final String[] EMPTY_STRINGS = new String[0]; // Just for readability.
 
@@ -154,6 +155,11 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase {
             // ignore.
             return null;
         }
+
+        @Override
+        public void unregisterReceiver(BroadcastReceiver receiver) {
+            // ignore.
+        }
     }
 
     /** Context used in the client side */
@@ -212,6 +218,11 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase {
         }
 
         @Override
+        public String injectGetLocaleTagsForUser(@UserIdInt int userId) {
+            return mInjectedLocale.toLanguageTag();
+        }
+
+        @Override
         boolean injectShouldPerformVerification() {
             return true; // Always verify during unit tests.
         }
@@ -715,11 +726,6 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase {
         // Start the service.
         initService();
         setCaller(CALLING_PACKAGE_1);
-
-        // In order to complicate the situation, we set mLocaleChangeSequenceNumber to 1 by
-        // calling this.  Running test with mLocaleChangeSequenceNumber == 0 might make us miss
-        // some edge cases.
-        mInternal.onSystemLocaleChangedNoLock();
     }
 
     /**
@@ -837,12 +843,6 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase {
         // Send boot sequence events.
         mService.onBootPhase(SystemService.PHASE_LOCK_SETTINGS_READY);
 
-        // Make sure a call to onSystemLocaleChangedNoLock() before PHASE_BOOT_COMPLETED will be
-        // ignored.
-        final long origSequenceNumber = mService.getLocaleChangeSequenceNumber();
-        mInternal.onSystemLocaleChangedNoLock();
-        assertEquals(origSequenceNumber, mService.getLocaleChangeSequenceNumber());
-
         mService.onBootPhase(SystemService.PHASE_BOOT_COMPLETED);
     }
 
index bf6c2ff..d563185 100644 (file)
@@ -5730,21 +5730,24 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
         mRunningUsers.put(USER_10, false);
         mUnlockedUsers.put(USER_10, false);
 
-                mService.mPackageMonitor.onReceive(getTestContext(),
+        mService.mPackageMonitor.onReceive(getTestContext(),
                 genPackageAddIntent(CALLING_PACKAGE_2, USER_10));
         runWithCaller(CALLING_PACKAGE_2, USER_10, () -> {
-            assertEmpty(mManager.getManifestShortcuts());
-            assertEmpty(mManager.getPinnedShortcuts());
+            // Don't use the mManager APIs to get shortcuts, because they'll trigger the package
+            // update check.
+            // So look the internal data directly using getCallerShortcuts().
+            assertEmpty(getCallerShortcuts());
         });
 
         // Try again, but the user is locked, so still ignored.
         mRunningUsers.put(USER_10, true);
-
                 mService.mPackageMonitor.onReceive(getTestContext(),
                 genPackageAddIntent(CALLING_PACKAGE_2, USER_10));
         runWithCaller(CALLING_PACKAGE_2, USER_10, () -> {
-            assertEmpty(mManager.getManifestShortcuts());
-            assertEmpty(mManager.getPinnedShortcuts());
+            // Don't use the mManager APIs to get shortcuts, because they'll trigger the package
+            // update check.
+            // So look the internal data directly using getCallerShortcuts().
+            assertEmpty(getCallerShortcuts());
         });
 
         // Unlock the user, now it should work.
@@ -6108,7 +6111,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
         });
     }
 
-    public void testManifestShortcuts_localeChange() {
+    public void testManifestShortcuts_localeChange() throws InterruptedException {
         mService.handleUnlockUser(USER_0);
 
         // Package 1 updated, which has one valid manifest shortcut and one invalid.
@@ -6164,8 +6167,15 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
 
         mInjectedCurrentTimeMillis++;
 
+        // Change the locale and send the broadcast, make sure the launcher gets a callback too.
         mInjectedLocale = Locale.JAPANESE;
-        mInternal.onSystemLocaleChangedNoLock();
+
+        setCaller(LAUNCHER_1, USER_0);
+
+        assertForLauncherCallback(mLauncherApps, () -> {
+            mService.mReceiver.onReceive(mServiceContext, new Intent(Intent.ACTION_LOCALE_CHANGED));
+        }).assertCallbackCalledForPackageAndUser(CALLING_PACKAGE_1, HANDLE_USER_0)
+                .haveIds("ms1", "ms2", "s1");
 
         runWithCaller(CALLING_PACKAGE_1, USER_0, () -> {
             // check first shortcut.
index f570ff2..d1caa4e 100644 (file)
@@ -47,6 +47,8 @@ import android.test.suitebuilder.annotation.SmallTest;
 import com.android.frameworks.servicestests.R;
 import com.android.server.pm.ShortcutService.ConfigConstants;
 
+import java.util.Locale;
+
 /**
  * Tests for ShortcutService and ShortcutManager.
  *
@@ -1331,13 +1333,12 @@ public class ShortcutManagerTest2 extends BaseShortcutManagerTest {
         mService.saveDirtyInfo();
         initService();
 
-        final long origSequenceNumber = mService.getLocaleChangeSequenceNumber();
-
-        mInternal.onSystemLocaleChangedNoLock();
-        assertEquals(origSequenceNumber + 1, mService.getLocaleChangeSequenceNumber());
+        mInjectedLocale = Locale.CHINA;
+        mService.mReceiver.onReceive(mServiceContext, new Intent(Intent.ACTION_LOCALE_CHANGED));
 
         // Note at this point only user-0 is loaded, and the counters are reset for this user,
-        // but it will work for other users too, because we persist when
+        // but it will work for other users too because we check the locale change at any
+        // API entry point.
 
         runWithCaller(CALLING_PACKAGE_1, USER_0, () -> {
             assertEquals(3, mManager.getRemainingCallCount());
@@ -1358,11 +1359,28 @@ public class ShortcutManagerTest2 extends BaseShortcutManagerTest {
             assertEquals(3, mManager.getRemainingCallCount());
         });
 
+        // Make sure even if we receive ACTION_LOCALE_CHANGED, if the locale hasn't actually
+        // changed, we don't reset throttling.
+        runWithCaller(CALLING_PACKAGE_1, USER_0, () -> {
+            mManager.updateShortcuts(list());
+            assertEquals(2, mManager.getRemainingCallCount());
+        });
+
+        mService.mReceiver.onReceive(mServiceContext, new Intent(Intent.ACTION_LOCALE_CHANGED));
+
+        runWithCaller(CALLING_PACKAGE_1, USER_0, () -> {
+            assertEquals(2, mManager.getRemainingCallCount()); // Still 2.
+        });
+
         mService.saveDirtyInfo();
         initService();
 
-        // Make sure the counter is persisted.
-        assertEquals(origSequenceNumber + 1, mService.getLocaleChangeSequenceNumber());
+        // The locale should be persisted, so it still shouldn't reset throttling.
+        mService.mReceiver.onReceive(mServiceContext, new Intent(Intent.ACTION_LOCALE_CHANGED));
+
+        runWithCaller(CALLING_PACKAGE_1, USER_0, () -> {
+            assertEquals(2, mManager.getRemainingCallCount()); // Still 2.
+        });
     }
 
     public void testThrottling_foreground() throws Exception {