Commit cea053c3 authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Tweak AutofillAction UI.

Before this patch, AutofillAction would behave as follows:

- when there are no cards or profile registered for autofill, the action
would hang forever, as AutofillAssistantUiDelegate.showProfiles and
showCards would return immediately without reporting any results.

- when a card or profile was selected, the button would remain, but
clicking on it would crash the application (as it would call a C++
callback that was already deleted).

This patch attempts to guarantee that the callbacks that correspond to
AutofillAssistantUiDelegate.Client::onAddressSelected and onCardSelected
are called exactly once.

With this patch:

- when there are no cards, the callback is called with an empty guid
which tells the action to fall back to manuall filling

- buttons disappear once they're clicked. If that fails, the C++ layer
ignores duplicate calls for the same callback, without crashing.

Bug: 806868
Change-Id: I15bb1b1e6ee528c18cbd76763827e69dfe3f1726
Reviewed-on: https://chromium-review.googlesource.com/c/1264475Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597105}
parent a4ac4892
......@@ -165,8 +165,10 @@ class AutofillAssistantUiDelegate {
for (int i = 0; i < scriptHandles.size(); i++) {
ScriptHandle scriptHandle = scriptHandles.get(i);
TextView chipView = createChipView(scriptHandle.getName());
chipView.setOnClickListener(
(unusedView) -> mClient.onScriptSelected(scriptHandle.getPath()));
chipView.setOnClickListener((unusedView) -> {
mChipsViewContainer.removeAllViews();
mClient.onScriptSelected(scriptHandle.getPath());
});
mChipsViewContainer.addView(chipView);
}
......@@ -204,16 +206,21 @@ class AutofillAssistantUiDelegate {
* @param profiles List of profiles to show.
*/
public void showProfiles(ArrayList<AutofillProfile> profiles) {
mChipsViewContainer.removeAllViews();
if (profiles.isEmpty()) {
mClient.onAddressSelected("");
return;
}
if (profiles.isEmpty()) return;
mChipsViewContainer.removeAllViews();
for (int i = 0; i < profiles.size(); i++) {
AutofillProfile profile = profiles.get(i);
// TODO(crbug.com/806868): Show more information than the street.
TextView chipView = createChipView(profile.getStreetAddress());
chipView.setOnClickListener(
(unusedView) -> mClient.onAddressSelected(profile.getGUID()));
chipView.setOnClickListener((unusedView) -> {
mChipsViewContainer.removeAllViews();
mClient.onAddressSelected(profile.getGUID());
});
mChipsViewContainer.addView(chipView);
}
......@@ -226,15 +233,21 @@ class AutofillAssistantUiDelegate {
* @param cards List of cards to show.
*/
public void showCards(ArrayList<CreditCard> cards) {
mChipsViewContainer.removeAllViews();
if (cards.isEmpty()) {
mClient.onCardSelected("");
return;
}
if (cards.isEmpty()) return;
mChipsViewContainer.removeAllViews();
for (int i = 0; i < cards.size(); i++) {
CreditCard card = cards.get(i);
// TODO(crbug.com/806868): Show more information than the card number.
TextView chipView = createChipView(card.getObfuscatedNumber());
chipView.setOnClickListener((unusedView) -> mClient.onCardSelected(card.getGUID()));
chipView.setOnClickListener((unusedView) -> {
mChipsViewContainer.removeAllViews();
mClient.onCardSelected(card.getGUID());
});
mChipsViewContainer.addView(chipView);
}
......
......@@ -128,7 +128,9 @@ void UiControllerAndroid::OnAddressSelected(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jstring>& jaddress_guid) {
DCHECK(address_or_card_callback_);
if (!address_or_card_callback_) // possibly duplicate call
return;
std::string guid;
base::android::ConvertJavaStringToUTF8(env, jaddress_guid, &guid);
std::move(address_or_card_callback_).Run(guid);
......@@ -138,7 +140,9 @@ void UiControllerAndroid::OnCardSelected(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jstring>& jcard_guid) {
DCHECK(address_or_card_callback_);
if (!address_or_card_callback_) // possibly duplicate call
return;
std::string guid;
base::android::ConvertJavaStringToUTF8(env, jcard_guid, &guid);
std::move(address_or_card_callback_).Run(guid);
......
......@@ -188,7 +188,6 @@ void AutofillAction::OnDataSelected(ActionDelegate* delegate,
}
if (guid.empty()) {
// User selected 'Fill manually'.
delegate->StopCurrentScript(fill_form_message_);
EndAction(/* successful= */ true);
return;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment