[Autofill Assistant] Add optional wait for ShowCast

This adds an optional waiting step for the ShowCast
action to ensure the element is stable before trying
to scroll to it.

This change also makes ElementPositionGetter configurable
in terms of rounds and interval.

This change also removes the settings from WebController
as they are no longer needed.

Bug: b/172334699
Change-Id: I5ffd532c5c7ae8eab4b5d0809d17d6fec9b82749
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532293
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826440}

chrome/browser/android/autofill_assistant/trigger_script_bridge_android.cc

@@ -62,9 +62,7 @@ void TriggerScriptBridgeAndroid::StartTriggerScript(
 
   ServerUrlFetcher url_fetcher{ServerUrlFetcher::GetDefaultServerUrl()};
   trigger_script_coordinator_ = std::make_unique<TriggerScriptCoordinator>(
-      client,
-      WebController::CreateForWebContents(client->GetWebContents(),
-                                          &client_settings_),
+      client, WebController::CreateForWebContents(client->GetWebContents()),
       std::move(service_request_sender),
       url_fetcher.GetTriggerScriptsEndpoint(),
       std::make_unique<StaticTriggerConditions>(),

chrome/browser/android/autofill_assistant/trigger_script_bridge_android.h

@@ -11,6 +11,7 @@
 
 #include "base/android/jni_android.h"
 #include "components/autofill_assistant/browser/client.h"
+#include "components/autofill_assistant/browser/client_settings.h"
 #include "components/autofill_assistant/browser/metrics.h"
 #include "components/autofill_assistant/browser/service.pb.h"
 #include "components/autofill_assistant/browser/trigger_context.h"

components/autofill_assistant/browser/actions/action_delegate.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/callback_forward.h"
+#include "base/time/time.h"
 #include "components/autofill_assistant/browser/batch_element_checker.h"
 #include "components/autofill_assistant/browser/details.h"
 #include "components/autofill_assistant/browser/info_box.h"
@@ -127,6 +128,8 @@ class ActionDelegate {
   // Wait for the |element| to stop moving on the page. Fails with
   // ELEMENT_UNSTABLE.
   virtual void WaitUntilElementIsStable(
+      int max_rounds,
+      base::TimeDelta check_interval,
       const ElementFinder::Result& element,
       base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
@@ -431,7 +434,7 @@ class ActionDelegate {
       base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
   // Returns the current client settings.
-  virtual const ClientSettings& GetSettings() = 0;
+  virtual const ClientSettings& GetSettings() const = 0;
 
   // Show a form to the user and call |changed_callback| with its values
   // whenever there is a change. |changed_callback| will be called directly with

components/autofill_assistant/browser/actions/action_delegate_util.cc

@@ -6,6 +6,7 @@
 
 #include "base/callback.h"
 #include "components/autofill_assistant/browser/actions/action_delegate.h"
+#include "components/autofill_assistant/browser/client_settings.h"
 #include "components/autofill_assistant/browser/selector.h"
 #include "components/autofill_assistant/browser/service.pb.h"
 #include "components/autofill_assistant/browser/string_conversions_util.h"
@@ -111,27 +112,13 @@ void AddClickOrTapSequence(const ActionDelegate* delegate,
       base::BindOnce(&ActionDelegate::ScrollIntoView, delegate->GetWeakPtr()));
   if (click_type != ClickType::JAVASCRIPT) {
     actions->emplace_back(base::BindOnce(
-        &ActionDelegate::WaitUntilElementIsStable, delegate->GetWeakPtr()));
-
-    switch (on_top) {
-      case STEP_UNSPECIFIED:
-        NOTREACHED() << __func__ << " unspecified on_top step";
-        FALLTHROUGH;
-
-      case SKIP_STEP:
-        break;
-
-      case REPORT_STEP_RESULT:
-        actions->emplace_back(base::BindOnce(
-            &SkipIfFail, base::BindOnce(&ActionDelegate::CheckOnTop,
-                                        delegate->GetWeakPtr())));
-        break;
-
-      case REQUIRE_STEP_SUCCESS:
-        actions->emplace_back(base::BindOnce(&ActionDelegate::CheckOnTop,
-                                             delegate->GetWeakPtr()));
-        break;
-    }
+        &ActionDelegate::WaitUntilElementIsStable, delegate->GetWeakPtr(),
+        delegate->GetSettings().box_model_check_count,
+        delegate->GetSettings().box_model_check_interval));
+    AddOptionalStep(
+        on_top,
+        base::BindOnce(&ActionDelegate::CheckOnTop, delegate->GetWeakPtr()),
+        actions);
   }
   actions->emplace_back(base::BindOnce(&ActionDelegate::ClickOrTapElement,
                                        delegate->GetWeakPtr(), click_type));
@@ -148,6 +135,27 @@ void PerformAll(std::unique_ptr<ElementActionVector> perform_actions,
       std::move(done), OkClientStatus());
 }
 
+void AddOptionalStep(OptionalStep optional_step,
+                     ElementActionCallback step,
+                     ElementActionVector* actions) {
+  switch (optional_step) {
+    case STEP_UNSPECIFIED:
+      NOTREACHED() << __func__ << " unspecified optional_step";
+      FALLTHROUGH;
+
+    case SKIP_STEP:
+      break;
+
+    case REPORT_STEP_RESULT:
+      actions->emplace_back(base::BindOnce(&SkipIfFail, std::move(step)));
+      break;
+
+    case REQUIRE_STEP_SUCCESS:
+      actions->emplace_back(std::move(step));
+      break;
+  }
+}
+
 void FindElementAndPerform(const ActionDelegate* delegate,
                            const Selector& selector,
                            ElementActionCallback perform,

components/autofill_assistant/browser/actions/action_delegate_util.h

@@ -9,6 +9,7 @@
 #include "components/autofill_assistant/browser/actions/action_delegate.h"
 #include "components/autofill_assistant/browser/selector.h"
 #include "components/autofill_assistant/browser/service.pb.h"
+#include "components/autofill_assistant/browser/web/element_finder.h"
 
 namespace autofill_assistant {
 namespace action_delegate_util {
@@ -80,6 +81,13 @@ void PerformAll(std::unique_ptr<ElementActionVector> perform_actions,
                 const ElementFinder::Result& element,
                 base::OnceCallback<void(const ClientStatus&)> done);
 
+// Adds an optional step to the |actions|. If the step is |SKIP_STEP|, it does
+// not add it. For |REPORT_STEP_RESULT| it adds the step ignoring a potential
+// failure. For |REQUIRE_STEP_SUCCESS| it binds the step as is.
+void AddOptionalStep(OptionalStep optional_step,
+                     ElementActionCallback step,
+                     ElementActionVector* actions);
+
 void ClickOrTapElement(const ActionDelegate* delegate,
                        const Selector& selector,
                        ClickType click_type,

components/autofill_assistant/browser/actions/click_action_unittest.cc

@@ -33,8 +33,8 @@ class ClickActionTest : public testing::Test {
         .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
     ON_CALL(mock_action_delegate_, ScrollIntoView(_, _))
         .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
-    ON_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _))
-        .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
+    ON_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _, _, _))
+        .WillByDefault(RunOnceCallback<3>(OkClientStatus()));
     ON_CALL(mock_action_delegate_, CheckOnTop(_, _))
         .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
     ON_CALL(mock_action_delegate_, ClickOrTapElement(_, _, _))
@@ -90,9 +90,10 @@ TEST_F(ClickActionTest, CheckExpectedCallChain) {
   EXPECT_CALL(mock_action_delegate_,
               ScrollIntoView(EqualsElement(expected_element), _))
       .WillOnce(RunOnceCallback<1>(OkClientStatus()));
-  EXPECT_CALL(mock_action_delegate_,
-              WaitUntilElementIsStable(EqualsElement(expected_element), _))
-      .WillOnce(RunOnceCallback<1>(OkClientStatus()));
+  EXPECT_CALL(
+      mock_action_delegate_,
+      WaitUntilElementIsStable(_, _, EqualsElement(expected_element), _))
+      .WillOnce(RunOnceCallback<3>(OkClientStatus()));
   EXPECT_CALL(
       mock_action_delegate_,
       ClickOrTapElement(ClickType::CLICK, EqualsElement(expected_element), _))
@@ -112,7 +113,8 @@ TEST_F(ClickActionTest, JavaScriptClickSkipsWaitForElementStable) {
   ElementFinder::Result expected_element =
       test_util::MockFindElement(mock_action_delegate_, expected_selector);
 
-  EXPECT_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _)).Times(0);
+  EXPECT_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _, _, _))
+      .Times(0);
   EXPECT_CALL(mock_action_delegate_, CheckOnTop(_, _)).Times(0);
   EXPECT_CALL(mock_action_delegate_,
               ClickOrTapElement(ClickType::JAVASCRIPT,
@@ -152,8 +154,9 @@ TEST_F(ClickActionTest, RequireCheckOnTop) {
       test_util::MockFindElement(mock_action_delegate_, expected_selector);
 
   InSequence seq;
-  EXPECT_CALL(mock_action_delegate_,
-              WaitUntilElementIsStable(EqualsElement(expected_element), _));
+  EXPECT_CALL(
+      mock_action_delegate_,
+      WaitUntilElementIsStable(_, _, EqualsElement(expected_element), _));
   EXPECT_CALL(mock_action_delegate_,
               CheckOnTop(EqualsElement(expected_element), _));
   EXPECT_CALL(
@@ -177,8 +180,9 @@ TEST_F(ClickActionTest, OptionalCheckOnTop) {
       test_util::MockFindElement(mock_action_delegate_, expected_selector);
 
   InSequence seq;
-  EXPECT_CALL(mock_action_delegate_,
-              WaitUntilElementIsStable(EqualsElement(expected_element), _));
+  EXPECT_CALL(
+      mock_action_delegate_,
+      WaitUntilElementIsStable(_, _, EqualsElement(expected_element), _));
   EXPECT_CALL(mock_action_delegate_,
               CheckOnTop(EqualsElement(expected_element), _));
   EXPECT_CALL(

components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler_unittest.cc

@@ -55,8 +55,8 @@ class RequiredFieldsFallbackHandlerTest : public testing::Test {
         .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
     ON_CALL(mock_action_delegate_, ScrollIntoView(_, _))
         .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
-    ON_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _))
-        .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
+    ON_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _, _, _))
+        .WillByDefault(RunOnceCallback<3>(OkClientStatus()));
   }
 
  protected:

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "base/time/time.h"
 #include "components/autofill/core/browser/data_model/credit_card.h"
 #include "components/autofill/core/common/form_data.h"
 #include "components/autofill_assistant/browser/actions/action_delegate.h"
@@ -90,8 +91,10 @@ class MockActionDelegate : public ActionDelegate {
                void(const ElementFinder::Result& element,
                     base::OnceCallback<void(const ClientStatus&)> callback));
 
-  MOCK_METHOD2(WaitUntilElementIsStable,
-               void(const ElementFinder::Result& element,
+  MOCK_METHOD4(WaitUntilElementIsStable,
+               void(int,
+                    base::TimeDelta,
+                    const ElementFinder::Result& element,
                     base::OnceCallback<void(const ClientStatus&)> callback));
 
   MOCK_METHOD2(CheckOnTop,
@@ -362,7 +365,9 @@ class MockActionDelegate : public ActionDelegate {
     return weak_ptr_factory_.GetWeakPtr();
   }
 
-  const ClientSettings& GetSettings() override { return client_settings_; }
+  const ClientSettings& GetSettings() const override {
+    return client_settings_;
+  }
 
   ClientSettings client_settings_;
 

components/autofill_assistant/browser/actions/prompt_action.cc

@@ -13,7 +13,6 @@
 #include "base/callback.h"
 #include "base/strings/string_number_conversions.h"
 #include "components/autofill_assistant/browser/actions/action_delegate.h"
-#include "components/autofill_assistant/browser/client_settings.h"
 #include "components/autofill_assistant/browser/element_precondition.h"
 #include "url/gurl.h"
 

components/autofill_assistant/browser/actions/set_form_field_value_action_unittest.cc

@@ -73,8 +73,8 @@ class SetFormFieldValueActionTest : public testing::Test {
         .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
     ON_CALL(mock_action_delegate_, ScrollIntoView(_, _))
         .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
-    ON_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _))
-        .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
+    ON_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _, _, _))
+        .WillByDefault(RunOnceCallback<3>(OkClientStatus()));
     ON_CALL(mock_action_delegate_, ClickOrTapElement(_, _, _))
         .WillByDefault(RunOnceCallback<2>(OkClientStatus()));
     ON_CALL(mock_action_delegate_, OnSendKeyboardInput(_, _, _, _))

components/autofill_assistant/browser/actions/show_cast_action.cc

@@ -25,6 +25,8 @@ ShowCastAction::ShowCastAction(ActionDelegate* delegate,
 ShowCastAction::~ShowCastAction() {}
 
 void ShowCastAction::InternalProcessAction(ProcessActionCallback callback) {
+  process_action_callback_ = std::move(callback);
+
   const ShowCastProto& show_cast = proto_.show_cast();
   if (show_cast.has_title()) {
     // TODO(crbug.com/806868): Deprecate and remove message from this action and
@@ -34,8 +36,7 @@ void ShowCastAction::InternalProcessAction(ProcessActionCallback callback) {
   Selector selector = Selector(show_cast.element_to_present());
   if (selector.empty()) {
     VLOG(1) << __func__ << ": empty selector";
-    UpdateProcessedAction(INVALID_SELECTOR);
-    std::move(callback).Run(std::move(processed_action_proto_));
+    EndAction(ClientStatus(INVALID_SELECTOR));
     return;
   }
 
@@ -60,17 +61,14 @@ void ShowCastAction::InternalProcessAction(ProcessActionCallback callback) {
                                weak_ptr_factory_.GetWeakPtr(),
                                base::BindOnce(&ShowCastAction::OnWaitForElement,
                                               weak_ptr_factory_.GetWeakPtr(),
-                                              std::move(callback), selector,
-                                              top_padding)));
+                                              selector, top_padding)));
 }
 
-void ShowCastAction::OnWaitForElement(ProcessActionCallback callback,
-                                      const Selector& selector,
+void ShowCastAction::OnWaitForElement(const Selector& selector,
                                       const TopPadding& top_padding,
                                       const ClientStatus& element_status) {
   if (!element_status.ok()) {
-    UpdateProcessedAction(element_status.proto_status());
-    std::move(callback).Run(std::move(processed_action_proto_));
+    EndAction(element_status);
     return;
   }
 
@@ -78,6 +76,18 @@ void ShowCastAction::OnWaitForElement(ProcessActionCallback callback,
   actions->emplace_back(
       base::BindOnce(&ActionDelegate::WaitForDocumentToBecomeInteractive,
                      delegate_->GetWeakPtr()));
+  auto wait_for_stable_element = proto_.show_cast().wait_for_stable_element();
+  if (wait_for_stable_element == STEP_UNSPECIFIED) {
+    wait_for_stable_element = SKIP_STEP;
+  }
+  action_delegate_util::AddOptionalStep(
+      wait_for_stable_element,
+      base::BindOnce(&ActionDelegate::WaitUntilElementIsStable,
+                     delegate_->GetWeakPtr(),
+                     proto_.show_cast().stable_check_max_rounds(),
+                     base::TimeDelta::FromMilliseconds(
+                         proto_.show_cast().stable_check_interval_ms())),
+      actions.get());
   actions->emplace_back(base::BindOnce(&ActionDelegate::ScrollToElementPosition,
                                        delegate_->GetWeakPtr(), selector,
                                        top_padding));
@@ -85,15 +95,18 @@ void ShowCastAction::OnWaitForElement(ProcessActionCallback callback,
       delegate_, selector,
       base::BindOnce(&action_delegate_util::PerformAll, std::move(actions)),
       base::BindOnce(&ShowCastAction::OnScrollToElementPosition,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
-void ShowCastAction::OnScrollToElementPosition(ProcessActionCallback callback,
-                                               const ClientStatus& status) {
+void ShowCastAction::OnScrollToElementPosition(const ClientStatus& status) {
   delegate_->SetTouchableElementArea(
       proto().show_cast().touchable_element_area());
+  EndAction(status);
+}
+
+void ShowCastAction::EndAction(const ClientStatus& status) {
   UpdateProcessedAction(status);
-  std::move(callback).Run(std::move(processed_action_proto_));
+  std::move(process_action_callback_).Run(std::move(processed_action_proto_));
 }
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/actions/show_cast_action.h

@@ -8,6 +8,7 @@
 #include "components/autofill_assistant/browser/actions/action.h"
 #include "components/autofill_assistant/browser/top_padding.h"
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 
@@ -24,12 +25,14 @@ class ShowCastAction : public Action {
   // Overrides Action:
   void InternalProcessAction(ProcessActionCallback callback) override;
 
-  void OnWaitForElement(ProcessActionCallback callback,
-                        const Selector& selector,
+  void OnWaitForElement(const Selector& selector,
                         const TopPadding& top_padding,
                         const ClientStatus& element_status);
-  void OnScrollToElementPosition(ProcessActionCallback callback,
-                                 const ClientStatus& status);
+  void OnScrollToElementPosition(const ClientStatus& status);
+
+  void EndAction(const ClientStatus& status);
+
+  ProcessActionCallback process_action_callback_;
 
   base::WeakPtrFactory<ShowCastAction> weak_ptr_factory_{this};
 

components/autofill_assistant/browser/actions/show_cast_action_unittest.cc

@@ -68,10 +68,9 @@ TEST_F(ShowCastActionTest, ActionFailsForNonExistentElement) {
 TEST_F(ShowCastActionTest, CheckExpectedCallChain) {
   InSequence sequence;
 
-  Selector selector({"#focus"});
-  *proto_.mutable_element_to_present() = selector.proto;
+  Selector expected_selector({"#focus"});
+  *proto_.mutable_element_to_present() = expected_selector.proto;
 
-  Selector expected_selector = selector;
   EXPECT_CALL(mock_action_delegate_,
               OnShortWaitForElement(expected_selector, _))
       .WillOnce(RunOnceCallback<1>(OkClientStatus(),
@@ -93,6 +92,38 @@ TEST_F(ShowCastActionTest, CheckExpectedCallChain) {
   Run();
 }
 
+TEST_F(ShowCastActionTest, WaitsForStableElementIfSpecified) {
+  InSequence sequence;
+
+  Selector expected_selector({"#focus"});
+  *proto_.mutable_element_to_present() = expected_selector.proto;
+  proto_.set_wait_for_stable_element(REQUIRE_STEP_SUCCESS);
+
+  EXPECT_CALL(mock_action_delegate_,
+              OnShortWaitForElement(expected_selector, _))
+      .WillOnce(RunOnceCallback<1>(OkClientStatus(),
+                                   base::TimeDelta::FromSeconds(0)));
+  auto expected_element =
+      test_util::MockFindElement(mock_action_delegate_, expected_selector);
+  EXPECT_CALL(mock_action_delegate_, WaitForDocumentToBecomeInteractive(
+                                         EqualsElement(expected_element), _))
+      .WillOnce(RunOnceCallback<1>(OkClientStatus()));
+  EXPECT_CALL(
+      mock_action_delegate_,
+      WaitUntilElementIsStable(_, _, EqualsElement(expected_element), _))
+      .WillOnce(RunOnceCallback<3>(OkClientStatus()));
+  EXPECT_CALL(mock_action_delegate_,
+              ScrollToElementPosition(expected_selector, _,
+                                      EqualsElement(expected_element), _))
+      .WillOnce(RunOnceCallback<3>(OkClientStatus()));
+  EXPECT_CALL(mock_action_delegate_, SetTouchableElementArea(_));
+
+  EXPECT_CALL(
+      callback_,
+      Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
+  Run();
+}
+
 TEST_F(ShowCastActionTest, SetsTitleIfSpecified) {
   ON_CALL(mock_action_delegate_, OnShortWaitForElement(_, _))
       .WillByDefault(RunOnceCallback<1>(OkClientStatus(),

components/autofill_assistant/browser/actions/use_address_action_unittest.cc

@@ -596,8 +596,8 @@ TEST_F(UseAddressActionTest, ForcedFallbackWithKeystrokes) {
   EXPECT_CALL(mock_action_delegate_,
               ScrollIntoView(EqualsElement(expected_element), _))
       .WillOnce(RunOnceCallback<1>(OkClientStatus()));
-  EXPECT_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _))
-      .WillOnce(RunOnceCallback<1>(OkClientStatus()));
+  EXPECT_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _, _, _))
+      .WillOnce(RunOnceCallback<3>(OkClientStatus()));
   EXPECT_CALL(
       mock_action_delegate_,
       ClickOrTapElement(ClickType::CLICK, EqualsElement(expected_element), _))

components/autofill_assistant/browser/actions/use_credit_card_action_unittest.cc

@@ -417,8 +417,8 @@ TEST_F(UseCreditCardActionTest, ForcedFallbackWithKeystrokes) {
   EXPECT_CALL(mock_action_delegate_,
               ScrollIntoView(EqualsElement(expected_element), _))
       .WillOnce(RunOnceCallback<1>(OkClientStatus()));
-  EXPECT_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _))
-      .WillOnce(RunOnceCallback<1>(OkClientStatus()));
+  EXPECT_CALL(mock_action_delegate_, WaitUntilElementIsStable(_, _, _, _))
+      .WillOnce(RunOnceCallback<3>(OkClientStatus()));
   EXPECT_CALL(
       mock_action_delegate_,
       ClickOrTapElement(ClickType::CLICK, EqualsElement(expected_element), _))

components/autofill_assistant/browser/controller.cc

@@ -139,8 +139,7 @@ Service* Controller::GetService() {
 
 WebController* Controller::GetWebController() {
   if (!web_controller_) {
-    web_controller_ =
-        WebController::CreateForWebContents(web_contents(), &settings_);
+    web_controller_ = WebController::CreateForWebContents(web_contents());
   }
   return web_controller_.get();
 }

components/autofill_assistant/browser/script_executor.cc

@@ -347,10 +347,12 @@ void ScriptExecutor::ScrollIntoView(
 }
 
 void ScriptExecutor::WaitUntilElementIsStable(
+    int max_rounds,
+    base::TimeDelta check_interval,
     const ElementFinder::Result& element,
     base::OnceCallback<void(const ClientStatus&)> callback) {
-  delegate_->GetWebController()->WaitUntilElementIsStable(element,
-                                                          std::move(callback));
+  delegate_->GetWebController()->WaitUntilElementIsStable(
+      element, max_rounds, check_interval, std::move(callback));
 }
 
 void ScriptExecutor::CheckOnTop(
@@ -822,7 +824,7 @@ void ScriptExecutor::WaitForWindowHeightChange(
   delegate_->GetWebController()->WaitForWindowHeightChange(std::move(callback));
 }
 
-const ClientSettings& ScriptExecutor::GetSettings() {
+const ClientSettings& ScriptExecutor::GetSettings() const {
   return delegate_->GetSettings();
 }
 

components/autofill_assistant/browser/script_executor.h

@@ -136,6 +136,8 @@ class ScriptExecutor : public ActionDelegate,
       const ElementFinder::Result& element,
       base::OnceCallback<void(const ClientStatus&)> callback) override;
   void WaitUntilElementIsStable(
+      int max_rounds,
+      base::TimeDelta check_interval,
       const ElementFinder::Result& element,
       base::OnceCallback<void(const ClientStatus&)> callback) override;
   void CheckOnTop(
@@ -276,7 +278,7 @@ class ScriptExecutor : public ActionDelegate,
   void CollapseBottomSheet() override;
   void WaitForWindowHeightChange(
       base::OnceCallback<void(const ClientStatus&)> callback) override;
-  const ClientSettings& GetSettings() override;
+  const ClientSettings& GetSettings() const override;
   bool SetForm(
       std::unique_ptr<FormProto> form,
       base::RepeatingCallback<void(const FormProto::Result*)> changed_callback,

components/autofill_assistant/browser/script_executor_unittest.cc

@@ -74,8 +74,8 @@ class ScriptExecutorTest : public testing::Test,
                                           base::TimeDelta::FromSeconds(0)));
     ON_CALL(mock_web_controller_, OnScrollIntoView(_, _))
         .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
-    ON_CALL(mock_web_controller_, WaitUntilElementIsStable(_, _))
-        .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
+    ON_CALL(mock_web_controller_, WaitUntilElementIsStable(_, _, _, _))
+        .WillByDefault(RunOnceCallback<3>(OkClientStatus()));
     ON_CALL(mock_web_controller_, OnClickOrTapElement(_, _))
         .WillByDefault(RunOnceCallback<1>(ClientStatus(UNEXPECTED_JS_ERROR)));
   }

components/autofill_assistant/browser/service.proto

@@ -1354,6 +1354,20 @@ message ShowCastProto {
 
   // The padding that will be added between the focused element and the top.
   optional TopPadding top_padding = 7;
+
+  // Configure whether the scrolling should wait for the element to be stable
+  // before scrolling.
+  //
+  // If set to REQUIRE_STEP_SUCCESS, scrolling may fail with ELEMENT_UNSTABLE.
+  optional OptionalStep wait_for_stable_element = 9;
+
+  // Maximum rounds to stable check.
+  optional int32 stable_check_max_rounds = 10 [default = 50];
+
+  // Interval for stable check in ms.
+  optional int32 stable_check_interval_ms = 11 [default = 200];
+
+  reserved 3, 4, 8;
 }
 
 // An area made up of rectangles whole border are made defined by the position

components/autofill_assistant/browser/ui_delegate.h

@@ -11,6 +11,7 @@
 
 #include "base/optional.h"
 #include "bottom_sheet_state.h"
+#include "components/autofill_assistant/browser/client_settings.h"
 #include "components/autofill_assistant/browser/event_handler.h"
 #include "components/autofill_assistant/browser/metrics.h"
 #include "components/autofill_assistant/browser/rectf.h"
@@ -24,7 +25,6 @@ class ControllerObserver;
 class Details;
 class InfoBox;
 class BasicInteractions;
-struct ClientSettings;
 
 // UI delegate called for script executions.
 class UiDelegate {

components/autofill_assistant/browser/web/element_position_getter.cc

@@ -25,10 +25,11 @@ namespace autofill_assistant {
 
 ElementPositionGetter::ElementPositionGetter(
     DevtoolsClient* devtools_client,
-    const ClientSettings& settings,
+    int max_rounds,
+    base::TimeDelta check_interval,
     const std::string& optional_node_frame_id)
-    : check_interval_(settings.box_model_check_interval),
-      max_rounds_(settings.box_model_check_count),
+    : check_interval_(check_interval),
+      max_rounds_(max_rounds),
       devtools_client_(devtools_client),
       node_frame_id_(optional_node_frame_id),
       weak_ptr_factory_(this) {}

components/autofill_assistant/browser/web/element_position_getter.h

@@ -11,7 +11,6 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "components/autofill_assistant/browser/client_settings.h"
 #include "components/autofill_assistant/browser/devtools/devtools/domains/types_dom.h"
 #include "components/autofill_assistant/browser/devtools/devtools/domains/types_runtime.h"
 #include "components/autofill_assistant/browser/devtools/devtools_client.h"
@@ -30,7 +29,8 @@ class ElementPositionGetter : public WebControllerWorker {
  public:
   // |devtools_client| must be valid for the lifetime of the instance.
   ElementPositionGetter(DevtoolsClient* devtools_client,
-                        const ClientSettings& settings,
+                        int max_rounds,
+                        base::TimeDelta check_interval,
                         const std::string& optional_node_frame_id);
   ~ElementPositionGetter() override;
 
@@ -41,8 +41,6 @@ class ElementPositionGetter : public WebControllerWorker {
   // If the operation succeeded, check the coordinate in the getter.
   using Callback = base::OnceCallback<void(const ClientStatus&)>;
 
-  void DisableWaitForElementStable() { max_rounds_ = 1; }
-
   // The X coordinate of the center of the element, only valid after getting a
   // successful callback.
   int x() { return point_x_; }

components/autofill_assistant/browser/web/mock_web_controller.cc

@@ -6,8 +6,7 @@
 
 namespace autofill_assistant {
 
-MockWebController::MockWebController()
-    : WebController(nullptr, nullptr, nullptr) {}
+MockWebController::MockWebController() : WebController(nullptr, nullptr) {}
 MockWebController::~MockWebController() {}
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/web/mock_web_controller.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "base/time/time.h"
 #include "components/autofill_assistant/browser/top_padding.h"
 #include "components/autofill_assistant/browser/web/element_finder.h"
 #include "components/autofill_assistant/browser/web/element_rect_getter.h"
@@ -43,8 +44,10 @@ class MockWebController : public WebController {
                void(const ElementFinder::Result&,
                     base::OnceCallback<void(const ClientStatus&)>&));
 
-  MOCK_METHOD2(WaitUntilElementIsStable,
+  MOCK_METHOD4(WaitUntilElementIsStable,
                void(const ElementFinder::Result& element,
+                    int,
+                    base::TimeDelta,
                     base::OnceCallback<void(const ClientStatus&)> callback));
 
   void ClickOrTapElement(

components/autofill_assistant/browser/web/web_controller.cc

@@ -23,7 +23,6 @@
 #include "components/autofill/core/browser/data_model/credit_card.h"
 #include "components/autofill/core/common/autofill_constants.h"
 #include "components/autofill/core/common/form_data.h"
-#include "components/autofill_assistant/browser/client_settings.h"
 #include "components/autofill_assistant/browser/client_status.h"
 #include "components/autofill_assistant/browser/rectf.h"
 #include "components/autofill_assistant/browser/string_conversions_util.h"
@@ -286,21 +285,17 @@ void DecorateControllerStatusWithValue(
 
 // static
 std::unique_ptr<WebController> WebController::CreateForWebContents(
-    content::WebContents* web_contents,
-    const ClientSettings* settings) {
+    content::WebContents* web_contents) {
   return std::make_unique<WebController>(
       web_contents,
       std::make_unique<DevtoolsClient>(
-          content::DevToolsAgentHost::GetOrCreateFor(web_contents)),
-      settings);
+          content::DevToolsAgentHost::GetOrCreateFor(web_contents)));
 }
 
 WebController::WebController(content::WebContents* web_contents,
-                             std::unique_ptr<DevtoolsClient> devtools_client,
-                             const ClientSettings* settings)
+                             std::unique_ptr<DevtoolsClient> devtools_client)
     : web_contents_(web_contents),
-      devtools_client_(std::move(devtools_client)),
-      settings_(settings) {}
+      devtools_client_(std::move(devtools_client)) {}
 
 WebController::~WebController() {}
 
@@ -436,13 +431,16 @@ void WebController::OnCheckOnTop(
 
 void WebController::WaitUntilElementIsStable(
     const ElementFinder::Result& element,
+    int max_rounds,
+    base::TimeDelta check_interval,
     base::OnceCallback<void(const ClientStatus&)> callback) {
   auto wrapped_callback = GetAssistantActionRunningStateRetainingCallback(
       element, std::move(callback));
 
   std::unique_ptr<ElementPositionGetter> getter =
-      std::make_unique<ElementPositionGetter>(
-          devtools_client_.get(), *settings_, element.node_frame_id);
+      std::make_unique<ElementPositionGetter>(devtools_client_.get(),
+                                              max_rounds, check_interval,
+                                              element.node_frame_id);
   auto* ptr = getter.get();
   pending_workers_.emplace_back(std::move(getter));
   ptr->Start(element.container_frame_host, element.object_id,
@@ -495,8 +493,9 @@ void WebController::ClickOrTapElement(
 
   std::unique_ptr<ElementPositionGetter> getter =
       std::make_unique<ElementPositionGetter>(
-          devtools_client_.get(), *settings_, element.node_frame_id);
-  getter->DisableWaitForElementStable();
+          devtools_client_.get(), /* max_rounds= */ 1,
+          /* check_interval= */ base::TimeDelta::FromMilliseconds(0),
+          element.node_frame_id);
   auto* ptr = getter.get();
   pending_workers_.emplace_back(std::move(getter));
   ptr->Start(

components/autofill_assistant/browser/web/web_controller.h

@@ -12,6 +12,7 @@
 #include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/time/time.h"
 #include "components/autofill_assistant/browser/batch_element_checker.h"
 #include "components/autofill_assistant/browser/client_status.h"
 #include "components/autofill_assistant/browser/devtools/devtools/domains/types_dom.h"
@@ -44,7 +45,6 @@ class RenderFrameHost;
 }  // namespace content
 
 namespace autofill_assistant {
-struct ClientSettings;
 
 // Controller to interact with the web pages.
 //
@@ -60,13 +60,11 @@ class WebController {
   // Create web controller for a given |web_contents|. |settings| must be valid
   // for the lifetime of the controller.
   static std::unique_ptr<WebController> CreateForWebContents(
-      content::WebContents* web_contents,
-      const ClientSettings* settings);
+      content::WebContents* web_contents);
 
   // |web_contents| and |settings| must outlive this web controller.
   WebController(content::WebContents* web_contents,
-                std::unique_ptr<DevtoolsClient> devtools_client,
-                const ClientSettings* settings);
+                std::unique_ptr<DevtoolsClient> devtools_client);
   virtual ~WebController();
 
   // Load |url| in the current tab. Returns immediately, before the new page has
@@ -103,6 +101,8 @@ class WebController {
   // the element position doesn't stabilize quickly enough.
   virtual void WaitUntilElementIsStable(
       const ElementFinder::Result& element,
+      int max_rounds,
+      base::TimeDelta check_interval,
       base::OnceCallback<void(const ClientStatus&)> callback);
 
   // Check whether the center given element is on top. Fail with
@@ -450,7 +450,6 @@ class WebController {
   // is guaranteed by the owner of this object.
   content::WebContents* web_contents_;
   std::unique_ptr<DevtoolsClient> devtools_client_;
-  const ClientSettings* const settings_;
 
   // Currently running workers.
   std::vector<std::unique_ptr<WebControllerWorker>> pending_workers_;

components/autofill_assistant/browser/web/web_controller_browsertest.cc

@@ -8,7 +8,6 @@
 #include "base/memory/ref_counted.h"
 #include "base/strings/strcat.h"
 #include "base/test/bind.h"
-#include "components/autofill_assistant/browser/client_settings.h"
 #include "components/autofill_assistant/browser/service.pb.h"
 #include "components/autofill_assistant/browser/string_conversions_util.h"
 #include "components/autofill_assistant/browser/top_padding.h"
@@ -59,8 +58,8 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
     ASSERT_TRUE(http_server_->Start(8080));
     ASSERT_TRUE(
         NavigateToURL(shell(), http_server_->GetURL(kTargetWebsitePath)));
-    web_controller_ = WebController::CreateForWebContents(
-        shell()->web_contents(), &settings_);
+    web_controller_ =
+        WebController::CreateForWebContents(shell()->web_contents());
     Observe(shell()->web_contents());
   }
 
@@ -1003,7 +1002,6 @@ document.getElementById("overlay_in_frame").style.visibility='hidden';
 
  protected:
   std::unique_ptr<WebController> web_controller_;
-  ClientSettings settings_;
 
  private:
   std::unique_ptr<net::EmbeddedTestServer> http_server_;