Commit 5bf6f1b1 authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Bind the script tracker with the controller.

With this change, the controller tells the script tracker to run checks
when necessary, calls the UI controller to choose a script to run and
run the script that was chosen. The implementation of the UI controller
for choosing the script is not included into this change.

Integration is preliminary, just to get something to get started until
we have a usable UI to try it out. The UI part, implementing script
selection, is missing from this change.

Change-Id: I867bec0e554dfdd87fc2f95e7c9088391621c5c9
Bug: 806868
Reviewed-on: https://chromium-review.googlesource.com/1194034
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarGanggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589135}
parent ccefc737
...@@ -72,6 +72,11 @@ void UiControllerAndroid::ChooseCard( ...@@ -72,6 +72,11 @@ void UiControllerAndroid::ChooseCard(
std::move(callback).Run(""); std::move(callback).Run("");
} }
void UiControllerAndroid::UpdateScripts(
const std::vector<ScriptHandle>& scripts) {
// TODO(crbug.com/806868): Implement UpdateScripts.
}
std::string UiControllerAndroid::GetApiKey() { std::string UiControllerAndroid::GetApiKey() {
std::string api_key; std::string api_key;
if (google_apis::IsGoogleChromeAPIKeyUsed()) { if (google_apis::IsGoogleChromeAPIKeyUsed()) {
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/android/scoped_java_ref.h" #include "base/android/scoped_java_ref.h"
#include "base/macros.h" #include "base/macros.h"
#include "components/autofill_assistant/browser/client.h" #include "components/autofill_assistant/browser/client.h"
#include "components/autofill_assistant/browser/script.h"
#include "components/autofill_assistant/browser/ui_controller.h" #include "components/autofill_assistant/browser/ui_controller.h"
namespace autofill_assistant { namespace autofill_assistant {
...@@ -30,6 +31,7 @@ class UiControllerAndroid : public UiController, public Client { ...@@ -30,6 +31,7 @@ class UiControllerAndroid : public UiController, public Client {
base::OnceCallback<void(const std::string&)> callback) override; base::OnceCallback<void(const std::string&)> callback) override;
void ChooseCard( void ChooseCard(
base::OnceCallback<void(const std::string&)> callback) override; base::OnceCallback<void(const std::string&)> callback) override;
void UpdateScripts(const std::vector<ScriptHandle>& scripts) override;
// Overrides Client: // Overrides Client:
std::string GetApiKey() override; std::string GetApiKey() override;
......
...@@ -63,6 +63,7 @@ jumbo_static_library("browser") { ...@@ -63,6 +63,7 @@ jumbo_static_library("browser") {
source_set("unit_tests") { source_set("unit_tests") {
testonly = true testonly = true
sources = [ sources = [
"controller_unittest.cc",
"mock_run_once_callback.h", "mock_run_once_callback.h",
"mock_service.cc", "mock_service.cc",
"mock_service.h", "mock_service.h",
...@@ -79,6 +80,7 @@ source_set("unit_tests") { ...@@ -79,6 +80,7 @@ source_set("unit_tests") {
":proto", ":proto",
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//content/test:test_support",
"//testing/gmock", "//testing/gmock",
"//testing/gtest", "//testing/gtest",
] ]
......
...@@ -17,7 +17,11 @@ namespace autofill_assistant { ...@@ -17,7 +17,11 @@ namespace autofill_assistant {
void Controller::CreateAndStartForWebContents( void Controller::CreateAndStartForWebContents(
content::WebContents* web_contents, content::WebContents* web_contents,
std::unique_ptr<Client> client) { std::unique_ptr<Client> client) {
new Controller(web_contents, std::move(client)); const std::string& api_key = client->GetApiKey();
new Controller(
web_contents, std::move(client),
WebController::CreateForWebContents(web_contents),
std::make_unique<Service>(api_key, web_contents->GetBrowserContext()));
} }
Service* Controller::GetService() { Service* Controller::GetService() {
...@@ -37,41 +41,68 @@ ClientMemory* Controller::GetClientMemory() { ...@@ -37,41 +41,68 @@ ClientMemory* Controller::GetClientMemory() {
} }
Controller::Controller(content::WebContents* web_contents, Controller::Controller(content::WebContents* web_contents,
std::unique_ptr<Client> client) std::unique_ptr<Client> client,
std::unique_ptr<WebController> web_controller,
std::unique_ptr<Service> service)
: content::WebContentsObserver(web_contents), : content::WebContentsObserver(web_contents),
client_(std::move(client)), client_(std::move(client)),
web_controller_(WebController::CreateForWebContents(web_contents)), web_controller_(std::move(web_controller)),
service_(std::make_unique<Service>(client_->GetApiKey(), service_(std::move(service)),
web_contents->GetBrowserContext())),
script_tracker_(std::make_unique<ScriptTracker>(this, this)) { script_tracker_(std::make_unique<ScriptTracker>(this, this)) {
GetUiController()->SetUiDelegate(this); GetUiController()->SetUiDelegate(this);
GetUiController()->ShowOverlay(); GetUiController()->ShowOverlay();
if (!web_contents->IsLoading()) { if (!web_contents->IsLoading()) {
GetScripts(); GetOrCheckScripts(web_contents->GetLastCommittedURL());
} }
} }
Controller::~Controller() {} Controller::~Controller() {}
void Controller::GetScripts() { void Controller::GetOrCheckScripts(const GURL& url) {
service_->GetScriptsForUrl( if (script_tracker_->running())
web_contents()->GetLastCommittedURL(), return;
base::BindOnce(&Controller::OnGetScripts, base::Unretained(this)));
if (script_domain_ != url.host()) {
script_domain_ = url.host();
service_->GetScriptsForUrl(
url,
base::BindOnce(&Controller::OnGetScripts, base::Unretained(this), url));
} else {
script_tracker_->CheckScripts();
}
} }
void Controller::OnGetScripts(bool result, const std::string& response) { void Controller::OnGetScripts(const GURL& url,
bool result,
const std::string& response) {
// If the domain of the current URL changed since the request was sent, the
// response is not relevant anymore and can be safely discarded.
if (url.host() != script_domain_)
return;
if (!result) { if (!result) {
LOG(ERROR) << "Failed to get assistant scripts for URL " LOG(ERROR) << "Failed to get assistant scripts for URL " << url.spec();
<< web_contents()->GetLastCommittedURL().spec();
// TODO(crbug.com/806868): Terminate Autofill Assistant. // TODO(crbug.com/806868): Terminate Autofill Assistant.
return; return;
} }
std::vector<std::unique_ptr<Script>> scripts; std::vector<std::unique_ptr<Script>> scripts;
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));
} }
void Controller::OnScriptExecuted(const std::string& script_path,
bool success) {
GetUiController()->HideOverlay();
if (!success) {
LOG(ERROR) << "Failed to execute script " << script_path;
// TODO(crbug.com/806868): Handle script execution failure.
}
GetOrCheckScripts(web_contents()->GetLastCommittedURL());
}
void Controller::OnClickOverlay() { void Controller::OnClickOverlay() {
GetUiController()->HideOverlay(); GetUiController()->HideOverlay();
// TODO(crbug.com/806868): Stop executing scripts. // TODO(crbug.com/806868): Stop executing scripts.
...@@ -81,16 +112,41 @@ void Controller::OnDestroy() { ...@@ -81,16 +112,41 @@ void Controller::OnDestroy() {
delete this; delete this;
} }
void Controller::OnScriptSelected(const std::string& script_path) {
DCHECK(!script_path.empty());
GetUiController()->ShowOverlay();
script_tracker_->ExecuteScript(
script_path, base::BindOnce(&Controller::OnScriptExecuted,
// script_tracker_ is owned by Controller.
base::Unretained(this), script_path));
}
void Controller::OnRunnableScriptsChanged() { void Controller::OnRunnableScriptsChanged() {
// TODO(crbug.com/806868): Take the set of runnable script from the tracker if (script_tracker_->running())
// and make them available for selection. Run the selected script. return;
GetUiController()->UpdateScripts(script_tracker_->runnable_scripts());
}
void Controller::DidGetUserInteraction(const blink::WebInputEvent::Type type) {
switch (type) {
case blink::WebInputEvent::kGestureLongTap:
case blink::WebInputEvent::kGestureTap:
if (!script_tracker_->running())
script_tracker_->CheckScripts();
break;
default:
break;
}
} }
void Controller::DidFinishLoad(content::RenderFrameHost* render_frame_host, void Controller::DidFinishLoad(content::RenderFrameHost* render_frame_host,
const GURL& validated_url) { const GURL& validated_url) {
// TODO(crbug.com/806868): Find a better time to get and update assistant // TODO(crbug.com/806868): Find a better time to get and update assistant
// scripts. // scripts.
GetScripts(); GetOrCheckScripts(validated_url);
} }
void Controller::WebContentsDestroyed() { void Controller::WebContentsDestroyed() {
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <memory> #include <memory>
#include <string> #include <string>
#include "base/macros.h"
#include "components/autofill_assistant/browser/client.h" #include "components/autofill_assistant/browser/client.h"
#include "components/autofill_assistant/browser/client_memory.h" #include "components/autofill_assistant/browser/client_memory.h"
#include "components/autofill_assistant/browser/script.h" #include "components/autofill_assistant/browser/script.h"
...@@ -24,6 +25,8 @@ class WebContents; ...@@ -24,6 +25,8 @@ class WebContents;
} // namespace content } // namespace content
namespace autofill_assistant { namespace autofill_assistant {
class ControllerTest;
// Autofill assistant controller controls autofill assistant action detection, // Autofill assistant controller controls autofill assistant action detection,
// display, execution and so on. The instance of this object self deletes when // display, execution and so on. The instance of this object self deletes when
// the web contents is being destroyed. // the web contents is being destroyed.
...@@ -42,21 +45,29 @@ class Controller : public ScriptExecutorDelegate, ...@@ -42,21 +45,29 @@ class Controller : public ScriptExecutorDelegate,
ClientMemory* GetClientMemory() override; ClientMemory* GetClientMemory() override;
private: private:
friend ControllerTest;
Controller(content::WebContents* web_contents, Controller(content::WebContents* web_contents,
std::unique_ptr<Client> client); std::unique_ptr<Client> client,
std::unique_ptr<WebController> web_controller,
std::unique_ptr<Service> service);
~Controller() override; ~Controller() override;
void GetScripts(); void GetOrCheckScripts(const GURL& url);
void OnGetScripts(bool result, const std::string& response); void OnGetScripts(const GURL& url, bool result, const std::string& response);
void OnScriptChosen(const std::string& script_path);
void OnScriptExecuted(const std::string& script_path, bool success);
// Overrides content::UiDelegate: // Overrides content::UiDelegate:
void OnClickOverlay() override; void OnClickOverlay() override;
void OnDestroy() override; void OnDestroy() override;
void OnScriptSelected(const std::string& script_path) override;
// Overrides ScriptTracker::Listener: // Overrides ScriptTracker::Listener:
void OnRunnableScriptsChanged() override; void OnRunnableScriptsChanged() override;
// Overrides content::WebContentsObserver: // Overrides content::WebContentsObserver:
void DidGetUserInteraction(const blink::WebInputEvent::Type type) override;
void DidFinishLoad(content::RenderFrameHost* render_frame_host, void DidFinishLoad(content::RenderFrameHost* render_frame_host,
const GURL& validated_url) override; const GURL& validated_url) override;
void WebContentsDestroyed() override; void WebContentsDestroyed() override;
...@@ -65,6 +76,9 @@ class Controller : public ScriptExecutorDelegate, ...@@ -65,6 +76,9 @@ class Controller : public ScriptExecutorDelegate,
std::unique_ptr<WebController> web_controller_; std::unique_ptr<WebController> web_controller_;
std::unique_ptr<Service> service_; std::unique_ptr<Service> service_;
std::unique_ptr<ScriptTracker> script_tracker_; std::unique_ptr<ScriptTracker> script_tracker_;
// Domain of the last URL the controller requested scripts from.
std::string script_domain_;
ClientMemory memory_; ClientMemory memory_;
DISALLOW_COPY_AND_ASSIGN(Controller); DISALLOW_COPY_AND_ASSIGN(Controller);
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/autofill_assistant/browser/controller.h"
#include <memory>
#include <utility>
#include "base/test/mock_callback.h"
#include "components/autofill_assistant/browser/mock_run_once_callback.h"
#include "components/autofill_assistant/browser/mock_service.h"
#include "components/autofill_assistant/browser/mock_ui_controller.h"
#include "components/autofill_assistant/browser/mock_web_controller.h"
#include "components/autofill_assistant/browser/service.h"
#include "content/public/test/test_renderer_host.h"
#include "content/public/test/web_contents_tester.h"
#include "testing/gmock/include/gmock/gmock.h"
namespace autofill_assistant {
using ::testing::_;
using ::testing::ElementsAre;
using ::testing::Eq;
using ::testing::NiceMock;
using ::testing::SizeIs;
using ::testing::StrEq;
namespace {
class FakeClient : public Client {
public:
FakeClient(std::unique_ptr<UiController> ui_controller)
: ui_controller_(std::move(ui_controller)) {}
// Implements Client
std::string GetApiKey() override { return ""; }
UiController* GetUiController() override { return ui_controller_.get(); }
private:
std::unique_ptr<UiController> ui_controller_;
};
} // namespace
class ControllerTest : public content::RenderViewHostTestHarness {
public:
ControllerTest() {}
~ControllerTest() override {}
void SetUp() override {
content::RenderViewHostTestHarness::SetUp();
auto ui_controller = std::make_unique<NiceMock<MockUiController>>();
mock_ui_controller_ = ui_controller.get();
auto web_controller = std::make_unique<NiceMock<MockWebController>>();
mock_web_controller_ = web_controller.get();
auto service = std::make_unique<NiceMock<MockService>>();
mock_service_ = service.get();
controller_ = new Controller(
web_contents(), std::make_unique<FakeClient>(std::move(ui_controller)),
std::move(web_controller), std::move(service));
// Fetching scripts succeeds for all URLs, but return nothing.
ON_CALL(*mock_service_, OnGetScriptsForUrl(_, _))
.WillByDefault(RunOnceCallback<1>(true, ""));
// Scripts run, but have no actions.
ON_CALL(*mock_service_, OnGetActions(_, _))
.WillByDefault(RunOnceCallback<1>(true, ""));
// Element "exists" exists.
ON_CALL(*mock_web_controller_, OnElementExists(ElementsAre("exists"), _))
.WillByDefault(RunOnceCallback<1>(true));
tester_ = content::WebContentsTester::For(web_contents());
}
void TearDown() override {
controller_->OnDestroy(); // deletes the controller and mocks
content::RenderViewHostTestHarness::TearDown();
}
protected:
static void AddRunnableScript(SupportsScriptResponseProto* response,
const std::string& name,
const std::string& path) {
SupportedScriptProto* script = response->add_scripts();
script->set_path(path);
script->mutable_presentation()->set_name(name);
script->mutable_presentation()
->mutable_precondition()
->add_elements_exist()
->add_selectors("exists");
}
// Updates the current url of the controller and forces a refresh, without
// bothering with actually rendering any page content.
void SimulateNavigateToUrl(const GURL& url) {
tester_->SetLastCommittedURL(url);
controller_->DidFinishLoad(nullptr, url);
}
// Sets up the next call to the service for scripts to return |response|.
void SetNextScriptResponse(const SupportsScriptResponseProto& response) {
std::string response_str;
response.SerializeToString(&response_str);
EXPECT_CALL(*mock_service_, OnGetScriptsForUrl(_, _))
.WillOnce(RunOnceCallback<1>(true, response_str));
}
UiDelegate* GetUiDelegate() { return controller_; }
MockService* mock_service_;
MockWebController* mock_web_controller_;
MockUiController* mock_ui_controller_;
content::WebContentsTester* tester_;
Controller* controller_;
};
TEST_F(ControllerTest, FetchAndRunScripts) {
// Going to the URL triggers a whole flow:
// 1. loading scripts
SupportsScriptResponseProto script_response;
AddRunnableScript(&script_response, "script1 name", "script1");
AddRunnableScript(&script_response, "script2 name", "script2");
SetNextScriptResponse(script_response);
// 2. checking the scripts
// 3. offering the choices: script1 and script2
// 4. script1 is chosen
EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(2)))
.WillOnce([this](const std::vector<ScriptHandle>& scripts) {
EXPECT_EQ("script1", scripts[0].path);
EXPECT_EQ("script2", scripts[1].path);
GetUiDelegate()->OnScriptSelected("script1");
});
// 5. script1 run successfully (no actions).
EXPECT_CALL(*mock_service_, OnGetActions(StrEq("script1"), _))
.WillOnce(RunOnceCallback<1>(true, ""));
// 6. offering the choice: script2
EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(1)))
.WillOnce([](const std::vector<ScriptHandle>& scripts) {
EXPECT_EQ("script2", scripts[0].path);
});
// 7. As nothing is selected from the 2nd UpdateScripts call, the flow
// terminates.
// Start the flow.
SimulateNavigateToUrl(GURL("http://a.example.com/path"));
}
TEST_F(ControllerTest, RefreshScriptWhenDomainChanges) {
EXPECT_CALL(*mock_service_,
OnGetScriptsForUrl(Eq(GURL("http://a.example.com/path1")), _))
.WillOnce(RunOnceCallback<1>(true, ""));
EXPECT_CALL(*mock_service_,
OnGetScriptsForUrl(Eq(GURL("http://b.example.com/path1")), _))
.WillOnce(RunOnceCallback<1>(true, ""));
SimulateNavigateToUrl(GURL("http://a.example.com/path1"));
SimulateNavigateToUrl(GURL("http://a.example.com/path2"));
SimulateNavigateToUrl(GURL("http://b.example.com/path1"));
SimulateNavigateToUrl(GURL("http://b.example.com/path2"));
}
} // namespace autofill_assistant
...@@ -34,6 +34,8 @@ class MockUiController : public UiController { ...@@ -34,6 +34,8 @@ class MockUiController : public UiController {
} }
MOCK_METHOD1(OnChooseCard, MOCK_METHOD1(OnChooseCard,
void(base::OnceCallback<void(const std::string&)>& callback)); void(base::OnceCallback<void(const std::string&)>& callback));
MOCK_METHOD1(UpdateScripts, void(const std::vector<ScriptHandle>& scripts));
}; };
} // namespace autofill_assistant } // namespace autofill_assistant
......
...@@ -18,7 +18,6 @@ ...@@ -18,7 +18,6 @@
namespace autofill_assistant { namespace autofill_assistant {
class ScriptExecutorDelegate; class ScriptExecutorDelegate;
class ScriptTrackerTest;
// The script tracker keeps track of which scripts are available, which are // The script tracker keeps track of which scripts are available, which are
// running, which have run, which are runnable whose preconditions are met. // running, which have run, which are runnable whose preconditions are met.
...@@ -63,15 +62,13 @@ class ScriptTracker { ...@@ -63,15 +62,13 @@ class ScriptTracker {
// script running at a time. // script running at a time.
bool running() const { return executor_ != nullptr; } bool running() const { return executor_ != nullptr; }
private:
friend ScriptTrackerTest;
// Returns a set of scripts that can be run, according to the last round of // Returns a set of scripts that can be run, according to the last round of
// checks. // checks.
const std::vector<ScriptHandle>& runnable_scripts() const { const std::vector<ScriptHandle>& runnable_scripts() const {
return runnable_scripts_; return runnable_scripts_;
} }
private:
void OnScriptRun(base::OnceCallback<void(bool)> original_callback, void OnScriptRun(base::OnceCallback<void(bool)> original_callback,
bool success); bool success);
void UpdateRunnableScriptsIfNecessary(); void UpdateRunnableScriptsIfNecessary();
......
...@@ -5,9 +5,11 @@ ...@@ -5,9 +5,11 @@
#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_UI_CONTROLLER_H_ #ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_UI_CONTROLLER_H_
#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_UI_CONTROLLER_H_ #define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_UI_CONTROLLER_H_
#include "components/autofill_assistant/browser/script.h"
#include "components/autofill_assistant/browser/ui_delegate.h" #include "components/autofill_assistant/browser/ui_delegate.h"
#include <string> #include <string>
#include <vector>
#include "base/callback_forward.h" #include "base/callback_forward.h"
...@@ -41,6 +43,10 @@ class UiController { ...@@ -41,6 +43,10 @@ class UiController {
virtual void ChooseCard( virtual void ChooseCard(
base::OnceCallback<void(const std::string&)> callback) = 0; base::OnceCallback<void(const std::string&)> callback) = 0;
// Show or update the UI to propose these scripts for execution. The set of
// scripts might be empty if there are no more runnable scripts.
virtual void UpdateScripts(const std::vector<ScriptHandle>& scripts);
protected: protected:
UiController() = default; UiController() = default;
}; };
......
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_UI_DELEGATE_H_ #ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_UI_DELEGATE_H_
#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_UI_DELEGATE_H_ #define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_UI_DELEGATE_H_
#include <string>
namespace autofill_assistant { namespace autofill_assistant {
// UI delegate called for script executions. // UI delegate called for script executions.
class UiDelegate { class UiDelegate {
...@@ -18,6 +20,9 @@ class UiDelegate { ...@@ -18,6 +20,9 @@ class UiDelegate {
// detached from the associated activity. // detached from the associated activity.
virtual void OnDestroy() = 0; virtual void OnDestroy() = 0;
// Called when a script was selected for execution.
virtual void OnScriptSelected(const std::string& script_path) = 0;
protected: protected:
UiDelegate() = default; UiDelegate() = default;
}; };
......
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