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

This reverts commit e195437f.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 595445 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2UxOTU0MzdmMzBiZjMzZWI0OWIyZTM4MzhmZjFhYmI1ZjUyY2ZkZGEM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Android%20CFI/3124

Sample Failed Step: compile

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}

Change-Id: Iefb7112349dd74ca20ff2190ed661f3c6098f89b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 806868
Reviewed-on: https://chromium-review.googlesource.com/1255502
Cr-Commit-Position: refs/heads/master@{#595480}

components/autofill_assistant/browser/actions/action_delegate.h

@@ -103,9 +103,6 @@ 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);
-
   // Return the current ClientMemory.
   virtual ClientMemory* GetClientMemory() = 0;
 

components/autofill_assistant/browser/actions/autofill_action.cc

@@ -27,8 +27,6 @@ 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;
@@ -38,8 +36,6 @@ 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();
   }
 }
 
@@ -61,14 +57,19 @@ void AutofillAction::ProcessAction(ActionDelegate* delegate,
     const std::string& guid = selected_data.value();
     if (guid.empty()) {
       // User selected 'Fill manually'.
-      delegate->StopCurrentScript(fill_form_message_);
-      EndAction(/* successful= */ true);
+      // 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_));
       return;
     }
 
     if (selectors_.empty()) {
       // If there is no selector, finish the action directly.
-      EndAction(/* successful= */ true);
+      UpdateProcessedAction(true);
+      std::move(process_action_callback_)
+          .Run(std::move(processed_action_proto_));
       return;
     }
 
@@ -93,11 +94,6 @@ 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.
@@ -111,17 +107,21 @@ 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.
-    EndAction(/* successful= */ true);
+    UpdateProcessedAction(true);
+    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
     return;
   }
 
   if (guid.empty()) {
     // User selected 'Fill manually'.
-    delegate->StopCurrentScript(fill_form_message_);
-    EndAction(/* successful= */ true);
+    // 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_));
     return;
   }
 
+  // TODO(crbug.com/806868): Validate form and use fallback if needed.
   FillFormWithData(guid, delegate);
 }
 
@@ -145,9 +145,11 @@ void AutofillAction::FillFormWithData(const std::string& guid,
 void AutofillAction::OnFormFilled(const std::string& guid,
                                   ActionDelegate* delegate,
                                   bool successful) {
-  // In case Autofill failed, we fail the action.
+  // In case Autofill failed, we stop the action.
   if (!successful) {
-    EndAction(/* successful= */ false);
+    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.
     return;
   }
 
@@ -159,13 +161,15 @@ void AutofillAction::CheckRequiredFields(const std::string& guid,
                                          bool allow_fallback) {
   if (is_autofill_card_) {
     // TODO(crbug.com/806868): Implement required fields checking for cards.
-    EndAction(/* successful= */ true);
+    UpdateProcessedAction(true);
+    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
     return;
   }
 
   // If there are no required fields, finish the action successfully.
   if (proto_.use_address().required_fields().empty()) {
-    EndAction(/* successful= */ true);
+    UpdateProcessedAction(true);
+    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
     return;
   }
 
@@ -215,10 +219,12 @@ 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 stop
-        // the script.
-        delegate->StopCurrentScript(check_form_message_);
-        EndAction(/* successful= */ true);
+        // 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.
         return;
       }
 
@@ -242,15 +248,17 @@ void AutofillAction::OnGetRequiredFieldValue(
   DCHECK_EQ(failed_selectors.size(), fallback_values.size());
 
   if (validation_successful) {
-    EndAction(/* successful= */ true);
+    UpdateProcessedAction(true);
+    std::move(process_action_callback_).Run(std::move(processed_action_proto_));
     return;
   }
 
   if (fallback_values.empty()) {
     // One or more required fields is empty but there is no fallback value, so
-    // we stop the script.
-    delegate->StopCurrentScript(check_form_message_);
-    EndAction(/* successful= */ true);
+    // 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.
     return;
   }
 
@@ -311,10 +319,12 @@ 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 stop the script without checking the fields.
+    // Fallback failed: we fail the action without checking the fields.
     if (process_action_callback_) {
-      delegate->StopCurrentScript(check_form_message_);
-      EndAction(/* successful= */ true);
+      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.
     }
     return;
   }

components/autofill_assistant/browser/actions/autofill_action.h

@@ -72,14 +72,10 @@ 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

@@ -74,25 +74,19 @@ 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;
-    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);
+    action.mutable_use_card()->mutable_form_field_element()->add_selectors(
+        kFakeSelector);
     return action;
   }
 
@@ -106,15 +100,6 @@ class AutofillActionTest : public testing::Test {
            ProcessedActionStatus::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_;
   autofill::AutofillProfile autofill_profile_;
@@ -140,7 +125,7 @@ TEST_F(AutofillActionTest, FillManually) {
   // We save the selection in memory.
   EXPECT_CALL(mock_client_memory_, set_selected_address(kAddressName, ""));
 
-  ExpectActionToStopScript(action_proto, kFillForm);
+  EXPECT_FALSE(ProcessAction(action_proto));
 }
 
 TEST_F(AutofillActionTest, FillCardFormFails) {
@@ -248,7 +233,7 @@ TEST_F(AutofillActionTest, FallbackFails) {
               OnSetFieldValue(ElementsAre(kFakeSelector), kFirstName, _))
       .WillOnce(RunOnceCallback<2>(false));
 
-  ExpectActionToStopScript(action_proto, kCheckForm);
+  EXPECT_FALSE(ProcessAction(action_proto));
 }
 
 TEST_F(AutofillActionTest, FallbackSucceeds) {

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -101,7 +101,6 @@ class MockActionDelegate : public ActionDelegate {
   MOCK_METHOD1(LoadURL, void(const GURL& url));
   MOCK_METHOD0(Shutdown, void());
   MOCK_METHOD0(Restart, void());
-  MOCK_METHOD1(StopCurrentScript, void(const std::string& message));
   MOCK_METHOD0(GetClientMemory, ClientMemory*());
 };
 

components/autofill_assistant/browser/script_executor.cc

@@ -24,7 +24,6 @@ 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_);
 }
@@ -128,13 +127,6 @@ 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();
 }
@@ -217,14 +209,6 @@ 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

@@ -84,7 +84,6 @@ class ScriptExecutor : public ActionDelegate {
   void Shutdown() override;
   void Restart() override;
   ClientMemory* GetClientMemory() override;
-  void StopCurrentScript(const std::string& message) override;
 
  private:
   void OnGetActions(bool result, const std::string& response);
@@ -102,7 +101,6 @@ 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,14 +289,6 @@ 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
@@ -336,8 +328,6 @@ 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.
@@ -348,8 +338,6 @@ 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