Commit 6964c68b authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Re-check scripts regularly after a new page load.

This change is a quick and dirty way of making sure that scripts that
expect a specific element on the page will get triggered, by checking
for 20 seconds after a page load or after a script has finished.

This is just enough to unblock further manual tests while we work on a
proper solution.

Bug: 806868
Change-Id: I218ba905a2eef586463c6335435e97cdca1657d0
Reviewed-on: https://chromium-review.googlesource.com/c/1256691Reviewed-by: default avatarGanggui Tang <gogerald@chromium.org>
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596162}
parent 41d7dc9b
...@@ -6,13 +6,28 @@ ...@@ -6,13 +6,28 @@
#include <utility> #include <utility>
#include "base/task/post_task.h"
#include "components/autofill_assistant/browser/protocol_utils.h" #include "components/autofill_assistant/browser/protocol_utils.h"
#include "components/autofill_assistant/browser/ui_controller.h" #include "components/autofill_assistant/browser/ui_controller.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace autofill_assistant { namespace autofill_assistant {
namespace {
// Time between two periodic script checks.
static constexpr base::TimeDelta kPeriodicScriptCheckInterval =
base::TimeDelta::FromSeconds(2);
// Number of script checks to run after a call to StartPeriodicScriptChecks.
static constexpr int kPeriodicScriptCheckCount = 10;
} // namespace
// static // static
void Controller::CreateAndStartForWebContents( void Controller::CreateAndStartForWebContents(
content::WebContents* web_contents, content::WebContents* web_contents,
...@@ -71,7 +86,10 @@ Controller::Controller( ...@@ -71,7 +86,10 @@ Controller::Controller(
/* listener= */ this)), /* listener= */ this)),
parameters_(std::move(parameters)), parameters_(std::move(parameters)),
memory_(std::make_unique<ClientMemory>()), memory_(std::make_unique<ClientMemory>()),
allow_autostart_(true) { allow_autostart_(true),
periodic_script_check_scheduled_(false),
periodic_script_check_count_(false),
weak_ptr_factory_(this) {
DCHECK(parameters_); DCHECK(parameters_);
GetUiController()->SetUiDelegate(this); GetUiController()->SetUiDelegate(this);
...@@ -88,13 +106,48 @@ void Controller::GetOrCheckScripts(const GURL& url) { ...@@ -88,13 +106,48 @@ void Controller::GetOrCheckScripts(const GURL& url) {
return; return;
if (script_domain_ != url.host()) { if (script_domain_ != url.host()) {
StopPeriodicScriptChecks();
script_domain_ = url.host(); script_domain_ = url.host();
service_->GetScriptsForUrl( service_->GetScriptsForUrl(
url, *parameters_, url, *parameters_,
base::BindOnce(&Controller::OnGetScripts, base::Unretained(this), url)); base::BindOnce(&Controller::OnGetScripts, base::Unretained(this), url));
} else { } else {
script_tracker_->CheckScripts(); script_tracker_->CheckScripts();
StartPeriodicScriptChecks();
}
}
void Controller::StartPeriodicScriptChecks() {
periodic_script_check_count_ = kPeriodicScriptCheckCount;
// If periodic checks are running, setting periodic_script_check_count_ keeps
// them running longer.
if (periodic_script_check_scheduled_)
return;
periodic_script_check_scheduled_ = true;
base::PostDelayedTaskWithTraits(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&Controller::OnPeriodicScriptCheck,
weak_ptr_factory_.GetWeakPtr()),
kPeriodicScriptCheckInterval);
}
void Controller::StopPeriodicScriptChecks() {
periodic_script_check_count_ = 0;
}
void Controller::OnPeriodicScriptCheck() {
if (periodic_script_check_count_ <= 0) {
DCHECK_EQ(0, periodic_script_check_count_);
periodic_script_check_scheduled_ = false;
return;
} }
periodic_script_check_count_--;
script_tracker_->CheckScripts();
base::PostDelayedTaskWithTraits(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&Controller::OnPeriodicScriptCheck,
weak_ptr_factory_.GetWeakPtr()),
kPeriodicScriptCheckInterval);
} }
void Controller::OnGetScripts(const GURL& url, void Controller::OnGetScripts(const GURL& url,
...@@ -115,6 +168,7 @@ void Controller::OnGetScripts(const GURL& url, ...@@ -115,6 +168,7 @@ void Controller::OnGetScripts(const GURL& url,
bool parse_result = ProtocolUtils::ParseScripts(response, &scripts); bool parse_result = ProtocolUtils::ParseScripts(response, &scripts);
DCHECK(parse_result); DCHECK(parse_result);
script_tracker_->SetAndCheckScripts(std::move(scripts)); script_tracker_->SetAndCheckScripts(std::move(scripts));
StartPeriodicScriptChecks();
} }
void Controller::OnScriptExecuted(const std::string& script_path, void Controller::OnScriptExecuted(const std::string& script_path,
...@@ -157,6 +211,7 @@ void Controller::OnScriptSelected(const std::string& script_path) { ...@@ -157,6 +211,7 @@ void Controller::OnScriptSelected(const std::string& script_path) {
GetUiController()->ShowOverlay(); GetUiController()->ShowOverlay();
allow_autostart_ = false; // Only ever autostart the very first script. allow_autostart_ = false; // Only ever autostart the very first script.
StopPeriodicScriptChecks();
script_tracker_->ExecuteScript( script_tracker_->ExecuteScript(
script_path, base::BindOnce(&Controller::OnScriptExecuted, script_path, base::BindOnce(&Controller::OnScriptExecuted,
// script_tracker_ is owned by Controller. // script_tracker_ is owned by Controller.
......
...@@ -67,6 +67,15 @@ class Controller : public ScriptExecutorDelegate, ...@@ -67,6 +67,15 @@ class Controller : public ScriptExecutorDelegate,
void OnScriptExecuted(const std::string& script_path, void OnScriptExecuted(const std::string& script_path,
ScriptExecutor::Result result); ScriptExecutor::Result result);
// Check script preconditions every few seconds for a certain number of times.
// If checks are already running, StartPeriodicScriptChecks resets the count.
//
// TODO(crbug.com/806868): Find a better solution. This is a brute-force
// solution that reacts slowly to changes.
void StartPeriodicScriptChecks();
void StopPeriodicScriptChecks();
void OnPeriodicScriptCheck();
// Overrides content::UiDelegate: // Overrides content::UiDelegate:
void OnClickOverlay() override; void OnClickOverlay() override;
void OnDestroy() override; void OnDestroy() override;
...@@ -93,6 +102,14 @@ class Controller : public ScriptExecutorDelegate, ...@@ -93,6 +102,14 @@ class Controller : public ScriptExecutorDelegate,
std::unique_ptr<ClientMemory> memory_; std::unique_ptr<ClientMemory> memory_;
bool allow_autostart_; bool allow_autostart_;
// Whether a task for periodic checks is scheduled.
bool periodic_script_check_scheduled_;
// Number of remaining periodic checks.
int periodic_script_check_count_;
base::WeakPtrFactory<Controller> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(Controller); DISALLOW_COPY_AND_ASSIGN(Controller);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment