[Autofill Assistant] Add skip option for UseAddressProto

This is a proposal to add a boolean to skip Autofill, entirely relying
on the required fields. The purpose of this change is to give our client
full, deterministic control over what happens in a |UseAddressAction|.
The same change is introduced for |UseCreditCardAction|.

Bug: none
Change-Id: Iae4ce4fd00fec6038f5394dff7bf7dd88e2a80a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207458Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#772623}

components/autofill_assistant/browser/actions/use_address_action.cc

@@ -36,11 +36,17 @@ void UseAddressAction::InternalProcessAction(
     ProcessActionCallback action_callback) {
   process_action_callback_ = std::move(action_callback);
 
-  if (selector_.empty()) {
+  if (selector_.empty() && !proto_.use_address().skip_autofill()) {
     VLOG(1) << "UseAddress failed: |selector| empty";
     EndAction(ClientStatus(INVALID_ACTION));
     return;
   }
+  if (proto_.use_address().skip_autofill() &&
+      proto_.use_address().required_fields().empty()) {
+    VLOG(1) << "UseAddress failed: |skip_autofill| without required fields";
+    EndAction(ClientStatus(INVALID_ACTION));
+    return;
+  }
 
   // Ensure data already selected in a previous action.
   switch (proto_.use_address().address_source_case()) {
@@ -121,19 +127,7 @@ void UseAddressAction::EndAction(
 }
 
 void UseAddressAction::FillFormWithData() {
-  delegate_->ShortWaitForElement(
-      selector_, base::BindOnce(&UseAddressAction::OnWaitForElement,
-                                weak_ptr_factory_.GetWeakPtr()));
-}
-
-void UseAddressAction::OnWaitForElement(const ClientStatus& element_status) {
-  if (!element_status.ok()) {
-    EndAction(ClientStatus(element_status.proto_status()));
-    return;
-  }
-  DCHECK(!selector_.empty());
   DCHECK(profile_ != nullptr);
-
   std::vector<RequiredField> required_fields;
   for (const auto& required_field_proto :
        proto_.use_address().required_fields()) {
@@ -153,6 +147,25 @@ void UseAddressAction::OnWaitForElement(const ClientStatus& element_status) {
                                                        /* locale = */ "en-US"),
       delegate_);
 
+  if (proto_.use_address().skip_autofill()) {
+    fallback_handler_->CheckAndFallbackRequiredFields(
+        OkClientStatus(), base::BindOnce(&UseAddressAction::EndAction,
+                                         weak_ptr_factory_.GetWeakPtr()));
+    return;
+  }
+
+  delegate_->ShortWaitForElement(
+      selector_, base::BindOnce(&UseAddressAction::OnWaitForElement,
+                                weak_ptr_factory_.GetWeakPtr()));
+}
+
+void UseAddressAction::OnWaitForElement(const ClientStatus& element_status) {
+  if (!element_status.ok()) {
+    EndAction(element_status);
+    return;
+  }
+
+  DCHECK(!selector_.empty());
   delegate_->FillAddressForm(profile_.get(), selector_,
                              base::BindOnce(&UseAddressAction::OnFormFilled,
                                             weak_ptr_factory_.GetWeakPtr()));

components/autofill_assistant/browser/actions/use_address_action_unittest.cc

@@ -137,6 +137,14 @@ TEST_F(UseAddressActionTest, InvalidActionNameSetButEmpty) {
   EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action));
 }
 
+TEST_F(UseAddressActionTest, InvalidActionSkipAutofillWithoutRequiredFields) {
+  ActionProto action;
+  UseAddressProto* use_address = action.mutable_use_address();
+  use_address->set_name(kAddressName);
+  use_address->set_skip_autofill(true);
+  EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action));
+}
+
 TEST_F(UseAddressActionTest, PreconditionFailedNoProfileForName) {
   ActionProto action;
   UseAddressProto* use_address = action.mutable_use_address();
@@ -496,5 +504,44 @@ TEST_F(UseAddressActionTest, ForcedFallbackWithKeystrokes) {
             ProcessAction(action_proto));
 }
 
+TEST_F(UseAddressActionTest, SkippingAutofill) {
+  ON_CALL(mock_action_delegate_, GetElementTag(_, _))
+      .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "INPUT"));
+
+  ActionProto action_proto;
+  action_proto.mutable_use_address()->set_name(kAddressName);
+  AddRequiredField(&action_proto,
+                   base::NumberToString(
+                       static_cast<int>(autofill::ServerFieldType::NAME_FIRST)),
+                   "#first_name");
+  action_proto.mutable_use_address()->set_skip_autofill(true);
+
+  EXPECT_CALL(mock_action_delegate_, OnFillAddressForm(_, _, _)).Times(0);
+
+  // First validation fails.
+  EXPECT_CALL(mock_web_controller_,
+              OnGetFieldValue(Selector({"#first_name"}), _))
+      .WillOnce(RunOnceCallback<1>(OkClientStatus(), ""));
+  // Fill first name.
+  Expectation set_first_name =
+      EXPECT_CALL(mock_action_delegate_,
+                  OnSetFieldValue(Selector({"#first_name"}), kFirstName, _))
+          .WillOnce(RunOnceCallback<2>(OkClientStatus()));
+  // Second validation succeeds.
+  EXPECT_CALL(mock_web_controller_,
+              OnGetFieldValue(Selector({"#first_name"}), _))
+      .After(set_first_name)
+      .WillOnce(RunOnceCallback<1>(OkClientStatus(), "not empty"));
+
+  ProcessedActionProto processed_action;
+  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::ACTION_APPLIED);
+}
+
 }  // namespace
 }  // namespace autofill_assistant

components/autofill_assistant/browser/actions/use_credit_card_action.cc

@@ -40,11 +40,17 @@ void UseCreditCardAction::InternalProcessAction(
     ProcessActionCallback action_callback) {
   process_action_callback_ = std::move(action_callback);
 
-  if (selector_.empty()) {
+  if (selector_.empty() && !proto_.use_card().skip_autofill()) {
     VLOG(1) << "UseCreditCard failed: |selector| empty";
     EndAction(ClientStatus(INVALID_ACTION));
     return;
   }
+  if (proto_.use_card().skip_autofill() &&
+      proto_.use_card().required_fields().empty()) {
+    VLOG(1) << "UseCreditCard failed: |skip_autofill| without required fields";
+    EndAction(ClientStatus(INVALID_ACTION));
+    return;
+  }
 
   // Ensure data already selected in a previous action.
   if (proto_.use_card().has_model_identifier()) {
@@ -102,6 +108,14 @@ void UseCreditCardAction::EndAction(
 }
 
 void UseCreditCardAction::FillFormWithData() {
+  if (proto_.use_card().skip_autofill()) {
+    DCHECK(credit_card_ != nullptr);
+    delegate_->GetFullCard(credit_card_.get(),
+                           base::BindOnce(&UseCreditCardAction::OnGetFullCard,
+                                          weak_ptr_factory_.GetWeakPtr()));
+    return;
+  }
+
   delegate_->ShortWaitForElement(
       selector_, base::BindOnce(&UseCreditCardAction::OnWaitForElement,
                                 weak_ptr_factory_.GetWeakPtr()));
@@ -109,7 +123,7 @@ void UseCreditCardAction::FillFormWithData() {
 
 void UseCreditCardAction::OnWaitForElement(const ClientStatus& element_status) {
   if (!element_status.ok()) {
-    EndAction(ClientStatus(element_status.proto_status()));
+    EndAction(element_status);
     return;
   }
 
@@ -117,7 +131,6 @@ void UseCreditCardAction::OnWaitForElement(const ClientStatus& element_status) {
   delegate_->GetFullCard(credit_card_.get(),
                          base::BindOnce(&UseCreditCardAction::OnGetFullCard,
                                         weak_ptr_factory_.GetWeakPtr()));
-  return;
 }
 
 void UseCreditCardAction::OnGetFullCard(
@@ -128,16 +141,6 @@ void UseCreditCardAction::OnGetFullCard(
     return;
   }
 
-  std::map<int, std::string> fallback_values =
-      autofill_field_formatter::CreateAutofillMappings(*card,
-                                                       /* locale = */ "en-US");
-  fallback_values.emplace(
-      static_cast<int>(AutofillFormatProto::CREDIT_CARD_VERIFICATION_CODE),
-      base::UTF16ToUTF8(cvc));
-  fallback_values.emplace(
-      (int)AutofillFormatProto::CREDIT_CARD_RAW_NUMBER,
-      base::UTF16ToUTF8(card->GetRawInfo(autofill::CREDIT_CARD_NUMBER)));
-
   std::vector<RequiredField> required_fields;
   for (const auto& required_field_proto : proto_.use_card().required_fields()) {
     if (required_field_proto.value_expression().empty()) {
@@ -149,10 +152,27 @@ void UseCreditCardAction::OnGetFullCard(
     required_fields.emplace_back(required_field);
   }
 
+  std::map<int, std::string> fallback_values =
+      autofill_field_formatter::CreateAutofillMappings(*card,
+                                                       /* locale = */ "en-US");
+  fallback_values.emplace(
+      static_cast<int>(AutofillFormatProto::CREDIT_CARD_VERIFICATION_CODE),
+      base::UTF16ToUTF8(cvc));
+  fallback_values.emplace(
+      (int)AutofillFormatProto::CREDIT_CARD_RAW_NUMBER,
+      base::UTF16ToUTF8(card->GetRawInfo(autofill::CREDIT_CARD_NUMBER)));
+
   DCHECK(fallback_handler_ == nullptr);
   fallback_handler_ = std::make_unique<RequiredFieldsFallbackHandler>(
       required_fields, fallback_values, delegate_);
 
+  if (proto_.use_card().skip_autofill()) {
+    fallback_handler_->CheckAndFallbackRequiredFields(
+        OkClientStatus(), base::BindOnce(&UseCreditCardAction::EndAction,
+                                         weak_ptr_factory_.GetWeakPtr()));
+    return;
+  }
+
   delegate_->FillCardForm(std::move(card), cvc, selector_,
                           base::BindOnce(&UseCreditCardAction::OnFormFilled,
                                          weak_ptr_factory_.GetWeakPtr()));

components/autofill_assistant/browser/actions/use_credit_card_action_unittest.cc

@@ -132,6 +132,14 @@ TEST_F(UseCreditCardActionTest, InvalidActionNoSelectorSet) {
   EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action));
 }
 
+TEST_F(UseCreditCardActionTest,
+       InvalidActionnSkipAutofillWithoutRequiredFields) {
+  ActionProto action;
+  auto* use_card = action.mutable_use_card();
+  use_card->set_skip_autofill(true);
+  EXPECT_EQ(ProcessedActionStatusProto::INVALID_ACTION, ProcessAction(action));
+}
+
 TEST_F(UseCreditCardActionTest, PreconditionFailedNoCreditCardInUserData) {
   ActionProto action;
   auto* use_card = action.mutable_use_card();
@@ -380,6 +388,35 @@ TEST_F(UseCreditCardActionTest, ForcedFallbackWithKeystrokes) {
   EXPECT_EQ(ProcessedActionStatusProto::ACTION_APPLIED, ProcessAction(action));
 }
 
+TEST_F(UseCreditCardActionTest, SkippingAutofill) {
+  ON_CALL(mock_action_delegate_, GetElementTag(_, _))
+      .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "INPUT"));
+
+  ActionProto action = CreateUseCreditCardAction();
+  AddRequiredField(&action,
+                   base::NumberToString(static_cast<int>(
+                       AutofillFormatProto::CREDIT_CARD_VERIFICATION_CODE)),
+                   "#cvc");
+  action.mutable_use_card()->set_skip_autofill(true);
+
+  EXPECT_CALL(mock_action_delegate_, OnFillCardForm(_, _, _, _)).Times(0);
+
+  // First validation fails.
+  EXPECT_CALL(mock_web_controller_, OnGetFieldValue(Selector({"#cvc"}), _))
+      .WillOnce(RunOnceCallback<1>(OkClientStatus(), ""));
+  // Fill cvc.
+  Expectation set_cvc =
+      EXPECT_CALL(mock_action_delegate_,
+                  OnSetFieldValue(Selector({"#cvc"}), kFakeCvc, _))
+          .WillOnce(RunOnceCallback<2>(OkClientStatus()));
+  // Second validation succeeds.
+  EXPECT_CALL(mock_web_controller_, OnGetFieldValue(Selector({"#cvc"}), _))
+      .After(set_cvc)
+      .WillOnce(RunOnceCallback<1>(OkClientStatus(), "not empty"));
+
+  EXPECT_EQ(ProcessedActionStatusProto::ACTION_APPLIED, ProcessAction(action));
+}
+
 TEST_F(UseCreditCardActionTest, AutofillFailureWithoutRequiredFieldsIsFatal) {
   ActionProto action_proto = CreateUseCreditCardAction();
 

components/autofill_assistant/browser/service.proto

@@ -884,7 +884,7 @@ message UseAddressProto {
     // The client memory key from which to retrieve the address.
     string name = 1;
     // The client model identifier from which to retrieve the address.
-    string model_identifier = 8;
+    string model_identifier = 9;
   }
 
   // Reference to an element in the form that should be filled.
@@ -893,7 +893,11 @@ message UseAddressProto {
   // An optional list of fields that should be filled by this action.
   repeated RequiredField required_fields = 6;
 
-  reserved 2;
+  // If true, this skips the Autofill step jumping straight to the
+  // |required_fields|.
+  optional bool skip_autofill = 10;
+
+  reserved 2, 7, 8;
 }
 
 // Fill a form with a credit card if there is one stored in client memory,
@@ -949,7 +953,11 @@ message UseCreditCardProto {
 
   repeated RequiredField required_fields = 7;
 
-  reserved 1;
+  // If true, this skips the Autofill step jumping straight to the
+  // |required_fields|.
+  optional bool skip_autofill = 8;
+
+  reserved 1, 5, 6;
 }
 
 // Ask Chrome to wait for an element in the DOM. This can be used to only