Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Use more fine-grained lock when persisting shortcuts to disk" into tm-dev

Browse Source
This commit is contained in:
Pinyao Ting
2022-04-29 16:49:24 +00:00
committed by Android (Google) Code Review
parent 4a1fc38e11 9cb1e9a7c3
commit 80ed971262
7 changed files with 102 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
services/core/java/com/android/server/pm/ShortcutLauncher.java
View File

@@ -420,4 +420,14 @@ class ShortcutLauncher extends ShortcutPackageItem {
ArraySet<String> getAllPinnedShortcutsForTest(String packageName, int packageUserId) {
return new ArraySet<>(mPinnedShortcuts.get(PackageWithUser.of(packageUserId, packageName)));
}
@Override
protected File getShortcutPackageItemFile() {
final File path = new File(mShortcutUser.mService.injectUserDataPath(
mShortcutUser.getUserId()), ShortcutUser.DIRECTORY_LUANCHERS);
// Package user id and owner id can have different values for ShortcutLaunchers. Adding
// user Id to the file name to create a unique path. Owner id is used in the root path.
final String fileName = getPackageName() + getPackageUserId() + ".xml";
return new File(path, fileName);
}
}

38
services/core/java/com/android/server/pm/ShortcutPackage.java
View File

@@ -160,8 +160,6 @@ class ShortcutPackage extends ShortcutPackageItem {
private static final String KEY_BITMAPS = "bitmaps";
private static final String KEY_BITMAP_BYTES = "bitmapBytes";
private final Object mLock = new Object();
private final Executor mExecutor;
/**
@@ -779,7 +777,7 @@ class ShortcutPackage extends ShortcutPackageItem {
return false;
}
mApiCallCount++;
s.scheduleSaveUser(getOwnerUserId());
scheduleSave();
return true;
}
@@ -789,7 +787,7 @@ class ShortcutPackage extends ShortcutPackageItem {
}
if (mApiCallCount > 0) {
mApiCallCount = 0;
mShortcutUser.mService.scheduleSaveUser(getOwnerUserId());
scheduleSave();
}
}
@@ -1890,15 +1888,12 @@ class ShortcutPackage extends ShortcutPackageItem {
final ShortcutPackage ret = new ShortcutPackage(shortcutUser,
shortcutUser.getUserId(), packageName);
synchronized (ret.mLock) {
ret.mIsAppSearchSchemaUpToDate = ShortcutService.parseIntAttribute(
parser, ATTR_SCHEMA_VERSON, 0) == AppSearchShortcutInfo.SCHEMA_VERSION;
}
ret.mApiCallCount =
ShortcutService.parseIntAttribute(parser, ATTR_CALL_COUNT);
ret.mLastResetTime =
ShortcutService.parseLongAttribute(parser, ATTR_LAST_RESET);
ret.mApiCallCount = ShortcutService.parseIntAttribute(parser, ATTR_CALL_COUNT);
ret.mLastResetTime = ShortcutService.parseLongAttribute(parser, ATTR_LAST_RESET);
final int outerDepth = parser.getDepth();
@@ -2440,16 +2435,15 @@ class ShortcutPackage extends ShortcutPackageItem {
})));
}
void persistsAllShortcutsAsync() {
synchronized (mLock) {
final Map<String, ShortcutInfo> copy = mShortcuts;
if (!mTransientShortcuts.isEmpty()) {
copy.putAll(mTransientShortcuts);
mTransientShortcuts.clear();
}
saveShortcutsAsync(copy.values().stream().filter(ShortcutInfo::usesQuota).collect(
Collectors.toList()));
@Override
void scheduleSaveToAppSearchLocked() {
final Map<String, ShortcutInfo> copy = new ArrayMap<>(mShortcuts);
if (!mTransientShortcuts.isEmpty()) {
copy.putAll(mTransientShortcuts);
mTransientShortcuts.clear();
}
saveShortcutsAsync(copy.values().stream().filter(ShortcutInfo::usesQuota).collect(
Collectors.toList()));
}
private void saveShortcutsAsync(
@@ -2548,4 +2542,12 @@ class ShortcutPackage extends ShortcutPackageItem {
Binder.restoreCallingIdentity(callingIdentity);
}
}
@Override
protected File getShortcutPackageItemFile() {
final File path = new File(mShortcutUser.mService.injectUserDataPath(
mShortcutUser.getUserId()), ShortcutUser.DIRECTORY_PACKAGES);
final String fileName = getPackageName() + ".xml";
return new File(path, fileName);
}
}

49
services/core/java/com/android/server/pm/ShortcutPackageItem.java
View File

@@ -23,6 +23,7 @@ import android.util.Slog;
import android.util.TypedXmlSerializer;
import android.util.Xml;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.util.Preconditions;
import org.json.JSONException;
@@ -36,7 +37,7 @@ import java.nio.charset.StandardCharsets;
import java.util.Objects;
/**
* All methods should be guarded by {@code #mShortcutUser.mService.mLock}.
* All methods should be either guarded by {@code #mShortcutUser.mService.mLock} or {@code #mLock}.
*/
abstract class ShortcutPackageItem {
private static final String TAG = ShortcutService.TAG;
@@ -49,6 +50,8 @@ abstract class ShortcutPackageItem {
protected ShortcutUser mShortcutUser;
protected final Object mLock = new Object();
protected ShortcutPackageItem(@NonNull ShortcutUser shortcutUser,
int packageUserId, @NonNull String packageName,
@NonNull ShortcutPackageInfo packageInfo) {
@@ -98,7 +101,7 @@ abstract class ShortcutPackageItem {
}
final ShortcutService s = mShortcutUser.mService;
mPackageInfo.refreshSignature(s, this);
s.scheduleSaveUser(getOwnerUserId());
scheduleSave();
}
public void attemptToRestoreIfNeededAndSave() {
@@ -138,7 +141,7 @@ abstract class ShortcutPackageItem {
// Either way, it's no longer a shadow.
mPackageInfo.setShadow(false);
s.scheduleSaveUser(mPackageUserId);
scheduleSave();
}
protected abstract boolean canRestoreAnyVersion();
@@ -148,7 +151,8 @@ abstract class ShortcutPackageItem {
public abstract void saveToXml(@NonNull TypedXmlSerializer out, boolean forBackup)
throws IOException, XmlPullParserException;
public void saveToFile(File path, boolean forBackup) {
@GuardedBy("mLock")
public void saveToFileLocked(File path, boolean forBackup) {
final AtomicFile file = new AtomicFile(path);
FileOutputStream os = null;
try {
@@ -176,6 +180,11 @@ abstract class ShortcutPackageItem {
}
}
@GuardedBy("mLock")
void scheduleSaveToAppSearchLocked() {
}
public JSONObject dumpCheckin(boolean clear) throws JSONException {
final JSONObject result = new JSONObject();
result.put(KEY_NAME, mPackageName);
@@ -187,4 +196,36 @@ abstract class ShortcutPackageItem {
*/
public void verifyStates() {
}
public void scheduleSave() {
mShortcutUser.mService.injectPostToHandlerDebounced(
mSaveShortcutPackageRunner, mSaveShortcutPackageRunner);
}
private final Runnable mSaveShortcutPackageRunner = this::saveShortcutPackageItem;
void saveShortcutPackageItem() {
// Wait for bitmap saves to conclude before proceeding to saving shortcuts.
mShortcutUser.mService.waitForBitmapSaves();
// Save each ShortcutPackageItem in a separate Xml file.
final File path = getShortcutPackageItemFile();
if (ShortcutService.DEBUG || ShortcutService.DEBUG_REBOOT) {
Slog.d(TAG, "Saving package item " + getPackageName() + " to " + path);
}
synchronized (mLock) {
path.getParentFile().mkdirs();
// TODO: Since we are persisting shortcuts into AppSearch, we should read from/write to
// AppSearch as opposed to maintaining a separate XML file.
saveToFileLocked(path, false /*forBackup*/);
scheduleSaveToAppSearchLocked();
}
}
void removeShortcutPackageItem() {
synchronized (mLock) {
getShortcutPackageItemFile().delete();
}
}
protected abstract File getShortcutPackageItemFile();
}

32
services/core/java/com/android/server/pm/ShortcutService.java
View File

@@ -748,7 +748,7 @@ public class ShortcutService extends IShortcutService.Stub {
getUserShortcutsLocked(userId).cancelAllInFlightTasks();
// Save all dirty information.
saveDirtyInfo(false);
saveDirtyInfo();
// Unload
mUsers.delete(userId);
@@ -1203,10 +1203,6 @@ public class ShortcutService extends IShortcutService.Stub {
@VisibleForTesting
void saveDirtyInfo() {
saveDirtyInfo(true);
}
private void saveDirtyInfo(boolean saveShortcutsInAppSearch) {
if (DEBUG || DEBUG_REBOOT) {
Slog.d(TAG, "saveDirtyInfo");
}
@@ -1221,10 +1217,6 @@ public class ShortcutService extends IShortcutService.Stub {
if (userId == UserHandle.USER_NULL) { // USER_NULL for base state.
saveBaseStateLocked();
} else {
if (saveShortcutsInAppSearch) {
getUserShortcutsLocked(userId).forAllPackages(
ShortcutPackage::persistsAllShortcutsAsync);
}
saveUserLocked(userId);
}
}
@@ -1816,7 +1808,7 @@ public class ShortcutService extends IShortcutService.Stub {
}
injectPostToHandlerDebounced(sp, notifyListenerRunnable(packageName, userId));
notifyShortcutChangeCallbacks(packageName, userId, changedShortcuts, removedShortcuts);
scheduleSaveUser(userId);
sp.scheduleSave();
}
private void notifyListeners(@NonNull final String packageName, @UserIdInt final int userId) {
@@ -2878,12 +2870,11 @@ public class ShortcutService extends IShortcutService.Stub {
final ShortcutUser user = getUserShortcutsLocked(owningUserId);
boolean doNotify = false;
// First, remove the package from the package list (if the package is a publisher).
if (packageUserId == owningUserId) {
if (user.removePackage(packageName) != null) {
doNotify = true;
}
final ShortcutPackage sp = (packageUserId == owningUserId)
? user.removePackage(packageName) : null;
if (sp != null) {
doNotify = true;
}
// Also remove from the launcher list (if the package is a launcher).
@@ -2906,6 +2897,10 @@ public class ShortcutService extends IShortcutService.Stub {
// notifyListeners.
user.rescanPackageIfNeeded(packageName, /* forceRescan=*/ true);
}
if (!appStillExists && (packageUserId == owningUserId) && sp != null) {
// If the app is removed altogether, we can get rid of the xml as well
injectPostToHandler(() -> sp.removeShortcutPackageItem());
}
if (!wasUserLoaded) {
// Note this will execute the scheduled save.
@@ -3788,7 +3783,7 @@ public class ShortcutService extends IShortcutService.Stub {
if (mHandler.hasCallbacks(mSaveDirtyInfoRunner)) {
mHandler.removeCallbacks(mSaveDirtyInfoRunner);
forEachLoadedUserLocked(ShortcutUser::cancelAllInFlightTasks);
saveDirtyInfo(false);
saveDirtyInfo();
}
mShutdown.set(true);
}
@@ -4457,7 +4452,7 @@ public class ShortcutService extends IShortcutService.Stub {
// Save to the filesystem.
scheduleSaveUser(userId);
saveDirtyInfo(false);
saveDirtyInfo();
// Note, in case of backup, we don't have to wait on bitmap saving, because we don't
// back up bitmaps anyway.
@@ -5352,8 +5347,7 @@ public class ShortcutService extends IShortcutService.Stub {
}
}
@VisibleForTesting
void waitForBitmapSavesForTest() {
void waitForBitmapSaves() {
synchronized (mLock) {
mShortcutBitmapSaver.waitForAllSavesLocked();
}

27
services/core/java/com/android/server/pm/ShortcutUser.java
View File

@@ -407,35 +407,10 @@ class ShortcutUser {
}
spi.saveToXml(out, forBackup);
} else {
// Save each ShortcutPackageItem in a separate Xml file.
final File path = getShortcutPackageItemFile(spi);
if (ShortcutService.DEBUG || ShortcutService.DEBUG_REBOOT) {
Slog.d(TAG, "Saving package item " + spi.getPackageName() + " to " + path);
}
path.getParentFile().mkdirs();
spi.saveToFile(path, forBackup);
spi.saveShortcutPackageItem();
}
}
private File getShortcutPackageItemFile(ShortcutPackageItem spi) {
boolean isShortcutLauncher = spi instanceof ShortcutLauncher;
final File path = new File(mService.injectUserDataPath(mUserId),
isShortcutLauncher ? DIRECTORY_LUANCHERS : DIRECTORY_PACKAGES);
final String fileName;
if (isShortcutLauncher) {
// Package user id and owner id can have different values for ShortcutLaunchers. Adding
// user Id to the file name to create a unique path. Owner id is used in the root path.
fileName = spi.getPackageName() + spi.getPackageUserId() + ".xml";
} else {
fileName = spi.getPackageName() + ".xml";
}
return new File(path, fileName);
}
public static ShortcutUser loadFromXml(ShortcutService s, TypedXmlPullParser parser, int userId,
boolean fromBackup) throws IOException, XmlPullParserException, InvalidFileFormatException {
final ShortcutUser ret = new ShortcutUser(s, userId);

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

@@ -1975,7 +1975,7 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase {
if (si == null) {
return null;
}
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
return new File(si.getBitmapPath()).getName();
}
@@ -1984,7 +1984,7 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase {
if (si == null) {
return null;
}
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
return new File(si.getBitmapPath()).getAbsolutePath();
}
@@ -2139,7 +2139,7 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase {
}
protected boolean bitmapDirectoryExists(String packageName, int userId) {
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
final File path = new File(mService.getUserBitmapFilePath(userId), packageName);
return path.isDirectory();
}

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

@@ -1040,7 +1040,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
dumpsysOnLogcat();
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
// Check files and directories.
// Package 3 has no bitmaps, so we don't create a directory.
assertBitmapDirectories(USER_0, CALLING_PACKAGE_1, CALLING_PACKAGE_2);
@@ -1096,7 +1096,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
makeFile(mService.getUserBitmapFilePath(USER_10), CALLING_PACKAGE_2, "3").createNewFile();
makeFile(mService.getUserBitmapFilePath(USER_10), CALLING_PACKAGE_2, "4").createNewFile();
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
assertBitmapDirectories(USER_0, CALLING_PACKAGE_1, CALLING_PACKAGE_2, CALLING_PACKAGE_3,
"a.b.c", "d.e.f");
@@ -1111,7 +1111,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
// The below check is the same as above, except this time USER_0 use the CALLING_PACKAGE_3
// directory.
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
assertBitmapDirectories(USER_0, CALLING_PACKAGE_1, CALLING_PACKAGE_2, CALLING_PACKAGE_3);
assertBitmapDirectories(USER_10, CALLING_PACKAGE_1, CALLING_PACKAGE_2);
@@ -1390,7 +1390,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
.setIcon(Icon.createWithContentUri("test_uri"))
.build()
)));
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
assertWith(getCallerShortcuts())
.forShortcutWithId("s1", si -> {
assertTrue(si.hasIconUri());
@@ -1402,13 +1402,13 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
.setIcon(Icon.createWithResource(getTestContext(), R.drawable.black_32x32))
.build()
)));
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
assertWith(getCallerShortcuts())
.forShortcutWithId("s1", si -> {
assertTrue(si.hasIconResource());
assertEquals(R.drawable.black_32x32, si.getIconResourceId());
});
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
mInjectedCurrentTimeMillis += INTERVAL; // reset throttling
@@ -1419,7 +1419,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
getTestContext().getResources(), R.drawable.black_64x64)))
.build()
)));
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
assertWith(getCallerShortcuts())
.forShortcutWithId("s1", si -> {
assertTrue(si.hasIconFile());
@@ -1437,7 +1437,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
getTestContext().getResources(), R.drawable.black_64x64)))
.build()
)));
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
assertWith(getCallerShortcuts())
.forShortcutWithId("s1", si -> {
assertTrue(si.hasIconFile());
@@ -1451,7 +1451,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
.setIcon(Icon.createWithResource(getTestContext(), R.drawable.black_32x32))
.build()
)));
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
assertWith(getCallerShortcuts())
.forShortcutWithId("s1", si -> {
assertTrue(si.hasIconResource());
@@ -1463,7 +1463,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
.setIcon(Icon.createWithContentUri("test_uri"))
.build()
)));
mService.waitForBitmapSavesForTest();
mService.waitForBitmapSaves();
assertWith(getCallerShortcuts())
.forShortcutWithId("s1", si -> {
assertTrue(si.hasIconUri());