[PM-37255] feat: Integrate fill-assist targeting rules into autofill parser #7066
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,17 +3,22 @@ package com.x8bit.bitwarden.data.autofill.parser | |
| import android.app.assist.AssistStructure | ||
| import android.service.autofill.FillRequest | ||
| import android.view.autofill.AutofillId | ||
| import androidx.core.net.toUri | ||
| import com.bitwarden.core.data.manager.model.FlagKey | ||
| import com.x8bit.bitwarden.data.autofill.manager.FillAssistManager | ||
| import com.x8bit.bitwarden.data.autofill.model.AutofillAppInfo | ||
| import com.x8bit.bitwarden.data.autofill.model.AutofillPartition | ||
| import com.x8bit.bitwarden.data.autofill.model.AutofillRequest | ||
| import com.x8bit.bitwarden.data.autofill.model.AutofillView | ||
| import com.x8bit.bitwarden.data.autofill.model.ViewNodeTraversalData | ||
| import com.x8bit.bitwarden.data.autofill.util.buildFillAssistViews | ||
| import com.x8bit.bitwarden.data.autofill.util.buildPackageNameOrNull | ||
| import com.x8bit.bitwarden.data.autofill.util.buildUriOrNull | ||
| import com.x8bit.bitwarden.data.autofill.util.getInlinePresentationSpecs | ||
| import com.x8bit.bitwarden.data.autofill.util.getMaxInlineSuggestionsCount | ||
| import com.x8bit.bitwarden.data.autofill.util.toAutofillView | ||
| import com.x8bit.bitwarden.data.autofill.util.website | ||
| import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager | ||
| import com.x8bit.bitwarden.data.platform.repository.SettingsRepository | ||
| import timber.log.Timber | ||
|
|
||
|
|
@@ -50,12 +55,30 @@ private val URL_BARS: Map<String, String> = mapOf( | |
| "com.brave.browser_nightly" to "url_bar", | ||
| ) | ||
|
|
||
| /** | ||
| * A list of categories from Fill Assist that are used for [AutofillView.Login] | ||
| */ | ||
| private val LOGIN_FILL_ASSIST_CATEGORIES: List<String> = listOf( | ||
| "account-login", | ||
| "account-creation", | ||
| "account-update", | ||
| ) | ||
|
|
||
| /** | ||
| * A list of categories from Fill Assist that are used for [AutofillView.Card] | ||
| */ | ||
| private val CARD_FILL_ASSIST_CATEGORIES: List<String> = listOf( | ||
| "payment-card", | ||
| ) | ||
|
aj-rosado marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * The default [AutofillParser] implementation for the app. This is a tool for parsing autofill data | ||
| * from the OS into domain models. | ||
| */ | ||
| class AutofillParserImpl( | ||
| private val settingsRepository: SettingsRepository, | ||
| private val fillAssistManager: FillAssistManager, | ||
| private val featureFlagManager: FeatureFlagManager, | ||
| ) : AutofillParser { | ||
| override fun parse( | ||
| autofillAppInfo: AutofillAppInfo, | ||
|
|
@@ -100,56 +123,49 @@ class AutofillParserImpl( | |
| val urlBarWebsite = traversalDataList | ||
| .flatMap { it.urlBarWebsites } | ||
| .firstOrNull() | ||
| val autofillViews = traversalDataList.toAutofillViews(urlBarWebsite = urlBarWebsite) | ||
|
Collaborator
There was a problem hiding this comment. The new
Contributor
Author
There was a problem hiding this comment. It is possible but as It adds complexity to the traverse, not sure if it a real improvement. |
||
|
|
||
| // Find the focused view, or fallback to the first fillable item on the screen (so | ||
| // we at least have something to hook into) | ||
|
Collaborator
There was a problem hiding this comment. Can we leave in the comments in place? Autofill is confusing enough as it is, I want to make sure we leave goo notes for the next person. // 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.
?: return AutofillRequest.Unfillable |
||
| val focusedView = autofillViews | ||
| .firstOrNull { it.data.isFocused } | ||
| ?: autofillViews.firstOrNull() | ||
| // The view is unfillable if there are no focused views. | ||
| ?: return AutofillRequest.Unfillable | ||
|
|
||
| val packageName = traversalDataList.buildPackageNameOrNull( | ||
| assistStructure = assistStructure, | ||
| ) | ||
| val uri = focusedView.buildUriOrNull(packageName = packageName) | ||
|
|
||
|
david-livefront marked this conversation as resolved.
|
||
| // The view is unfillable if the URI is block listed. | ||
| if ((settingsRepository.blockedAutofillUris + BLOCK_LISTED_URIS).contains(uri)) { | ||
| return AutofillRequest.Unfillable | ||
| } | ||
|
|
||
| val effectiveViews = autofillViews.toEffectiveViews( | ||
| assistStructure = assistStructure, | ||
| uri = uri, | ||
| focusedView = focusedView, | ||
| urlBarWebsite = urlBarWebsite, | ||
| ) | ||
|
|
||
| val effectiveFocusedView = effectiveViews | ||
| .firstOrNull { it.data.isFocused } | ||
| ?: effectiveViews.firstOrNull() | ||
| ?: return AutofillRequest.Unfillable | ||
|
|
||
| // Choose the first focused partition of data for fulfillment. | ||
| val partition = when (effectiveFocusedView) { | ||
| is AutofillView.Card -> { | ||
| AutofillPartition.Card( | ||
| views = effectiveViews.filterIsInstance<AutofillView.Card>(), | ||
| ) | ||
| } | ||
|
|
||
| is AutofillView.Login -> { | ||
| AutofillPartition.Login( | ||
| views = effectiveViews.filterIsInstance<AutofillView.Login>(), | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -185,6 +201,45 @@ class AutofillParserImpl( | |
| uri = uri, | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Returns the effective [AutofillView] list for filling. Applies fill-assist targeting rules | ||
| * when the feature flag is enabled and the host rules cover the current partition type; | ||
| * otherwise returns the heuristic autofillViews [this]. | ||
| */ | ||
| private fun List<AutofillView>.toEffectiveViews( | ||
| assistStructure: AssistStructure, | ||
| uri: String?, | ||
| focusedView: AutofillView, | ||
| urlBarWebsite: String?, | ||
| ): List<AutofillView> { | ||
| val hostRules = uri | ||
| ?.takeUnless { it.startsWith("androidapp://") } | ||
| ?.toUri() | ||
| ?.host | ||
| ?.takeIf { featureFlagManager.getFeatureFlag(FlagKey.FillAssistTargetingRules) } | ||
|
Collaborator
There was a problem hiding this comment. Can we just return early instead of putting this check in the chain? if (!featureFlagManager.getFeatureFlag(FlagKey.FillAssistTargetingRules)) {
return autofillViews
}
Contributor
Author
There was a problem hiding this comment. Next PR in chain will remove that. Are you ok if we keep it here for now to avoid more conflicts when merging?
Collaborator
There was a problem hiding this comment. Why is it being removed in that PR?
Contributor
Author
There was a problem hiding this comment. Moved the validation to outside this method |
||
| ?.let { host -> | ||
| fillAssistManager.getFillAssistRules()?.hostRules?.get(host.removePrefix("www.")) | ||
|
Contributor
There was a problem hiding this comment. 🧹 DEBT: // FillAssistDiskSourceImpl
override fun getFillAssistRules(serverUrl: String): FillAssistRules? =
getString(FILL_ASSIST_RULES_KEY.appendIdentifier(serverUrl))
?.let { json.decodeFromStringOrNull(it) }Since the cached blob can contain rules for many hosts, this JSON parse repeats on every autofill interaction with no in-memory memoization. Consider caching the deserialized
Contributor
There was a problem hiding this comment. Claude finished @aj-rosado's task in 1m 20s —— View job Verifying the caching update
✅ Resolved. The DEBT concern is addressed.
Test coverage is solid — the three new cases cover the cache hit (disk read One carry-over note: this newly-added |
||
| } | ||
| ?: return this | ||
|
|
||
| val coversCurrentPartition = hostRules.any { rule -> | ||
| 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 | ||
| } | ||
| } | ||
|
|
||
| return if (coversCurrentPartition) { | ||
| assistStructure.buildFillAssistViews( | ||
| hostRules = hostRules, | ||
| urlBarWebsite = urlBarWebsite, | ||
| ) | ||
| } else { | ||
| this | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -201,6 +256,27 @@ private fun AssistStructure.traverse(): List<ViewNodeTraversalData> = | |
| ?.updateForMissingUsernameFields() | ||
| } | ||
|
|
||
| /** | ||
| * 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. | ||
| */ | ||
| private fun List<ViewNodeTraversalData>.toAutofillViews( | ||
| urlBarWebsite: String?, | ||
| ): List<AutofillView> { | ||
| val viewsLists = map { it.autofillViews } | ||
| val autofillViewLists = viewsLists | ||
| .filter { views -> views.any { it.data.isFocused } } | ||
| .flatten() | ||
| .filter { it !is AutofillView.Unused } | ||
| .takeUnless { it.isEmpty() } | ||
| ?: viewsLists | ||
| .flatten() | ||
| .filter { it !is AutofillView.Unused } | ||
| return autofillViewLists.map { it.updateWebsiteIfNecessary(website = urlBarWebsite) } | ||
| } | ||
|
|
||
| /** | ||
| * 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 | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.