From 31ab19f71db752b54861719cedb5c23737bfdeb4 Mon Sep 17 00:00:00 2001 From: Sunny Goyal Date: Wed, 17 Jul 2019 14:30:04 -0700 Subject: [PATCH] Removing global state from DeepShortcutManager This makes DeepShortcutManager thread-safe and avoids any locks Change-Id: If4593b3541da6259591ff7a607efa58158006481 --- .../shortcuts/DeepShortcutManager.java | 53 ++++++------- .../android/launcher3/model/LoaderTask.java | 4 +- .../model/UserLockStateChangedTask.java | 5 +- .../shortcuts/DeepShortcutManager.java | 76 +++++++++---------- 4 files changed, 66 insertions(+), 72 deletions(-) diff --git a/go/src/com/android/launcher3/shortcuts/DeepShortcutManager.java b/go/src/com/android/launcher3/shortcuts/DeepShortcutManager.java index 1e449108d8..c20ed129ce 100644 --- a/go/src/com/android/launcher3/shortcuts/DeepShortcutManager.java +++ b/go/src/com/android/launcher3/shortcuts/DeepShortcutManager.java @@ -26,57 +26,46 @@ import android.os.UserHandle; import com.android.launcher3.ItemInfo; -import java.util.Collections; +import java.util.ArrayList; import java.util.List; /** * Performs operations related to deep shortcuts, such as querying for them, pinning them, etc. */ public class DeepShortcutManager { - private static DeepShortcutManager sInstance; - private static final Object sInstanceLock = new Object(); + + private static final DeepShortcutManager sInstance = new DeepShortcutManager(); public static DeepShortcutManager getInstance(Context context) { - synchronized (sInstanceLock) { - if (sInstance == null) { - sInstance = new DeepShortcutManager(context.getApplicationContext()); - } - return sInstance; - } + return sInstance; } - private DeepShortcutManager(Context context) { - } + private final QueryResult mFailure = new QueryResult(); + + private DeepShortcutManager() { } public static boolean supportsShortcuts(ItemInfo info) { return false; } - public boolean wasLastCallSuccess() { - return false; - } - - public void onShortcutsChanged(List shortcuts) { - } - /** * Queries for the shortcuts with the package name and provided ids. * * This method is intended to get the full details for shortcuts when they are added or updated, * because we only get "key" fields in onShortcutsChanged(). */ - public List queryForFullDetails(String packageName, + public QueryResult queryForFullDetails(String packageName, List shortcutIds, UserHandle user) { - return Collections.emptyList(); + return mFailure; } /** * Gets all the manifest and dynamic shortcuts associated with the given package and user, * to be displayed in the shortcuts container on long press. */ - public List queryForShortcutsContainer(ComponentName activity, + public QueryResult queryForShortcutsContainer(ComponentName activity, UserHandle user) { - return Collections.emptyList(); + return mFailure; } /** @@ -106,20 +95,28 @@ public class DeepShortcutManager { * * If packageName is null, returns all pinned shortcuts regardless of package. */ - public List queryForPinnedShortcuts(String packageName, UserHandle user) { - return Collections.emptyList(); + public QueryResult queryForPinnedShortcuts(String packageName, UserHandle user) { + return mFailure; } - public List queryForPinnedShortcuts(String packageName, + public QueryResult queryForPinnedShortcuts(String packageName, List shortcutIds, UserHandle user) { - return Collections.emptyList(); + return mFailure; } - public List queryForAllShortcuts(UserHandle user) { - return Collections.emptyList(); + public QueryResult queryForAllShortcuts(UserHandle user) { + return mFailure; } public boolean hasHostPermission() { return false; } + + + public static class QueryResult extends ArrayList { + + public boolean wasSuccess() { + return true; + } + } } diff --git a/src/com/android/launcher3/model/LoaderTask.java b/src/com/android/launcher3/model/LoaderTask.java index 0138572d0c..9cab4b0423 100644 --- a/src/com/android/launcher3/model/LoaderTask.java +++ b/src/com/android/launcher3/model/LoaderTask.java @@ -317,9 +317,9 @@ public class LoaderTask implements Runnable { // We can only query for shortcuts when the user is unlocked. if (userUnlocked) { - List pinnedShortcuts = + DeepShortcutManager.QueryResult pinnedShortcuts = mShortcutManager.queryForPinnedShortcuts(null, user); - if (mShortcutManager.wasLastCallSuccess()) { + if (pinnedShortcuts.wasSuccess()) { for (ShortcutInfo shortcut : pinnedShortcuts) { shortcutKeyToPinnedShortcuts.put(ShortcutKey.fromInfo(shortcut), shortcut); diff --git a/src/com/android/launcher3/model/UserLockStateChangedTask.java b/src/com/android/launcher3/model/UserLockStateChangedTask.java index 2cb256e092..50fff2653d 100644 --- a/src/com/android/launcher3/model/UserLockStateChangedTask.java +++ b/src/com/android/launcher3/model/UserLockStateChangedTask.java @@ -37,7 +37,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; -import java.util.List; /** * Task to handle changing of lock state of the user @@ -58,9 +57,9 @@ public class UserLockStateChangedTask extends BaseModelUpdateTask { HashMap pinnedShortcuts = new HashMap<>(); if (isUserUnlocked) { - List shortcuts = + DeepShortcutManager.QueryResult shortcuts = deepShortcutManager.queryForPinnedShortcuts(null, mUser); - if (deepShortcutManager.wasLastCallSuccess()) { + if (shortcuts.wasSuccess()) { for (ShortcutInfo shortcut : shortcuts) { pinnedShortcuts.put(ShortcutKey.fromInfo(shortcut), shortcut); } diff --git a/src_shortcuts_overrides/com/android/launcher3/shortcuts/DeepShortcutManager.java b/src_shortcuts_overrides/com/android/launcher3/shortcuts/DeepShortcutManager.java index 6b6f70d7b6..84a59b2280 100644 --- a/src_shortcuts_overrides/com/android/launcher3/shortcuts/DeepShortcutManager.java +++ b/src_shortcuts_overrides/com/android/launcher3/shortcuts/DeepShortcutManager.java @@ -45,19 +45,15 @@ public class DeepShortcutManager { | ShortcutQuery.FLAG_MATCH_MANIFEST | ShortcutQuery.FLAG_MATCH_PINNED; private static DeepShortcutManager sInstance; - private static final Object sInstanceLock = new Object(); public static DeepShortcutManager getInstance(Context context) { - synchronized (sInstanceLock) { - if (sInstance == null) { - sInstance = new DeepShortcutManager(context.getApplicationContext()); - } - return sInstance; + if (sInstance == null) { + sInstance = new DeepShortcutManager(context.getApplicationContext()); } + return sInstance; } private final LauncherApps mLauncherApps; - private boolean mWasLastCallSuccess; private DeepShortcutManager(Context context) { mLauncherApps = (LauncherApps) context.getSystemService(Context.LAUNCHER_APPS_SERVICE); @@ -70,17 +66,13 @@ public class DeepShortcutManager { && !info.isDisabled() && !isItemPromise; } - public boolean wasLastCallSuccess() { - return mWasLastCallSuccess; - } - /** * Queries for the shortcuts with the package name and provided ids. * * This method is intended to get the full details for shortcuts when they are added or updated, * because we only get "key" fields in onShortcutsChanged(). */ - public List queryForFullDetails(String packageName, + public QueryResult queryForFullDetails(String packageName, List shortcutIds, UserHandle user) { return query(FLAG_GET_ALL, packageName, null, shortcutIds, user); } @@ -89,7 +81,7 @@ public class DeepShortcutManager { * Gets all the manifest and dynamic shortcuts associated with the given package and user, * to be displayed in the shortcuts container on long press. */ - public List queryForShortcutsContainer(ComponentName activity, + public QueryResult queryForShortcutsContainer(ComponentName activity, UserHandle user) { return query(ShortcutQuery.FLAG_MATCH_MANIFEST | ShortcutQuery.FLAG_MATCH_DYNAMIC, activity.getPackageName(), activity, null, user); @@ -107,10 +99,8 @@ public class DeepShortcutManager { pinnedIds.remove(id); try { mLauncherApps.pinShortcuts(packageName, pinnedIds, user); - mWasLastCallSuccess = true; } catch (SecurityException|IllegalStateException e) { Log.w(TAG, "Failed to unpin shortcut", e); - mWasLastCallSuccess = false; } } @@ -126,10 +116,8 @@ public class DeepShortcutManager { pinnedIds.add(id); try { mLauncherApps.pinShortcuts(packageName, pinnedIds, user); - mWasLastCallSuccess = true; } catch (SecurityException|IllegalStateException e) { Log.w(TAG, "Failed to pin shortcut", e); - mWasLastCallSuccess = false; } } @@ -138,23 +126,18 @@ public class DeepShortcutManager { try { mLauncherApps.startShortcut(packageName, id, sourceBounds, startActivityOptions, user); - mWasLastCallSuccess = true; } catch (SecurityException|IllegalStateException e) { Log.e(TAG, "Failed to start shortcut", e); - mWasLastCallSuccess = false; } } public Drawable getShortcutIconDrawable(ShortcutInfo shortcutInfo, int density) { try { - Drawable icon = mLauncherApps.getShortcutIconDrawable(shortcutInfo, density); - mWasLastCallSuccess = true; - return icon; + return mLauncherApps.getShortcutIconDrawable(shortcutInfo, density); } catch (SecurityException|IllegalStateException e) { Log.e(TAG, "Failed to get shortcut icon", e); - mWasLastCallSuccess = false; + return null; } - return null; } /** @@ -162,20 +145,20 @@ public class DeepShortcutManager { * * If packageName is null, returns all pinned shortcuts regardless of package. */ - public List queryForPinnedShortcuts(String packageName, UserHandle user) { + public QueryResult queryForPinnedShortcuts(String packageName, UserHandle user) { return queryForPinnedShortcuts(packageName, null, user); } - public List queryForPinnedShortcuts(String packageName, - List shortcutIds, UserHandle user) { + public QueryResult queryForPinnedShortcuts(String packageName, List shortcutIds, + UserHandle user) { return query(ShortcutQuery.FLAG_MATCH_PINNED, packageName, null, shortcutIds, user); } - public List queryForAllShortcuts(UserHandle user) { + public QueryResult queryForAllShortcuts(UserHandle user) { return query(FLAG_GET_ALL, null, null, null, user); } - private List extractIds(List shortcuts) { + private static List extractIds(List shortcuts) { List shortcutIds = new ArrayList<>(shortcuts.size()); for (ShortcutInfo shortcut : shortcuts) { shortcutIds.add(shortcut.getId()); @@ -189,8 +172,8 @@ public class DeepShortcutManager { * * TODO: Use the cache to optimize this so we don't make an RPC every time. */ - private List query(int flags, String packageName, - ComponentName activity, List shortcutIds, UserHandle user) { + private QueryResult query(int flags, String packageName, ComponentName activity, + List shortcutIds, UserHandle user) { ShortcutQuery q = new ShortcutQuery(); q.setQueryFlags(flags); if (packageName != null) { @@ -198,18 +181,12 @@ public class DeepShortcutManager { q.setActivity(activity); q.setShortcutIds(shortcutIds); } - List shortcutInfos = null; try { - shortcutInfos = mLauncherApps.getShortcuts(q, user); - mWasLastCallSuccess = true; + return new QueryResult(mLauncherApps.getShortcuts(q, user)); } catch (SecurityException|IllegalStateException e) { Log.e(TAG, "Failed to query for shortcuts", e); - mWasLastCallSuccess = false; + return QueryResult.FAILURE; } - if (shortcutInfos == null) { - return Collections.EMPTY_LIST; - } - return shortcutInfos; } public boolean hasHostPermission() { @@ -220,4 +197,25 @@ public class DeepShortcutManager { } return false; } + + public static class QueryResult extends ArrayList { + + static QueryResult FAILURE = new QueryResult(); + + private final boolean mWasSuccess; + + QueryResult(List result) { + super(result == null ? Collections.emptyList() : result); + mWasSuccess = true; + } + + QueryResult() { + mWasSuccess = false; + } + + + public boolean wasSuccess() { + return mWasSuccess; + } + } }