[Autofill Assistant] Replace label generation [chromium/src : main]
Sandro Maggi (Gerrit)
Attention is currently required from: Florian Gauger, Luca Hunkeler.
Patch set 2:Commit-Queue +1
1 comment:
Patchset:
PTAL.
To view, visit change 3480016. To unsubscribe, or for help writing mail filters, visit settings.
Sandro Maggi (Gerrit)
Attention is currently required from: Florian Gauger, Luca Hunkeler.
1 comment:
Patchset:
PTAL.
To view, visit change 3480016. To unsubscribe, or for help writing mail filters, visit settings.
Luca Hunkeler (Gerrit)
Attention is currently required from: Florian Gauger, Sandro Maggi.
Patch set 2:Code-Review +1
1 comment:
Patchset:
LGTM
To view, visit change 3480016. To unsubscribe, or for help writing mail filters, visit settings.
Florian Gauger (Gerrit)
Attention is currently required from: Sandro Maggi.
6 comments:
Patchset:
lgtm - comments
Thanks for the change!
File chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantCollectUserDataUiTest.java:
PersonalDataManager.getInstance()
.getShippingAddressLabelWithCountryForPaymentRequest(profile)nit: use variable instead of inline comment please - same below
File chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantAddressEditorAutofill.java:
PersonalDataManager.getInstance()
.getShippingAddressLabelWithCountryForPaymentRequest(
editedAddress.getProfile())nit: dito and below
File chrome/browser/android/autofill_assistant/ui_controller_android.cc:
Patch Set #2, Line 259:
base::android::ConvertUTF8ToJavaStringnit: ::using please - also below
size_t fields_size = base::size(kLabelFields);
if (!with_country) {
--fields_size;
}Commment here please. This is a slightly weird pattern.
Patch Set #2, Line 282:
ields_size, fields_sizecomments here please
To view, visit change 3480016. To unsubscribe, or for help writing mail filters, visit settings.
Sandro Maggi (Gerrit)
Attention is currently required from: Florian Gauger.
6 comments:
Patchset:
Thanks for the reviews 😊
File chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantCollectUserDataUiTest.java:
PersonalDataManager.getInstance()
.getShippingAddressLabelWithCountryForPaymentRequest(profile)nit: use variable instead of inline comment please - same below
Done
File chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantAddressEditorAutofill.java:
PersonalDataManager.getInstance()
.getShippingAddressLabelWithCountryForPaymentRequest(
editedAddress.getProfile())nit: dito and below
Done
File chrome/browser/android/autofill_assistant/ui_controller_android.cc:
Patch Set #2, Line 259:
base::android::ConvertUTF8ToJavaStringnit: ::using please - also below
Done
size_t fields_size = base::size(kLabelFields);
if (!with_country) {
--fields_size;
}Commment here please. This is a slightly weird pattern.
Done
Patch Set #2, Line 282:
ields_size, fields_sizecomments here please
Done
To view, visit change 3480016. To unsubscribe, or for help writing mail filters, visit settings.
Sandro Maggi (Gerrit)
Attention is currently required from: Florian Gauger.
Patch set 3:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: chrome/browser/android/autofill_assistant/ui_controller_android.cc
Insertions: 9, Deletions: 5.
@@ -58,6 +58,7 @@
ConvertNativeOptionalStringToJava;
using ::autofill_assistant::ui_controller_android_utils::CreateJavaInfoPopup;
using ::base::android::AttachCurrentThread;
+using ::base::android::ConvertUTF16ToJavaString;
using ::base::android::ConvertUTF8ToJavaString;
using ::base::android::JavaParamRef;
using ::base::android::JavaRef;
@@ -256,7 +257,7 @@
const autofill::AutofillProfile* profile,
bool with_country) {
if (!profile) {
- return base::android::ConvertUTF8ToJavaString(env, std::string());
+ return ConvertUTF8ToJavaString(env, std::string());
}
// The full name is not included in the label for shipping address. It is
@@ -273,14 +274,17 @@
autofill::ServerFieldType::ADDRESS_HOME_COUNTRY,
};
size_t fields_size = base::size(kLabelFields);
+
+ // For the case where country is omitted, do not use the last field.
if (!with_country) {
--fields_size;
}
- return base::android::ConvertUTF16ToJavaString(
- env,
- profile->ConstructInferredLabel(kLabelFields, fields_size, fields_size,
- base::android::GetDefaultLocaleString()));
+ return ConvertUTF16ToJavaString(
+ env, profile->ConstructInferredLabel(
+ kLabelFields, /* label_fields_size= */ fields_size,
+ /* num_fields_to_include= */ fields_size,
+ base::android::GetDefaultLocaleString()));
}
} // namespace
```
```
The name of the file: chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantCollectUserDataUiTest.java
Insertions: 5, Deletions: 4.
@@ -1140,11 +1140,12 @@
}
private AddressModel createAddressModel(PersonalDataManager.AutofillProfile profile) {
- return new AddressModel(createDummyAddress(profile), /* fullDescription= */
+ String fullDescription =
PersonalDataManager.getInstance()
- .getShippingAddressLabelWithCountryForPaymentRequest(profile),
- /* summaryDescription= */
+ .getShippingAddressLabelWithCountryForPaymentRequest(profile);
+ String summaryDescription =
PersonalDataManager.getInstance()
- .getShippingAddressLabelWithoutCountryForPaymentRequest(profile));
+ .getShippingAddressLabelWithoutCountryForPaymentRequest(profile);
+ return new AddressModel(createDummyAddress(profile), fullDescription, summaryDescription);
}
}
```
```
The name of the file: chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantAddressEditorAutofill.java
Insertions: 8, Deletions: 8.
@@ -47,17 +47,17 @@
Callback<AutofillAddress> editorDoneCallback = editedAddress -> {
assert (editedAddress != null && editedAddress.isComplete()
&& editedAddress.getProfile() != null);
+ String fullDescription = PersonalDataManager.getInstance()
+ .getShippingAddressLabelWithCountryForPaymentRequest(
+ editedAddress.getProfile());
+ String summaryDescription =
+ PersonalDataManager.getInstance()
+ .getShippingAddressLabelWithoutCountryForPaymentRequest(
+ editedAddress.getProfile());
doneCallback.onResult(new AddressModel(
AssistantAutofillUtilChrome.autofillProfileToAssistantAutofillProfile(
editedAddress.getProfile()),
- /* fullDescription= */
- PersonalDataManager.getInstance()
- .getShippingAddressLabelWithCountryForPaymentRequest(
- editedAddress.getProfile()),
- /* summaryDescription= */
- PersonalDataManager.getInstance()
- .getShippingAddressLabelWithoutCountryForPaymentRequest(
- editedAddress.getProfile())));
+ fullDescription, summaryDescription));
};
Callback<AutofillAddress> editorCancelCallback =
```
[Autofill Assistant] Replace label generation
This replaces the label generation for shipping addresses
such that it no longer depends on Chrome code.
Bug: b:220829993
Change-Id: Ib502480b25bc155aa183f45fb6390b367c3ded82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3480016
Reviewed-by: Luca Hunkeler <hl...@google.com>
Commit-Queue: Sandro Maggi <sandr...@google.com>
Cr-Commit-Position: refs/heads/main@{#974110}
---
M chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/user_data/AssistantCollectUserDataModel.java
M chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/user_data/AssistantShippingAddressSection.java
M chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantCollectUserDataUiTest.java
M chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantAddressEditorAutofill.java
M chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantAutofillUtilChrome.java
M chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantOptionModel.java
M chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantPaymentInstrumentEditorAutofill.java
M chrome/browser/android/autofill_assistant/ui_controller_android.cc
8 files changed, 126 insertions(+), 55 deletions(-)