Readability cleanup in AppPrediction comparator.
(i.e., in AppPredictionServiceResolverComparator)
This is believed to be a pure refactoring; no behavioral changes.
This is relatively simple compared to the ongoing cleanup of the
other comparator (e.g. in ag/17697713) because the AppPrediction
comparator already used simple immutable model data. This should
be more clear after this CL, which shows that the updated model
instance is derived from the sorted list we receive from the
service, along with immutable data that was provided when the
AppPredictionServiceResolverComparator was initialized.
The only parts that remain in the legacy ...ResolverComparator
code are responsible either for fetching the model data from the
remote service (which uses a somewhat sloppy/ad-hoc concurrency
model that should be cleaned up in a future CL as part of removing
the AbstractResolverComparator's Handler design), or for implementing
a fallback to use a ResolverRankerServiceResolverComparator if
there's a problem with the AppPredictionService (which should also
be cleaned up in a future CL, since the AppPredictionService
comparator really doesn't need to take on ownership responsibilities
for some completely different model).
Note that this CL also removes the bookkeeping of mTargetScores
(including the `checkAppTargetRankValid()` method) because it
wasn't used for anything.
Test: atest ResolverActivityTest ChooserActivityTest
Bug: 227486788
Change-Id: I419a73bf094c14276b0c5791edacaa27797b215c
diff --git a/core/java/com/android/internal/app/AppPredictionServiceResolverComparator.java b/core/java/com/android/internal/app/AppPredictionServiceResolverComparator.java
index b19ac2f..115a9d7 100644
--- a/core/java/com/android/internal/app/AppPredictionServiceResolverComparator.java
+++ b/core/java/com/android/internal/app/AppPredictionServiceResolverComparator.java
@@ -18,7 +18,6 @@
import static android.app.prediction.AppTargetEvent.ACTION_LAUNCH;
-import android.annotation.Nullable;
import android.app.prediction.AppPredictor;
import android.app.prediction.AppTarget;
import android.app.prediction.AppTargetEvent;
@@ -34,6 +33,7 @@
import com.android.internal.app.ResolverActivity.ResolvedComponentInfo;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
@@ -51,16 +51,17 @@
private final AppPredictor mAppPredictor;
private final Context mContext;
- private final Map<ComponentName, Integer> mTargetRanks = new HashMap<>();
- private final Map<ComponentName, Integer> mTargetScores = new HashMap<>();
private final UserHandle mUser;
private final Intent mIntent;
private final String mReferrerPackage;
+
+ private final ModelBuilder mModelBuilder;
+ private ResolverComparatorModel mComparatorModel;
+
// If this is non-null (and this is not destroyed), it means APS is disabled and we should fall
// back to using the ResolverRankerService.
// TODO: responsibility for this fallback behavior can live outside of the AppPrediction client.
private ResolverRankerServiceResolverComparator mResolverRankerService;
- private AppPredictionServiceComparatorModel mComparatorModel;
AppPredictionServiceResolverComparator(
Context context,
@@ -76,7 +77,20 @@
mUser = user;
mReferrerPackage = referrerPackage;
setChooserActivityLogger(chooserActivityLogger);
- mComparatorModel = buildUpdatedModel();
+
+ mModelBuilder = new ModelBuilder(appPredictor, user);
+ mComparatorModel = mModelBuilder.buildFromRankedList(Collections.emptyList());
+ }
+
+ @Override
+ void destroy() {
+ if (mResolverRankerService != null) {
+ mResolverRankerService.destroy();
+ mResolverRankerService = null;
+
+ // TODO: may not be necessary to build a new model, since we're destroying anyways.
+ mComparatorModel = mModelBuilder.buildFallbackModel(mResolverRankerService);
+ }
}
@Override
@@ -85,6 +99,29 @@
}
@Override
+ float getScore(ComponentName name) {
+ return mComparatorModel.getScore(name);
+ }
+
+ @Override
+ void updateModel(ComponentName componentName) {
+ mComparatorModel.notifyOnTargetSelected(componentName);
+ }
+
+ @Override
+ void handleResultMessage(Message msg) {
+ // Null value is okay if we have defaulted to the ResolverRankerService.
+ if (msg.what == RANKER_SERVICE_RESULT && msg.obj != null) {
+ // TODO: this probably never happens? The sorting callback circumvents the Handler
+ // design to call handleResult() directly instead of sending the list through a Message.
+ // (OK to leave as-is since the Handler design is going away soon.)
+ mComparatorModel = mModelBuilder.buildFromRankedList((List<AppTarget>) msg.obj);
+ } else if (msg.obj == null && mResolverRankerService == null) {
+ Log.e(TAG, "Unexpected null result");
+ }
+ }
+
+ @Override
void doCompute(List<ResolvedComponentInfo> targets) {
if (targets.isEmpty()) {
mHandler.sendEmptyMessage(RANKER_SERVICE_RESULT);
@@ -104,121 +141,113 @@
sortedAppTargets -> {
if (sortedAppTargets.isEmpty()) {
Log.i(TAG, "AppPredictionService disabled. Using resolver.");
- // APS for chooser is disabled. Fallback to resolver.
- mResolverRankerService =
- new ResolverRankerServiceResolverComparator(
- mContext, mIntent, mReferrerPackage,
- () -> mHandler.sendEmptyMessage(RANKER_SERVICE_RESULT),
- getChooserActivityLogger());
- mComparatorModel = buildUpdatedModel();
- mResolverRankerService.compute(targets);
+ setupFallbackModel(targets);
} else {
Log.i(TAG, "AppPredictionService response received");
// Skip sending to Handler which takes extra time to dispatch messages.
+ // TODO: the Handler guards some concurrency conditions, so this could
+ // probably result in a race (we're not currently on the Handler thread?).
+ // We'll leave this as-is since we intend to remove the Handler design
+ // shortly, but this is still an unsound shortcut.
handleResult(sortedAppTargets);
}
}
);
}
- @Override
- void handleResultMessage(Message msg) {
- // Null value is okay if we have defaulted to the ResolverRankerService.
- if (msg.what == RANKER_SERVICE_RESULT && msg.obj != null) {
- final List<AppTarget> sortedAppTargets = (List<AppTarget>) msg.obj;
- handleSortedAppTargets(sortedAppTargets);
- } else if (msg.obj == null && mResolverRankerService == null) {
- Log.e(TAG, "Unexpected null result");
- }
+ private void setupFallbackModel(List<ResolvedComponentInfo> targets) {
+ mResolverRankerService =
+ new ResolverRankerServiceResolverComparator(
+ mContext, mIntent, mReferrerPackage,
+ () -> mHandler.sendEmptyMessage(RANKER_SERVICE_RESULT),
+ getChooserActivityLogger());
+ mComparatorModel = mModelBuilder.buildFallbackModel(mResolverRankerService);
+ mResolverRankerService.compute(targets);
}
private void handleResult(List<AppTarget> sortedAppTargets) {
if (mHandler.hasMessages(RANKER_RESULT_TIMEOUT)) {
- handleSortedAppTargets(sortedAppTargets);
+ mComparatorModel = mModelBuilder.buildFromRankedList(sortedAppTargets);
mHandler.removeMessages(RANKER_RESULT_TIMEOUT);
afterCompute();
}
}
- private void handleSortedAppTargets(List<AppTarget> sortedAppTargets) {
- if (checkAppTargetRankValid(sortedAppTargets)) {
- sortedAppTargets.forEach(target -> mTargetScores.put(
- new ComponentName(target.getPackageName(), target.getClassName()),
- target.getRank()));
- }
- for (int i = 0; i < sortedAppTargets.size(); i++) {
- ComponentName componentName = new ComponentName(
- sortedAppTargets.get(i).getPackageName(),
- sortedAppTargets.get(i).getClassName());
- mTargetRanks.put(componentName, i);
- Log.i(TAG, "handleSortedAppTargets, sortedAppTargets #" + i + ": " + componentName);
- }
- mComparatorModel = buildUpdatedModel();
- }
+ static class ModelBuilder {
+ private final AppPredictor mAppPredictor;
+ private final UserHandle mUser;
- private boolean checkAppTargetRankValid(List<AppTarget> sortedAppTargets) {
- for (AppTarget target : sortedAppTargets) {
- if (target.getRank() != 0) {
- return true;
+ ModelBuilder(AppPredictor appPredictor, UserHandle user) {
+ mAppPredictor = appPredictor;
+ mUser = user;
+ }
+
+ ResolverComparatorModel buildFromRankedList(List<AppTarget> sortedAppTargets) {
+ return new AppPredictionServiceComparatorModel(
+ mAppPredictor, mUser, buildTargetRanksMapFromSortedTargets(sortedAppTargets));
+ }
+
+ ResolverComparatorModel buildFallbackModel(
+ ResolverRankerServiceResolverComparator fallback) {
+ return adaptLegacyResolverComparatorToComparatorModel(fallback);
+ }
+
+ // The remaining methods would be static if this weren't an inner class (i.e., they don't
+ // depend on any ivars, not even the ones in ModelBuilder).
+
+ private Map<ComponentName, Integer> buildTargetRanksMapFromSortedTargets(
+ List<AppTarget> sortedAppTargets) {
+ Map<ComponentName, Integer> targetRanks = new HashMap<>();
+ for (int i = 0; i < sortedAppTargets.size(); i++) {
+ ComponentName componentName = new ComponentName(
+ sortedAppTargets.get(i).getPackageName(),
+ sortedAppTargets.get(i).getClassName());
+ targetRanks.put(componentName, i);
+ Log.i(TAG, "handleSortedAppTargets, sortedAppTargets #" + i + ": " + componentName);
}
+ return targetRanks;
}
- return false;
- }
- @Override
- float getScore(ComponentName name) {
- return mComparatorModel.getScore(name);
- }
+ // TODO: when the refactoring is further along we'll probably have access to the
+ // comparator's new ResolverComparatorModel API, so we won't have to adapt from the legacy
+ // interface here. On the other hand, AppPredictionServiceResolverComparatorModel (or its
+ // replacement counterpart) shouldn't still be responsible for implementing the
+ // ResolverRankerService fallback logic at that time.
+ private ResolverComparatorModel adaptLegacyResolverComparatorToComparatorModel(
+ AbstractResolverComparator comparator) {
+ return new ResolverComparatorModel() {
+ @Override
+ public Comparator<ResolveInfo> getComparator() {
+ // Adapt the base type, which doesn't declare itself to be an implementation
+ // of {@code Comparator<ResolveInfo>} even though it has the one method.
+ return (lhs, rhs) -> comparator.compare(lhs, rhs);
+ }
- @Override
- void updateModel(ComponentName componentName) {
- mComparatorModel.notifyOnTargetSelected(componentName);
- }
+ @Override
+ public float getScore(ComponentName componentName) {
+ return comparator.getScore(componentName);
+ }
- @Override
- void destroy() {
- if (mResolverRankerService != null) {
- mResolverRankerService.destroy();
- mResolverRankerService = null;
- mComparatorModel = buildUpdatedModel();
+ @Override
+ public void notifyOnTargetSelected(ComponentName componentName) {
+ comparator.updateModel(componentName);
+ }
+ };
}
}
- /**
- * Re-construct an {@code AppPredictionServiceComparatorModel} to replace the current model
- * instance (if any) using the up-to-date {@code AppPredictionServiceResolverComparator} ivar
- * values.
- *
- * TODO: each time we replace the model instance, we're either updating the model to use
- * adjusted data (which is appropriate), or we're providing a (late) value for one of our ivars
- * that wasn't available the last time the model was updated. For those latter cases, we should
- * just avoid creating the model altogether until we have all the prerequisites we'll need. Then
- * we can probably simplify the logic in {@code AppPredictionServiceComparatorModel} since we
- * won't need to handle edge cases when the model data isn't fully prepared.
- * (In some cases, these kinds of "updates" might interleave -- e.g., we might have finished
- * initializing the first time and now want to adjust some data, but still need to wait for
- * changes to propagate to the other ivars before rebuilding the model.)
- */
- private AppPredictionServiceComparatorModel buildUpdatedModel() {
- return new AppPredictionServiceComparatorModel(
- mAppPredictor, mResolverRankerService, mUser, mTargetRanks);
- }
-
// TODO: Finish separating behaviors of AbstractResolverComparator, then (probably) make this a
// standalone class once clients are written in terms of ResolverComparatorModel.
static class AppPredictionServiceComparatorModel implements ResolverComparatorModel {
private final AppPredictor mAppPredictor;
- private final ResolverRankerServiceResolverComparator mResolverRankerService;
private final UserHandle mUser;
private final Map<ComponentName, Integer> mTargetRanks; // Treat as immutable.
AppPredictionServiceComparatorModel(
AppPredictor appPredictor,
- @Nullable ResolverRankerServiceResolverComparator resolverRankerService,
UserHandle user,
Map<ComponentName, Integer> targetRanks) {
mAppPredictor = appPredictor;
- mResolverRankerService = resolverRankerService;
mUser = user;
mTargetRanks = targetRanks;
}
@@ -226,9 +255,6 @@
@Override
public Comparator<ResolveInfo> getComparator() {
return (lhs, rhs) -> {
- if (mResolverRankerService != null) {
- return mResolverRankerService.compare(lhs, rhs);
- }
Integer lhsRank = mTargetRanks.get(new ComponentName(lhs.activityInfo.packageName,
lhs.activityInfo.name));
Integer rhsRank = mTargetRanks.get(new ComponentName(rhs.activityInfo.packageName,
@@ -246,9 +272,6 @@
@Override
public float getScore(ComponentName name) {
- if (mResolverRankerService != null) {
- return mResolverRankerService.getScore(name);
- }
Integer rank = mTargetRanks.get(name);
if (rank == null) {
Log.w(TAG, "Score requested for unknown component. Did you call compute yet?");
@@ -260,10 +283,6 @@
@Override
public void notifyOnTargetSelected(ComponentName componentName) {
- if (mResolverRankerService != null) {
- mResolverRankerService.updateModel(componentName);
- return;
- }
mAppPredictor.notifyAppTargetEvent(
new AppTargetEvent.Builder(
new AppTarget.Builder(
