[Autofill Assistant] Re-introduce the wait for Use*Action

This re-introduced the ShortWaitForElement for the Use*Action, if the
|form_field_element| is specified. This is not a hard restriction, if
|skip_autofill| is true, the element does not need to be specified. If
it's not, the |RequiredFieldsFallbackHandler| will be called
immediately without waiting.

Bug: none
Change-Id: I269ac0735a060357a6679e66fdf41f7a315f402a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2228720Reviewed-by: Clemens Arbesser <arbesser@google.com>
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#774633}

components/autofill_assistant/browser/actions/use_address_action.cc

@@ -127,6 +127,36 @@ void UseAddressAction::EndAction(
 }
 
 void UseAddressAction::FillFormWithData() {
+  if (selector_.empty()) {
+    DCHECK(proto_.use_address().skip_autofill());
+    OnWaitForElement(OkClientStatus());
+    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;
+  }
+
+  if (proto_.use_address().skip_autofill()) {
+    ExecuteFallback(OkClientStatus());
+    return;
+  }
+
+  DCHECK(!selector_.empty());
+  DCHECK(profile_ != nullptr);
+  delegate_->FillAddressForm(profile_.get(), selector_,
+                             base::BindOnce(&UseAddressAction::ExecuteFallback,
+                                            weak_ptr_factory_.GetWeakPtr()));
+}
+
+void UseAddressAction::ExecuteFallback(const ClientStatus& status) {
   DCHECK(profile_ != nullptr);
   std::vector<RequiredField> required_fields;
   for (const auto& required_field_proto :
@@ -147,32 +177,6 @@ void UseAddressAction::FillFormWithData() {
                                                        /* 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()));
-}
-
-void UseAddressAction::OnFormFilled(const ClientStatus& status) {
-  DCHECK(fallback_handler_ != nullptr);
   fallback_handler_->CheckAndFallbackRequiredFields(
       status, base::BindOnce(&UseAddressAction::EndAction,
                              weak_ptr_factory_.GetWeakPtr()));

components/autofill_assistant/browser/actions/use_address_action.h

@@ -44,7 +44,7 @@ class UseAddressAction : public Action {
   void OnWaitForElement(const ClientStatus& element_status);
 
   // Called when the address has been filled.
-  void OnFormFilled(const ClientStatus& status);
+  void ExecuteFallback(const ClientStatus& status);
 
   // Note: |fallback_handler_| must be a member, because checking for fallbacks
   // is asynchronous and the existence of the handler must be ensured.

components/autofill_assistant/browser/actions/use_address_action_unittest.cc

@@ -516,6 +516,7 @@ TEST_F(UseAddressActionTest, SkippingAutofill) {
                    "#first_name");
   action_proto.mutable_use_address()->set_skip_autofill(true);
 
+  EXPECT_CALL(mock_action_delegate_, OnShortWaitForElement(_, _)).Times(0);
   EXPECT_CALL(mock_action_delegate_, OnFillAddressForm(_, _, _)).Times(0);
 
   // First validation fails.

components/autofill_assistant/browser/actions/use_credit_card_action.cc

@@ -108,11 +108,9 @@ 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()));
+  if (selector_.empty()) {
+    DCHECK(proto_.use_card().skip_autofill());
+    OnWaitForElement(OkClientStatus());
     return;
   }
 
@@ -167,18 +165,17 @@ void UseCreditCardAction::OnGetFullCard(
       required_fields, fallback_values, delegate_);
 
   if (proto_.use_card().skip_autofill()) {
-    fallback_handler_->CheckAndFallbackRequiredFields(
-        OkClientStatus(), base::BindOnce(&UseCreditCardAction::EndAction,
-                                         weak_ptr_factory_.GetWeakPtr()));
+    ExecuteFallback(OkClientStatus());
     return;
   }
 
+  DCHECK(!selector_.empty());
   delegate_->FillCardForm(std::move(card), cvc, selector_,
-                          base::BindOnce(&UseCreditCardAction::OnFormFilled,
+                          base::BindOnce(&UseCreditCardAction::ExecuteFallback,
                                          weak_ptr_factory_.GetWeakPtr()));
 }
 
-void UseCreditCardAction::OnFormFilled(const ClientStatus& status) {
+void UseCreditCardAction::ExecuteFallback(const ClientStatus& status) {
   DCHECK(fallback_handler_ != nullptr);
   fallback_handler_->CheckAndFallbackRequiredFields(
       status, base::BindOnce(&UseCreditCardAction::EndAction,

components/autofill_assistant/browser/actions/use_credit_card_action.h

@@ -48,7 +48,7 @@ class UseCreditCardAction : public Action {
                      const base::string16& cvc);
 
   // Called when the form credit card has been filled.
-  void OnFormFilled(const ClientStatus& status);
+  void ExecuteFallback(const ClientStatus& status);
 
   // Note: |fallback_handler_| must be a member, because checking for fallbacks
   // is asynchronous and the existence of the handler must be ensured.

components/autofill_assistant/browser/actions/use_credit_card_action_unittest.cc

@@ -392,13 +392,14 @@ TEST_F(UseCreditCardActionTest, SkippingAutofill) {
   ON_CALL(mock_action_delegate_, GetElementTag(_, _))
       .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "INPUT"));
 
-  ActionProto action = CreateUseCreditCardAction();
+  ActionProto action;
   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_, OnShortWaitForElement(_, _)).Times(0);
   EXPECT_CALL(mock_action_delegate_, OnFillCardForm(_, _, _, _)).Times(0);
 
   // First validation fails.