[PM-40995] fix: Allow fill-assist to rescue autofill requests when heuristics find no fields (#7214)

2026-08-01 18:53:39 -05:00 · 2026-07-28 19:17:40 +00:00
parent bb71a0e950
commit d87432e982
2 changed files with 277 additions and 37 deletions
@@ -123,14 +123,29 @@ class AutofillParserImpl(
        val urlBarWebsite = traversalDataList
            .flatMap { it.urlBarWebsites }
            .firstOrNull()
-        val autofillViews = traversalDataList.toAutofillViews(urlBarWebsite = urlBarWebsite)
+        // Heuristic views: the focused node's candidates with unfillable (Unused) fields removed,
+        // falling back to all fillable views when nothing has focus.
+        val autofillViews = traversalDataList
+            .selectCandidateAutofillViews(urlBarWebsite = urlBarWebsite) {
+                it !is AutofillView.Unused
+            }
+
+        val isFillAssistEnabled = featureFlagManager
+            .getFeatureFlag(FlagKey.FillAssistTargetingRules) &&
+            settingsRepository.isFillAssistEnabled

        // Find the focused view, or fallback to the first fillable item on the screen (so
-        // we at least have something to hook into)
-        val focusedView = autofillViews
-            .firstOrNull { it.data.isFocused }
-            ?: autofillViews.firstOrNull()
-            // The view is unfillable if there are no focused views.
+        // we at least have something to hook into). If heuristics found nothing at all, only
+        // give fill-assist a chance to rescue the page when it's actually enabled -- otherwise
+        // the view is unfillable since there are no focused views.
+        val focusedView = autofillViews.firstFocusedOrNull()
+            ?: if (isFillAssistEnabled) {
+                traversalDataList
+                    .selectCandidateAutofillViews(urlBarWebsite = urlBarWebsite)
+                    .firstFocusedOrNull()
+            } else {
+                null
+            }
            ?: return AutofillRequest.Unfillable

        val packageName = traversalDataList.buildPackageNameOrNull(
@@ -143,9 +158,6 @@ class AutofillParserImpl(
            return AutofillRequest.Unfillable
        }

-        val isFillAssistEnabled = featureFlagManager
-            .getFeatureFlag(FlagKey.FillAssistTargetingRules) &&
-            settingsRepository.isFillAssistEnabled
        val effectiveViews = if (isFillAssistEnabled) {
            autofillViews.toEffectiveViews(
                assistStructure = assistStructure,
@@ -157,9 +169,7 @@ class AutofillParserImpl(
            autofillViews
        }

-        val effectiveFocusedView = effectiveViews
-            .firstOrNull { it.data.isFocused }
-            ?: effectiveViews.firstOrNull()
+        val effectiveFocusedView = effectiveViews.firstFocusedOrNull()
            ?: return AutofillRequest.Unfillable

        // Choose the first focused partition of data for fulfillment.
@@ -177,8 +187,9 @@ class AutofillParserImpl(
            }

            is AutofillView.Unused -> {
-                // The view is unfillable since the field is not meant to be used for autofill.
-                // This will never happen since we filter out all unused views above.
+                // This will never happen: the heuristic path filters out Unused views, and the
+                // fill-assist path never constructs one (toAutofillViewForFieldKey has no Unused
+                // case).
                return AutofillRequest.Unfillable
            }
        }
@@ -233,19 +244,25 @@ class AutofillParserImpl(
            when (focusedView) {
                is AutofillView.Card -> rule.category in CARD_FILL_ASSIST_CATEGORIES
                is AutofillView.Login -> rule.category in LOGIN_FILL_ASSIST_CATEGORIES
-                is AutofillView.Unused -> false
+                is AutofillView.Unused -> {
+                    rule.category in LOGIN_FILL_ASSIST_CATEGORIES ||
+                        rule.category in CARD_FILL_ASSIST_CATEGORIES
+                }
            }
        }
+        if (!coversCurrentPartition) return this

-        return if (coversCurrentPartition) {
-            Timber.d("FillAssist invoked for this autofill attempt")
-            assistStructure.buildFillAssistViews(
-                hostRules = hostRules,
-                urlBarWebsite = urlBarWebsite,
-            )
-        } else {
-            this
-        }
+        val fillAssistViews = assistStructure.buildFillAssistViews(
+            hostRules = hostRules,
+            urlBarWebsite = urlBarWebsite,
+        )
+        // Fill-assist is authoritative for a partition its rules cover (guarded by
+        // coversCurrentPartition above), so its views are used even when empty: for Login/Card
+        // that discards the already heuristically-confirmed views, and for the Unused rescue path
+        // there were no heuristic views to fall back to. Reaching here means fill-assist has taken
+        // over this attempt (an empty result leaves the request Unfillable).
+        Timber.d("FillAssist invoked for this autofill attempt")
+        return fillAssistViews
    }
 }

@@ -264,26 +281,32 @@ private fun AssistStructure.traverse(): List<ViewNodeTraversalData> =
        }

 /**
- * Assembles the [AutofillView] list from this [ViewNodeTraversalData] list.
- * Take only the autofill views from the node that currently has focus.
- * Then remove all the fields that cannot be filled with data.
- * We fall back to taking all the fillable views if nothing has focus.
+ * Selects the autofill views from the node that currently has focus, or falls back to all
+ * fillable views if nothing has focus. The optional [predicate] filters the views *before* the
+ * emptiness/fallback check, so callers that only want fillable fields (e.g. excluding
+ * [AutofillView.Unused]) still get the multi-window fallback applied to the filtered set. By
+ * default no views are filtered out.
 */
-private fun List<ViewNodeTraversalData>.toAutofillViews(
+private fun List<ViewNodeTraversalData>.selectCandidateAutofillViews(
    urlBarWebsite: String?,
+    predicate: (AutofillView) -> Boolean = { true },
 ): List<AutofillView> {
    val viewsLists = map { it.autofillViews }
-    val autofillViewLists = viewsLists
+    val candidates = viewsLists
        .filter { views -> views.any { it.data.isFocused } }
        .flatten()
-        .filter { it !is AutofillView.Unused }
+        .filter(predicate)
        .takeUnless { it.isEmpty() }
-        ?: viewsLists
-            .flatten()
-            .filter { it !is AutofillView.Unused }
-    return autofillViewLists.map { it.updateWebsiteIfNecessary(website = urlBarWebsite) }
+        ?: viewsLists.flatten().filter(predicate)
+    return candidates.map { it.updateWebsiteIfNecessary(website = urlBarWebsite) }
 }

+/**
+ * Returns the focused [AutofillView], or falls back to the first entry if none is focused.
+ */
+private fun List<AutofillView>.firstFocusedOrNull(): AutofillView? =
+    firstOrNull { it.data.isFocused } ?: firstOrNull()
+
 /**
 * This helper function updates the [ViewNodeTraversalData] if necessary for missing password
 * fields that were marked invalid because they contained a specific `hint` or `idEntry`. If the
@@ -106,17 +106,19 @@ class AutofillParserTests {
        // ViewNodeExtensionsKt class, so one mockkStatic call covers all three.
        mockkStatic(AssistStructure.ViewNode::toAutofillView)
        // Default stub for toAutofillViewData (same mocked class — no separate mockkStatic needed).
+        // Note: this is a mocked extension function, so the receiver occupies arg position 0 --
+        // `autofillId` and `website` are therefore the second and third args, not first/second.
        every {
            any<AssistStructure.ViewNode>().toAutofillViewData(autofillId = any(), website = any())
        } answers {
            AutofillView.Data(
-                autofillId = firstArg(),
+                autofillId = secondArg(),
                autofillOptions = emptyList(),
                autofillType = AUTOFILL_TYPE,
                isFocused = false,
                textValue = null,
                hasPasswordTerms = false,
-                website = secondArg(),
+                website = thirdArg(),
            )
        }
        mockkStatic(
@@ -809,6 +811,71 @@ class AutofillParserTests {
        }
    }

+    @Suppress("MaxLineLength")
+    @Test
+    fun `parse should fall back to another window's fillable view when the focused window yields only Unused views`() {
+        // Setup: two window nodes. The focused window (e.g. a browser url bar EditText with no
+        // hint) contributes only an Unused view, while a second, unfocused window holds the real
+        // Login.Username field -- a shape that occurs for the browsers in URL_BARS. The focused
+        // window's emptiness check must run AFTER Unused views are filtered out, so the request
+        // falls back to the fillable view in the other window instead of short-circuiting to
+        // Unfillable. Fill-assist stays disabled (default), so there is no rescue path.
+        val urlBarAutofillId: AutofillId = mockk()
+        val urlBarViewNode: AssistStructure.ViewNode = mockk {
+            every { this@mockk.autofillHints } returns emptyArray()
+            every { this@mockk.autofillId } returns urlBarAutofillId
+            every { this@mockk.childCount } returns 0
+            every { this@mockk.htmlInfo } returns mockk(relaxed = true)
+            every { this@mockk.idPackage } returns ID_PACKAGE
+            every { this@mockk.idEntry } returns null
+            every { this@mockk.website } returns null
+        }
+        val urlBarWindowNode: AssistStructure.WindowNode = mockk {
+            every { this@mockk.rootViewNode } returns urlBarViewNode
+        }
+        every { assistStructure.windowNodeCount } returns 2
+        every { assistStructure.getWindowNodeAt(0) } returns urlBarWindowNode
+        every { assistStructure.getWindowNodeAt(1) } returns loginWindowNode
+        val unusedFocusedView = AutofillView.Unused(
+            data = AutofillView.Data(
+                autofillId = urlBarAutofillId,
+                autofillOptions = emptyList(),
+                autofillType = AUTOFILL_TYPE,
+                isFocused = true,
+                textValue = null,
+                hasPasswordTerms = false,
+                website = null,
+            ),
+        )
+        val loginAutofillView = AutofillView.Login.Username(
+            data = AutofillView.Data(
+                autofillId = loginAutofillId,
+                autofillOptions = emptyList(),
+                autofillType = AUTOFILL_TYPE,
+                isFocused = false,
+                textValue = null,
+                hasPasswordTerms = false,
+                website = URI,
+            ),
+        )
+        every { urlBarViewNode.toAutofillView(parentWebsite = any()) } returns unusedFocusedView
+        every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
+
+        // Test
+        val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest)
+
+        // Verify: the request falls back to the fillable Login.Username in the unfocused window.
+        val expected = AutofillRequest.Fillable(
+            ignoreAutofillIds = emptyList(),
+            inlinePresentationSpecs = inlinePresentationSpecs,
+            maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT,
+            packageName = PACKAGE_NAME,
+            partition = AutofillPartition.Login(views = listOf(loginAutofillView)),
+            uri = URI,
+        )
+        assertEquals(expected, actual)
+    }
+
    @Test
    fun `parse should return empty inline suggestions when inline autofill is disabled`() {
        // Setup
@@ -1263,6 +1330,156 @@ class AutofillParserTests {
        assertEquals(expected, actual)
    }

+    @Suppress("MaxLineLength")
+    @Test
+    fun `parse should use fill-assist views when heuristics classify the focused view as Unused`() {
+        // Setup: heuristics found nothing recognizable (focused view is Unused), but fill-assist
+        // rules exist for this host and match. Fill-assist should rescue the request instead of
+        // it dying as Unfillable before fill-assist is ever consulted.
+        mutableFillAssistFlagFlow.value = true
+        mockIsFillAssistEnabled = true
+        every { any<AutofillView>().buildUriOrNull(PACKAGE_NAME) } returns FILL_ASSIST_URI
+        every { fillAssistManager.getFillAssistRules() } returns FillAssistRules(
+            hostRules = mapOf(
+                FILL_ASSIST_URI to listOf(
+                    FillAssistRules.HostRule(
+                        category = "account-login",
+                        fields = mapOf(
+                            "username" to listOf(
+                                FillAssistRules.SelectorClause(
+                                    tag = "input",
+                                    id = "user",
+                                    name = null,
+                                    type = null,
+                                    role = null,
+                                ),
+                            ),
+                        ),
+                    ),
+                ),
+            ),
+        )
+        val unusedLoginView = AutofillView.Unused(
+            data = AutofillView.Data(
+                autofillId = loginAutofillId,
+                autofillOptions = emptyList(),
+                autofillType = AUTOFILL_TYPE,
+                isFocused = true,
+                textValue = null,
+                hasPasswordTerms = false,
+                website = null,
+            ),
+        )
+        every { any<HtmlInfo>().matchesSelectorClause(any()) } returns true
+        every { assistStructure.windowNodeCount } returns 1
+        every { assistStructure.getWindowNodeAt(0) } returns loginWindowNode
+        every { loginViewNode.toAutofillView(parentWebsite = any()) } returns unusedLoginView
+
+        // Test
+        val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest)
+
+        // Verify: fill-assist rescued a page heuristics found nothing on.
+        val fillAssistData = AutofillView.Data(
+            autofillId = loginAutofillId,
+            autofillOptions = emptyList(),
+            autofillType = AUTOFILL_TYPE,
+            isFocused = false,
+            textValue = null,
+            hasPasswordTerms = false,
+            website = WEBSITE,
+        )
+        val expected = AutofillRequest.Fillable(
+            ignoreAutofillIds = emptyList(),
+            inlinePresentationSpecs = inlinePresentationSpecs,
+            maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT,
+            packageName = PACKAGE_NAME,
+            partition = AutofillPartition.Login(
+                views = listOf(AutofillView.Login.Username(data = fillAssistData)),
+            ),
+            uri = FILL_ASSIST_URI,
+        )
+        assertEquals(expected, actual)
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `parse should return Unfillable when heuristics classify the focused view as Unused and fill-assist matches nothing`() {
+        // Fill-assist takes over the attempt but matches no fields, and the Unused rescue path has
+        // no heuristic views to fall back to -- so the request ends up Unfillable.
+        mutableFillAssistFlagFlow.value = true
+        mockIsFillAssistEnabled = true
+        every { any<AutofillView>().buildUriOrNull(PACKAGE_NAME) } returns FILL_ASSIST_URI
+        every { fillAssistManager.getFillAssistRules() } returns FillAssistRules(
+            hostRules = mapOf(
+                FILL_ASSIST_URI to listOf(
+                    FillAssistRules.HostRule(
+                        category = "account-login",
+                        fields = mapOf(
+                            "username" to listOf(
+                                FillAssistRules.SelectorClause(
+                                    tag = "input",
+                                    id = "user",
+                                    name = null,
+                                    type = null,
+                                    role = null,
+                                ),
+                            ),
+                        ),
+                    ),
+                ),
+            ),
+        )
+        val unusedLoginView = AutofillView.Unused(
+            data = AutofillView.Data(
+                autofillId = loginAutofillId,
+                autofillOptions = emptyList(),
+                autofillType = AUTOFILL_TYPE,
+                isFocused = true,
+                textValue = null,
+                hasPasswordTerms = false,
+                website = null,
+            ),
+        )
+        // matchesSelectorClause defaults to false per setup -- fill-assist matches nothing.
+        every { assistStructure.windowNodeCount } returns 1
+        every { assistStructure.getWindowNodeAt(0) } returns loginWindowNode
+        every { loginViewNode.toAutofillView(parentWebsite = any()) } returns unusedLoginView
+
+        // Test
+        val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest)
+
+        // Verify
+        assertEquals(AutofillRequest.Unfillable, actual)
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `parse should return Unfillable when fill-assist is disabled even though heuristics classify the focused view as Unused`() {
+        // Setup: fill-assist stays disabled (default). The rescue path must never activate when
+        // the feature flag/setting are off, regardless of what fill-assist rules would say.
+        every { any<AutofillView>().buildUriOrNull(PACKAGE_NAME) } returns FILL_ASSIST_URI
+        val unusedLoginView = AutofillView.Unused(
+            data = AutofillView.Data(
+                autofillId = loginAutofillId,
+                autofillOptions = emptyList(),
+                autofillType = AUTOFILL_TYPE,
+                isFocused = true,
+                textValue = null,
+                hasPasswordTerms = false,
+                website = null,
+            ),
+        )
+        every { assistStructure.windowNodeCount } returns 1
+        every { assistStructure.getWindowNodeAt(0) } returns loginWindowNode
+        every { loginViewNode.toAutofillView(parentWebsite = any()) } returns unusedLoginView
+
+        // Test
+        val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest)
+
+        // Verify
+        assertEquals(AutofillRequest.Unfillable, actual)
+    }
+
    @Suppress("MaxLineLength")
    @Test
    fun `parse should deduplicate when a child node returns an AutofillView with the same autofillId already claimed by a non-Unused parent view`() {