Skip to content

GitLab

T
tangled
Commit 6b7d6578 authored by sandromaggi's avatar sandromaggi Committed by Commit Bot
Browse files
Options

[Autofill Assistant] Send autofill error details 

Before this change we lost autofill error details when the Autofill
action itself succeeded but the RequiredFieldsFallbackHandler did not.

Bug: b/160651587
Change-Id: I6a72e36e01117ae428a7add4cc6bdfe6af2ad6d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2283593
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#786186}
parent e8cf4cd3
Hide whitespace changes
Inline Side-by-side
Showing with 125 additions and 4 deletions
chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantCollectUserDataIntegrationTest.java
View file @ 6b7d6578
...@@ -156,6 +156,7 @@ public class AutofillAssistantCollectUserDataIntegrationTest { ...@@ -156,6 +156,7 @@ public class AutofillAssistantCollectUserDataIntegrationTest {
RequiredField fallbackTextField = RequiredField fallbackTextField =
(RequiredField) RequiredField.newBuilder() (RequiredField) RequiredField.newBuilder()
.setForced(true) // Make sure we do actual work.
.setValueExpression("${57}") .setValueExpression("${57}")
.setElement(SelectorProto.newBuilder().addFilters( .setElement(SelectorProto.newBuilder().addFilters(
SelectorProto.Filter.newBuilder().setCssSelector( SelectorProto.Filter.newBuilder().setCssSelector(
...@@ -219,6 +220,84 @@ public class AutofillAssistantCollectUserDataIntegrationTest { ...@@ -219,6 +220,84 @@ public class AutofillAssistantCollectUserDataIntegrationTest {
assertThat(getElementValue(getWebContents(), "js_dropdown_value"), is("2050")); assertThat(getElementValue(getWebContents(), "js_dropdown_value"), is("2050"));
} }
@Test
@MediumTest
public void testFailingAutofillSendsProperError() throws Exception {
String profileId = mHelper.addDummyProfile("John Doe", "johndoe@gmail.com");
mHelper.addDummyCreditCard(profileId);
ArrayList<ActionProto> list = new ArrayList<>();
list.add(
(ActionProto) ActionProto.newBuilder()
.setCollectUserData(CollectUserDataProto.newBuilder()
.setRequestPaymentMethod(true)
.setBillingAddressName("billing_address")
.addSupportedBasicCardNetworks("visa")
.setPrivacyNoticeText("3rd party privacy text")
.setShowTermsAsCheckbox(true)
.setRequestTermsAndConditions(true)
.setAcceptTermsAndConditionsText("accept terms")
.setTermsAndConditionsState(
TermsAndConditionsState.ACCEPTED))
.build());
RequiredField fallbackTextField =
(RequiredField) RequiredField.newBuilder()
.setForced(true) // Make sure we fail here while trying to fill the field.
.setValueExpression("${-99}") // Use non-existent key to force an error.
.setElement(SelectorProto.newBuilder().addFilters(
SelectorProto.Filter.newBuilder().setCssSelector(
"#fallback_entry")))
.setFillStrategy(KeyboardValueFillStrategy.SIMULATE_KEY_PRESSES)
.build();
list.add((ActionProto) ActionProto.newBuilder()
.setUseCard(
UseCreditCardProto.newBuilder()
.setFormFieldElement(SelectorProto.newBuilder().addFilters(
SelectorProto.Filter.newBuilder().setCssSelector(
"#card_number")))
.addRequiredFields(fallbackTextField))
.build());
AutofillAssistantTestScript script = new AutofillAssistantTestScript(
(SupportedScriptProto) SupportedScriptProto.newBuilder()
.setPath("form_target_website.html")
.setPresentation(PresentationProto.newBuilder().setAutostart(true).setChip(
ChipProto.newBuilder().setText("Payment")))
.build(),
list);
AutofillAssistantTestService testService =
new AutofillAssistantTestService(Collections.singletonList(script));
startAutofillAssistant(mTestRule.getActivity(), testService);
waitUntilViewMatchesCondition(withText("Continue"), isCompletelyDisplayed());
testService.setNextActions(new ArrayList<>());
onView(withText("Continue")).perform(click());
waitUntilViewMatchesCondition(withId(R.id.card_unmask_input), isCompletelyDisplayed());
onView(withId(R.id.card_unmask_input)).perform(typeText("123"));
onView(withId(R.id.positive_button)).perform(click());
testService.waitUntilGetNextActions(1);
List<ProcessedActionProto> processedActions = testService.getProcessedActions();
assertThat(processedActions, iterableWithSize(2));
assertThat(
processedActions.get(0).getStatus(), is(ProcessedActionStatusProto.ACTION_APPLIED));
ProcessedActionProto processedUseCard = processedActions.get(1);
assertThat(
processedUseCard.getStatus(), is(ProcessedActionStatusProto.AUTOFILL_INCOMPLETE));
assertThat(processedUseCard.hasStatusDetails(), is(true));
assertThat(processedUseCard.getStatusDetails().hasAutofillErrorInfo(), is(true));
assertThat(processedUseCard.getStatusDetails()
.getAutofillErrorInfo()
.getAutofillFieldErrorList(),
iterableWithSize(1));
assertThat(processedUseCard.getStatusDetails()
.getAutofillErrorInfo()
.getAutofillFieldError(0)
.getNoFallbackValue(),
is(true));
}
/** /**
* Showcasts an element of the webpage and checks that it can be interacted with. * Showcasts an element of the webpage and checks that it can be interacted with.
*/ */
......
components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.cc
View file @ 6b7d6578
...@@ -24,6 +24,8 @@ const char kSelectElementTag[] = "SELECT"; ...@@ -24,6 +24,8 @@ const char kSelectElementTag[] = "SELECT";
AutofillErrorInfoProto::AutofillFieldError* AddAutofillError( AutofillErrorInfoProto::AutofillFieldError* AddAutofillError(
const RequiredField& required_field, const RequiredField& required_field,
ClientStatus* client_status) { ClientStatus* client_status) {
client_status->set_proto_status(
ProcessedActionStatusProto::AUTOFILL_INCOMPLETE);
auto* field_error = client_status->mutable_details() auto* field_error = client_status->mutable_details()
->mutable_autofill_error_info() ->mutable_autofill_error_info()
->add_autofill_field_error(); ->add_autofill_field_error();
......
components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler_unittest.cc
View file @ 6b7d6578
...@@ -108,6 +108,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, ...@@ -108,6 +108,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest,
const base::Optional<ClientStatus>& detail_status) { const base::Optional<ClientStatus>& detail_status) {
EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE);
ASSERT_TRUE(detail_status.has_value()); ASSERT_TRUE(detail_status.has_value());
ASSERT_EQ(detail_status.value().proto_status(),
AUTOFILL_INCOMPLETE);
ASSERT_EQ(detail_status.value() ASSERT_EQ(detail_status.value()
.details() .details()
.autofill_error_info() .autofill_error_info()
...@@ -181,6 +183,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, AddsFirstFieldFillingError) { ...@@ -181,6 +183,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, AddsFirstFieldFillingError) {
const base::Optional<ClientStatus>& detail_status) { const base::Optional<ClientStatus>& detail_status) {
EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE);
ASSERT_TRUE(detail_status.has_value()); ASSERT_TRUE(detail_status.has_value());
ASSERT_EQ(detail_status.value().proto_status(),
AUTOFILL_INCOMPLETE);
ASSERT_EQ(detail_status.value() ASSERT_EQ(detail_status.value()
.details() .details()
.autofill_error_info() .autofill_error_info()
...@@ -232,6 +236,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, ...@@ -232,6 +236,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest,
const base::Optional<ClientStatus>& detail_status) { const base::Optional<ClientStatus>& detail_status) {
EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE);
ASSERT_TRUE(detail_status.has_value()); ASSERT_TRUE(detail_status.has_value());
ASSERT_EQ(detail_status.value().proto_status(),
AUTOFILL_INCOMPLETE);
ASSERT_EQ(detail_status.value() ASSERT_EQ(detail_status.value()
.details() .details()
.autofill_error_info() .autofill_error_info()
...@@ -370,6 +376,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, FailsIfForcedFieldDidNotGetFilled) { ...@@ -370,6 +376,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, FailsIfForcedFieldDidNotGetFilled) {
const base::Optional<ClientStatus>& detail_status) { const base::Optional<ClientStatus>& detail_status) {
EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE);
ASSERT_TRUE(detail_status.has_value()); ASSERT_TRUE(detail_status.has_value());
ASSERT_EQ(detail_status.value().proto_status(),
AUTOFILL_INCOMPLETE);
ASSERT_EQ(detail_status.value() ASSERT_EQ(detail_status.value()
.details() .details()
.autofill_error_info() .autofill_error_info()
...@@ -461,6 +469,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest, ...@@ -461,6 +469,8 @@ TEST_F(RequiredFieldsFallbackHandlerTest,
const base::Optional<ClientStatus>& detail_status) { const base::Optional<ClientStatus>& detail_status) {
EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE); EXPECT_EQ(status.proto_status(), AUTOFILL_INCOMPLETE);
ASSERT_TRUE(detail_status.has_value()); ASSERT_TRUE(detail_status.has_value());
ASSERT_EQ(detail_status.value().proto_status(),
AUTOFILL_INCOMPLETE);
ASSERT_EQ(detail_status.value() ASSERT_EQ(detail_status.value()
.details() .details()
.autofill_error_info() .autofill_error_info()
......
components/autofill_assistant/browser/actions/use_address_action_unittest.cc
View file @ 6b7d6578
...@@ -335,8 +335,23 @@ TEST_F(UseAddressActionTest, FallbackFails) { ...@@ -335,8 +335,23 @@ TEST_F(UseAddressActionTest, FallbackFails) {
kFirstName, _)) kFirstName, _))
.WillOnce(RunOnceCallback<2>(ClientStatus(OTHER_ACTION_STATUS))); .WillOnce(RunOnceCallback<2>(ClientStatus(OTHER_ACTION_STATUS)));
EXPECT_EQ(ProcessedActionStatusProto::AUTOFILL_INCOMPLETE, ProcessedActionProto processed_action;
ProcessAction(action_proto)); EXPECT_CALL(callback_, Run(_)).WillOnce(SaveArgPointee<0>(&processed_action));
UseAddressAction action(&mock_action_delegate_, action_proto);
action.ProcessAction(callback_.Get());
EXPECT_EQ(processed_action.status(),
ProcessedActionStatusProto::AUTOFILL_INCOMPLETE);
EXPECT_TRUE(processed_action.has_status_details());
EXPECT_EQ(processed_action.status_details()
.autofill_error_info()
.autofill_field_error_size(),
1);
EXPECT_EQ(OTHER_ACTION_STATUS, processed_action.status_details()
.autofill_error_info()
.autofill_field_error(0)
.status());
} }
TEST_F(UseAddressActionTest, FallbackSucceeds) { TEST_F(UseAddressActionTest, FallbackSucceeds) {
......
components/autofill_assistant/browser/actions/use_credit_card_action_unittest.cc
View file @ 6b7d6578
...@@ -612,8 +612,23 @@ TEST_F(UseCreditCardActionTest, FallbackFails) { ...@@ -612,8 +612,23 @@ TEST_F(UseCreditCardActionTest, FallbackFails) {
"09/2050", _)) "09/2050", _))
.WillOnce(RunOnceCallback<2>(ClientStatus(OTHER_ACTION_STATUS))); .WillOnce(RunOnceCallback<2>(ClientStatus(OTHER_ACTION_STATUS)));
EXPECT_EQ(ProcessedActionStatusProto::AUTOFILL_INCOMPLETE, ProcessedActionProto processed_action;
ProcessAction(action_proto)); EXPECT_CALL(callback_, Run(_)).WillOnce(SaveArgPointee<0>(&processed_action));
UseCreditCardAction action(&mock_action_delegate_, action_proto);
action.ProcessAction(callback_.Get());
EXPECT_EQ(processed_action.status(),
ProcessedActionStatusProto::AUTOFILL_INCOMPLETE);
EXPECT_TRUE(processed_action.has_status_details());
EXPECT_EQ(processed_action.status_details()
.autofill_error_info()
.autofill_field_error_size(),
1);
EXPECT_EQ(OTHER_ACTION_STATUS, processed_action.status_details()
.autofill_error_info()
.autofill_field_error(0)
.status());
} }
} // namespace } // namespace
......
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