Debounce MutationObserver notifications in SelectorObserver

Now it will only check selectors when it hasn't received a mutation
notification for |debonuce_interval| ms (configurable), we still have
the usual unconditional checks every |min_check_interval| ms.

Change-Id: I49e0409c8cef69164e2b26472479b31222460ecd
Bug: b:205676462
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3629240
Commit-Queue: David Bengoa <dbengoa@google.com>
Reviewed-by: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/main@{#1001988}

components/autofill_assistant/browser/batch_element_checker.cc

@@ -76,19 +76,13 @@ void BatchElementChecker::AddAllDoneCallback(
 }
 
 void BatchElementChecker::EnableObserver(
-    base::TimeDelta max_wait_time,
-    base::TimeDelta periodic_check_interval,
-    base::TimeDelta extra_timeout) {
-  DCHECK(!use_observers_);
+    const SelectorObserver::Settings& settings) {
+  DCHECK(!observer_settings_);
   DCHECK(!started_);
   DCHECK(get_field_value_callbacks_.empty())
       << "Observer-based BatchElementChecker doesn't work with "
          "AddFieldValueCheck";
-
-  use_observers_ = true;
-  observer_max_wait_time_ = max_wait_time;
-  observer_periodic_check_interval_ = periodic_check_interval;
-  observer_extra_timeout_ = extra_timeout;
+  observer_settings_.emplace(settings);
 }
 
 void BatchElementChecker::Run(WebController* web_controller) {
@@ -97,7 +91,7 @@ void BatchElementChecker::Run(WebController* web_controller) {
   for (size_t i = 0; i < element_condition_checks_.size(); ++i) {
     AddElementConditionResults(element_condition_checks_[i].proto, i);
   }
-  if (use_observers_) {
+  if (observer_settings_) {
     RunWithObserver(web_controller);
     return;
   }
@@ -149,6 +143,7 @@ void BatchElementChecker::RunWithObserver(WebController* web_controller) {
   DCHECK(get_field_value_callbacks_.empty())
       << "Observer-based BatchElementChecker doesn't work with "
          "AddFieldValueCheck";
+  DCHECK(observer_settings_);
   DCHECK(!started_);
   std::vector<SelectorObserver::ObservableSelector> selectors;
 
@@ -164,8 +159,7 @@ void BatchElementChecker::RunWithObserver(WebController* web_controller) {
   }
   started_ = true;
   auto result = web_controller->ObserveSelectors(
-      selectors, observer_max_wait_time_, observer_periodic_check_interval_,
-      observer_extra_timeout_,
+      selectors, *observer_settings_,
       base::BindRepeating(&BatchElementChecker::OnResultsUpdated,
                           weak_ptr_factory_.GetWeakPtr())
 

components/autofill_assistant/browser/batch_element_checker.h

@@ -84,9 +84,7 @@ class BatchElementChecker {
 
   // Turns on observer mode. When BatchElementChecker runs in observer mode, it
   // waits until any element condition checks or element checks become true.
-  void EnableObserver(base::TimeDelta max_wait_time,
-                      base::TimeDelta periodic_check_interval,
-                      base::TimeDelta extra_timeout);
+  void EnableObserver(const SelectorObserver::Settings& settings);
 
   // Runs the checks. Once all checks are done, calls the callbacks registered
   // to AddAllDoneCallback().
@@ -207,11 +205,10 @@ class BatchElementChecker {
   // Run() was called. Checking elements might or might not have finished yet.
   bool started_ = false;
 
-  // Whether to wait until one of the conditions becomes true.
-  bool use_observers_ = false;
-  base::TimeDelta observer_max_wait_time_;
-  base::TimeDelta observer_periodic_check_interval_;
-  base::TimeDelta observer_extra_timeout_;
+  // Whether to wait until one of the conditions becomes true. If it's a nullopt
+  // it will check only once, if it has a value it will observe the DOM until
+  // one of the conditions becomes true or the observation times out.
+  absl::optional<const SelectorObserver::Settings> observer_settings_;
 
   std::vector<base::OnceCallback<void()>> all_done_;
 

components/autofill_assistant/browser/client_settings.cc

@@ -176,6 +176,10 @@ void ClientSettings::UpdateFromProto(const ClientSettingsProto& proto) {
     selector_observer_extra_timeout =
         base::Milliseconds(proto.selector_observer_extra_timeout_ms());
   }
+  if (proto.has_selector_observer_debounce_interval_ms()) {
+    selector_observer_debounce_interval =
+        base::Milliseconds(proto.selector_observer_debounce_interval_ms());
+  }
 }
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/client_settings.h

@@ -147,6 +147,10 @@ struct ClientSettings {
   // time spent waiting so a extra delay of 1 to 10 seconds for javascript
   // execution and checking selectors is conceivable.
   base::TimeDelta selector_observer_extra_timeout = base::Seconds(15);
+
+  // Wait until no DOM changes are received for this amount of time to check
+  // the selectors. An interval of 0 effectively disables debouncing.
+  base::TimeDelta selector_observer_debounce_interval = base::Milliseconds(100);
 };
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/mock_script_executor_delegate.cc

@@ -11,6 +11,7 @@ using ::testing::ReturnRef;
 MockScriptExecutorDelegate::MockScriptExecutorDelegate() {
   ON_CALL(*this, GetSettings).WillByDefault(ReturnRef(client_settings_));
   ON_CALL(*this, GetLogInfo).WillByDefault(ReturnRef(log_info_));
+  ON_CALL(*this, GetCurrentURL).WillByDefault(ReturnRef(default_url_));
 }
 
 MockScriptExecutorDelegate::~MockScriptExecutorDelegate() = default;

components/autofill_assistant/browser/mock_script_executor_delegate.h

@@ -100,6 +100,7 @@ class MockScriptExecutorDelegate : public ScriptExecutorDelegate {
  private:
   ClientSettings client_settings_;
   ProcessedActionStatusDetailsProto log_info_;
+  const GURL default_url_ = GURL("https://example.com/");
 };
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/service.proto

@@ -324,7 +324,7 @@ message OverlayImageProto {
   optional ClientDimensionProto text_size = 7;
 }
 
-// Next ID: 25
+// Next ID: 26
 message ClientSettingsProto {
   message IntegrationTestSettings {
     // Disables animations for the poodle and the progress bar.
@@ -505,6 +505,10 @@ message ClientSettingsProto {
   // wrong and fails with a |TIMED_OUT| error.
   optional int32 selector_observer_extra_timeout_ms = 24;
 
+  // SelectorObserver will wait until no DOM mutation notifications are received
+  // for this amount of time to check the selectors.
+  optional int32 selector_observer_debounce_interval_ms = 25;
+
   reserved 8 to 11;
 }
 

components/autofill_assistant/browser/wait_for_dom_operation.cc

@@ -165,12 +165,14 @@ void WaitForDomOperation::RunChecks(
                      base::Unretained(this), std::move(report_attempt_result)));
   if (use_observers_) {
     batch_element_checker_->EnableObserver(
-        /* max_wait_time= */ max_wait_time_ -
-            wait_time_stopwatch_.TotalElapsed(),
-        /* periodic_check_interval= */
-        delegate_->GetSettings().periodic_element_check_interval,
-        /* extra_timeout= */
-        delegate_->GetSettings().selector_observer_extra_timeout);
+        {/* max_wait_time= */ max_wait_time_ -
+             wait_time_stopwatch_.TotalElapsed(),
+         /* min_check_interval= */
+         delegate_->GetSettings().periodic_element_check_interval,
+         /* extra_timeout= */
+         delegate_->GetSettings().selector_observer_extra_timeout,
+         /* debounce_interval */
+         delegate_->GetSettings().selector_observer_debounce_interval});
   }
   batch_element_checker_->Run(delegate_->GetWebController());
 }

components/autofill_assistant/browser/web/batch_element_checker_browsertest.cc

@@ -101,6 +101,12 @@ class BatchElementCheckerBrowserTest
     return proto;
   }
 
+  static SelectorObserver::Settings SelectorObserverDefaultSettings(
+      base::TimeDelta max_wait_time) {
+    return {max_wait_time, base::Seconds(1), base::Seconds(15),
+            base::Milliseconds(100)};
+  }
+
   // Run Observer BatchElementChecker on the provided conditions. The second
   // value in the pairs (bool) is the match expectation.
   void RunObserverBatchElementChecker(
@@ -123,8 +129,7 @@ class BatchElementCheckerBrowserTest
             ObserverBatchElementCheckerAllDoneCallback,
         run_loop.QuitClosure(), &expected_results, &actual_results));
 
-    checker.EnableObserver(base::Seconds(30), base::Seconds(1),
-                           base::Seconds(15));
+    checker.EnableObserver(SelectorObserverDefaultSettings(base::Seconds(30)));
     checker.Run(web_controller_.get());
     run_loop.Run();
     EXPECT_EQ(web_controller_->pending_workers_.size(), 0u);
@@ -335,7 +340,7 @@ IN_PROC_BROWSER_TEST_F(BatchElementCheckerBrowserTest, SelectorObserver) {
         /* proto = */
         Selector({"#iframeExternal", ".dynamic.about-2-seconds"}).proto,
         /* strict = */ true}},
-      base::Seconds(30), base::Seconds(1), base::Seconds(15), update_callback);
+      SelectorObserverDefaultSettings(base::Seconds(30)), update_callback);
 
   run_loop.Run();
   ASSERT_TRUE(expected_updates.empty());
@@ -375,7 +380,7 @@ IN_PROC_BROWSER_TEST_F(BatchElementCheckerBrowserTest,
       {{/* selector_id = */ button_id,
         /* proto = */ Selector({"#iframeRedirecting", "#button"}).proto,
         /* strict = */ true}},
-      base::Seconds(30), base::Seconds(1), base::Seconds(15), update_callback);
+      SelectorObserverDefaultSettings(base::Seconds(30)), update_callback);
 
   run_loop.Run();
 }
@@ -411,7 +416,7 @@ IN_PROC_BROWSER_TEST_F(BatchElementCheckerBrowserTest,
       {{/* selector_id = */ SelectorObserver::SelectorId(1),
         /* proto = */ Selector({"#does_not_exist"}).proto,
         /* strict = */ true}},
-      base::Milliseconds(300), base::Seconds(1), base::Seconds(15),
+      SelectorObserverDefaultSettings(base::Milliseconds(300)),
       mock_callback.Get());
 
   run_loop.Run();
@@ -454,7 +459,7 @@ IN_PROC_BROWSER_TEST_F(BatchElementCheckerBrowserTest,
       {{/* selector_id = */ button_id,
         /* proto = */ Selector({"#iframe", "#button"}).proto,
         /* strict = */ true}},
-      base::Milliseconds(1), base::Seconds(1), base::Seconds(15),
+      SelectorObserverDefaultSettings(base::Milliseconds(1)),
       update_callback.Get());
 
   run_loop.Run();

components/autofill_assistant/browser/web/selector_observer.cc

@@ -61,23 +61,31 @@ SelectorObserver::RequestedElement::~RequestedElement() = default;
 SelectorObserver::RequestedElement::RequestedElement(const RequestedElement&) =
     default;
 
+SelectorObserver::Settings::Settings(const base::TimeDelta& max_wait_time,
+                                     const base::TimeDelta& min_check_interval,
+                                     const base::TimeDelta& extra_timeout,
+                                     const base::TimeDelta& debounce_interval)
+    : max_wait_time(max_wait_time),
+      min_check_interval(min_check_interval),
+      extra_timeout(extra_timeout),
+      debounce_interval(debounce_interval) {}
+SelectorObserver::Settings::~Settings() = default;
+SelectorObserver::Settings::Settings(const Settings&) = default;
+
 SelectorObserver::SelectorObserver(
     const std::vector<ObservableSelector>& selectors,
-    base::TimeDelta max_wait_time,
-    base::TimeDelta periodic_check_interval,
-    base::TimeDelta extra_timeout,
+    const Settings& settings,
     content::WebContents* web_contents,
     DevtoolsClient* devtools_client,
     const UserData* user_data,
     Callback update_callback)
-    : periodic_check_interval_(periodic_check_interval),
-      extra_timeout_(extra_timeout),
+    : settings_(settings),
       devtools_client_(devtools_client),
       web_contents_(web_contents),
       user_data_(user_data),
       update_callback_(update_callback) {
   const DomRoot root(/* frame_id = */ "", DomRoot::kUseMainDoc);
-  wait_time_remaining_ms_[root] = max_wait_time.InMilliseconds();
+  wait_time_remaining_ms_[root] = settings.max_wait_time.InMilliseconds();
   for (auto& selector : selectors) {
     selectors_.emplace(std::make_pair(selector.selector_id, selector));
     // Every selector starts in the root frame
@@ -106,7 +114,7 @@ ClientStatus SelectorObserver::Start(base::OnceClosure finished_callback) {
   ResolveObjectIdAndInjectFrame(root, 0);
 
   timeout_timer_ = std::make_unique<base::OneShotTimer>();
-  timeout_timer_->Start(FROM_HERE, MaxTimeRemaining() + extra_timeout_,
+  timeout_timer_->Start(FROM_HERE, MaxTimeRemaining() + settings_.extra_timeout,
                         base::BindOnce(&SelectorObserver::OnHardTimeout,
                                        weak_ptr_factory_.GetWeakPtr()));
 
@@ -814,15 +822,21 @@ std::string SelectorObserver::BuildExpression(const DomRoot& dom_root) const {
   snippet.AddLine("(function selectorObserver() {");
   snippet.AddLine(
       {"const pollInterval = ",
-       base::NumberToString(periodic_check_interval_.InMilliseconds()), ";"});
+       base::NumberToString(settings_.min_check_interval.InMilliseconds()),
+       ";"});
   int max_wait_time = wait_time_remaining_ms_.at(dom_root);
-  snippet.AddLine({"const maxRuntime = ",
-                   base::NumberToString(base::saturated_cast<int>(
-                       (base::Milliseconds(max_wait_time) + extra_timeout_)
-                           .InMilliseconds())),
-                   ";"});
+  snippet.AddLine(
+      {"const maxRuntime = ",
+       base::NumberToString(base::saturated_cast<int>(
+           (base::Milliseconds(max_wait_time) + settings_.extra_timeout)
+               .InMilliseconds())),
+       ";"});
   snippet.AddLine(
       {"const maxWaitTime = ", base::NumberToString(max_wait_time), ";"});
+  snippet.AddLine(
+      {"const debounceInterval = ",
+       base::NumberToString(settings_.debounce_interval.InMilliseconds()),
+       ";"});
   snippet.AddLine("const selectors = [");
 
   size_t depth = frame_depth_.at(dom_root);

components/autofill_assistant/browser/web/selector_observer.h

@@ -60,6 +60,7 @@ class SelectorObserver : public WebControllerWorker {
     // If true, match will fail if more that one element matches the selector.
     bool strict;
   };
+
   // An update to the match status of a selector.
   struct Update {
     Update();
@@ -73,6 +74,7 @@ class SelectorObserver : public WebControllerWorker {
     // fetch the element later.
     int element_id;
   };
+
   struct RequestedElement {
     RequestedElement(const SelectorId& selector_id, int element_id);
     ~RequestedElement();
@@ -85,18 +87,37 @@ class SelectorObserver : public WebControllerWorker {
     // end.
     int element_id;
   };
+
+  // Settings to configure the selector observer.
+  struct Settings {
+    Settings(const base::TimeDelta& max_wait_time,
+             const base::TimeDelta& min_check_interval,
+             const base::TimeDelta& extra_timeout,
+             const base::TimeDelta& debounce_interval);
+    ~Settings();
+    Settings(const Settings&);
+    // Maximum amount of time it will wait for an element.
+    const base::TimeDelta max_wait_time;
+    // Selector checks will run at least this often, even if no DOM changes are
+    // detected.
+    const base::TimeDelta min_check_interval;
+    // Extra wait time before assuming something has failed and giving up.
+    const base::TimeDelta extra_timeout;
+    // Wait until no DOM changes are received for this amount of time to check
+    // the selectors. An interval of 0 effectively disables debouncing.
+    const base::TimeDelta debounce_interval;
+  };
+
   using Callback = base::RepeatingCallback<
       void(const ClientStatus&, const std::vector<Update>&, SelectorObserver*)>;
 
   // |content::WebContents| and |DevtoolsClient| need to outlive this instance.
   // |UserData| needs to exist until Start() is called.
   explicit SelectorObserver(const std::vector<ObservableSelector>& selectors,
-                            base::TimeDelta max_wait_time,
-                            base::TimeDelta periodic_check_interval,
-                            base::TimeDelta extra_timeout,
-                            content::WebContents*,
-                            DevtoolsClient*,
-                            const UserData*,
+                            const Settings& settings,
+                            content::WebContents* web_contents,
+                            DevtoolsClient* devtools_client,
+                            const UserData* user_data,
                             Callback update_callback);
 
   ~SelectorObserver() override;
@@ -143,9 +164,7 @@ class SelectorObserver : public WebControllerWorker {
     ERROR_STATE = 4,
   };
   State state_ = State::INITIALIZED;
-  const base::TimeDelta periodic_check_interval_;
-  const base::TimeDelta extra_timeout_;
-  base::TimeDelta max_wait_time_;
+  const Settings settings_;
   base::TimeTicks started_;
   std::unique_ptr<base::OneShotTimer> timeout_timer_;