Handle locale change and pacakge change in different way
- 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
diff --git a/core/java/android/content/pm/ShortcutServiceInternal.java b/core/java/android/content/pm/ShortcutServiceInternal.java
index 3f8bad1..f994d7e 100644
--- a/core/java/android/content/pm/ShortcutServiceInternal.java
+++ b/core/java/android/content/pm/ShortcutServiceInternal.java
@@ -67,10 +67,4 @@
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();
}
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 4762a67..1c86d7c 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -140,7 +140,6 @@
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 @@
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) {
diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java
index b94d0f0..3c18198 100644
--- a/services/core/java/com/android/server/pm/ShortcutPackage.java
+++ b/services/core/java/com/android/server/pm/ShortcutPackage.java
@@ -34,6 +34,7 @@
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 @@
* locale changes.
*/
public int getApiCallCount() {
- mShortcutUser.resetThrottlingIfNeeded();
-
final ShortcutService s = mShortcutUser.mService;
// Reset the counter if:
@@ -598,7 +597,37 @@
}
/**
- * 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 @@
* 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 @@
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 @@
}
}
if (changed) {
- s.scheduleSaveUser(getPackageUserId());
+ s.packageShortcutsChanged(getPackageName(), getPackageUserId());
}
}
diff --git a/services/core/java/com/android/server/pm/ShortcutPackageItem.java b/services/core/java/com/android/server/pm/ShortcutPackageItem.java
index 757dd19..26b52e9 100644
--- a/services/core/java/com/android/server/pm/ShortcutPackageItem.java
+++ b/services/core/java/com/android/server/pm/ShortcutPackageItem.java
@@ -48,6 +48,10 @@
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
diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java
index 5f8cbbf..e2f656e 100644
--- a/services/core/java/com/android/server/pm/ShortcutService.java
+++ b/services/core/java/com/android/server/pm/ShortcutService.java
@@ -24,9 +24,11 @@
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.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.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 @@
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 @@
@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 @@
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 @@
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 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 @@
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 @@
// 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 @@
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 @@
synchronized (mLock) {
final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+ ps.getUser().onCalledByPublisher(packageName);
ps.ensureImmutableShortcutsNotIncluded(newShortcuts);
@@ -1546,6 +1554,7 @@
synchronized (mLock) {
final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+ ps.getUser().onCalledByPublisher(packageName);
ps.ensureImmutableShortcutsNotIncluded(newShortcuts);
@@ -1624,6 +1633,7 @@
synchronized (mLock) {
final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+ ps.getUser().onCalledByPublisher(packageName);
ps.ensureImmutableShortcutsNotIncluded(newShortcuts);
@@ -1671,6 +1681,7 @@
synchronized (mLock) {
final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+ ps.getUser().onCalledByPublisher(packageName);
ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds);
@@ -1698,6 +1709,7 @@
synchronized (mLock) {
final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+ ps.getUser().onCalledByPublisher(packageName);
ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds);
@@ -1718,6 +1730,7 @@
synchronized (mLock) {
final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
+ ps.getUser().onCalledByPublisher(packageName);
ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds);
@@ -1739,7 +1752,9 @@
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 @@
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 @@
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 @@
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 @@
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 @@
@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 @@
final long token = injectClearCallingIdentity();
try {
- forEachLoadedUserLocked(u -> u.forAllPackages(p -> p.resolveResourceStrings()));
+ forEachLoadedUserLocked(user -> user.detectLocaleChange());
} finally {
injectRestoreCallingIdentity(token);
}
@@ -2327,53 +2330,65 @@
* 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);
+
+ if (!mUserManager.isUserUnlocked(userId)) {
+ if (DEBUG) {
+ Slog.d(TAG, "Ignoring package broadcast " + action
+ + " for locked/stopped user " + userId);
+ }
+ return;
+ }
+
+ 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;
+ }
+
+ final boolean replacing = intent.getBooleanExtra(Intent.EXTRA_REPLACING, false);
+
+ 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);
+
+ break;
+ case Intent.ACTION_PACKAGE_DATA_CLEARED:
+ handlePackageDataCleared(packageName, userId);
+ break;
+ }
} finally {
injectRestoreCallingIdentity(token);
}
}
-
- @Override
- public void onPackageAdded(String packageName, int uid) {
- if (!isUserUnlocked()) return;
- handlePackageAdded(packageName, getChangingUserId());
- }
-
- @Override
- public void onPackageUpdateFinished(String packageName, int uid) {
- if (!isUserUnlocked()) return;
- handlePackageUpdateFinished(packageName, getChangingUserId());
- }
-
- @Override
- public void onPackageRemoved(String packageName, int uid) {
- if (!isUserUnlocked()) return;
- handlePackageRemoved(packageName, getChangingUserId());
- }
-
- @Override
- public void onPackageDataCleared(String packageName, int uid) {
- if (!isUserUnlocked()) return;
- handlePackageDataCleared(packageName, getChangingUserId());
- }
-
- @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.
- }
};
/**
@@ -2423,7 +2438,7 @@
// 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 @@
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 @@
user.attemptToRestoreIfNeededAndSave(this, packageName, userId);
if (isPackageInstalled(packageName, userId)) {
- user.handlePackageAddedOrUpdated(packageName, /* forceRescan=*/ false);
+ user.rescanPackageIfNeeded(packageName, /* forceRescan=*/ false);
}
}
verifyStates();
@@ -2495,7 +2510,7 @@
synchronized (mLock) {
final ShortcutUser user = getUserShortcutsLocked(packageUserId);
- user.handlePackageAddedOrUpdated(packageName, /* forceRescan=*/ true);
+ user.rescanPackageIfNeeded(packageName, /* forceRescan=*/ true);
}
verifyStates();
@@ -2971,10 +2986,6 @@
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 @@
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();
diff --git a/services/core/java/com/android/server/pm/ShortcutUser.java b/services/core/java/com/android/server/pm/ShortcutUser.java
index 7ea89c9..9649641 100644
--- a/services/core/java/com/android/server/pm/ShortcutUser.java
+++ b/services/core/java/com/android/server/pm/ShortcutUser.java
@@ -19,6 +19,8 @@
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 @@
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 @@
/** 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 @@
}
/**
- * 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());
-
- mService.scheduleSaveUser(mUserId);
- }
+ public void onCalledByPublisher(@NonNull String packageName) {
+ detectLocaleChange();
+ rescanPackageIfNeeded(packageName, /*forceRescan=*/ false);
}
- public void handlePackageAddedOrUpdated(@NonNull String packageName, boolean forceRescan) {
+ 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 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 @@
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 @@
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 @@
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("] ");
diff --git a/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java
index 1be57bc..2652b8f 100644
--- a/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java
@@ -110,13 +110,14 @@
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 @@
// ignore.
return null;
}
+
+ @Override
+ public void unregisterReceiver(BroadcastReceiver receiver) {
+ // ignore.
+ }
}
/** Context used in the client side */
@@ -212,6 +218,11 @@
}
@Override
+ public String injectGetLocaleTagsForUser(@UserIdInt int userId) {
+ return mInjectedLocale.toLanguageTag();
+ }
+
+ @Override
boolean injectShouldPerformVerification() {
return true; // Always verify during unit tests.
}
@@ -715,11 +726,6 @@
// 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 @@
// 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);
}
diff --git a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java
index bf6c2ff..d563185 100644
--- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java
+++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java
@@ -5730,21 +5730,24 @@
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 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 @@
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.
diff --git a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java
index f570ff2..d1caa4e 100644
--- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java
+++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java
@@ -47,6 +47,8 @@
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 @@
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 @@
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 {
diff --git a/services/tests/shortcutmanagerutils/src/com/android/server/pm/shortcutmanagertest/ShortcutManagerTestUtils.java b/services/tests/shortcutmanagerutils/src/com/android/server/pm/shortcutmanagertest/ShortcutManagerTestUtils.java
index f89c4e4..2f64ad7 100644
--- a/services/tests/shortcutmanagerutils/src/com/android/server/pm/shortcutmanagertest/ShortcutManagerTestUtils.java
+++ b/services/tests/shortcutmanagerutils/src/com/android/server/pm/shortcutmanagertest/ShortcutManagerTestUtils.java
@@ -973,6 +973,8 @@
waitOnMainThread();
+ launcherApps.unregisterCallback(asserter.getMockCallback());
+
return asserter;
}
}
