diff options
4 files changed, 56 insertions, 27 deletions
diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index d5767b4cad8a..c1fc7f114c67 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -878,6 +878,9 @@ public class ShortcutService extends IShortcutService.Stub { saveUserInternalLocked(userId, os, /* forBackup= */ false); file.finishWrite(os); + + // Remove all dangling bitmap files. + cleanupDanglingBitmapDirectoriesLocked(userId); } catch (XmlPullParserException | IOException e) { Slog.e(TAG, "Failed to write to file " + file.getBaseFile(), e); file.failWrite(os); @@ -929,7 +932,6 @@ public class ShortcutService extends IShortcutService.Stub { } try { final ShortcutUser ret = loadUserInternal(userId, in, /* forBackup= */ false); - cleanupDanglingBitmapDirectoriesLocked(userId, ret); return ret; } catch (IOException | XmlPullParserException e) { Slog.e(TAG, "Failed to read file " + file.getBaseFile(), e); @@ -1062,9 +1064,22 @@ public class ShortcutService extends IShortcutService.Stub { } } - // Requires mLock held, but "Locked" prefix would look weired so we jsut say "L". + // Requires mLock held, but "Locked" prefix would look weired so we just say "L". protected boolean isUserUnlockedL(@UserIdInt int userId) { - return mUnlockedUsers.get(userId); + // First, check the local copy. + if (mUnlockedUsers.get(userId)) { + return true; + } + // If the local copy says the user is locked, check with AM for the actual state, since + // the user might just have been unlocked. + // Note we just don't use isUserUnlockingOrUnlocked() here, because it'll return false + // when the user is STOPPING, which we still want to consider as "unlocked". + final long token = injectClearCallingIdentity(); + try { + return mUserManager.isUserUnlockingOrUnlocked(userId); + } finally { + injectRestoreCallingIdentity(token); + } } // Requires mLock held, but "Locked" prefix would look weired so we jsut say "L". @@ -1125,14 +1140,8 @@ public class ShortcutService extends IShortcutService.Stub { // === Caller validation === void removeIcon(@UserIdInt int userId, ShortcutInfo shortcut) { - if (shortcut.getBitmapPath() != null) { - if (DEBUG) { - Slog.d(TAG, "Removing " + shortcut.getBitmapPath()); - } - new File(shortcut.getBitmapPath()).delete(); - - shortcut.setBitmapPath(null); - } + // Do not remove the actual bitmap file yet, because if the device crashes before saving + // he XML we'd lose the icon. We just remove all dangling files after saving the XML. shortcut.setIconResourceId(0); shortcut.setIconResName(null); shortcut.clearFlags(ShortcutInfo.FLAG_HAS_ICON_FILE | ShortcutInfo.FLAG_HAS_ICON_RES); @@ -1148,13 +1157,14 @@ public class ShortcutService extends IShortcutService.Stub { } } - private void cleanupDanglingBitmapDirectoriesLocked( - @UserIdInt int userId, @NonNull ShortcutUser user) { + private void cleanupDanglingBitmapDirectoriesLocked(@UserIdInt int userId) { if (DEBUG) { Slog.d(TAG, "cleanupDanglingBitmaps: userId=" + userId); } final long start = injectElapsedRealtime(); + final ShortcutUser user = getUserShortcutsLocked(userId); + final File bitmapDir = getUserBitmapFilePath(userId); final File[] children = bitmapDir.listFiles(); if (children == null) { 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 e7f33457bb5f..1c7a138468cf 100644 --- a/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java @@ -247,20 +247,6 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { } @Override - protected boolean isUserUnlockedL(@UserIdInt int userId) { - // Note due to a late change, now ShortcutManager doesn't use - // UserManager.isUserUnlockingOrUnlocked(). But all unit tests are still using it, - // so we convert here. - - final long token = injectClearCallingIdentity(); - try { - return mMockUserManager.isUserUnlockingOrUnlocked(userId); - } finally { - injectRestoreCallingIdentity(token); - } - } - - @Override int injectDipToPixel(int dip) { return dip; } 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 75a34272d82b..253334eec9cf 100644 --- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java +++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java @@ -5863,6 +5863,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { assertEmpty(mManager.getPinnedShortcuts()); }); // Send add broadcast, but the user is not running, so should be ignored. + mService.handleCleanupUser(USER_10); mRunningUsers.put(USER_10, false); mUnlockedUsers.put(USER_10, false); 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 ad5bc7730833..d25923c019ca 100644 --- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java +++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java @@ -2005,4 +2005,36 @@ public class ShortcutManagerTest2 extends BaseShortcutManagerTest { }); }); } + + public void testIsUserUnlocked() { + mRunningUsers.clear(); + mUnlockedUsers.clear(); + + assertFalse(mService.isUserUnlockedL(USER_0)); + assertFalse(mService.isUserUnlockedL(USER_10)); + + // Start user 0, still locked. + mRunningUsers.put(USER_0, true); + assertFalse(mService.isUserUnlockedL(USER_0)); + assertFalse(mService.isUserUnlockedL(USER_10)); + + // Unlock user. + mUnlockedUsers.put(USER_0, true); + assertTrue(mService.isUserUnlockedL(USER_0)); + assertFalse(mService.isUserUnlockedL(USER_10)); + + // Clear again. + mRunningUsers.clear(); + mUnlockedUsers.clear(); + + // Directly call the lifecycle event. Now also locked. + mService.handleUnlockUser(USER_0); + assertTrue(mService.isUserUnlockedL(USER_0)); + assertFalse(mService.isUserUnlockedL(USER_10)); + + // Directly call the stop lifecycle event. Goes back to the initial state. + mService.handleCleanupUser(USER_0); + assertFalse(mService.isUserUnlockedL(USER_0)); + assertFalse(mService.isUserUnlockedL(USER_10)); + } } |