Commit 4a452f41 authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Include parameters in the initial requests.

This change forwards parameters taken from the Intent to SupportsSite
and the initial GetAction requests. Ownership of parameters moves up to
the controller, so it's available at the points where the requests for
these RPCs are built.

Bug: 806868
Change-Id: Id93c5d7d43997ba61fff10558bc010f9d859bbfd
Reviewed-on: https://chromium-review.googlesource.com/1233534
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593502}
parent 43086f6a
...@@ -42,6 +42,10 @@ ClientMemory* Controller::GetClientMemory() { ...@@ -42,6 +42,10 @@ ClientMemory* Controller::GetClientMemory() {
return memory_.get(); return memory_.get();
} }
const std::map<std::string, std::string>& Controller::GetParameters() {
return *parameters_;
}
Controller::Controller( Controller::Controller(
content::WebContents* web_contents, content::WebContents* web_contents,
std::unique_ptr<Client> client, std::unique_ptr<Client> client,
...@@ -52,11 +56,14 @@ Controller::Controller( ...@@ -52,11 +56,14 @@ Controller::Controller(
client_(std::move(client)), client_(std::move(client)),
web_controller_(std::move(web_controller)), web_controller_(std::move(web_controller)),
service_(std::move(service)), service_(std::move(service)),
script_tracker_( script_tracker_(std::make_unique<ScriptTracker>(this, this)),
std::make_unique<ScriptTracker>(this, this, std::move(parameters))), parameters_(std::move(parameters)),
memory_(std::make_unique<ClientMemory>()), memory_(std::make_unique<ClientMemory>()),
allow_autostart_(true) { allow_autostart_(true) {
DCHECK(parameters_);
GetUiController()->SetUiDelegate(this); GetUiController()->SetUiDelegate(this);
GetUiController()->ShowOverlay();
if (!web_contents->IsLoading()) { if (!web_contents->IsLoading()) {
GetOrCheckScripts(web_contents->GetLastCommittedURL()); GetOrCheckScripts(web_contents->GetLastCommittedURL());
} }
...@@ -71,7 +78,7 @@ void Controller::GetOrCheckScripts(const GURL& url) { ...@@ -71,7 +78,7 @@ void Controller::GetOrCheckScripts(const GURL& url) {
if (script_domain_ != url.host()) { if (script_domain_ != url.host()) {
script_domain_ = url.host(); script_domain_ = url.host();
service_->GetScriptsForUrl( service_->GetScriptsForUrl(
url, 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();
......
...@@ -47,6 +47,7 @@ class Controller : public ScriptExecutorDelegate, ...@@ -47,6 +47,7 @@ class Controller : public ScriptExecutorDelegate,
UiController* GetUiController() override; UiController* GetUiController() override;
WebController* GetWebController() override; WebController* GetWebController() override;
ClientMemory* GetClientMemory() override; ClientMemory* GetClientMemory() override;
const std::map<std::string, std::string>& GetParameters() override;
private: private:
friend ControllerTest; friend ControllerTest;
...@@ -82,6 +83,7 @@ class Controller : public ScriptExecutorDelegate, ...@@ -82,6 +83,7 @@ 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_;
std::unique_ptr<std::map<std::string, std::string>> parameters_;
// Domain of the last URL the controller requested scripts from. // Domain of the last URL the controller requested scripts from.
std::string script_domain_; std::string script_domain_;
......
...@@ -20,13 +20,15 @@ ...@@ -20,13 +20,15 @@
namespace autofill_assistant { namespace autofill_assistant {
using ::testing::_; using ::testing::_;
using ::testing::Contains;
using ::testing::ElementsAre; using ::testing::ElementsAre;
using ::testing::UnorderedElementsAre;
using ::testing::Eq; using ::testing::Eq;
using ::testing::NiceMock; using ::testing::NiceMock;
using ::testing::Pair;
using ::testing::ReturnRef;
using ::testing::SizeIs; using ::testing::SizeIs;
using ::testing::StrEq; using ::testing::StrEq;
using ::testing::ReturnRef; using ::testing::UnorderedElementsAre;
namespace { namespace {
...@@ -60,18 +62,20 @@ class ControllerTest : public content::RenderViewHostTestHarness { ...@@ -60,18 +62,20 @@ class ControllerTest : public content::RenderViewHostTestHarness {
mock_web_controller_ = web_controller.get(); mock_web_controller_ = web_controller.get();
auto service = std::make_unique<NiceMock<MockService>>(); auto service = std::make_unique<NiceMock<MockService>>();
mock_service_ = service.get(); mock_service_ = service.get();
auto parameters = std::make_unique<std::map<std::string, std::string>>();
parameters->insert(std::make_pair("a", "b"));
controller_ = new Controller( controller_ = new Controller(
web_contents(), std::make_unique<FakeClient>(std::move(ui_controller)), web_contents(), std::make_unique<FakeClient>(std::move(ui_controller)),
std::move(web_controller), std::move(service), std::move(web_controller), std::move(service), std::move(parameters));
std::make_unique<std::map<std::string, std::string>>());
// Fetching scripts succeeds for all URLs, but return nothing. // Fetching scripts succeeds for all URLs, but return nothing.
ON_CALL(*mock_service_, OnGetScriptsForUrl(_, _)) ON_CALL(*mock_service_, OnGetScriptsForUrl(_, _, _))
.WillByDefault(RunOnceCallback<1>(true, "")); .WillByDefault(RunOnceCallback<2>(true, ""));
// Scripts run, but have no actions. // Scripts run, but have no actions.
ON_CALL(*mock_service_, OnGetActions(_, _)) ON_CALL(*mock_service_, OnGetActions(_, _, _))
.WillByDefault(RunOnceCallback<1>(true, "")); .WillByDefault(RunOnceCallback<2>(true, ""));
// Make WebController::GetUrl accessible. // Make WebController::GetUrl accessible.
ON_CALL(*mock_web_controller_, GetUrl()).WillByDefault(ReturnRef(url_)); ON_CALL(*mock_web_controller_, GetUrl()).WillByDefault(ReturnRef(url_));
...@@ -110,8 +114,8 @@ class ControllerTest : public content::RenderViewHostTestHarness { ...@@ -110,8 +114,8 @@ class ControllerTest : public content::RenderViewHostTestHarness {
std::string response_str; std::string response_str;
response.SerializeToString(&response_str); response.SerializeToString(&response_str);
EXPECT_CALL(*mock_service_, OnGetScriptsForUrl(_, _)) EXPECT_CALL(*mock_service_, OnGetScriptsForUrl(_, _, _))
.WillOnce(RunOnceCallback<1>(true, response_str)); .WillOnce(RunOnceCallback<2>(true, response_str));
} }
UiDelegate* GetUiDelegate() { return controller_; } UiDelegate* GetUiDelegate() { return controller_; }
...@@ -121,6 +125,10 @@ class ControllerTest : public content::RenderViewHostTestHarness { ...@@ -121,6 +125,10 @@ class ControllerTest : public content::RenderViewHostTestHarness {
MockWebController* mock_web_controller_; MockWebController* mock_web_controller_;
MockUiController* mock_ui_controller_; MockUiController* mock_ui_controller_;
content::WebContentsTester* tester_; content::WebContentsTester* tester_;
// |controller_| deletes itself when OnDestroy is called from Setup. Outside
// of tests, the controller deletes itself when the web contents it observers
// is destroyed or when UiDelegate::OnDestroy is called.
Controller* controller_; Controller* controller_;
}; };
...@@ -146,8 +154,8 @@ TEST_F(ControllerTest, FetchAndRunScripts) { ...@@ -146,8 +154,8 @@ TEST_F(ControllerTest, FetchAndRunScripts) {
}); });
// 5. script1 run successfully (no actions). // 5. script1 run successfully (no actions).
EXPECT_CALL(*mock_service_, OnGetActions(StrEq("script1"), _)) EXPECT_CALL(*mock_service_, OnGetActions(StrEq("script1"), _, _))
.WillOnce(RunOnceCallback<1>(true, "")); .WillOnce(RunOnceCallback<2>(true, ""));
// 6. offering the choice: script2 // 6. offering the choice: script2
EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(1))) EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(1)))
...@@ -163,11 +171,11 @@ TEST_F(ControllerTest, FetchAndRunScripts) { ...@@ -163,11 +171,11 @@ TEST_F(ControllerTest, FetchAndRunScripts) {
TEST_F(ControllerTest, RefreshScriptWhenDomainChanges) { TEST_F(ControllerTest, RefreshScriptWhenDomainChanges) {
EXPECT_CALL(*mock_service_, EXPECT_CALL(*mock_service_,
OnGetScriptsForUrl(Eq(GURL("http://a.example.com/path1")), _)) OnGetScriptsForUrl(Eq(GURL("http://a.example.com/path1")), _, _))
.WillOnce(RunOnceCallback<1>(true, "")); .WillOnce(RunOnceCallback<2>(true, ""));
EXPECT_CALL(*mock_service_, EXPECT_CALL(*mock_service_,
OnGetScriptsForUrl(Eq(GURL("http://b.example.com/path1")), _)) OnGetScriptsForUrl(Eq(GURL("http://b.example.com/path1")), _, _))
.WillOnce(RunOnceCallback<1>(true, "")); .WillOnce(RunOnceCallback<2>(true, ""));
SimulateNavigateToUrl(GURL("http://a.example.com/path1")); SimulateNavigateToUrl(GURL("http://a.example.com/path1"));
SimulateNavigateToUrl(GURL("http://a.example.com/path2")); SimulateNavigateToUrl(GURL("http://a.example.com/path2"));
...@@ -175,6 +183,15 @@ TEST_F(ControllerTest, RefreshScriptWhenDomainChanges) { ...@@ -175,6 +183,15 @@ TEST_F(ControllerTest, RefreshScriptWhenDomainChanges) {
SimulateNavigateToUrl(GURL("http://b.example.com/path2")); SimulateNavigateToUrl(GURL("http://b.example.com/path2"));
} }
TEST_F(ControllerTest, ForwardParameters) {
// Parameter a=b is set in SetUp.
EXPECT_CALL(*mock_service_,
OnGetScriptsForUrl(_, Contains(Pair("a", "b")), _))
.WillOnce(RunOnceCallback<2>(true, ""));
SimulateNavigateToUrl(GURL("http://example.com/"));
}
TEST_F(ControllerTest, Autostart) { TEST_F(ControllerTest, Autostart) {
SupportsScriptResponseProto script_response; SupportsScriptResponseProto script_response;
AddRunnableScript(&script_response, "runnable") AddRunnableScript(&script_response, "runnable")
...@@ -182,8 +199,8 @@ TEST_F(ControllerTest, Autostart) { ...@@ -182,8 +199,8 @@ TEST_F(ControllerTest, Autostart) {
->set_autostart(true); ->set_autostart(true);
SetNextScriptResponse(script_response); SetNextScriptResponse(script_response);
EXPECT_CALL(*mock_service_, OnGetActions(StrEq("runnable"), _)) EXPECT_CALL(*mock_service_, OnGetActions(StrEq("runnable"), _, _))
.WillOnce(RunOnceCallback<1>(true, "")); .WillOnce(RunOnceCallback<2>(true, ""));
EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(0))); EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(0)));
SimulateNavigateToUrl(GURL("http://a.example.com/path")); SimulateNavigateToUrl(GURL("http://a.example.com/path"));
...@@ -197,7 +214,7 @@ TEST_F(ControllerTest, AutostartFallsBackToUpdateScriptAfterTap) { ...@@ -197,7 +214,7 @@ TEST_F(ControllerTest, AutostartFallsBackToUpdateScriptAfterTap) {
SetNextScriptResponse(script_response); SetNextScriptResponse(script_response);
EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(1))); EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(1)));
EXPECT_CALL(*mock_service_, OnGetActions(StrEq("runnable"), _)).Times(0); EXPECT_CALL(*mock_service_, OnGetActions(StrEq("runnable"), _, _)).Times(0);
SimulateUserInteraction(blink::WebInputEvent::kGestureTap); SimulateUserInteraction(blink::WebInputEvent::kGestureTap);
SimulateNavigateToUrl(GURL("http://a.example.com/path")); SimulateNavigateToUrl(GURL("http://a.example.com/path"));
...@@ -210,11 +227,11 @@ TEST_F(ControllerTest, AutostartFallsBackToUpdateScriptAfterExecution) { ...@@ -210,11 +227,11 @@ TEST_F(ControllerTest, AutostartFallsBackToUpdateScriptAfterExecution) {
->set_autostart(true); ->set_autostart(true);
SetNextScriptResponse(script_response); SetNextScriptResponse(script_response);
EXPECT_CALL(*mock_service_, OnGetActions(StrEq("script1"), _)); EXPECT_CALL(*mock_service_, OnGetActions(StrEq("script1"), _, _));
GetUiDelegate()->OnScriptSelected("script1"); GetUiDelegate()->OnScriptSelected("script1");
EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(1))); EXPECT_CALL(*mock_ui_controller_, UpdateScripts(SizeIs(1)));
EXPECT_CALL(*mock_service_, OnGetActions(StrEq("runnable"), _)).Times(0); EXPECT_CALL(*mock_service_, OnGetActions(StrEq("runnable"), _, _)).Times(0);
SimulateNavigateToUrl(GURL("http://a.example.com/path")); SimulateNavigateToUrl(GURL("http://a.example.com/path"));
} }
......
...@@ -5,7 +5,9 @@ ...@@ -5,7 +5,9 @@
#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_MOCK_SERVICE_H_ #ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_MOCK_SERVICE_H_
#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_MOCK_SERVICE_H_ #define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_MOCK_SERVICE_H_
#include <map>
#include <string> #include <string>
#include <vector>
#include "components/autofill_assistant/browser/service.h" #include "components/autofill_assistant/browser/service.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
...@@ -17,20 +19,26 @@ class MockService : public Service { ...@@ -17,20 +19,26 @@ class MockService : public Service {
MockService(); MockService();
~MockService() override; ~MockService() override;
void GetScriptsForUrl(const GURL& url, ResponseCallback callback) override { void GetScriptsForUrl(const GURL& url,
const std::map<std::string, std::string>& parameters,
ResponseCallback callback) override {
// Transforming callback into a references allows using RunOnceCallback on // Transforming callback into a references allows using RunOnceCallback on
// the argument. // the argument.
OnGetScriptsForUrl(url, callback); OnGetScriptsForUrl(url, parameters, callback);
} }
MOCK_METHOD2(OnGetScriptsForUrl, MOCK_METHOD3(OnGetScriptsForUrl,
void(const GURL& url, ResponseCallback& callback)); void(const GURL& url,
const std::map<std::string, std::string>& parameters,
ResponseCallback& callback));
void GetActions(const std::string& script_path, void GetActions(const std::string& script_path,
const std::map<std::string, std::string>& parameters,
ResponseCallback callback) override { ResponseCallback callback) override {
OnGetActions(script_path, callback); OnGetActions(script_path, parameters, callback);
} }
MOCK_METHOD2(OnGetActions, MOCK_METHOD3(OnGetActions,
void(const std::string& script_path, void(const std::string& script_path,
const std::map<std::string, std::string>& parameters,
ResponseCallback& callback)); ResponseCallback& callback));
void GetNextActions( void GetNextActions(
......
...@@ -20,14 +20,34 @@ ...@@ -20,14 +20,34 @@
namespace autofill_assistant { namespace autofill_assistant {
namespace {
// Fills the destination proto field with script parameters from the given
// parameter map.
void AddScriptParametersToProto(
const std::map<std::string, std::string>& source,
::google::protobuf::RepeatedPtrField<ScriptParameterProto>* destination) {
for (const auto& param_entry : source) {
ScriptParameterProto* parameter = destination->Add();
parameter->set_name(param_entry.first);
parameter->set_value(param_entry.second);
}
}
} // namespace
// static // static
std::string ProtocolUtils::CreateGetScriptsRequest(const GURL& url) { std::string ProtocolUtils::CreateGetScriptsRequest(
const GURL& url,
const std::map<std::string, std::string>& parameters) {
DCHECK(!url.is_empty()); DCHECK(!url.is_empty());
SupportsScriptRequestProto script_proto; SupportsScriptRequestProto script_proto;
script_proto.set_url(url.spec()); script_proto.set_url(url.spec());
script_proto.mutable_client_context()->mutable_chrome()->set_chrome_version( script_proto.mutable_client_context()->mutable_chrome()->set_chrome_version(
version_info::GetProductNameAndVersionForUserAgent()); version_info::GetProductNameAndVersionForUserAgent());
AddScriptParametersToProto(parameters,
script_proto.mutable_script_parameters());
std::string serialized_script_proto; std::string serialized_script_proto;
bool success = script_proto.SerializeToString(&serialized_script_proto); bool success = script_proto.SerializeToString(&serialized_script_proto);
DCHECK(success); DCHECK(success);
...@@ -72,12 +92,17 @@ bool ProtocolUtils::ParseScripts( ...@@ -72,12 +92,17 @@ bool ProtocolUtils::ParseScripts(
// static // static
std::string ProtocolUtils::CreateInitialScriptActionsRequest( std::string ProtocolUtils::CreateInitialScriptActionsRequest(
const std::string& script_path) { const std::string& script_path,
const std::map<std::string, std::string>& parameters) {
ScriptActionRequestProto request_proto; ScriptActionRequestProto request_proto;
InitialScriptActionsRequestProto* initial_request_proto =
request_proto.mutable_initial_request();
InitialScriptActionsRequestProto::QueryProto* query = InitialScriptActionsRequestProto::QueryProto* query =
request_proto.mutable_initial_request()->mutable_query(); initial_request_proto->mutable_query();
query->add_script_path(script_path); query->add_script_path(script_path);
query->set_policy(PolicyType::SCRIPT); query->set_policy(PolicyType::SCRIPT);
AddScriptParametersToProto(
parameters, initial_request_proto->mutable_script_parameters());
request_proto.mutable_client_context()->mutable_chrome()->set_chrome_version( request_proto.mutable_client_context()->mutable_chrome()->set_chrome_version(
version_info::GetProductNameAndVersionForUserAgent()); version_info::GetProductNameAndVersionForUserAgent());
......
...@@ -23,7 +23,9 @@ class ProtocolUtils { ...@@ -23,7 +23,9 @@ class ProtocolUtils {
public: public:
// Create getting autofill assistant scripts request for the given // Create getting autofill assistant scripts request for the given
// |url|. // |url|.
static std::string CreateGetScriptsRequest(const GURL& url); static std::string CreateGetScriptsRequest(
const GURL& url,
const std::map<std::string, std::string>& parameters);
using Scripts = std::map<Script*, std::unique_ptr<Script>>; using Scripts = std::map<Script*, std::unique_ptr<Script>>;
// Parse assistant scripts from the given |response|, which should not be an // Parse assistant scripts from the given |response|, which should not be an
...@@ -39,7 +41,8 @@ class ProtocolUtils { ...@@ -39,7 +41,8 @@ class ProtocolUtils {
// Create initial request to get script actions for the given |script_path|. // Create initial request to get script actions for the given |script_path|.
static std::string CreateInitialScriptActionsRequest( static std::string CreateInitialScriptActionsRequest(
const std::string& script_path); const std::string& script_path,
const std::map<std::string, std::string>& parameters);
// Create request to get next sequence of actions for a script. // Create request to get next sequence of actions for a script.
static std::string CreateNextScriptActionsRequest( static std::string CreateNextScriptActionsRequest(
......
...@@ -7,12 +7,15 @@ ...@@ -7,12 +7,15 @@
#include "base/macros.h" #include "base/macros.h"
#include "components/autofill_assistant/browser/service.pb.h" #include "components/autofill_assistant/browser/service.pb.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "url/gurl.h"
namespace autofill_assistant { namespace autofill_assistant {
namespace { namespace {
using ::testing::SizeIs; using ::testing::SizeIs;
using ::testing::Not;
using ::testing::IsEmpty; using ::testing::IsEmpty;
using ::testing::ElementsAre;
TEST(ProtocolUtilsTest, NoScripts) { TEST(ProtocolUtilsTest, NoScripts) {
std::vector<std::unique_ptr<Script>> scripts; std::vector<std::unique_ptr<Script>> scripts;
...@@ -62,5 +65,46 @@ TEST(ProtocolUtilsTest, OneFullyFeaturedScript) { ...@@ -62,5 +65,46 @@ TEST(ProtocolUtilsTest, OneFullyFeaturedScript) {
EXPECT_NE(nullptr, scripts[0]->precondition); EXPECT_NE(nullptr, scripts[0]->precondition);
} }
TEST(ProtocolUtilsTest, CreateInitialScriptActionsRequest) {
std::map<std::string, std::string> parameters;
parameters["a"] = "b";
parameters["c"] = "d";
ScriptActionRequestProto request;
EXPECT_TRUE(
request.ParseFromString(ProtocolUtils::CreateInitialScriptActionsRequest(
"script_path", parameters)));
EXPECT_THAT(request.client_context().chrome().chrome_version(),
Not(IsEmpty()));
const InitialScriptActionsRequestProto& initial = request.initial_request();
EXPECT_THAT(initial.query().script_path(), ElementsAre("script_path"));
ASSERT_EQ(2, initial.script_parameters_size());
EXPECT_EQ("a", initial.script_parameters(0).name());
EXPECT_EQ("b", initial.script_parameters(0).value());
EXPECT_EQ("c", initial.script_parameters(1).name());
EXPECT_EQ("d", initial.script_parameters(1).value());
}
TEST(ProtocolUtilsTest, CreateGetScriptsRequest) {
std::map<std::string, std::string> parameters;
parameters["a"] = "b";
parameters["c"] = "d";
SupportsScriptRequestProto request;
EXPECT_TRUE(request.ParseFromString(ProtocolUtils::CreateGetScriptsRequest(
GURL("http://example.com/"), parameters)));
EXPECT_EQ("http://example.com/", request.url());
EXPECT_THAT(request.client_context().chrome().chrome_version(),
Not(IsEmpty()));
ASSERT_EQ(2, request.script_parameters_size());
EXPECT_EQ("a", request.script_parameters(0).name());
EXPECT_EQ("b", request.script_parameters(0).value());
EXPECT_EQ("c", request.script_parameters(1).name());
EXPECT_EQ("d", request.script_parameters(1).value());
}
} // namespace } // namespace
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -31,8 +31,9 @@ void ScriptExecutor::Run(RunScriptCallback callback) { ...@@ -31,8 +31,9 @@ void ScriptExecutor::Run(RunScriptCallback callback) {
DCHECK(delegate_->GetService()); DCHECK(delegate_->GetService());
delegate_->GetService()->GetActions( delegate_->GetService()->GetActions(
script_path_, base::BindOnce(&ScriptExecutor::OnGetActions, script_path_, delegate_->GetParameters(),
weak_ptr_factory_.GetWeakPtr())); base::BindOnce(&ScriptExecutor::OnGetActions,
weak_ptr_factory_.GetWeakPtr()));
} }
void ScriptExecutor::ShowStatusMessage(const std::string& message) { void ScriptExecutor::ShowStatusMessage(const std::string& message) {
......
...@@ -5,6 +5,9 @@ ...@@ -5,6 +5,9 @@
#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SCRIPT_EXECUTOR_DELEGATE_H_ #ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SCRIPT_EXECUTOR_DELEGATE_H_
#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SCRIPT_EXECUTOR_DELEGATE_H_ #define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SCRIPT_EXECUTOR_DELEGATE_H_
#include <map>
#include <string>
namespace autofill_assistant { namespace autofill_assistant {
class Service; class Service;
...@@ -22,6 +25,8 @@ class ScriptExecutorDelegate { ...@@ -22,6 +25,8 @@ class ScriptExecutorDelegate {
virtual ClientMemory* GetClientMemory() = 0; virtual ClientMemory* GetClientMemory() = 0;
virtual const std::map<std::string, std::string>& GetParameters() = 0;
protected: protected:
virtual ~ScriptExecutorDelegate() {} virtual ~ScriptExecutorDelegate() {}
}; };
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "components/autofill_assistant/browser/script_executor.h" #include "components/autofill_assistant/browser/script_executor.h"
#include <map>
#include "base/test/mock_callback.h" #include "base/test/mock_callback.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "components/autofill_assistant/browser/client_memory.h" #include "components/autofill_assistant/browser/client_memory.h"
...@@ -18,11 +20,14 @@ namespace autofill_assistant { ...@@ -18,11 +20,14 @@ namespace autofill_assistant {
namespace { namespace {
using ::testing::AllOf;
using ::testing::Contains;
using ::testing::DoAll; using ::testing::DoAll;
using ::testing::NiceMock;
using ::testing::Pair;
using ::testing::SaveArg; using ::testing::SaveArg;
using ::testing::StrEq; using ::testing::StrEq;
using ::testing::StrictMock; using ::testing::StrictMock;
using ::testing::NiceMock;
using ::testing::_; using ::testing::_;
class ScriptExecutorTest : public testing::Test, public ScriptExecutorDelegate { class ScriptExecutorTest : public testing::Test, public ScriptExecutorDelegate {
...@@ -49,6 +54,10 @@ class ScriptExecutorTest : public testing::Test, public ScriptExecutorDelegate { ...@@ -49,6 +54,10 @@ class ScriptExecutorTest : public testing::Test, public ScriptExecutorDelegate {
ClientMemory* GetClientMemory() override { return &memory_; } ClientMemory* GetClientMemory() override { return &memory_; }
const std::map<std::string, std::string>& GetParameters() override {
return parameters_;
}
std::string Serialize(const google::protobuf::MessageLite& message) { std::string Serialize(const google::protobuf::MessageLite& message) {
std::string output; std::string output;
message.SerializeToString(&output); message.SerializeToString(&output);
...@@ -64,17 +73,32 @@ class ScriptExecutorTest : public testing::Test, public ScriptExecutorDelegate { ...@@ -64,17 +73,32 @@ class ScriptExecutorTest : public testing::Test, public ScriptExecutorDelegate {
NiceMock<MockWebController> mock_web_controller_; NiceMock<MockWebController> mock_web_controller_;
NiceMock<MockUiController> mock_ui_controller_; NiceMock<MockUiController> mock_ui_controller_;
std::unique_ptr<ScriptExecutor> executor_; std::unique_ptr<ScriptExecutor> executor_;
std::map<std::string, std::string> parameters_;
StrictMock<base::MockCallback<ScriptExecutor::RunScriptCallback>> StrictMock<base::MockCallback<ScriptExecutor::RunScriptCallback>>
executor_callback_; executor_callback_;
}; };
TEST_F(ScriptExecutorTest, GetActionsFails) { TEST_F(ScriptExecutorTest, GetActionsFails) {
EXPECT_CALL(mock_service_, OnGetActions(_, _)) EXPECT_CALL(mock_service_, OnGetActions(_, _, _))
.WillOnce(RunOnceCallback<1>(false, "")); .WillOnce(RunOnceCallback<2>(false, ""));
EXPECT_CALL(executor_callback_, Run(false)); EXPECT_CALL(executor_callback_, Run(false));
executor_->Run(executor_callback_.Get()); executor_->Run(executor_callback_.Get());
} }
TEST_F(ScriptExecutorTest, ForwardParameters) {
parameters_["param1"] = "value1";
parameters_["param2"] = "value2";
EXPECT_CALL(mock_service_,
OnGetActions(StrEq("script path"),
AllOf(Contains(Pair("param1", "value1")),
Contains(Pair("param2", "value2"))),
_))
.WillOnce(RunOnceCallback<2>(true, ""));
EXPECT_CALL(executor_callback_, Run(true));
executor_->Run(executor_callback_.Get());
}
TEST_F(ScriptExecutorTest, RunOneActionReportFailureAndStop) { TEST_F(ScriptExecutorTest, RunOneActionReportFailureAndStop) {
ActionsResponseProto actions_response; ActionsResponseProto actions_response;
actions_response.set_server_payload("payload"); actions_response.set_server_payload("payload");
...@@ -83,8 +107,8 @@ TEST_F(ScriptExecutorTest, RunOneActionReportFailureAndStop) { ...@@ -83,8 +107,8 @@ TEST_F(ScriptExecutorTest, RunOneActionReportFailureAndStop) {
->mutable_element_to_click() ->mutable_element_to_click()
->add_selectors("will fail"); ->add_selectors("will fail");
EXPECT_CALL(mock_service_, OnGetActions(_, _)) EXPECT_CALL(mock_service_, OnGetActions(_, _, _))
.WillOnce(RunOnceCallback<1>(true, Serialize(actions_response))); .WillOnce(RunOnceCallback<2>(true, Serialize(actions_response)));
std::vector<ProcessedActionProto> processed_actions_capture; std::vector<ProcessedActionProto> processed_actions_capture;
EXPECT_CALL(mock_service_, OnGetNextActions(_, _, _)) EXPECT_CALL(mock_service_, OnGetNextActions(_, _, _))
...@@ -102,8 +126,8 @@ TEST_F(ScriptExecutorTest, RunMultipleActions) { ...@@ -102,8 +126,8 @@ TEST_F(ScriptExecutorTest, RunMultipleActions) {
initial_actions_response.set_server_payload("payload1"); initial_actions_response.set_server_payload("payload1");
initial_actions_response.add_actions()->mutable_tell()->set_message("1"); initial_actions_response.add_actions()->mutable_tell()->set_message("1");
initial_actions_response.add_actions()->mutable_tell()->set_message("2"); initial_actions_response.add_actions()->mutable_tell()->set_message("2");
EXPECT_CALL(mock_service_, OnGetActions(StrEq("script path"), _)) EXPECT_CALL(mock_service_, OnGetActions(StrEq("script path"), _, _))
.WillOnce(RunOnceCallback<1>(true, Serialize(initial_actions_response))); .WillOnce(RunOnceCallback<2>(true, Serialize(initial_actions_response)));
ActionsResponseProto next_actions_response; ActionsResponseProto next_actions_response;
next_actions_response.set_server_payload("payload2"); next_actions_response.set_server_payload("payload2");
...@@ -135,8 +159,8 @@ TEST_F(ScriptExecutorTest, InterruptActionListOnError) { ...@@ -135,8 +159,8 @@ TEST_F(ScriptExecutorTest, InterruptActionListOnError) {
initial_actions_response.add_actions()->mutable_tell()->set_message( initial_actions_response.add_actions()->mutable_tell()->set_message(
"never run"); "never run");
EXPECT_CALL(mock_service_, OnGetActions(_, _)) EXPECT_CALL(mock_service_, OnGetActions(_, _, _))
.WillOnce(RunOnceCallback<1>(true, Serialize(initial_actions_response))); .WillOnce(RunOnceCallback<2>(true, Serialize(initial_actions_response)));
ActionsResponseProto next_actions_response; ActionsResponseProto next_actions_response;
next_actions_response.set_server_payload("payload2"); next_actions_response.set_server_payload("payload2");
...@@ -171,8 +195,8 @@ TEST_F(ScriptExecutorTest, RunDelayedAction) { ...@@ -171,8 +195,8 @@ TEST_F(ScriptExecutorTest, RunDelayedAction) {
action->mutable_tell()->set_message("delayed"); action->mutable_tell()->set_message("delayed");
action->set_action_delay_ms(1000); action->set_action_delay_ms(1000);
EXPECT_CALL(mock_service_, OnGetActions(_, _)) EXPECT_CALL(mock_service_, OnGetActions(_, _, _))
.WillOnce(RunOnceCallback<1>(true, Serialize(actions_response))); .WillOnce(RunOnceCallback<2>(true, Serialize(actions_response)));
std::vector<ProcessedActionProto> processed_actions_capture; std::vector<ProcessedActionProto> processed_actions_capture;
EXPECT_CALL(mock_service_, OnGetNextActions(_, _, _)) EXPECT_CALL(mock_service_, OnGetNextActions(_, _, _))
......
...@@ -13,18 +13,14 @@ ...@@ -13,18 +13,14 @@
namespace autofill_assistant { namespace autofill_assistant {
ScriptTracker::ScriptTracker( ScriptTracker::ScriptTracker(ScriptExecutorDelegate* delegate,
ScriptExecutorDelegate* delegate, ScriptTracker::Listener* listener)
ScriptTracker::Listener* listener,
std::unique_ptr<std::map<std::string, std::string>> parameters)
: delegate_(delegate), : delegate_(delegate),
listener_(listener), listener_(listener),
parameters_(std::move(parameters)),
pending_precondition_check_count_(0), pending_precondition_check_count_(0),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
DCHECK(delegate_); DCHECK(delegate_);
DCHECK(listener_); DCHECK(listener_);
DCHECK(parameters_);
} }
ScriptTracker::~ScriptTracker() = default; ScriptTracker::~ScriptTracker() = default;
...@@ -63,7 +59,7 @@ void ScriptTracker::CheckScripts() { ...@@ -63,7 +59,7 @@ void ScriptTracker::CheckScripts() {
} else { } else {
for (Script* script : scripts_to_check) { for (Script* script : scripts_to_check) {
script->precondition->Check( script->precondition->Check(
delegate_->GetWebController(), *parameters_.get(), delegate_->GetWebController(), delegate_->GetParameters(),
base::BindOnce(&ScriptTracker::OnPreconditionCheck, base::BindOnce(&ScriptTracker::OnPreconditionCheck,
weak_ptr_factory_.GetWeakPtr(), script)); weak_ptr_factory_.GetWeakPtr(), script));
} }
......
...@@ -40,8 +40,7 @@ class ScriptTracker { ...@@ -40,8 +40,7 @@ class ScriptTracker {
// |delegate| and |listener| should outlive this object and should not be // |delegate| and |listener| should outlive this object and should not be
// nullptr. // nullptr.
ScriptTracker(ScriptExecutorDelegate* delegate, ScriptTracker(ScriptExecutorDelegate* delegate,
ScriptTracker::Listener* listener, ScriptTracker::Listener* listener);
std::unique_ptr<std::map<std::string, std::string>> parameters);
~ScriptTracker(); ~ScriptTracker();
......
...@@ -40,16 +40,12 @@ class ScriptTrackerTest : public testing::Test, ...@@ -40,16 +40,12 @@ class ScriptTrackerTest : public testing::Test,
ON_CALL(mock_web_controller_, GetUrl()).WillByDefault(ReturnRef(url_)); ON_CALL(mock_web_controller_, GetUrl()).WillByDefault(ReturnRef(url_));
// Scripts run, but have no actions. // Scripts run, but have no actions.
ON_CALL(mock_service_, OnGetActions(_, _)) ON_CALL(mock_service_, OnGetActions(_, _, _))
.WillByDefault(RunOnceCallback<1>(true, "")); .WillByDefault(RunOnceCallback<2>(true, ""));
} }
protected: protected:
ScriptTrackerTest() ScriptTrackerTest() : runnable_scripts_changed_(0), tracker_(this, this) {}
: runnable_scripts_changed_(0),
tracker_(this,
this,
std::make_unique<std::map<std::string, std::string>>()) {}
// Overrides ScriptTrackerDelegate // Overrides ScriptTrackerDelegate
Service* GetService() override { return &mock_service_; } Service* GetService() override { return &mock_service_; }
...@@ -60,6 +56,10 @@ class ScriptTrackerTest : public testing::Test, ...@@ -60,6 +56,10 @@ class ScriptTrackerTest : public testing::Test,
ClientMemory* GetClientMemory() override { return &client_memory_; } ClientMemory* GetClientMemory() override { return &client_memory_; }
const std::map<std::string, std::string>& GetParameters() override {
return parameters_;
}
// Overrides ScriptTracker::Listener // Overrides ScriptTracker::Listener
void OnRunnableScriptsChanged( void OnRunnableScriptsChanged(
const std::vector<ScriptHandle>& runnable_scripts) override { const std::vector<ScriptHandle>& runnable_scripts) override {
...@@ -112,6 +112,7 @@ class ScriptTrackerTest : public testing::Test, ...@@ -112,6 +112,7 @@ class ScriptTrackerTest : public testing::Test,
NiceMock<MockWebController> mock_web_controller_; NiceMock<MockWebController> mock_web_controller_;
NiceMock<MockUiController> mock_ui_controller_; NiceMock<MockUiController> mock_ui_controller_;
ClientMemory client_memory_; ClientMemory client_memory_;
std::map<std::string, std::string> parameters_;
// Number of times OnRunnableScriptsChanged was called. // Number of times OnRunnableScriptsChanged was called.
int runnable_scripts_changed_; int runnable_scripts_changed_;
......
...@@ -81,18 +81,22 @@ Service::Service(const std::string& api_key, content::BrowserContext* context) ...@@ -81,18 +81,22 @@ Service::Service(const std::string& api_key, content::BrowserContext* context)
Service::~Service() {} Service::~Service() {}
void Service::GetScriptsForUrl(const GURL& url, ResponseCallback callback) { void Service::GetScriptsForUrl(
const GURL& url,
const std::map<std::string, std::string>& parameters,
ResponseCallback callback) {
DCHECK(url.is_valid()); DCHECK(url.is_valid());
std::unique_ptr<Loader> loader = std::make_unique<Loader>(); std::unique_ptr<Loader> loader = std::make_unique<Loader>();
loader->callback = std::move(callback); loader->callback = std::move(callback);
loader->loader = CreateAndStartLoader( loader->loader = CreateAndStartLoader(
script_server_url_, ProtocolUtils::CreateGetScriptsRequest(url), script_server_url_,
loader.get()); ProtocolUtils::CreateGetScriptsRequest(url, parameters), loader.get());
loaders_[loader.get()] = std::move(loader); loaders_[loader.get()] = std::move(loader);
} }
void Service::GetActions(const std::string& script_path, void Service::GetActions(const std::string& script_path,
const std::map<std::string, std::string>& parameters,
ResponseCallback callback) { ResponseCallback callback) {
DCHECK(!script_path.empty()); DCHECK(!script_path.empty());
...@@ -100,7 +104,7 @@ void Service::GetActions(const std::string& script_path, ...@@ -100,7 +104,7 @@ void Service::GetActions(const std::string& script_path,
loader->callback = std::move(callback); loader->callback = std::move(callback);
loader->loader = CreateAndStartLoader( loader->loader = CreateAndStartLoader(
script_action_server_url_, script_action_server_url_,
ProtocolUtils::CreateInitialScriptActionsRequest(script_path), ProtocolUtils::CreateInitialScriptActionsRequest(script_path, parameters),
loader.get()); loader.get());
loaders_[loader.get()] = std::move(loader); loaders_[loader.get()] = std::move(loader);
} }
......
...@@ -31,10 +31,14 @@ class Service { ...@@ -31,10 +31,14 @@ class Service {
using ResponseCallback = using ResponseCallback =
base::OnceCallback<void(bool result, const std::string&)>; base::OnceCallback<void(bool result, const std::string&)>;
// Get scripts for a given |url|, which should be a valid URL. // Get scripts for a given |url|, which should be a valid URL.
virtual void GetScriptsForUrl(const GURL& url, ResponseCallback callback); virtual void GetScriptsForUrl(
const GURL& url,
const std::map<std::string, std::string>& parameters,
ResponseCallback callback);
// Get actions. // Get actions.
virtual void GetActions(const std::string& script_path, virtual void GetActions(const std::string& script_path,
const std::map<std::string, std::string>& parameters,
ResponseCallback callback); ResponseCallback callback);
// Get next sequence of actions according to server payload in previous // Get next sequence of actions according to server payload in previous
......
...@@ -16,8 +16,18 @@ message ClientContextProto { ...@@ -16,8 +16,18 @@ message ClientContextProto {
// Get the list of scripts that can potentially be run on a url. // Get the list of scripts that can potentially be run on a url.
message SupportsScriptRequestProto { message SupportsScriptRequestProto {
required string url = 1; optional string url = 1;
required ClientContextProto client_context = 3;
// Parameters that can be used to filter the scripts suitable for execution.
repeated ScriptParameterProto script_parameters = 2;
optional ClientContextProto client_context = 3;
}
message ScriptParameterProto {
// Parameter name, as found in the Intent, without prefix.
optional string name = 3;
optional string value = 2;
} }
// Response of the list of supported scripts. // Response of the list of supported scripts.
...@@ -99,6 +109,8 @@ message InitialScriptActionsRequestProto { ...@@ -99,6 +109,8 @@ message InitialScriptActionsRequestProto {
optional PolicyType policy = 3; optional PolicyType policy = 3;
} }
optional QueryProto query = 3; optional QueryProto query = 3;
repeated ScriptParameterProto script_parameters = 2;
} }
// Next request to get a script's actions. // Next request to get a script's actions.
......
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