From 4d63dbda5103b7e3f6028a0d0541f7e8dc2bd35a Mon Sep 17 00:00:00 2001 From: Haoran Zhang Date: Tue, 7 Mar 2023 18:59:25 +0000 Subject: [PATCH] Adding experiment flags for optimization part of autofill for all apps For experiemnt, we added two flags: 1. Enable heuristic check on important views. This flag is default to false. If this flag is enabled, the imeAction check will also be applied on important for autofill views. We want to see if enabling this would decrease non-autofillable requests received by autofill providers, also check if there is any regressions. 2. Flag to indicate whether to have view type check on not important views. It's default to enabled. We wnat to see if disabling this would cause any regression. For potential bug line, we will be only checking package partially denied if package is not fully denied. In current logic, if package is fully denied, it will always be partially denied. Because whether it's fully denied is checked by if "package:;" is in denylist and partial denied is checked by if "package:" is in denylist. bug:265186343 Change-Id: I613d3a76af1252903cd5a5b12746e1ae8ad33615 --- core/java/android/view/View.java | 34 +++++++-- .../view/autofill/AutofillFeatureFlags.java | 38 ++++++++++ .../view/autofill/AutofillManager.java | 70 +++++++++++++++---- 3 files changed, 123 insertions(+), 19 deletions(-) diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java index ef76ce36ec0f8..800fc97d03a6e 100644 --- a/core/java/android/view/View.java +++ b/core/java/android/view/View.java @@ -163,6 +163,7 @@ import android.view.translation.ViewTranslationCallback; import android.view.translation.ViewTranslationRequest; import android.view.translation.ViewTranslationResponse; import android.widget.Checkable; +import android.widget.EditText; import android.widget.FrameLayout; import android.widget.ScrollBarDrawable; import android.window.OnBackInvokedDispatcher; @@ -10356,7 +10357,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback, final AutofillManager afm = getAutofillManager(); // keep default behavior if (afm == null) return false; - return afm.isMatchingAutofillableHeuristics(this); + return afm.isMatchingAutofillableHeuristicsForNotImportantViews(this); } private boolean isAutofillable() { @@ -10372,26 +10373,47 @@ public class View implements Drawable.Callback, KeyEvent.Callback, return false; } + // Experiment imeAction heuristic on important views. If the important view doesn't pass + // heuristic check, also check augmented autofill in case augmented autofill is enabled + // for the activity + // TODO: refactor to have both important views and not important views use the same + // heuristic check + if (isImportantForAutofill() + && afm.isTriggerFillRequestOnFilteredImportantViewsEnabled() + && this instanceof EditText + && !afm.isPassingImeActionCheck((EditText) this) + && !notifyAugmentedAutofillIfNeeded(afm)) { + // TODO: add a log to indicate what has filtered out the view + return false; + } + if (!isImportantForAutofill()) { // If view matches heuristics and is not denied, it will be treated same as view that's // important for autofill - if (afm.isMatchingAutofillableHeuristics(this) + if (afm.isMatchingAutofillableHeuristicsForNotImportantViews(this) && !afm.isActivityDeniedForAutofillForUnimportantView()) { return getAutofillViewId() > LAST_APP_AUTOFILL_ID; } // View is not important for "regular" autofill, so we must check if Augmented Autofill // is enabled for the activity - final AutofillOptions options = mContext.getAutofillOptions(); - if (options == null || !options.isAugmentedAutofillEnabled(mContext)) { + if (!notifyAugmentedAutofillIfNeeded(afm)){ return false; } - - afm.notifyViewEnteredForAugmentedAutofill(this); } return getAutofillViewId() > LAST_APP_AUTOFILL_ID; } + /** @hide **/ + public boolean notifyAugmentedAutofillIfNeeded(AutofillManager afm) { + final AutofillOptions options = mContext.getAutofillOptions(); + if (options == null || !options.isAugmentedAutofillEnabled(mContext)) { + return false; + } + afm.notifyViewEnteredForAugmentedAutofill(this); + return true; + } + /** @hide */ public boolean canNotifyAutofillEnterExitEvent() { return isAutofillable() && isAttachedToWindow(); diff --git a/core/java/android/view/autofill/AutofillFeatureFlags.java b/core/java/android/view/autofill/AutofillFeatureFlags.java index 6d78e60e93581..7da69e7d819b8 100644 --- a/core/java/android/view/autofill/AutofillFeatureFlags.java +++ b/core/java/android/view/autofill/AutofillFeatureFlags.java @@ -155,6 +155,14 @@ public class AutofillFeatureFlags { public static final String DEVICE_CONFIG_TRIGGER_FILL_REQUEST_ON_UNIMPORTANT_VIEW = "trigger_fill_request_on_unimportant_view"; + /** + * Whether to apply heuristic check on important views. + * + * @hide + */ + public static final String DEVICE_CONFIG_TRIGGER_FILL_REQUEST_ON_FILTERED_IMPORTANT_VIEWS = + "trigger_fill_request_on_filtered_important_views"; + /** * Continas imeAction ids that is irrelevant for autofill. For example, ime_action_search. We * use this to avoid trigger fill request on unimportant views. @@ -167,6 +175,14 @@ public class AutofillFeatureFlags { @SuppressLint("IntentName") public static final String DEVICE_CONFIG_NON_AUTOFILLABLE_IME_ACTION_IDS = "non_autofillable_ime_action_ids"; + + /** + * Whether to enable autofill on all view types (not just checkbox, spinner, datepicker etc...) + * + * @hide + */ + public static final String DEVICE_CONFIG_SHOULD_ENABLE_AUTOFILL_ON_ALL_VIEW_TYPES = + "should_enable_autofill_on_all_view_types"; // END AUTOFILL FOR ALL APPS FLAGS // @@ -314,6 +330,28 @@ public class AutofillFeatureFlags { DEVICE_CONFIG_TRIGGER_FILL_REQUEST_ON_UNIMPORTANT_VIEW, false); } + /** + * Whether to apply heuristic check on important views before triggering fill request + * + * @hide + */ + public static boolean isTriggerFillRequestOnFilteredImportantViewsEnabled() { + return DeviceConfig.getBoolean( + DeviceConfig.NAMESPACE_AUTOFILL, + DEVICE_CONFIG_TRIGGER_FILL_REQUEST_ON_FILTERED_IMPORTANT_VIEWS, false); + } + + /** + * Whether to enable autofill on all view types. + * + * @hide + */ + public static boolean shouldEnableAutofillOnAllViewTypes(){ + return DeviceConfig.getBoolean( + DeviceConfig.NAMESPACE_AUTOFILL, + DEVICE_CONFIG_SHOULD_ENABLE_AUTOFILL_ON_ALL_VIEW_TYPES, false); + } + /** * Get the non-autofillable ime actions from flag. This will be used in filtering * condition to trigger fill request. diff --git a/core/java/android/view/autofill/AutofillManager.java b/core/java/android/view/autofill/AutofillManager.java index 14c781bb5560e..0deaa76363106 100644 --- a/core/java/android/view/autofill/AutofillManager.java +++ b/core/java/android/view/autofill/AutofillManager.java @@ -674,6 +674,12 @@ public final class AutofillManager { // Indicate whether trigger fill request on unimportant views is enabled private boolean mIsTriggerFillRequestOnUnimportantViewEnabled = false; + // Indicate whether to apply heuristic check on important views before trigger fill request + private boolean mIsTriggerFillRequestOnFilteredImportantViewsEnabled; + + // Indicate whether to enable autofill for all view types + private boolean mShouldEnableAutofillOnAllViewTypes; + // A set containing all non-autofillable ime actions passed by flag private Set mNonAutofillableImeActionIdSet = new ArraySet<>(); @@ -855,6 +861,12 @@ public final class AutofillManager { mIsTriggerFillRequestOnUnimportantViewEnabled = AutofillFeatureFlags.isTriggerFillRequestOnUnimportantViewEnabled(); + mIsTriggerFillRequestOnFilteredImportantViewsEnabled = + AutofillFeatureFlags.isTriggerFillRequestOnFilteredImportantViewsEnabled(); + + mShouldEnableAutofillOnAllViewTypes = + AutofillFeatureFlags.shouldEnableAutofillOnAllViewTypes(); + mNonAutofillableImeActionIdSet = AutofillFeatureFlags.getNonAutofillableImeActionIdSetFromFlag(); @@ -865,14 +877,39 @@ public final class AutofillManager { mIsPackageFullyDeniedForAutofillForUnimportantView = isPackageFullyDeniedForAutofillForUnimportantView(denyListString, packageName); - mIsPackagePartiallyDeniedForAutofillForUnimportantView = - isPackagePartiallyDeniedForAutofillForUnimportantView(denyListString, packageName); + if (!mIsPackageFullyDeniedForAutofillForUnimportantView) { + mIsPackagePartiallyDeniedForAutofillForUnimportantView = + isPackagePartiallyDeniedForAutofillForUnimportantView(denyListString, packageName); + } if (mIsPackagePartiallyDeniedForAutofillForUnimportantView) { setDeniedActivitySetWithDenyList(denyListString, packageName); } } + /** + * Whether to apply heuristic check on important views before triggering fill request + * + * @hide + */ + public boolean isTriggerFillRequestOnFilteredImportantViewsEnabled() { + return mIsTriggerFillRequestOnFilteredImportantViewsEnabled; + } + + /** + * Whether view passes the imeAction check + * + * @hide + */ + public boolean isPassingImeActionCheck(EditText editText) { + final int actionId = editText.getImeOptions(); + if (mNonAutofillableImeActionIdSet.contains(String.valueOf(actionId))) { + // TODO: add a log to indicate what has filtered out the view + return false; + } + return true; + } + private boolean isPackageFullyDeniedForAutofillForUnimportantView( @NonNull String denyListString, @NonNull String packageName) { // If "PackageName:;" is in the string, then it means the package name is in denylist @@ -957,25 +994,32 @@ public final class AutofillManager { * * @hide */ - public final boolean isMatchingAutofillableHeuristics(@NonNull View view) { + public final boolean isMatchingAutofillableHeuristicsForNotImportantViews(@NonNull View view) { if (!mIsTriggerFillRequestOnUnimportantViewEnabled) { return false; } + + // TODO: remove the autofill type check when this function is applied on both important and + // not important views. + // This check is needed here because once the view type check is lifted, addiditional + // unimportant views will be added to the assist structure which may cuase system health + // regression (viewGroup#populateChidlrenForAutofill() calls this function to decide whether + // to include child view) + if (view.getAutofillType() == View.AUTOFILL_TYPE_NONE) return false; + if (view instanceof EditText) { - final int actionId = ((EditText) view).getImeOptions(); - if (mNonAutofillableImeActionIdSet.contains(String.valueOf(actionId))) { - return false; - } - return true; + return isPassingImeActionCheck((EditText) view); } + if (view instanceof CheckBox - || view instanceof Spinner - || view instanceof DatePicker - || view instanceof TimePicker - || view instanceof RadioGroup) { + || view instanceof Spinner + || view instanceof DatePicker + || view instanceof TimePicker + || view instanceof RadioGroup) { return true; } - return false; + + return mShouldEnableAutofillOnAllViewTypes; }