Merge "Autofill For All Apps - refactor to have important and not impotant views call the same isAutofillable() function in AutofillManager" into udc-dev

3 files changed, 73 insertions, 63 deletions

diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java
index aec3487910d8..6cd894113ca6 100644
--- a/core/java/android/view/View.java
+++ b/core/java/android/view/View.java
@@ -163,7 +163,6 @@ 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;
@@ -10347,24 +10346,29 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
     }
 
     /**
-     * Check whether current activity / package is in denylist.If it's in the denylist,
-     * then the views marked as not important for autofill are not eligible for autofill.
+     * Check whether current activity / package is in autofill denylist.
+     *
+     * Called by viewGroup#populateChildrenForAutofill() to determine whether to include view in
+     * assist structure
      */
     final boolean isActivityDeniedForAutofillForUnimportantView() {
         final AutofillManager afm = getAutofillManager();
-        // keep behavior same with denylist feature not enabled
-        if (afm == null) return true;
-        return afm.isActivityDeniedForAutofillForUnimportantView();
+        if (afm == null) return false;
+        return afm.isActivityDeniedForAutofill();
     }
 
     /**
      * Check whether current view matches autofillable heuristics
+     *
+     * Called by viewGroup#populateChildrenForAutofill() to determine whether to include view in
+     * assist structure
      */
     final boolean isMatchingAutofillableHeuristics() {
         final AutofillManager afm = getAutofillManager();
-        // keep default behavior
         if (afm == null) return false;
-        return afm.isMatchingAutofillableHeuristicsForNotImportantViews(this);
+        // check the flag to see if trigger fill request on not important views is enabled
+        return afm.isTriggerFillRequestOnUnimportantViewEnabled()
+            ? afm.isAutofillable(this) : false;
     }
 
     private boolean isAutofillable() {
@@ -10380,39 +10384,26 @@ 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
+        // Check whether view is not part of an activity. If it's not, return false.
+        if (getAutofillViewId() <= LAST_APP_AUTOFILL_ID) {
             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.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
-            if (!notifyAugmentedAutofillIfNeeded(afm)){
-                return false;
-            }
+        // If view is important and filter important view flag is turned on, or view is not
+        // important and trigger fill request on not important view flag is turned on, then use
+        // AutofillManager.isAutofillable() to decide whether view is autofillable instead.
+        if ((isImportantForAutofill() && afm.isTriggerFillRequestOnFilteredImportantViewsEnabled())
+                || (!isImportantForAutofill()
+                    && afm.isTriggerFillRequestOnUnimportantViewEnabled())) {
+            return afm.isAutofillable(this) ? true : notifyAugmentedAutofillIfNeeded(afm);
         }
 
-        return getAutofillViewId() > LAST_APP_AUTOFILL_ID;
+        // If the previous condition is not met, fall back to the previous way to trigger fill
+        // request based on autofill importance instead.
+        return isImportantForAutofill() ? true : notifyAugmentedAutofillIfNeeded(afm);
     }
 
-    /** @hide **/
-    public boolean notifyAugmentedAutofillIfNeeded(AutofillManager afm) {
+    private boolean notifyAugmentedAutofillIfNeeded(AutofillManager afm) {
         final AutofillOptions options = mContext.getAutofillOptions();
         if (options == null || !options.isAugmentedAutofillEnabled(mContext)) {
             return false;
diff --git a/core/java/android/view/autofill/AutofillFeatureFlags.java b/core/java/android/view/autofill/AutofillFeatureFlags.java
index 7da69e7d819b..e267a7f1e248 100644
--- a/core/java/android/view/autofill/AutofillFeatureFlags.java
+++ b/core/java/android/view/autofill/AutofillFeatureFlags.java
@@ -365,7 +365,10 @@ public class AutofillFeatureFlags {
     }
 
     /**
-     * Get denylist string from flag
+     * Get denylist string from flag.
+     *
+     * Note: This denylist works both on important view and not important views. The flag used here
+     * is legacy flag which will be replaced with soon.
      *
      * @hide
      */
diff --git a/core/java/android/view/autofill/AutofillManager.java b/core/java/android/view/autofill/AutofillManager.java
index 508c20a37215..781a4b63f889 100644
--- a/core/java/android/view/autofill/AutofillManager.java
+++ b/core/java/android/view/autofill/AutofillManager.java
@@ -687,11 +687,11 @@ public final class AutofillManager {
 
     // If a package is fully denied, then all views that marked as not
     // important for autofill will not trigger fill request
-    private boolean mIsPackageFullyDeniedForAutofillForUnimportantView = false;
+    private boolean mIsPackageFullyDeniedForAutofill = false;
 
     // If a package is partially denied, autofill manager will check whether
     // current activity is in deny set to decide whether to trigger fill request
-    private boolean mIsPackagePartiallyDeniedForAutofillForUnimportantView = false;
+    private boolean mIsPackagePartiallyDeniedForAutofill = false;
 
     // A deny set read from device config
     private Set<String> mDeniedActivitiySet = new ArraySet<>();
@@ -876,15 +876,15 @@ public final class AutofillManager {
 
         final String packageName = mContext.getPackageName();
 
-        mIsPackageFullyDeniedForAutofillForUnimportantView =
-            isPackageFullyDeniedForAutofillForUnimportantView(denyListString, packageName);
+        mIsPackageFullyDeniedForAutofill =
+            isPackageFullyDeniedForAutofill(denyListString, packageName);
 
-        if (!mIsPackageFullyDeniedForAutofillForUnimportantView) {
-            mIsPackagePartiallyDeniedForAutofillForUnimportantView =
-                isPackagePartiallyDeniedForAutofillForUnimportantView(denyListString, packageName);
+        if (!mIsPackageFullyDeniedForAutofill) {
+            mIsPackagePartiallyDeniedForAutofill =
+                isPackagePartiallyDeniedForAutofill(denyListString, packageName);
         }
 
-        if (mIsPackagePartiallyDeniedForAutofillForUnimportantView) {
+        if (mIsPackagePartiallyDeniedForAutofill) {
             setDeniedActivitySetWithDenyList(denyListString, packageName);
         }
     }
@@ -899,6 +899,15 @@ public final class AutofillManager {
     }
 
     /**
+     * Whether to trigger fill request on not important views that passes heuristic check
+     *
+     * @hide
+     */
+    public boolean isTriggerFillRequestOnUnimportantViewEnabled() {
+        return mIsTriggerFillRequestOnUnimportantViewEnabled;
+    }
+
+    /**
      * Whether view passes the imeAction check
      *
      * @hide
@@ -906,13 +915,13 @@ public final class AutofillManager {
     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
+            Log.d(TAG, "view not autofillable - not passing ime action check");
             return false;
         }
         return true;
     }
 
-    private boolean isPackageFullyDeniedForAutofillForUnimportantView(
+    private boolean isPackageFullyDeniedForAutofill(
             @NonNull String denyListString, @NonNull String packageName) {
         // If "PackageName:;" is in the string, then it means the package name is in denylist
         // and there are no activities specified under it. That means the package is fully
@@ -920,7 +929,7 @@ public final class AutofillManager {
         return denyListString.indexOf(packageName + ":;") != -1;
     }
 
-    private boolean isPackagePartiallyDeniedForAutofillForUnimportantView(
+    private boolean isPackagePartiallyDeniedForAutofill(
             @NonNull String denyListString, @NonNull String packageName) {
         // This check happens after checking package is not fully denied. If "PackageName:" instead
         // is in denylist, then it means there are specific activities to be denied. So the package
@@ -968,17 +977,16 @@ public final class AutofillManager {
     }
 
     /**
-     * Check whether autofill is denied for current activity or package. Used when a view is marked
-     * as not important for autofill, if current activity or package is denied, then the view won't
-     * trigger fill request.
+     * Check whether autofill is denied for current activity or package. If current activity or
+     * package is denied, then the view won't trigger fill request.
      *
      * @hide
      */
-    public final boolean isActivityDeniedForAutofillForUnimportantView() {
-        if (mIsPackageFullyDeniedForAutofillForUnimportantView) {
+    public boolean isActivityDeniedForAutofill() {
+        if (mIsPackageFullyDeniedForAutofill) {
             return true;
         }
-        if (mIsPackagePartiallyDeniedForAutofillForUnimportantView) {
+        if (mIsPackagePartiallyDeniedForAutofill) {
             final AutofillClient client = getClient();
             if (client == null) {
                 return false;
@@ -992,27 +1000,36 @@ public final class AutofillManager {
     }
 
     /**
-     * Check whether view matches autofill-able heuristics
+     * Check heuristics and other rules to determine if view is autofillable
+     *
+     * Note: this function should be only called only when autofill for all apps is turned on. The
+     * calling method needs to check the corresponding flag to make sure that before calling into
+     * this function.
      *
      * @hide
      */
-    public final boolean isMatchingAutofillableHeuristicsForNotImportantViews(@NonNull View view) {
-        if (!mIsTriggerFillRequestOnUnimportantViewEnabled) {
+    public boolean isAutofillable(View view) {
+        if (isActivityDeniedForAutofill()) {
+            Log.d(TAG, "view is not autofillable - activity denied for autofill");
             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)
+        // Duplicate the autofill type check here because ViewGroup will call this function to
+        // decide whether to include view in assist structure.
+        // Also keep the autofill type check inside View#IsAutofillable() to serve as an early out
+        // or if other functions need to call it.
         if (view.getAutofillType() == View.AUTOFILL_TYPE_NONE) return false;
 
         if (view instanceof EditText) {
             return isPassingImeActionCheck((EditText) view);
         }
 
+        // Skip view type check if view is important for autofill or
+        // shouldEnableAutofillOnAllViewTypes flag is turned on
+        if (view.isImportantForAutofill() || mShouldEnableAutofillOnAllViewTypes) {
+            return true;
+        }
+
         if (view instanceof CheckBox
             || view instanceof Spinner
             || view instanceof DatePicker
@@ -1021,10 +1038,9 @@ public final class AutofillManager {
             return true;
         }
 
-        return mShouldEnableAutofillOnAllViewTypes;
+        return false;
     }
 
-
     /**
      * @hide
      */