Reland "[Autofill Assistant] Add StopCurrentScript() in ActionDelegate."

This is a reland of e195437f

The first version of this change was broken because ActionDelegate::StopCurrentScript() was not abstract (pure virtual).

This was fixed in this CL by adding "= 0"; at the function declaration in action_delegate.h.

Original change's description:
> [Autofill Assistant] Add StopCurrentScript() in ActionDelegate.
>
> Bug: 806868
> Change-Id: Ic3deb3353377e950fadc0d011c75f7ed91ca5366
> Reviewed-on: https://chromium-review.googlesource.com/1250923
> Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
> Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#595445}

Bug: 806868
Change-Id: Ic57f8607844267c98d0ab5ece8a253ae853ec325
Reviewed-on: https://chromium-review.googlesource.com/1256184Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Jordan Demeulenaere <jdemeulenaere@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595826}

components/autofill_assistant/browser/actions/action_delegate.h

@@ -105,6 +105,9 @@ class ActionDelegate {
   // state.
   virtual void Restart() = 0;
 
+  // Stop the current script and show |message| if it is not empty .
+  virtual void StopCurrentScript(const std::string& message) = 0;
+
   // Return the current ClientMemory.
   virtual ClientMemory* GetClientMemory() = 0;
 

components/autofill_assistant/browser/actions/autofill_action.cc

@@ -97,6 +97,8 @@ AutofillAction::AutofillAction(const ActionProto& proto)
          proto.use_address().form_field_element().selectors()) {
       selectors_.emplace_back(selector);
     }
+    fill_form_message_ = proto.use_address().strings().fill_form();
+    check_form_message_ = proto.use_address().strings().check_form();
   } else {
     DCHECK(proto.has_use_card());
     is_autofill_card_ = true;
@@ -106,6 +108,8 @@ AutofillAction::AutofillAction(const ActionProto& proto)
          proto.use_card().form_field_element().selectors()) {
       selectors_.emplace_back(selector);
     }
+    fill_form_message_ = proto.use_card().strings().fill_form();
+    check_form_message_ = proto.use_card().strings().check_form();
   }
 }
 
@@ -127,19 +131,14 @@ void AutofillAction::ProcessAction(ActionDelegate* delegate,
     const std::string& guid = selected_data.value();
     if (guid.empty()) {
       // User selected 'Fill manually'.
-      // TODO(crbug.com/806868): We need to differentiate between action failure
-      // and stopping an action to let the user fill a form (expected stop).
-      UpdateProcessedAction(false);
-      std::move(process_action_callback_)
-          .Run(std::move(processed_action_proto_));
+      delegate->StopCurrentScript(fill_form_message_);
+      EndAction(/* successful= */ true);
       return;
     }
 
     if (selectors_.empty()) {
       // If there is no selector, finish the action directly.
-      UpdateProcessedAction(true);
-      std::move(process_action_callback_)
-          .Run(std::move(processed_action_proto_));
+      EndAction(/* successful= */ true);
       return;
     }
 
@@ -164,6 +163,11 @@ void AutofillAction::ProcessAction(ActionDelegate* delegate,
   delegate->ChooseAddress(std::move(selection_callback));
 }
 
+void AutofillAction::EndAction(bool successful) {
+  UpdateProcessedAction(successful);
+  std::move(process_action_callback_).Run(std::move(processed_action_proto_));
+}
+
 void AutofillAction::OnDataSelected(ActionDelegate* delegate,
                                     const std::string& guid) {
   // Remember the selection.
@@ -177,21 +181,17 @@ void AutofillAction::OnDataSelected(ActionDelegate* delegate,
     // If there is no selector, finish the action directly. This can be the case
     // when we want to trigger the selection of address or card at the beginning
     // of the script and use it later.
-    UpdateProcessedAction(true);
-    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
+    EndAction(/* successful= */ true);
     return;
   }
 
   if (guid.empty()) {
     // User selected 'Fill manually'.
-    // TODO(crbug.com/806868): We need to differentiate between action failure
-    // and stopping an action to let the user fill a form (expected stop).
-    UpdateProcessedAction(false);
-    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
+    delegate->StopCurrentScript(fill_form_message_);
+    EndAction(/* successful= */ true);
     return;
   }
 
-  // TODO(crbug.com/806868): Validate form and use fallback if needed.
   FillFormWithData(guid, delegate);
 }
 
@@ -238,11 +238,9 @@ void AutofillAction::OnGetFullCard(ActionDelegate* delegate,
 void AutofillAction::OnFormFilled(const std::string& guid,
                                   ActionDelegate* delegate,
                                   bool successful) {
-  // In case Autofill failed, we stop the action.
+  // In case Autofill failed, we fail the action.
   if (!successful) {
-    UpdateProcessedAction(false);
-    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
-    // TODO(crbug.com/806868): Tell the user to fill the form manually.
+    EndAction(/* successful= */ false);
     return;
   }
 
@@ -254,15 +252,13 @@ void AutofillAction::CheckRequiredFields(const std::string& guid,
                                          bool allow_fallback) {
   if (is_autofill_card_) {
     // TODO(crbug.com/806868): Implement required fields checking for cards.
-    UpdateProcessedAction(true);
-    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
+    EndAction(/* successful= */ true);
     return;
   }
 
   // If there are no required fields, finish the action successfully.
   if (proto_.use_address().required_fields().empty()) {
-    UpdateProcessedAction(true);
-    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
+    EndAction(/* successful= */ true);
     return;
   }
 
@@ -313,12 +309,10 @@ void AutofillAction::OnGetRequiredFieldValue(
   for (size_t i = 0; i < required_fields_value_status_.size(); i++) {
     if (required_fields_value_status_[i] == EMPTY) {
       if (!allow_fallback) {
-        // Validation failed and we don't want to try the fallback, so we fail
-        // the action.
-        UpdateProcessedAction(false);
-        std::move(process_action_callback_)
-            .Run(std::move(processed_action_proto_));
-        // TODO(crbug.com/806868): Tell the user to fill the form manually.
+        // Validation failed and we don't want to try the fallback, so we stop
+        // the script.
+        delegate->StopCurrentScript(check_form_message_);
+        EndAction(/* successful= */ true);
         return;
       }
 
@@ -342,17 +336,15 @@ void AutofillAction::OnGetRequiredFieldValue(
   DCHECK_EQ(failed_selectors.size(), fallback_values.size());
 
   if (validation_successful) {
-    UpdateProcessedAction(true);
-    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
+    EndAction(/* successful= */ true);
     return;
   }
 
   if (fallback_values.empty()) {
     // One or more required fields is empty but there is no fallback value, so
-    // we fail the action.
-    UpdateProcessedAction(false);
-    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
-    // TODO(crbug.com/806868): Tell the user to fill the form manually.
+    // we stop the script.
+    delegate->StopCurrentScript(check_form_message_);
+    EndAction(/* successful= */ true);
     return;
   }
 
@@ -413,12 +405,10 @@ void AutofillAction::OnSetFieldValue(const std::string& guid,
   // We can ignore the other SetFieldValue callbacks given that an action is
   // processed only once.
   if (!successful) {
-    // Fallback failed: we fail the action without checking the fields.
+    // Fallback failed: we stop the script without checking the fields.
     if (process_action_callback_) {
-      UpdateProcessedAction(false);
-      std::move(process_action_callback_)
-          .Run(std::move(processed_action_proto_));
-      // TODO(crbug.com/806868): Tell the user to fill the form manually.
+      delegate->StopCurrentScript(check_form_message_);
+      EndAction(/* successful= */ true);
     }
     return;
   }

components/autofill_assistant/browser/actions/autofill_action.h

@@ -78,10 +78,14 @@ class AutofillAction : public Action {
                        ActionDelegate* delegate,
                        bool successful);
 
+  void EndAction(bool successful);
+
   // Usage of the autofilled address. Ignored if autofilling a card.
   std::string name_;
   std::string prompt_;
   std::vector<std::string> selectors_;
+  std::string fill_form_message_;
+  std::string check_form_message_;
 
   // True if autofilling a card, otherwise we are autofilling an address.
   bool is_autofill_card_;

components/autofill_assistant/browser/actions/autofill_action_unittest.cc

@@ -112,19 +112,25 @@ class AutofillActionTest : public testing::Test {
   const char* const kFirstName = "Foo";
   const char* const kLastName = "Bar";
   const char* const kEmail = "foobar@gmail.com";
+  const char* const kFillForm = "fill_form";
+  const char* const kCheckForm = "check_form";
 
   ActionProto CreateUseAddressAction() {
     ActionProto action;
     UseAddressProto* use_address = action.mutable_use_address();
     use_address->set_name(kAddressName);
     use_address->mutable_form_field_element()->add_selectors(kFakeSelector);
+    use_address->mutable_strings()->set_fill_form(kFillForm);
+    use_address->mutable_strings()->set_check_form(kCheckForm);
     return action;
   }
 
   ActionProto CreateUseCardAction() {
     ActionProto action;
-    action.mutable_use_card()->mutable_form_field_element()->add_selectors(
-        kFakeSelector);
+    UseCreditCardProto* use_card = action.mutable_use_card();
+    use_card->mutable_form_field_element()->add_selectors(kFakeSelector);
+    use_card->mutable_strings()->set_fill_form(kFillForm);
+    use_card->mutable_strings()->set_check_form(kCheckForm);
     return action;
   }
 
@@ -138,6 +144,15 @@ class AutofillActionTest : public testing::Test {
            ProcessedActionStatusProto::ACTION_APPLIED;
   }
 
+  void ExpectActionToStopScript(const ActionProto& action_proto,
+                                const std::string& expected_message) {
+    EXPECT_CALL(mock_action_delegate_, StopCurrentScript(expected_message));
+
+    // The AutofillAction should finish successfully even when stopping the
+    // current script.
+    EXPECT_TRUE(ProcessAction(action_proto));
+  }
+
   MockActionDelegate mock_action_delegate_;
   MockClientMemory mock_client_memory_;
   std::string autofill_profile_guid_;
@@ -164,7 +179,7 @@ TEST_F(AutofillActionTest, FillManually) {
   // We save the selection in memory.
   EXPECT_CALL(mock_client_memory_, set_selected_address(kAddressName, ""));
 
-  EXPECT_FALSE(ProcessAction(action_proto));
+  ExpectActionToStopScript(action_proto, kFillForm);
 }
 
 TEST_F(AutofillActionTest, ValidationSucceeds) {
@@ -237,7 +252,7 @@ TEST_F(AutofillActionTest, FallbackFails) {
               OnSetFieldValue(ElementsAre(kFakeSelector), kFirstName, _))
       .WillOnce(RunOnceCallback<2>(false));
 
-  EXPECT_FALSE(ProcessAction(action_proto));
+  ExpectActionToStopScript(action_proto, kCheckForm);
 }
 
 TEST_F(AutofillActionTest, FallbackSucceeds) {

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -104,6 +104,7 @@ class MockActionDelegate : public ActionDelegate {
   MOCK_METHOD0(GetClientMemory, ClientMemory*());
   MOCK_METHOD0(GetPersonalDataManager, autofill::PersonalDataManager*());
   MOCK_METHOD0(GetWebContents, content::WebContents*());
+  MOCK_METHOD1(StopCurrentScript, void(const std::string& message));
 };
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/script_executor.cc

@@ -24,6 +24,7 @@ ScriptExecutor::ScriptExecutor(const std::string& script_path,
     : script_path_(script_path),
       delegate_(delegate),
       at_end_(CONTINUE),
+      should_stop_script_(false),
       weak_ptr_factory_(this) {
   DCHECK(delegate_);
 }
@@ -122,6 +123,13 @@ void ScriptExecutor::Restart() {
   at_end_ = RESTART;
 }
 
+void ScriptExecutor::StopCurrentScript(const std::string& message) {
+  if (!message.empty()) {
+    delegate_->GetUiController()->ShowStatusMessage(message);
+  }
+  should_stop_script_ = true;
+}
+
 ClientMemory* ScriptExecutor::GetClientMemory() {
   return delegate_->GetClientMemory();
 }
@@ -212,6 +220,14 @@ void ScriptExecutor::OnProcessedAction(
     GetNextActions();
     return;
   }
+
+  if (should_stop_script_) {
+    // Last action called StopCurrentScript(). We simulate a successful end of
+    // script to make sure we don't display any errors.
+    RunCallback(true);
+    return;
+  }
+
   ProcessNextAction();
 }
 

components/autofill_assistant/browser/script_executor.h

@@ -85,6 +85,7 @@ class ScriptExecutor : public ActionDelegate {
   ClientMemory* GetClientMemory() override;
   autofill::PersonalDataManager* GetPersonalDataManager() override;
   content::WebContents* GetWebContents() override;
+  void StopCurrentScript(const std::string& message) override;
 
  private:
   void OnGetActions(bool result, const std::string& response);
@@ -102,6 +103,7 @@ class ScriptExecutor : public ActionDelegate {
   std::vector<ProcessedActionProto> processed_actions_;
   std::string last_server_payload_;
   AtEnd at_end_;
+  bool should_stop_script_;
 
   base::WeakPtrFactory<ScriptExecutor> weak_ptr_factory_;
   DISALLOW_COPY_AND_ASSIGN(ScriptExecutor);

components/autofill_assistant/browser/service.proto

@@ -289,6 +289,14 @@ message FocusElementProto {
   optional string title = 2;
 }
 
+message AutofillStrings {
+  optional string fill_manually = 1;
+
+  optional string fill_form = 2;
+
+  optional string check_form = 3;
+}
+
 // Fill a form with an address if there is, otherwise fail this action.
 message UseAddressProto {
   // Message used to indicate what form fields should be filled with what
@@ -328,6 +336,8 @@ message UseAddressProto {
 
   // An optional list of fields that should be filled by this action.
   repeated RequiredField required_fields = 6;
+
+  optional AutofillStrings strings = 8;
 }
 
 // Fill a form with a credit card if there is, otherwise fail this action.
@@ -338,6 +348,8 @@ message UseCreditCardProto {
 
   // A reference to the card number field in the form that should be filled.
   optional ElementReferenceProto form_field_element = 3;
+
+  optional AutofillStrings strings = 8;
 }
 
 // Ask Chrome to wait for an element in the DOM. This can be used to only