[Autofill Assistant] Add support for interrupts.

This change introduces support for interrupts.

Interrupts can be run as part of wait_for_dom with
allow_interrupts=true.

Interrupts can also run manually, by clicking on a chip, but only if
they have a name.

Interrupts can run automatically if they have autostart=true. Autostart
interrupts are handled differently from normal autostart, as they can
happen at any time. Running an autostart interrupt does not prevent
runnig a normal interrupt.

I tried to keep changes to the main, non-interrupt, path and
refactorings to a minimum in this change.

The result puts all the complexity into the script executor. I'd like to
get it to work, then find a good code split in a follow-up change. The
code has resisted all my attempts at reorganization so far...

Change to the main, non-interrupt path:

 - The responsibility for updating the script status is moved to
 ScriptExecutor, even though the script status map stays in
 ScriptTracker.This allows tracking script state changes caused by
 interrupts running.

 - The WaitForDom now uses the same wait code as
 ActionDelegate::WaitForElement instead of rolling up its own

Limitations:

 - The payload from interrupts is never sent back to the server. This
 doesn't allow interrupts to change variables

 - If an interrupt fails, the failure of the main script is not
 reported to the server; from the server's point of view, the main
 script just never finishes.

 - There's no "prompt" action yet, so interrupts cannot ask questions.

Bug: 806868
Change-Id: I12165965783ba479688c4c5b44ab72477ad4ae2b
Reviewed-on: https://chromium-review.googlesource.com/c/1340309
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610382}

components/autofill_assistant/browser/actions/action_delegate.h

@@ -52,8 +52,20 @@ class ActionDelegate {
   //
   // TODO(crbug.com/806868): Consider embedding that wait right into
   // WebController and eliminate double-lookup.
-  virtual void WaitForElement(const std::vector<std::string>& selectors,
-                              base::OnceCallback<void(bool)> callback) = 0;
+  virtual void ShortWaitForElementExist(
+      const std::vector<std::string>& selectors,
+      base::OnceCallback<void(bool)> callback) = 0;
+
+  // Wait for up to |max_wait_time| for the element |selectors| to be visible on
+  // the page, then call |callback| with true if the element was visible, false
+  // otherwise.
+  //
+  // If |allow_interrupt| interrupts can run while waiting.
+  virtual void WaitForElementVisible(
+      base::TimeDelta max_wait_time,
+      bool allow_interrupt,
+      const std::vector<std::string>& selectors,
+      base::OnceCallback<void(bool)> callback) = 0;
 
   // Click or tap the element given by |selectors| on the web page.
   virtual void ClickOrTapElement(const std::vector<std::string>& selectors,

components/autofill_assistant/browser/actions/autofill_action.cc

@@ -212,10 +212,10 @@ void AutofillAction::OnDataSelected(ActionDelegate* delegate,
 }
 
 void AutofillAction::FillFormWithData(ActionDelegate* delegate) {
-  delegate->WaitForElement(selectors_,
-                           base::BindOnce(&AutofillAction::OnWaitForElement,
-                                          weak_ptr_factory_.GetWeakPtr(),
-                                          base::Unretained(delegate)));
+  delegate->ShortWaitForElementExist(
+      selectors_, base::BindOnce(&AutofillAction::OnWaitForElement,
+                                 weak_ptr_factory_.GetWeakPtr(),
+                                 base::Unretained(delegate)));
 }
 
 void AutofillAction::OnWaitForElement(ActionDelegate* delegate,

components/autofill_assistant/browser/actions/autofill_action_unittest.cc

@@ -114,7 +114,7 @@ class AutofillActionTest : public testing::Test {
         .WillByDefault(Invoke([this]() {
           return std::make_unique<BatchElementChecker>(&mock_web_controller_);
         }));
-    ON_CALL(mock_action_delegate_, OnWaitForElement(_, _))
+    ON_CALL(mock_action_delegate_, OnShortWaitForElementExist(_, _))
         .WillByDefault(RunOnceCallback<1>(true));
   }
 

components/autofill_assistant/browser/actions/click_action.cc

@@ -22,7 +22,7 @@ ClickAction::~ClickAction() {}
 void ClickAction::InternalProcessAction(ActionDelegate* delegate,
                                         ProcessActionCallback callback) {
   DCHECK_GT(proto_.click().element_to_click().selectors_size(), 0);
-  delegate->WaitForElement(
+  delegate->ShortWaitForElementExist(
       ExtractVector(proto_.click().element_to_click().selectors()),
       base::BindOnce(&ClickAction::OnWaitForElement,
                      weak_ptr_factory_.GetWeakPtr(), base::Unretained(delegate),

components/autofill_assistant/browser/actions/focus_element_action.cc

@@ -28,7 +28,7 @@ void FocusElementAction::InternalProcessAction(ActionDelegate* delegate,
   if (!focus_element.title().empty()) {
     delegate->ShowStatusMessage(focus_element.title());
   }
-  delegate->WaitForElement(
+  delegate->ShortWaitForElementExist(
       ExtractVector(focus_element.element().selectors()),
       base::BindOnce(&FocusElementAction::OnWaitForElement,
                      weak_ptr_factory_.GetWeakPtr(), base::Unretained(delegate),

components/autofill_assistant/browser/actions/highlight_element_action.cc

@@ -23,7 +23,7 @@ void HighlightElementAction::InternalProcessAction(
     ActionDelegate* delegate,
     ProcessActionCallback callback) {
   DCHECK_GT(proto_.highlight_element().element().selectors_size(), 0);
-  delegate->WaitForElement(
+  delegate->ShortWaitForElementExist(
       ExtractVector(proto_.highlight_element().element().selectors()),
       base::BindOnce(&HighlightElementAction::OnWaitForElement,
                      weak_ptr_factory_.GetWeakPtr(), base::Unretained(delegate),

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -24,15 +24,30 @@ class MockActionDelegate : public ActionDelegate {
   MOCK_METHOD0(CreateBatchElementChecker,
                std::unique_ptr<BatchElementChecker>());
 
-  void WaitForElement(const std::vector<std::string>& selectors,
-                      base::OnceCallback<void(bool)> callback) {
-    OnWaitForElement(selectors, callback);
+  void ShortWaitForElementExist(
+      const std::vector<std::string>& selectors,
+      base::OnceCallback<void(bool)> callback) override {
+    OnShortWaitForElementExist(selectors, callback);
   }
 
-  MOCK_METHOD2(OnWaitForElement,
+  MOCK_METHOD2(OnShortWaitForElementExist,
                void(const std::vector<std::string>&,
                     base::OnceCallback<void(bool)>&));
 
+  void WaitForElementVisible(base::TimeDelta max_wait_time,
+                             bool allow_interrupt,
+                             const std::vector<std::string>& selectors,
+                             base::OnceCallback<void(bool)> callback) override {
+    OnWaitForElementVisible(max_wait_time, allow_interrupt, selectors,
+                            callback);
+  }
+
+  MOCK_METHOD4(OnWaitForElementVisible,
+               void(base::TimeDelta,
+                    bool,
+                    const std::vector<std::string>&,
+                    base::OnceCallback<void(bool)>&));
+
   MOCK_METHOD1(ShowStatusMessage, void(const std::string& message));
   MOCK_METHOD2(ClickOrTapElement,
                void(const std::vector<std::string>& selectors,

components/autofill_assistant/browser/actions/select_option_action.cc

@@ -27,7 +27,7 @@ void SelectOptionAction::InternalProcessAction(ActionDelegate* delegate,
   DCHECK(select_option.has_selected_option());
   DCHECK_GT(select_option.element().selectors_size(), 0);
 
-  delegate->WaitForElement(
+  delegate->ShortWaitForElementExist(
       ExtractVector(select_option.element().selectors()),
       base::BindOnce(&SelectOptionAction::OnWaitForElement,
                      weak_ptr_factory_.GetWeakPtr(), base::Unretained(delegate),

components/autofill_assistant/browser/actions/set_attribute_action.cc

@@ -22,7 +22,7 @@ SetAttributeAction::~SetAttributeAction() {}
 
 void SetAttributeAction::InternalProcessAction(ActionDelegate* delegate,
                                                ProcessActionCallback callback) {
-  delegate->WaitForElement(
+  delegate->ShortWaitForElementExist(
       ExtractVector(proto_.set_attribute().element().selectors()),
       base::BindOnce(&SetAttributeAction::OnWaitForElement,
                      weak_ptr_factory_.GetWeakPtr(), base::Unretained(delegate),

components/autofill_assistant/browser/actions/set_form_field_value_action.cc

@@ -25,7 +25,7 @@ SetFormFieldValueAction::~SetFormFieldValueAction() {}
 void SetFormFieldValueAction::InternalProcessAction(
     ActionDelegate* delegate,
     ProcessActionCallback callback) {
-  delegate->WaitForElement(
+  delegate->ShortWaitForElementExist(
       ExtractVector(proto_.set_form_value().element().selectors()),
       base::BindOnce(&SetFormFieldValueAction::OnWaitForElement,
                      weak_ptr_factory_.GetWeakPtr(), base::Unretained(delegate),

components/autofill_assistant/browser/actions/upload_dom_action.cc

@@ -22,7 +22,7 @@ UploadDomAction::~UploadDomAction() {}
 void UploadDomAction::InternalProcessAction(ActionDelegate* delegate,
                                             ProcessActionCallback callback) {
   DCHECK_GT(proto_.upload_dom().tree_root().selectors_size(), 0);
-  delegate->WaitForElement(
+  delegate->ShortWaitForElementExist(
       ExtractVector(proto_.upload_dom().tree_root().selectors()),
       base::BindOnce(&UploadDomAction::OnWaitForElement,
                      weak_ptr_factory_.GetWeakPtr(), base::Unretained(delegate),

components/autofill_assistant/browser/actions/wait_for_dom_action.cc

@@ -9,7 +9,6 @@
 #include <utility>
 
 #include "base/bind.h"
-#include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/time/time.h"
 #include "components/autofill_assistant/browser/actions/action_delegate.h"
@@ -21,36 +20,30 @@ static constexpr base::TimeDelta kDefaultCheckDuration =
 
 namespace autofill_assistant {
 
-WaitForDomAction::WaitForDomAction(const ActionProto& proto) : Action(proto) {}
+WaitForDomAction::WaitForDomAction(const ActionProto& proto)
+    : Action(proto), weak_ptr_factory_(this) {}
 
 WaitForDomAction::~WaitForDomAction() {}
 
 void WaitForDomAction::InternalProcessAction(ActionDelegate* delegate,
                                              ProcessActionCallback callback) {
   DCHECK_GT(proto_.wait_for_dom().selectors_size(), 0);
-  batch_element_checker_ = delegate->CreateBatchElementChecker();
-  batch_element_checker_->AddElementCheck(
-      kVisibilityCheck, ExtractVector(proto_.wait_for_dom().selectors()),
-      base::DoNothing());
-
-  base::TimeDelta duration = kDefaultCheckDuration;
+  base::TimeDelta max_wait_time = kDefaultCheckDuration;
   int timeout_ms = proto_.wait_for_dom().timeout_ms();
   if (timeout_ms > 0)
-    duration = base::TimeDelta::FromMilliseconds(timeout_ms);
+    max_wait_time = base::TimeDelta::FromMilliseconds(timeout_ms);
 
-  batch_element_checker_->Run(
-      duration,
-      /* try_done= */ base::DoNothing(),
-      /* all_done= */
+  delegate->WaitForElementVisible(
+      max_wait_time, proto_.wait_for_dom().allow_interrupt(),
+      ExtractVector(proto_.wait_for_dom().selectors()),
       base::BindOnce(&WaitForDomAction::OnCheckDone,
-                     // batch_element_checker_ is owned by this
-                     base::Unretained(this), std::move(callback)));
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void WaitForDomAction::OnCheckDone(ProcessActionCallback callback) {
-  UpdateProcessedAction(batch_element_checker_->all_found()
-                            ? ACTION_APPLIED
-                            : ELEMENT_RESOLUTION_FAILED);
+void WaitForDomAction::OnCheckDone(ProcessActionCallback callback,
+                                   bool element_found) {
+  UpdateProcessedAction(element_found ? ACTION_APPLIED
+                                      : ELEMENT_RESOLUTION_FAILED);
   std::move(callback).Run(std::move(processed_action_proto_));
 }
 }  // namespace autofill_assistant

components/autofill_assistant/browser/actions/wait_for_dom_action.h

@@ -10,8 +10,8 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "components/autofill_assistant/browser/actions/action.h"
-#include "components/autofill_assistant/browser/batch_element_checker.h"
 #include "components/autofill_assistant/browser/service.pb.h"
 
 namespace autofill_assistant {
@@ -26,9 +26,10 @@ class WaitForDomAction : public Action {
   void InternalProcessAction(ActionDelegate* delegate,
                              ProcessActionCallback callback) override;
 
-  void OnCheckDone(ProcessActionCallback callback);
+  void OnCheckDone(ProcessActionCallback callback, bool element_found);
+
+  base::WeakPtrFactory<WaitForDomAction> weak_ptr_factory_;
 
-  std::unique_ptr<BatchElementChecker> batch_element_checker_;
   DISALLOW_COPY_AND_ASSIGN(WaitForDomAction);
 };
 

components/autofill_assistant/browser/controller.cc

@@ -242,9 +242,29 @@ void Controller::OnGetScripts(const GURL& url,
   StartPeriodicScriptChecks();
 }
 
+void Controller::ExecuteScript(const std::string& script_path) {
+  DCHECK(!script_tracker_->running());
+
+  GetUiController()->ShowOverlay();
+  touchable_element_area_.ClearElements();
+
+  StopPeriodicScriptChecks();
+  // Runnable scripts will be checked and reported if necessary after executing
+  // the script.
+  script_tracker_->ClearRunnableScripts();
+  GetUiController()->UpdateScripts({});  // Clear scripts.
+  // TODO(crbug.com/806868): Consider making ClearRunnableScripts part of
+  // ExecuteScripts to simplify the controller.
+  script_tracker_->ExecuteScript(
+      script_path, base::BindOnce(&Controller::OnScriptExecuted,
+                                  // script_tracker_ is owned by Controller.
+                                  base::Unretained(this), script_path));
+}
+
 void Controller::OnScriptExecuted(const std::string& script_path,
-                                  ScriptExecutor::Result result) {
-  GetUiController()->HideOverlay();
+                                  const ScriptExecutor::Result& result) {
+  if (!allow_autostart_)
+    GetUiController()->HideOverlay();
   if (!result.success) {
     LOG(ERROR) << "Failed to execute script " << script_path;
     GetUiController()->ShowStatusMessage(
@@ -289,6 +309,44 @@ void Controller::GiveUp() {
   GetUiController()->ShutdownGracefully();
 }
 
+bool Controller::MaybeAutostartScript(
+    const std::vector<ScriptHandle>& runnable_scripts) {
+  // We want to g through all runnable autostart interrupts first, one at a
+  // time. To do that, always run highest priority autostartable interrupt from
+  // runnable_script, which is ordered by priority.
+  for (const auto& script : runnable_scripts) {
+    if (script.autostart && script.interrupt) {
+      std::string script_path = script.path;
+      ExecuteScript(script_path);
+      // making a copy of script.path is necessary, as ExecuteScript clears
+      // runnable_scripts, so script.path will not survive until the end of
+      // ExecuteScript.
+      return true;
+    }
+  }
+
+  // Under specific conditions, we can directly run a non-interrupt script
+  // without first displaying it. This is meant to work only at the very
+  // beginning, when no non-interrupt scripts have run, and only if there's
+  // exactly one autostartable script.
+  if (allow_autostart_) {
+    int autostart_count = 0;
+    std::string autostart_path;
+    for (const auto& script : runnable_scripts) {
+      if (script.autostart && !script.interrupt) {
+        autostart_count++;
+        autostart_path = script.path;
+      }
+    }
+    if (autostart_count == 1) {
+      allow_autostart_ = false;
+      ExecuteScript(autostart_path);
+      return true;
+    }
+  }
+  return false;
+}
+
 void Controller::OnClickOverlay() {
   GetUiController()->HideOverlay();
   // TODO(crbug.com/806868): Stop executing scripts.
@@ -296,21 +354,11 @@ void Controller::OnClickOverlay() {
 
 void Controller::OnScriptSelected(const std::string& script_path) {
   DCHECK(!script_path.empty());
-  DCHECK(!script_tracker_->running());
 
-  GetUiController()->ShowOverlay();
-  touchable_element_area_.ClearElements();
+  // This is a script selected from the UI, so it should disable autostart.
+  allow_autostart_ = false;
 
-  StopPeriodicScriptChecks();
-  // Runnable scripts will be checked and reported if necessary after executing
-  // the script.
-  script_tracker_->ClearRunnableScripts();
-  GetUiController()->UpdateScripts({});  // Clear scripts.
-  allow_autostart_ = false;  // Only ever autostart the very first script.
-  script_tracker_->ExecuteScript(
-      script_path, base::BindOnce(&Controller::OnScriptExecuted,
-                                  // script_tracker_ is owned by Controller.
-                                  base::Unretained(this), script_path));
+  ExecuteScript(script_path);
 }
 
 std::string Controller::GetDebugContext() {
@@ -351,9 +399,6 @@ void Controller::DidGetUserInteraction(const blink::WebInputEvent::Type type) {
   switch (type) {
     case blink::WebInputEvent::kTouchStart:
     case blink::WebInputEvent::kGestureTapDown:
-      // Disable autostart after interaction with the web page.
-      allow_autostart_ = false;
-
       if (!script_tracker_->running()) {
         script_tracker_->CheckScripts(kPeriodicScriptCheckInterval);
         StartPeriodicScriptChecks();
@@ -387,23 +432,8 @@ void Controller::OnRunnableScriptsChanged(
     GetUiController()->HideOverlay();
   }
 
-  // Under specific conditions, we can directly run a script without first
-  // displaying it. This is meant to work only at the very beginning, when no
-  // scripts have run, there has been no interaction with the webpage and only
-  // if there's exactly one runnable autostartable script.
-  if (allow_autostart_) {
-    int autostart_count = 0;
-    std::string autostart_path;
-    for (const auto& script : runnable_scripts) {
-      if (script.autostart) {
-        autostart_count++;
-        autostart_path = script.path;
-      }
-    }
-    if (autostart_count == 1) {
-      OnScriptSelected(autostart_path);
-      return;
-    }
+  if (MaybeAutostartScript(runnable_scripts)) {
+    return;
   }
 
   // Show the initial prompt if available.
@@ -415,11 +445,10 @@ void Controller::OnRunnableScriptsChanged(
     }
   }
 
-  // Update the set of scripts. Only scripts that are not marked as autostart
-  // should be shown.
+  // Update the set of scripts in the UI.
   std::vector<ScriptHandle> scripts_to_update;
   for (const auto& script : runnable_scripts) {
-    if (!script.autostart) {
+    if (!script.autostart && !script.name.empty()) {
       scripts_to_update.emplace_back(script);
     }
   }

components/autofill_assistant/browser/controller.h

@@ -70,9 +70,9 @@ class Controller : public ScriptExecutorDelegate,
 
   void GetOrCheckScripts(const GURL& url);
   void OnGetScripts(const GURL& url, bool result, const std::string& response);
-  void OnScriptChosen(const std::string& script_path);
+  void ExecuteScript(const std::string& script_path);
   void OnScriptExecuted(const std::string& script_path,
-                        ScriptExecutor::Result result);
+                        const ScriptExecutor::Result& result);
 
   // Check script preconditions every few seconds for a certain number of times.
   // If checks are already running, StartPeriodicScriptChecks resets the count.
@@ -84,6 +84,10 @@ class Controller : public ScriptExecutorDelegate,
   void OnPeriodicScriptCheck();
   void GiveUp();
 
+  // Runs autostart scripts from |runnable_scripts|, if the conditions are
+  // right. Returns true if a script was auto-started.
+  bool MaybeAutostartScript(const std::vector<ScriptHandle>& runnable_scripts);
+
   // Overrides content::UiDelegate:
   void Start(const GURL& initialUrl) override;
   void OnClickOverlay() override;

components/autofill_assistant/browser/controller_unittest.cc

@@ -20,11 +20,13 @@
 namespace autofill_assistant {
 
 using ::testing::_;
+using ::testing::AtLeast;
 using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::InSequence;
 using ::testing::NiceMock;
+using ::testing::Not;
 using ::testing::Pair;
 using ::testing::ReturnRef;
 using ::testing::Sequence;
@@ -98,8 +100,8 @@ class ControllerTest : public content::RenderViewHostTestHarness {
         .WillByDefault(RunOnceCallback<2>(true, ""));
 
     // Scripts run, but have no actions.
-    ON_CALL(*mock_service_, OnGetActions(_, _, _))
-        .WillByDefault(RunOnceCallback<2>(true, ""));
+    ON_CALL(*mock_service_, OnGetActions(_, _, _, _, _))
+        .WillByDefault(RunOnceCallback<4>(true, ""));
 
     // Make WebController::GetUrl accessible.
     ON_CALL(*mock_web_controller_, GetUrl()).WillByDefault(ReturnRef(url_));
@@ -122,6 +124,14 @@ class ControllerTest : public content::RenderViewHostTestHarness {
     return script;
   }
 
+  static void RunOnce(SupportedScriptProto* proto) {
+    auto* run_once = proto->mutable_presentation()
+                         ->mutable_precondition()
+                         ->add_script_status_match();
+    run_once->set_script(proto->path());
+    run_once->set_status(SCRIPT_STATUS_NOT_RUN);
+  }
+
   void SetLastCommittedUrl(const GURL& url) {
     url_ = url;
     tester_->SetLastCommittedURL(url);
@@ -138,10 +148,6 @@ class ControllerTest : public content::RenderViewHostTestHarness {
     controller_->LoadProgressChanged(web_contents(), progress);
   }
 
-  void SimulateUserInteraction(const blink::WebInputEvent::Type type) {
-    controller_->DidGetUserInteraction(type);
-  }
-
   // Sets up the next call to the service for scripts to return |response|.
   void SetNextScriptResponse(const SupportsScriptResponseProto& response) {
     std::string response_str;
@@ -151,6 +157,15 @@ class ControllerTest : public content::RenderViewHostTestHarness {
         .WillOnce(RunOnceCallback<2>(true, response_str));
   }
 
+  // Sets up all calls to the service for scripts to return |response|.
+  void SetRepeatedScriptResponse(const SupportsScriptResponseProto& response) {
+    std::string response_str;
+    response.SerializeToString(&response_str);
+
+    EXPECT_CALL(*mock_service_, OnGetScriptsForUrl(_, _, _))
+        .WillRepeatedly(RunOnceCallback<2>(true, response_str));
+  }
+
   UiDelegate* GetUiDelegate() { return controller_; }
 
   GURL url_;
@@ -197,8 +212,8 @@ TEST_F(ControllerTest, FetchAndRunScripts) {
       });
 
   // 5. script1 run successfully (no actions).
-  EXPECT_CALL(*mock_service_, OnGetActions(StrEq("script1"), _, _))
-      .WillOnce(RunOnceCallback<2>(true, ""));
+  EXPECT_CALL(*mock_service_, OnGetActions(StrEq("script1"), _, _, _, _))
+      .WillOnce(RunOnceCallback<4>(true, ""));
 
   // 6. As nothing is selected the flow terminates.
 
@@ -240,8 +255,8 @@ TEST_F(ControllerTest, Stop) {
   actions_response.add_actions()->mutable_stop();
   std::string actions_response_str;
   actions_response.SerializeToString(&actions_response_str);
-  EXPECT_CALL(*mock_service_, OnGetActions(StrEq("stop"), _, _))
-      .WillOnce(RunOnceCallback<2>(true, actions_response_str));
+  EXPECT_CALL(*mock_service_, OnGetActions(StrEq("stop"), _, _, _, _))
+      .WillOnce(RunOnceCallback<4>(true, actions_response_str));
   EXPECT_CALL(*mock_service_, OnGetNextActions(_, _, _))
       .WillOnce(RunOnceCallback<2>(true, ""));
 
@@ -272,8 +287,8 @@ TEST_F(ControllerTest, Reset) {
     actions_response.add_actions()->mutable_reset();
     std::string actions_response_str;
     actions_response.SerializeToString(&actions_response_str);
-    EXPECT_CALL(*mock_service_, OnGetActions(StrEq("reset"), _, _))
-        .WillOnce(RunOnceCallback<2>(true, actions_response_str));
+    EXPECT_CALL(*mock_service_, OnGetActions(StrEq("reset"), _, _, _, _))
+        .WillOnce(RunOnceCallback<4>(true, actions_response_str));
 
     // 3. Report the result of running that action.
     EXPECT_CALL(*mock_service_, OnGetNextActions(_, _, _))
@@ -330,23 +345,72 @@ TEST_F(ControllerTest, Autostart) {
   AddRunnableScript(&script_response, "alsorunnable");
   SetNextScriptResponse(script_response);
 
-  EXPECT_CALL(*mock_service_, OnGetActions(StrEq("autostart"), _, _))
-      .WillOnce(RunOnceCallback<2>(true, ""));
+  EXPECT_CALL(*mock_service_, OnGetActions(StrEq("autostart"), _, _, _, _))
+      .WillOnce(RunOnceCallback<4>(true, ""));
 
   SimulateNavigateToUrl(GURL("http://a.example.com/path"));
 }
 
-TEST_F(ControllerTest, AutostartIsNotPassedToTheUi) {
+TEST_F(ControllerTest, AutostartFirstInterrupt) {
   SupportsScriptResponseProto script_response;
-  AddRunnableScript(&script_response, "runnable")
-      ->mutable_presentation()
-      ->set_autostart(true);
+  AddRunnableScript(&script_response, "runnable");
+
+  auto* interrupt1 =
+      AddRunnableScript(&script_response, "autostart interrupt 1");
+  interrupt1->mutable_presentation()->set_interrupt(true);
+  interrupt1->mutable_presentation()->set_priority(1);
+  interrupt1->mutable_presentation()->set_autostart(true);
+
+  auto* interrupt2 =
+      AddRunnableScript(&script_response, "autostart interrupt 2");
+  interrupt2->mutable_presentation()->set_interrupt(true);
+  interrupt2->mutable_presentation()->set_priority(2);
+  interrupt2->mutable_presentation()->set_autostart(true);
+
   SetNextScriptResponse(script_response);
 
-  EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(0)));
-  EXPECT_CALL(*mock_service_, OnGetActions(StrEq("runnable"), _, _)).Times(0);
+  EXPECT_CALL(*mock_service_,
+              OnGetActions(StrEq("autostart interrupt 1"), _, _, _, _))
+      .WillOnce(RunOnceCallback<4>(false, ""));
+  // The script fails, ending the flow. What matters is that the correct
+  // expectation is met.
+
+  SimulateNavigateToUrl(GURL("http://a.example.com/path"));
+}
+
+TEST_F(ControllerTest, InterruptThenAutostart) {
+  SupportsScriptResponseProto script_response;
+  AddRunnableScript(&script_response, "runnable");
+
+  auto* interrupt = AddRunnableScript(&script_response, "autostart interrupt");
+  interrupt->mutable_presentation()->set_interrupt(true);
+  interrupt->mutable_presentation()->set_autostart(true);
+  RunOnce(interrupt);
+
+  auto* autostart = AddRunnableScript(&script_response, "autostart");
+  autostart->mutable_presentation()->set_autostart(true);
+  RunOnce(autostart);
+
+  SetRepeatedScriptResponse(script_response);
+
+  {
+    testing::InSequence seq;
+    EXPECT_CALL(*mock_service_,
+                OnGetActions(StrEq("autostart interrupt"), _, _, _, _));
+    EXPECT_CALL(*mock_service_, OnGetActions(StrEq("autostart"), _, _, _, _));
+  }
+
+  SimulateNavigateToUrl(GURL("http://a.example.com/path"));
+}
+
+TEST_F(ControllerTest, AutostartIsNotPassedToTheUi) {
+  SupportsScriptResponseProto script_response;
+  auto* autostart = AddRunnableScript(&script_response, "runnable");
+  autostart->mutable_presentation()->set_autostart(true);
+  RunOnce(autostart);
+  SetRepeatedScriptResponse(script_response);
 
-  SimulateUserInteraction(blink::WebInputEvent::kTouchStart);
+  EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(0))).Times(AtLeast(1));
   SimulateNavigateToUrl(GURL("http://a.example.com/path"));
 }
 

components/autofill_assistant/browser/mock_service.h

@@ -32,15 +32,17 @@ class MockService : public Service {
                     ResponseCallback& callback));
 
   void GetActions(const std::string& script_path,
-                  const GURL& ignored_url,
+                  const GURL& url,
                   const std::map<std::string, std::string>& parameters,
                   const std::string& server_payload,
                   ResponseCallback callback) override {
-    OnGetActions(script_path, parameters, callback);
+    OnGetActions(script_path, url, parameters, server_payload, callback);
   }
-  MOCK_METHOD3(OnGetActions,
+  MOCK_METHOD5(OnGetActions,
                void(const std::string& script_path,
+                    const GURL& url,
                     const std::map<std::string, std::string>& parameters,
+                    const std::string& server_payload,
                     ResponseCallback& callback));
 
   void GetNextActions(

components/autofill_assistant/browser/protocol_utils.cc

@@ -85,14 +85,15 @@ bool ProtocolUtils::ParseScripts(
     const auto& presentation = script_proto.presentation();
     script->handle.name = presentation.name();
     script->handle.autostart = presentation.autostart();
+    script->handle.interrupt = presentation.interrupt();
     script->handle.initial_prompt = presentation.initial_prompt();
     script->handle.highlight = presentation.highlight();
     script->precondition = ScriptPrecondition::FromProto(
         script_proto.path(), presentation.precondition());
     script->priority = presentation.priority();
 
-    if (script->handle.name.empty() || script->handle.path.empty() ||
-        !script->precondition) {
+    if (script->handle.path.empty() || !script->precondition ||
+        (script->handle.name.empty() && !script->handle.interrupt)) {
       LOG(ERROR) << "Ignored invalid or incomplete script '"
                  << script->handle.path << "'";
       continue;

components/autofill_assistant/browser/protocol_utils_unittest.cc

@@ -77,6 +77,27 @@ TEST(ProtocolUtilsTest, OneFullyFeaturedScript) {
   EXPECT_NE(nullptr, scripts[0]->precondition);
 }
 
+TEST(ProtocolUtilsTest, AllowInterruptsWithNoName) {
+  SupportsScriptResponseProto proto;
+
+  SupportedScriptProto* script = proto.add_scripts();
+  script->set_path("path");
+  auto* presentation = script->mutable_presentation();
+  presentation->set_autostart(true);
+  presentation->set_initial_prompt("prompt");
+  presentation->set_interrupt(true);
+  presentation->mutable_precondition()->add_domain("www.example.com");
+
+  std::vector<std::unique_ptr<Script>> scripts;
+  std::string proto_str;
+  proto.SerializeToString(&proto_str);
+  EXPECT_TRUE(ProtocolUtils::ParseScripts(proto_str, &scripts));
+  ASSERT_THAT(scripts, SizeIs(1));
+  EXPECT_EQ("path", scripts[0]->handle.path);
+  EXPECT_EQ("", scripts[0]->handle.name);
+  EXPECT_TRUE(scripts[0]->handle.interrupt);
+}
+
 TEST(ProtocolUtilsTest, CreateInitialScriptActionsRequest) {
   std::map<std::string, std::string> parameters;
   parameters["a"] = "b";

components/autofill_assistant/browser/script.h

@@ -26,6 +26,10 @@ struct ScriptHandle {
   // be shown.
   bool autostart;
   bool highlight;
+
+  // If set, the script might be run during WaitForDom actions with
+  // allow_interrupt=true.
+  bool interrupt;
 };
 
 // Script represents a sequence of actions.

components/autofill_assistant/browser/script_executor.h

@@ -6,7 +6,9 @@
 #define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SCRIPT_EXECUTOR_H_
 
 #include <deque>
+#include <map>
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -33,11 +35,13 @@ class ScriptExecutor : public ActionDelegate {
     virtual void OnServerPayloadChanged(const std::string& server_payload) = 0;
   };
 
-  // |delegate| and |listener| should outlive this object and should not be
-  // nullptr.
+  // |delegate|, |listener|, |script_state| and |ordered_interrupts| should
+  // outlive this object and should not be nullptr.
   ScriptExecutor(const std::string& script_path,
                  const std::string& server_payload,
                  ScriptExecutor::Listener* listener,
+                 std::map<std::string, ScriptStatusProto>* scripts_state,
+                 const std::vector<Script*>* ordered_interrupts,
                  ScriptExecutorDelegate* delegate);
   ~ScriptExecutor() override;
 
@@ -70,13 +74,18 @@ class ScriptExecutor : public ActionDelegate {
     ~Result();
   };
 
-  using RunScriptCallback = base::OnceCallback<void(Result)>;
+  using RunScriptCallback = base::OnceCallback<void(const Result&)>;
   void Run(RunScriptCallback callback);
 
   // Override ActionDelegate:
   std::unique_ptr<BatchElementChecker> CreateBatchElementChecker() override;
-  void WaitForElement(const std::vector<std::string>& selectors,
-                      base::OnceCallback<void(bool)> callback) override;
+  void ShortWaitForElementExist(
+      const std::vector<std::string>& selectors,
+      base::OnceCallback<void(bool)> callback) override;
+  void WaitForElementVisible(base::TimeDelta max_wait_time,
+                             bool allow_interrupt,
+                             const std::vector<std::string>& selectors,
+                             base::OnceCallback<void(bool)> callback) override;
   void ShowStatusMessage(const std::string& message) override;
   void ClickOrTapElement(const std::vector<std::string>& selectors,
                          base::OnceCallback<void(bool)> callback) override;
@@ -136,16 +145,79 @@ class ScriptExecutor : public ActionDelegate {
   void HideOverlay() override;
 
  private:
+  // Helper for WaitForElementVisible that keeps track of the state required to
+  // run interrupts while waiting for a specific element.
+  class WaitWithInterrupts {
+   public:
+    // Let the caller know about either the result of looking for the element or
+    // of an abnormal result from an interrupt.
+    //
+    // If the given result is non-null, it should be forwarded as the result of
+    // the main script.
+    using Callback =
+        base::OnceCallback<void(bool, const ScriptExecutor::Result*)>;
+
+    // |main_script_| must not be null and outlive this instance.
+    WaitWithInterrupts(const ScriptExecutor* main_script,
+                       base::TimeDelta max_wait_time,
+                       ElementCheckType check_type,
+                       const std::vector<std::string>& selectors,
+                       WaitWithInterrupts::Callback callback);
+    ~WaitWithInterrupts();
+
+    void Run();
+
+   private:
+    void OnPreconditionCheckDone(const Script* interrupt,
+                                 bool precondition_match);
+    void OnElementCheckDone(bool found);
+    void OnTryDone();
+    void OnAllDone();
+    void RunInterrupt(const Script* interrupt);
+    void OnInterruptDone(const ScriptExecutor::Result& result);
+    void RunCallback(bool found, const ScriptExecutor::Result* result);
+
+    const ScriptExecutor* main_script_;
+    const base::TimeDelta max_wait_time_;
+    const ElementCheckType check_type_;
+    const std::vector<std::string> selectors_;
+    WaitWithInterrupts::Callback callback_;
+
+    std::unique_ptr<BatchElementChecker> batch_element_checker_;
+    std::set<const Script*> runnable_interrupts_;
+    bool element_found_;
+
+    // An empty vector of interrupts that can be passed to interrupt_executor_
+    // and outlives it. Interrupts must not run interrupts.
+    const std::vector<Script*> no_interrupts_;
+
+    // The interrupt that's currently running.
+    std::unique_ptr<ScriptExecutor> interrupt_executor_;
+
+    DISALLOW_COPY_AND_ASSIGN(WaitWithInterrupts);
+  };
+  friend class WaitWithInterrupts;
+
   void OnGetActions(bool result, const std::string& response);
   void RunCallback(bool success);
+  void RunCallbackWithResult(const Result& result);
   void ProcessNextAction();
   void ProcessAction(Action* action);
   void GetNextActions();
   void OnProcessedAction(std::unique_ptr<ProcessedActionProto> action);
+  void WaitForElement(base::TimeDelta max_wait_time,
+                      ElementCheckType check_type,
+                      const std::vector<std::string>& selectors,
+                      base::OnceCallback<void(bool)> callback);
+  void OnWaitForElementVisible(
+      base::OnceCallback<void(bool)> element_found_callback,
+      bool element_found,
+      const Result* interrupt_result);
   void OnChosen(base::OnceCallback<void(const std::string&)> callback,
                 const std::string& chosen);
 
   std::string script_path_;
+  const std::string initial_server_payload_;
   std::string last_server_payload_;
   ScriptExecutor::Listener* const listener_;
   ScriptExecutorDelegate* delegate_;
@@ -158,6 +230,14 @@ class ScriptExecutor : public ActionDelegate {
   bool should_clean_contextual_ui_on_finish_;
   ActionProto::ActionInfoCase previous_action_type_;
   std::vector<std::vector<std::string>> touchable_elements_;
+  std::map<std::string, ScriptStatusProto>* scripts_state_;
+
+  // Set of interrupts that might run during wait for dom actions with
+  // allow_interrupt. Sorted by priority; an interrupt that appears on the
+  // vector first should run first.
+  const std::vector<Script*>* ordered_interrupts_;
+
+  std::unique_ptr<WaitWithInterrupts> wait_with_interrupts_;
 
   base::WeakPtrFactory<ScriptExecutor> weak_ptr_factory_;
   DISALLOW_COPY_AND_ASSIGN(ScriptExecutor);

components/autofill_assistant/browser/script_tracker.cc

@@ -14,6 +14,24 @@
 
 namespace autofill_assistant {
 
+namespace {
+
+// Sort scripts by priority.
+void SortScripts(std::vector<Script*>* scripts) {
+  std::sort(scripts->begin(), scripts->end(),
+            [](const Script* a, const Script* b) {
+              // Runnable scripts with lowest priority value are displayed
+              // first, Interrupts with lowest priority values are run first.
+              // Order of scripts with the same priority is arbitrary. Fallback
+              // to ordering by name and path, arbitrarily, for the behavior to
+              // be consistent across runs.
+              return std::tie(a->priority, a->handle.name, a->handle.path) <
+                     std::tie(b->priority, b->handle.name, a->handle.path);
+            });
+}
+
+}  // namespace
+
 ScriptTracker::ScriptTracker(ScriptExecutorDelegate* delegate,
                              ScriptTracker::Listener* listener)
     : delegate_(delegate),
@@ -27,10 +45,26 @@ ScriptTracker::ScriptTracker(ScriptExecutorDelegate* delegate,
 ScriptTracker::~ScriptTracker() = default;
 
 void ScriptTracker::SetScripts(std::vector<std::unique_ptr<Script>> scripts) {
-  ClearAvailableScripts();
+  // The set of interrupts must not change while a script is running, as they're
+  // passed to ScriptExecutor.
+  if (running())
+    return;
+
+  TerminatePendingChecks();
+
+  available_scripts_.clear();
   for (auto& script : scripts) {
     available_scripts_[script.get()] = std::move(script);
   }
+
+  interrupts_.clear();
+  for (auto& entry : available_scripts_) {
+    Script* script = entry.first;
+    if (script->handle.interrupt) {
+      interrupts_.push_back(script);
+    }
+  }
+  SortScripts(&interrupts_);
 }
 
 void ScriptTracker::CheckScripts(const base::TimeDelta& max_duration) {
@@ -43,9 +77,12 @@ void ScriptTracker::CheckScripts(const base::TimeDelta& max_duration) {
       delegate_->GetWebController()->CreateBatchElementChecker();
   for (const auto& entry : available_scripts_) {
     Script* script = entry.first;
+    if (script->handle.name.empty() && !script->handle.autostart)
+      continue;
+
     script->precondition->Check(
         delegate_->GetWebController()->GetUrl(), batch_element_checker_.get(),
-        delegate_->GetParameters(), executed_scripts_,
+        delegate_->GetParameters(), scripts_state_,
         base::BindOnce(&ScriptTracker::OnPreconditionCheck,
                        weak_ptr_factory_.GetWeakPtr(), script));
   }
@@ -80,9 +117,9 @@ void ScriptTracker::ExecuteScript(const std::string& script_path,
     return;
   }
 
-  executed_scripts_[script_path] = SCRIPT_STATUS_RUNNING;
   executor_ = std::make_unique<ScriptExecutor>(
-      script_path, last_server_payload_, this, delegate_);
+      script_path, last_server_payload_, /* listener= */ this, &scripts_state_,
+      &interrupts_, delegate_);
   ScriptExecutor::RunScriptCallback run_script_callback = base::BindOnce(
       &ScriptTracker::OnScriptRun, weak_ptr_factory_.GetWeakPtr(), script_path,
       std::move(callback));
@@ -100,13 +137,13 @@ base::Value ScriptTracker::GetDebugContext() const {
   base::Base64Encode(last_server_payload_js, &last_server_payload_js);
   dict.SetKey("last-payload", base::Value(last_server_payload_js));
 
-  std::vector<base::Value> executed_scripts_js;
-  for (const auto& entry : executed_scripts_) {
+  std::vector<base::Value> scripts_state_js;
+  for (const auto& entry : scripts_state_) {
     base::Value script_js = base::Value(base::Value::Type::DICTIONARY);
     script_js.SetKey(entry.first, base::Value(entry.second));
-    executed_scripts_js.push_back(std::move(script_js));
+    scripts_state_js.push_back(std::move(script_js));
   }
-  dict.SetKey("executed-scripts", base::Value(executed_scripts_js));
+  dict.SetKey("executed-scripts", base::Value(scripts_state_js));
 
   std::vector<base::Value> available_scripts_js;
   for (const auto& entry : available_scripts_)
@@ -131,10 +168,8 @@ base::Value ScriptTracker::GetDebugContext() const {
 void ScriptTracker::OnScriptRun(
     const std::string& script_path,
     ScriptExecutor::RunScriptCallback original_callback,
-    ScriptExecutor::Result result) {
+    const ScriptExecutor::Result& result) {
   executor_.reset();
-  executed_scripts_[script_path] =
-      result.success ? SCRIPT_STATUS_SUCCESS : SCRIPT_STATUS_FAILURE;
   std::move(original_callback).Run(result);
 }
 
@@ -143,15 +178,7 @@ void ScriptTracker::UpdateRunnableScriptsIfNecessary() {
     return;
 
   runnable_scripts_.clear();
-  std::sort(pending_runnable_scripts_.begin(), pending_runnable_scripts_.end(),
-            [](const Script* a, const Script* b) {
-              // Runnable scripts with lowest priority value are displayed
-              // first. The display order of scripts with the same priority is
-              // arbitrary. Fallback to ordering by name, arbitrarily, for the
-              // behavior to be consistent.
-              return std::tie(a->priority, a->handle.name) <
-                     std::tie(b->priority, b->handle.name);
-            });
+  SortScripts(&pending_runnable_scripts_);
   for (Script* script : pending_runnable_scripts_) {
     runnable_scripts_.push_back(script->handle);
   }
@@ -194,13 +221,6 @@ void ScriptTracker::OnPreconditionCheck(Script* script,
     pending_runnable_scripts_.push_back(script);
 }
 
-void ScriptTracker::ClearAvailableScripts() {
-  available_scripts_.clear();
-  // Clearing available_scripts_ has cancelled any pending precondition checks,
-  // ending them.
-  TerminatePendingChecks();
-}
-
 void ScriptTracker::OnServerPayloadChanged(const std::string& server_payload) {
   last_server_payload_ = server_payload;
 }

components/autofill_assistant/browser/script_tracker.h

@@ -21,6 +21,7 @@
 
 namespace autofill_assistant {
 class ScriptExecutorDelegate;
+class ScriptTrackerTest;
 
 // The script tracker keeps track of which scripts are available, which are
 // running, which have run, which are runnable whose preconditions are met.
@@ -93,9 +94,11 @@ class ScriptTracker : public ScriptExecutor::Listener {
  private:
   typedef std::map<Script*, std::unique_ptr<Script>> AvailableScriptMap;
 
+  friend class ScriptTrackerTest;
+
   void OnScriptRun(const std::string& script_path,
                    ScriptExecutor::RunScriptCallback original_callback,
-                   ScriptExecutor::Result result);
+                   const ScriptExecutor::Result& result);
   void UpdateRunnableScriptsIfNecessary();
   void OnCheckDone();
 
@@ -110,7 +113,6 @@ class ScriptTracker : public ScriptExecutor::Listener {
   // Returns true if |runnable_| should be updated.
   bool RunnablesHaveChanged();
   void OnPreconditionCheck(Script* script, bool met_preconditions);
-  void ClearAvailableScripts();
 
   ScriptExecutorDelegate* const delegate_;
   ScriptTracker::Listener* const listener_;
@@ -134,8 +136,12 @@ class ScriptTracker : public ScriptExecutor::Listener {
   // any pending check.
   AvailableScriptMap available_scripts_;
 
+  // A subset of available_scripts that are interrupts.
+  std::vector<Script*> interrupts_;
+
   // List of scripts that have been executed and their corresponding statuses.
-  std::map<std::string, ScriptStatusProto> executed_scripts_;
+  std::map<std::string, ScriptStatusProto> scripts_state_;
+
   std::unique_ptr<BatchElementChecker> batch_element_checker_;
 
   // Scripts found to be runnable so far, in the current check, represented by

components/autofill_assistant/browser/script_tracker_unittest.cc

@@ -41,8 +41,8 @@ class ScriptTrackerTest : public testing::Test,
     ON_CALL(mock_web_controller_, GetUrl()).WillByDefault(ReturnRef(url_));
 
     // Scripts run, but have no actions.
-    ON_CALL(mock_service_, OnGetActions(_, _, _))
-        .WillByDefault(RunOnceCallback<2>(true, ""));
+    ON_CALL(mock_service_, OnGetActions(_, _, _, _, _))
+        .WillByDefault(RunOnceCallback<4>(true, ""));
   }
 
  protected:
@@ -168,6 +168,40 @@ TEST_F(ScriptTrackerTest, SomeRunnableScripts) {
   EXPECT_EQ("runnable path", runnable_scripts()[0].path);
 }
 
+TEST_F(ScriptTrackerTest, DoNotCheckInterruptWithNoName) {
+  SupportsScriptResponseProto scripts;
+
+  // The interrupt's preconditions would all be met, but it won't be reported
+  // since it doesn't have a name.
+  auto* no_name = AddScript(&scripts, "", "path1", "exists");
+  no_name->mutable_presentation()->set_interrupt(true);
+
+  // The interrupt's preconditions are met and it will be reported as a normal
+  // script.
+  auto* with_name = AddScript(&scripts, "with name", "path2", "exists");
+  with_name->mutable_presentation()->set_interrupt(true);
+
+  SetAndCheckScripts(scripts);
+
+  EXPECT_EQ(1, runnable_scripts_changed_);
+  ASSERT_THAT(runnable_scripts(), SizeIs(1));
+  EXPECT_EQ("with name", runnable_scripts()[0].name);
+}
+
+TEST_F(ScriptTrackerTest, ReportInterruptToAutostart) {
+  SupportsScriptResponseProto scripts;
+
+  // The interrupt's preconditions are met and it will be reported as runnable
+  // for autostart.
+  auto* autostart = AddScript(&scripts, "", "path2", "exists");
+  autostart->mutable_presentation()->set_interrupt(true);
+  autostart->mutable_presentation()->set_autostart(true);
+  SetAndCheckScripts(scripts);
+
+  EXPECT_EQ(1, runnable_scripts_changed_);
+  ASSERT_THAT(runnable_scripts(), SizeIs(1));
+}
+
 TEST_F(ScriptTrackerTest, OrderScriptsByPriority) {
   SupportsScriptResponseProto scripts;
 

components/autofill_assistant/browser/service.proto

@@ -84,6 +84,10 @@ message SupportedScriptProto {
     // When set to true this script can be run in 'autostart mode'. Script won't
     // be shown.
     optional bool autostart = 8;
+
+    // When set to true this script will be run from WaitForDom actions with
+    // allow_interrupt=true.
+    optional bool interrupt = 9;
   }
   optional PresentationProto presentation = 2;
 }
@@ -391,6 +395,10 @@ message WaitForDomProto {
 
   // The element to wait for.
   repeated string selectors = 2;
+
+  // If true, run scripts flagged with 'interrupt=true' as soon as their
+  // preconditions match.
+  optional bool allow_interrupt = 3;
 }
 
 // Volatile upload of a portion of the dom for backend analysis, does not store