Commit 8e9389b3 authored by Mathias Carlen's avatar Mathias Carlen Committed by Commit Bot

[Autofill Assistant] Introduce in-place scripts updates.

Sub-scripts part of larger scripts are not explicitly known to the
client, hence status match preconditions could not be applied in that
case. Before this patch, this kind of script filtering was not possible.

This patch introduces a way of updating the list of available scripts
mid script execution by reporting an updated list as part of the get
actions response. The updated list will be used when the script
execution finishes and another round of precondition checks is
triggered.

After this patch, the backend will be able to fine script eligibility
while still being backwards compatible with script status match
preconditions.

Note: This is an intermediate step towards removing the status match
preconditions from the client and handle that entirely in the backend.

R=szermatt@chromium.org

Bug: 806868
Change-Id: Ia29bd0a7bc5a3f00b4b0f736dd28a93f6b422d92
Reviewed-on: https://chromium-review.googlesource.com/c/1352763
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarStephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612119}
parent c52df073
...@@ -79,31 +79,37 @@ bool ProtocolUtils::ParseScripts( ...@@ -79,31 +79,37 @@ bool ProtocolUtils::ParseScripts(
scripts->clear(); scripts->clear();
for (const auto& script_proto : response_proto.scripts()) { for (const auto& script_proto : response_proto.scripts()) {
auto script = std::make_unique<Script>(); ProtocolUtils::AddScript(script_proto, scripts);
script->handle.path = script_proto.path();
const auto& presentation = script_proto.presentation();
script->handle.name = presentation.name();
script->handle.autostart = presentation.autostart();
script->handle.interrupt = presentation.interrupt();
script->handle.initial_prompt = presentation.initial_prompt();
script->handle.highlight = presentation.highlight();
script->precondition = ScriptPrecondition::FromProto(
script_proto.path(), presentation.precondition());
script->priority = presentation.priority();
if (script->handle.path.empty() || !script->precondition ||
(script->handle.name.empty() && !script->handle.interrupt)) {
LOG(ERROR) << "Ignored invalid or incomplete script '"
<< script->handle.path << "'";
continue;
}
scripts->emplace_back(std::move(script));
} }
return true; return true;
} }
// static
void ProtocolUtils::AddScript(const SupportedScriptProto& script_proto,
std::vector<std::unique_ptr<Script>>* scripts) {
auto script = std::make_unique<Script>();
script->handle.path = script_proto.path();
const auto& presentation = script_proto.presentation();
script->handle.name = presentation.name();
script->handle.autostart = presentation.autostart();
script->handle.interrupt = presentation.interrupt();
script->handle.initial_prompt = presentation.initial_prompt();
script->handle.highlight = presentation.highlight();
script->precondition = ScriptPrecondition::FromProto(
script_proto.path(), presentation.precondition());
script->priority = presentation.priority();
if (script->handle.path.empty() || !script->precondition ||
(script->handle.name.empty() && !script->handle.interrupt)) {
LOG(ERROR) << "Ignored invalid or incomplete script '"
<< script->handle.path << "'";
return;
}
scripts->emplace_back(std::move(script));
}
// static // static
std::string ProtocolUtils::CreateInitialScriptActionsRequest( std::string ProtocolUtils::CreateInitialScriptActionsRequest(
const std::string& script_path, const std::string& script_path,
...@@ -160,12 +166,14 @@ std::string ProtocolUtils::CreateNextScriptActionsRequest( ...@@ -160,12 +166,14 @@ std::string ProtocolUtils::CreateNextScriptActionsRequest(
} }
// static // static
bool ProtocolUtils::ParseActions( bool ProtocolUtils::ParseActions(const std::string& response,
const std::string& response, std::string* return_global_payload,
std::string* return_global_payload, std::string* return_script_payload,
std::string* return_script_payload, std::vector<std::unique_ptr<Action>>* actions,
std::vector<std::unique_ptr<Action>>* actions) { std::vector<std::unique_ptr<Script>>* scripts,
bool* should_update_scripts) {
DCHECK(actions); DCHECK(actions);
DCHECK(scripts);
ActionsResponseProto response_proto; ActionsResponseProto response_proto;
if (!response_proto.ParseFromString(response)) { if (!response_proto.ParseFromString(response)) {
...@@ -263,6 +271,12 @@ bool ProtocolUtils::ParseActions( ...@@ -263,6 +271,12 @@ bool ProtocolUtils::ParseActions(
} }
} }
*should_update_scripts = response_proto.has_update_script_list();
for (const auto& script_proto :
response_proto.update_script_list().scripts()) {
ProtocolUtils::AddScript(script_proto, scripts);
}
return true; return true;
} }
......
...@@ -40,6 +40,11 @@ class ProtocolUtils { ...@@ -40,6 +40,11 @@ class ProtocolUtils {
static bool ParseScripts(const std::string& response, static bool ParseScripts(const std::string& response,
std::vector<std::unique_ptr<Script>>* scripts); std::vector<std::unique_ptr<Script>>* scripts);
// Convert |script_proto| to a script struct and if the script is valid, add
// it to |scripts|.
static void AddScript(const SupportedScriptProto& script_proto,
std::vector<std::unique_ptr<Script>>* scripts);
// Create initial request to get script actions for the given |script_path|. // Create initial request to get script actions for the given |script_path|.
// //
// TODO(b/806868): Remove the script payload from initial requests once the // TODO(b/806868): Remove the script payload from initial requests once the
...@@ -63,12 +68,17 @@ class ProtocolUtils { ...@@ -63,12 +68,17 @@ class ProtocolUtils {
// //
// Pass in nullptr for |return_global_payload| or |return_script_payload| to // Pass in nullptr for |return_global_payload| or |return_script_payload| to
// indicate no need to return that payload. Parsed actions are returned // indicate no need to return that payload. Parsed actions are returned
// through |actions|, which should not be nullptr. Return false if parse // through |actions|, which should not be nullptr. Optionally, parsed scripts
// failed, otherwise return true. // are returned through |scripts| and used to update the list of cached
// scripts. The bool |should_update_scripts| makes clear the destinction
// between an empty list of |scripts| or the scripts field not even set in the
// proto. Return false if parse failed, otherwise return true.
static bool ParseActions(const std::string& response, static bool ParseActions(const std::string& response,
std::string* return_global_payload, std::string* return_global_payload,
std::string* return_script_payload, std::string* return_script_payload,
std::vector<std::unique_ptr<Action>>* actions); std::vector<std::unique_ptr<Action>>* actions,
std::vector<std::unique_ptr<Script>>* scripts,
bool* should_update_scripts);
private: private:
// To avoid instantiate this class by accident. // To avoid instantiate this class by accident.
......
...@@ -12,10 +12,11 @@ ...@@ -12,10 +12,11 @@
namespace autofill_assistant { namespace autofill_assistant {
namespace { namespace {
using ::testing::SizeIs;
using ::testing::Not;
using ::testing::IsEmpty;
using ::testing::ElementsAre; using ::testing::ElementsAre;
using ::testing::Eq;
using ::testing::IsEmpty;
using ::testing::Not;
using ::testing::SizeIs;
ClientContextProto CreateClientContextProto() { ClientContextProto CreateClientContextProto() {
ClientContextProto context; ClientContextProto context;
...@@ -142,5 +143,112 @@ TEST(ProtocolUtilsTest, CreateGetScriptsRequest) { ...@@ -142,5 +143,112 @@ TEST(ProtocolUtilsTest, CreateGetScriptsRequest) {
EXPECT_EQ("d", request.script_parameters(1).value()); EXPECT_EQ("d", request.script_parameters(1).value());
} }
TEST(ProtocolUtilsTest, AddScriptIgnoreInvalid) {
SupportedScriptProto script_proto;
std::vector<std::unique_ptr<Script>> scripts;
ProtocolUtils::AddScript(script_proto, &scripts);
EXPECT_TRUE(scripts.empty());
}
TEST(ProtocolUtilsTest, AddScriptValid) {
SupportedScriptProto script_proto;
script_proto.set_path("path");
auto* presentation = script_proto.mutable_presentation();
presentation->set_name("name");
presentation->set_autostart(true);
presentation->set_initial_prompt("prompt");
presentation->mutable_precondition()->add_domain("www.example.com");
std::vector<std::unique_ptr<Script>> scripts;
ProtocolUtils::AddScript(script_proto, &scripts);
std::unique_ptr<Script> script = std::move(scripts[0]);
EXPECT_NE(nullptr, script);
EXPECT_EQ("path", script->handle.path);
EXPECT_EQ("name", script->handle.name);
EXPECT_EQ("prompt", script->handle.initial_prompt);
EXPECT_TRUE(script->handle.autostart);
EXPECT_NE(nullptr, script->precondition);
}
TEST(ProtocolUtilsTest, ParseActionsParseError) {
bool unused;
std::vector<std::unique_ptr<Action>> unused_actions;
std::vector<std::unique_ptr<Script>> unused_scripts;
EXPECT_FALSE(ProtocolUtils::ParseActions(
"invalid", nullptr, nullptr, &unused_actions, &unused_scripts, &unused));
}
TEST(ProtocolUtilsTest, ParseActionsValid) {
ActionsResponseProto proto;
proto.set_global_payload("global_payload");
proto.set_script_payload("script_payload");
proto.add_actions()->mutable_tell();
proto.add_actions()->mutable_click();
std::string proto_str;
proto.SerializeToString(&proto_str);
std::string global_payload;
std::string script_payload;
bool should_update_scripts = true;
std::vector<std::unique_ptr<Action>> actions;
std::vector<std::unique_ptr<Script>> scripts;
EXPECT_TRUE(ProtocolUtils::ParseActions(proto_str, &global_payload,
&script_payload, &actions, &scripts,
&should_update_scripts));
EXPECT_EQ("global_payload", global_payload);
EXPECT_EQ("script_payload", script_payload);
EXPECT_THAT(actions, SizeIs(2));
EXPECT_FALSE(should_update_scripts);
EXPECT_TRUE(scripts.empty());
}
TEST(ProtocolUtilsTest, ParseActionsEmptyUpdateScriptList) {
ActionsResponseProto proto;
proto.mutable_update_script_list();
std::string proto_str;
proto.SerializeToString(&proto_str);
bool should_update_scripts = false;
std::vector<std::unique_ptr<Script>> scripts;
std::vector<std::unique_ptr<Action>> unused_actions;
EXPECT_TRUE(ProtocolUtils::ParseActions(
proto_str, /* global_payload= */ nullptr, /* script_payload */ nullptr,
&unused_actions, &scripts, &should_update_scripts));
EXPECT_TRUE(should_update_scripts);
EXPECT_TRUE(scripts.empty());
}
TEST(ProtocolUtilsTest, ParseActionsUpdateScriptListFullFeatured) {
ActionsResponseProto proto;
auto* script_list = proto.mutable_update_script_list();
auto* script_a = script_list->add_scripts();
script_a->set_path("a");
auto* presentation = script_a->mutable_presentation();
presentation->set_name("name");
presentation->mutable_precondition();
// One invalid script.
script_list->add_scripts();
std::string proto_str;
proto.SerializeToString(&proto_str);
bool should_update_scripts = false;
std::vector<std::unique_ptr<Script>> scripts;
std::vector<std::unique_ptr<Action>> unused_actions;
EXPECT_TRUE(ProtocolUtils::ParseActions(
proto_str, /* global_payload= */ nullptr, /* script_payload= */ nullptr,
&unused_actions, &scripts, &should_update_scripts));
EXPECT_TRUE(should_update_scripts);
EXPECT_THAT(scripts, SizeIs(1));
EXPECT_THAT("a", Eq(scripts[0]->handle.path));
EXPECT_THAT("name", Eq(scripts[0]->handle.name));
}
} // namespace } // namespace
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -312,13 +312,20 @@ void ScriptExecutor::OnGetActions(bool result, const std::string& response) { ...@@ -312,13 +312,20 @@ void ScriptExecutor::OnGetActions(bool result, const std::string& response) {
processed_actions_.clear(); processed_actions_.clear();
actions_.clear(); actions_.clear();
bool should_update_scripts = false;
std::vector<std::unique_ptr<Script>> scripts;
bool parse_result = ProtocolUtils::ParseActions( bool parse_result = ProtocolUtils::ParseActions(
response, &last_global_payload_, &last_script_payload_, &actions_); response, &last_global_payload_, &last_script_payload_, &actions_,
&scripts, &should_update_scripts);
if (!parse_result) { if (!parse_result) {
RunCallback(false); RunCallback(false);
return; return;
} }
ReportPayloadsToListener(); ReportPayloadsToListener();
if (should_update_scripts) {
ReportScriptsUpdateToListener(std::move(scripts));
}
if (actions_.empty()) { if (actions_.empty()) {
// Finished executing the script if there are no more actions. // Finished executing the script if there are no more actions.
...@@ -336,6 +343,14 @@ void ScriptExecutor::ReportPayloadsToListener() { ...@@ -336,6 +343,14 @@ void ScriptExecutor::ReportPayloadsToListener() {
listener_->OnServerPayloadChanged(last_global_payload_, last_script_payload_); listener_->OnServerPayloadChanged(last_global_payload_, last_script_payload_);
} }
void ScriptExecutor::ReportScriptsUpdateToListener(
std::vector<std::unique_ptr<Script>> scripts) {
if (!listener_)
return;
listener_->OnScriptListChanged(std::move(scripts));
}
void ScriptExecutor::RunCallback(bool success) { void ScriptExecutor::RunCallback(bool success) {
DCHECK(callback_); DCHECK(callback_);
if (should_clean_contextual_ui_on_finish_ || !success) { if (should_clean_contextual_ui_on_finish_ || !success) {
...@@ -506,6 +521,11 @@ void ScriptExecutor::WaitWithInterrupts::OnServerPayloadChanged( ...@@ -506,6 +521,11 @@ void ScriptExecutor::WaitWithInterrupts::OnServerPayloadChanged(
main_script_->ReportPayloadsToListener(); main_script_->ReportPayloadsToListener();
} }
void ScriptExecutor::WaitWithInterrupts::OnScriptListChanged(
std::vector<std::unique_ptr<Script>> scripts) {
main_script_->ReportScriptsUpdateToListener(std::move(scripts));
}
void ScriptExecutor::WaitWithInterrupts::OnPreconditionCheckDone( void ScriptExecutor::WaitWithInterrupts::OnPreconditionCheckDone(
const Script* interrupt, const Script* interrupt,
bool precondition_match) { bool precondition_match) {
......
...@@ -37,6 +37,10 @@ class ScriptExecutor : public ActionDelegate { ...@@ -37,6 +37,10 @@ class ScriptExecutor : public ActionDelegate {
// transitioned to global payloads. // transitioned to global payloads.
virtual void OnServerPayloadChanged(const std::string& global_payload, virtual void OnServerPayloadChanged(const std::string& global_payload,
const std::string& script_payload) = 0; const std::string& script_payload) = 0;
// Called when an update list of scripts is available.
virtual void OnScriptListChanged(
std::vector<std::unique_ptr<Script>> scripts) = 0;
}; };
// |delegate|, |listener|, |script_state| and |ordered_interrupts| should // |delegate|, |listener|, |script_state| and |ordered_interrupts| should
...@@ -180,6 +184,8 @@ class ScriptExecutor : public ActionDelegate { ...@@ -180,6 +184,8 @@ class ScriptExecutor : public ActionDelegate {
// Implements ScriptExecutor::Listener // Implements ScriptExecutor::Listener
void OnServerPayloadChanged(const std::string& global_payload, void OnServerPayloadChanged(const std::string& global_payload,
const std::string& script_payload) override; const std::string& script_payload) override;
void OnScriptListChanged(
std::vector<std::unique_ptr<Script>> scripts) override;
void OnPreconditionCheckDone(const Script* interrupt, void OnPreconditionCheckDone(const Script* interrupt,
bool precondition_match); bool precondition_match);
...@@ -230,6 +236,8 @@ class ScriptExecutor : public ActionDelegate { ...@@ -230,6 +236,8 @@ class ScriptExecutor : public ActionDelegate {
void OnGetActions(bool result, const std::string& response); void OnGetActions(bool result, const std::string& response);
void ReportPayloadsToListener(); void ReportPayloadsToListener();
void ReportScriptsUpdateToListener(
std::vector<std::unique_ptr<Script>> scripts);
void RunCallback(bool success); void RunCallback(bool success);
void RunCallbackWithResult(const Result& result); void RunCallbackWithResult(const Result& result);
void ProcessNextAction(); void ProcessNextAction();
......
...@@ -35,6 +35,7 @@ using ::testing::Not; ...@@ -35,6 +35,7 @@ using ::testing::Not;
using ::testing::Pair; using ::testing::Pair;
using ::testing::ReturnRef; using ::testing::ReturnRef;
using ::testing::SaveArg; using ::testing::SaveArg;
using ::testing::SizeIs;
using ::testing::StrEq; using ::testing::StrEq;
using ::testing::StrictMock; using ::testing::StrictMock;
...@@ -105,6 +106,13 @@ class ScriptExecutorTest : public testing::Test, ...@@ -105,6 +106,13 @@ class ScriptExecutorTest : public testing::Test,
last_script_payload_ = script_payload; last_script_payload_ = script_payload;
} }
void OnScriptListChanged(
std::vector<std::unique_ptr<Script>> scripts) override {
should_update_scripts_ = true;
scripts_update_ = std::move(scripts);
++scripts_update_count_;
}
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);
...@@ -170,6 +178,9 @@ class ScriptExecutorTest : public testing::Test, ...@@ -170,6 +178,9 @@ class ScriptExecutorTest : public testing::Test,
std::vector<Script*> ordered_interrupts_; std::vector<Script*> ordered_interrupts_;
std::string last_global_payload_; std::string last_global_payload_;
std::string last_script_payload_; std::string last_script_payload_;
bool should_update_scripts_ = false;
std::vector<std::unique_ptr<Script>> scripts_update_;
int scripts_update_count_ = 0;
std::unique_ptr<ScriptExecutor> executor_; std::unique_ptr<ScriptExecutor> executor_;
std::map<std::string, std::string> parameters_; std::map<std::string, std::string> parameters_;
StrictMock<base::MockCallback<ScriptExecutor::RunScriptCallback>> StrictMock<base::MockCallback<ScriptExecutor::RunScriptCallback>>
...@@ -717,5 +728,114 @@ TEST_F(ScriptExecutorTest, InterruptReturnsShutdown) { ...@@ -717,5 +728,114 @@ TEST_F(ScriptExecutorTest, InterruptReturnsShutdown) {
Contains(Pair("interrupt", SCRIPT_STATUS_SUCCESS))); Contains(Pair("interrupt", SCRIPT_STATUS_SUCCESS)));
} }
TEST_F(ScriptExecutorTest, UpdateScriptListGetNext) {
should_update_scripts_ = false;
scripts_update_.clear();
scripts_update_count_ = 0;
ActionsResponseProto initial_actions_response;
initial_actions_response.add_actions()->mutable_tell()->set_message("1");
EXPECT_CALL(mock_service_, OnGetActions(StrEq(kScriptPath), _, _, _, _, _))
.WillOnce(RunOnceCallback<5>(true, Serialize(initial_actions_response)));
ActionsResponseProto next_actions_response;
next_actions_response.add_actions()->mutable_tell()->set_message("2");
auto* script =
next_actions_response.mutable_update_script_list()->add_scripts();
script->set_path("path");
auto* presentation = script->mutable_presentation();
presentation->set_name("name");
presentation->mutable_precondition();
EXPECT_CALL(mock_service_, OnGetNextActions(_, _, _, _))
.WillOnce(RunOnceCallback<3>(true, Serialize(next_actions_response)))
.WillOnce(RunOnceCallback<3>(true, ""));
EXPECT_CALL(executor_callback_,
Run(Field(&ScriptExecutor::Result::success, true)));
executor_->Run(executor_callback_.Get());
EXPECT_TRUE(should_update_scripts_);
EXPECT_THAT(scripts_update_, SizeIs(1));
EXPECT_THAT(scripts_update_count_, Eq(1));
EXPECT_THAT("path", scripts_update_[0]->handle.path);
EXPECT_THAT("name", scripts_update_[0]->handle.name);
}
TEST_F(ScriptExecutorTest, UpdateScriptListShouldNotifyMultipleTimes) {
should_update_scripts_ = false;
scripts_update_.clear();
scripts_update_count_ = 0;
ActionsResponseProto actions_response;
actions_response.add_actions()->mutable_tell()->set_message("hi");
auto* script = actions_response.mutable_update_script_list()->add_scripts();
script->set_path("path");
auto* presentation = script->mutable_presentation();
presentation->set_name("name");
presentation->mutable_precondition();
EXPECT_CALL(mock_service_, OnGetActions(StrEq(kScriptPath), _, _, _, _, _))
.WillOnce(RunOnceCallback<5>(true, Serialize(actions_response)));
script->set_path("path2");
EXPECT_CALL(mock_service_, OnGetNextActions(_, _, _, _))
.WillOnce(RunOnceCallback<3>(true, Serialize(actions_response)))
.WillOnce(RunOnceCallback<3>(true, ""));
EXPECT_CALL(executor_callback_,
Run(Field(&ScriptExecutor::Result::success, true)));
executor_->Run(executor_callback_.Get());
EXPECT_TRUE(should_update_scripts_);
EXPECT_THAT(scripts_update_count_, Eq(2));
EXPECT_THAT(scripts_update_, SizeIs(1));
EXPECT_THAT("path2", scripts_update_[0]->handle.path);
}
TEST_F(ScriptExecutorTest, UpdateScriptListFromInterrupt) {
should_update_scripts_ = false;
scripts_update_.clear();
scripts_update_count_ = 0;
SetupInterruptibleScript(kScriptPath, "element");
RegisterInterrupt("interrupt", "interrupt_trigger");
ActionsResponseProto interrupt_actions;
interrupt_actions.add_actions()->mutable_tell()->set_message("abc");
EXPECT_CALL(mock_service_, OnGetActions(StrEq("interrupt"), _, _, _, _, _))
.WillOnce(RunOnceCallback<5>(true, Serialize(interrupt_actions)));
auto* script = interrupt_actions.mutable_update_script_list()->add_scripts();
script->set_path("path");
auto* presentation = script->mutable_presentation();
presentation->set_name("update_from_interrupt");
presentation->mutable_precondition();
// We expect a call from the interrupt which will update the script list and a
// second call from the interrupt to terminate. Then a call from the main
// script which will finish without running any actions.
EXPECT_CALL(mock_service_, OnGetNextActions(_, _, _, _))
.Times(3)
.WillOnce(RunOnceCallback<3>(true, Serialize(interrupt_actions)))
.WillRepeatedly(RunOnceCallback<3>(true, ""));
EXPECT_CALL(executor_callback_,
Run(Field(&ScriptExecutor::Result::success, true)));
executor_->Run(executor_callback_.Get());
EXPECT_THAT(scripts_state_,
Contains(Pair(kScriptPath, SCRIPT_STATUS_SUCCESS)));
EXPECT_THAT(scripts_state_,
Contains(Pair("interrupt", SCRIPT_STATUS_SUCCESS)));
EXPECT_TRUE(should_update_scripts_);
EXPECT_THAT(scripts_update_, SizeIs(1));
EXPECT_THAT(scripts_update_count_, Eq(1));
EXPECT_THAT("path", scripts_update_[0]->handle.path);
EXPECT_THAT("update_from_interrupt", scripts_update_[0]->handle.name);
}
} // namespace } // namespace
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -183,9 +183,17 @@ void ScriptTracker::OnScriptRun( ...@@ -183,9 +183,17 @@ void ScriptTracker::OnScriptRun(
ScriptExecutor::RunScriptCallback original_callback, ScriptExecutor::RunScriptCallback original_callback,
const ScriptExecutor::Result& result) { const ScriptExecutor::Result& result) {
executor_.reset(); executor_.reset();
MaybeSwapInScripts();
std::move(original_callback).Run(result); std::move(original_callback).Run(result);
} }
void ScriptTracker::MaybeSwapInScripts() {
if (scripts_update_) {
SetScripts(std::move(*scripts_update_));
scripts_update_.reset();
}
}
void ScriptTracker::UpdateRunnableScriptsIfNecessary() { void ScriptTracker::UpdateRunnableScriptsIfNecessary() {
if (!RunnablesHaveChanged()) if (!RunnablesHaveChanged())
return; return;
...@@ -240,4 +248,10 @@ void ScriptTracker::OnServerPayloadChanged(const std::string& global_payload, ...@@ -240,4 +248,10 @@ void ScriptTracker::OnServerPayloadChanged(const std::string& global_payload,
last_script_payload_ = script_payload; last_script_payload_ = script_payload;
} }
void ScriptTracker::OnScriptListChanged(
std::vector<std::unique_ptr<Script>> scripts) {
scripts_update_.reset(
new std::vector<std::unique_ptr<Script>>(std::move(scripts)));
}
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -113,9 +113,15 @@ class ScriptTracker : public ScriptExecutor::Listener { ...@@ -113,9 +113,15 @@ class ScriptTracker : public ScriptExecutor::Listener {
void UpdateRunnableScriptsIfNecessary(); void UpdateRunnableScriptsIfNecessary();
void OnCheckDone(); void OnCheckDone();
// Updates the list of available scripts if there is a pending update from
// when a script was still being executed.
void MaybeSwapInScripts();
// Overrides ScriptExecutor::Listener. // Overrides ScriptExecutor::Listener.
void OnServerPayloadChanged(const std::string& global_payload, void OnServerPayloadChanged(const std::string& global_payload,
const std::string& script_payload) override; const std::string& script_payload) override;
void OnScriptListChanged(
std::vector<std::unique_ptr<Script>> scripts) override;
// Stops running pending checks and cleans up any state used by pending // Stops running pending checks and cleans up any state used by pending
// checks. This can safely be called at any time, including when no checks are // checks. This can safely be called at any time, including when no checks are
...@@ -167,6 +173,10 @@ class ScriptTracker : public ScriptExecutor::Listener { ...@@ -167,6 +173,10 @@ class ScriptTracker : public ScriptExecutor::Listener {
std::string last_global_payload_; std::string last_global_payload_;
std::string last_script_payload_; std::string last_script_payload_;
// List of scripts to replace the currently available scripts. The replacement
// only occurse when |scripts_update| is not nullptr.
std::unique_ptr<std::vector<std::unique_ptr<Script>>> scripts_update_;
base::WeakPtrFactory<ScriptTracker> weak_ptr_factory_; base::WeakPtrFactory<ScriptTracker> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(ScriptTracker); DISALLOW_COPY_AND_ASSIGN(ScriptTracker);
......
...@@ -26,6 +26,8 @@ using ::testing::IsEmpty; ...@@ -26,6 +26,8 @@ using ::testing::IsEmpty;
using ::testing::NiceMock; using ::testing::NiceMock;
using ::testing::ReturnRef; using ::testing::ReturnRef;
using ::testing::SizeIs; using ::testing::SizeIs;
using ::testing::StrEq;
using ::testing::StrictMock;
using ::testing::UnorderedElementsAre; using ::testing::UnorderedElementsAre;
class ScriptTrackerTest : public testing::Test, class ScriptTrackerTest : public testing::Test,
...@@ -116,12 +118,6 @@ class ScriptTrackerTest : public testing::Test, ...@@ -116,12 +118,6 @@ class ScriptTrackerTest : public testing::Test,
return script; return script;
} }
static SupportedScriptProto* AddRunnableScript(
SupportsScriptResponseProto* response,
const std::string& name_and_path) {
return AddScript(response, name_and_path, name_and_path, "exists");
}
const std::vector<ScriptHandle>& runnable_scripts() { const std::vector<ScriptHandle>& runnable_scripts() {
return runnable_scripts_; return runnable_scripts_;
} }
...@@ -134,6 +130,12 @@ class ScriptTrackerTest : public testing::Test, ...@@ -134,6 +130,12 @@ class ScriptTrackerTest : public testing::Test,
return paths; return paths;
} }
std::string Serialize(const google::protobuf::MessageLite& message) {
std::string output;
message.SerializeToString(&output);
return output;
}
GURL url_; GURL url_;
NiceMock<MockService> mock_service_; NiceMock<MockService> mock_service_;
NiceMock<MockWebController> mock_web_controller_; NiceMock<MockWebController> mock_web_controller_;
...@@ -322,4 +324,89 @@ TEST_F(ScriptTrackerTest, CheckScriptsAfterDOMChange) { ...@@ -322,4 +324,89 @@ TEST_F(ScriptTrackerTest, CheckScriptsAfterDOMChange) {
ASSERT_THAT(runnable_script_paths(), ElementsAre("script path")); ASSERT_THAT(runnable_script_paths(), ElementsAre("script path"));
} }
TEST_F(ScriptTrackerTest, UpdateScriptList) {
// 1. Initialize runnable scripts with a single valid script.
SupportsScriptResponseProto scripts;
AddScript(&scripts, "runnable name", "runnable path", "exists");
SetAndCheckScripts(scripts);
EXPECT_EQ(1, runnable_scripts_changed_);
ASSERT_THAT(runnable_scripts(), SizeIs(1));
EXPECT_EQ("runnable name", runnable_scripts()[0].name);
EXPECT_EQ("runnable path", runnable_scripts()[0].path);
// 2. Run the action and trigger a script list update.
ActionsResponseProto actions_response;
actions_response.add_actions()->mutable_tell()->set_message("hi");
*actions_response.mutable_update_script_list()->add_scripts() =
*AddScript(&scripts, "update name", "update path", "exists");
*actions_response.mutable_update_script_list()->add_scripts() =
*AddScript(&scripts, "update name 2", "update path 2", "exists");
EXPECT_CALL(mock_service_,
OnGetActions(StrEq("runnable name"), _, _, _, _, _))
.WillOnce(RunOnceCallback<5>(true, Serialize(actions_response)));
EXPECT_CALL(mock_service_, OnGetNextActions(_, _, _, _))
.WillOnce(RunOnceCallback<3>(true, ""));
base::MockCallback<ScriptExecutor::RunScriptCallback> execute_callback;
EXPECT_CALL(execute_callback,
Run(Field(&ScriptExecutor::Result::success, true)));
tracker_.ExecuteScript("runnable name", execute_callback.Get());
tracker_.CheckScripts(base::TimeDelta::FromSeconds(0));
// 3. Verify that the runnable scripts have changed to the updated list.
EXPECT_EQ(2, runnable_scripts_changed_);
ASSERT_THAT(runnable_scripts(), SizeIs(2));
EXPECT_EQ("update name", runnable_scripts()[0].name);
EXPECT_EQ("update path", runnable_scripts()[0].path);
EXPECT_EQ("update name 2", runnable_scripts()[1].name);
EXPECT_EQ("update path 2", runnable_scripts()[1].path);
}
TEST_F(ScriptTrackerTest, UpdateScriptListFromInterrupt) {
// 1. Initialize runnable scripts with a single valid interrupt script.
SupportsScriptResponseProto scripts;
auto* script =
AddScript(&scripts, "runnable name", "runnable path", "exists");
script->mutable_presentation()->set_interrupt(true);
SetAndCheckScripts(scripts);
EXPECT_EQ(1, runnable_scripts_changed_);
ASSERT_THAT(runnable_scripts(), SizeIs(1));
EXPECT_EQ("runnable name", runnable_scripts()[0].name);
EXPECT_EQ("runnable path", runnable_scripts()[0].path);
// 2. Run the interrupt action and trigger a script list update from an
// interrupt.
ActionsResponseProto actions_response;
actions_response.add_actions()->mutable_tell()->set_message("hi");
*actions_response.mutable_update_script_list()->add_scripts() =
*AddScript(&scripts, "update name", "update path", "exists");
*actions_response.mutable_update_script_list()->add_scripts() =
*AddScript(&scripts, "update name 2", "update path 2", "exists");
EXPECT_CALL(mock_service_,
OnGetActions(StrEq("runnable name"), _, _, _, _, _))
.WillOnce(RunOnceCallback<5>(true, Serialize(actions_response)));
EXPECT_CALL(mock_service_, OnGetNextActions(_, _, _, _))
.WillOnce(RunOnceCallback<3>(true, ""));
base::MockCallback<ScriptExecutor::RunScriptCallback> execute_callback;
EXPECT_CALL(execute_callback,
Run(Field(&ScriptExecutor::Result::success, true)));
tracker_.ExecuteScript("runnable name", execute_callback.Get());
tracker_.CheckScripts(base::TimeDelta::FromSeconds(0));
// 3. Verify that the runnable scripts have changed to the updated list.
EXPECT_EQ(2, runnable_scripts_changed_);
ASSERT_THAT(runnable_scripts(), SizeIs(2));
EXPECT_EQ("update name", runnable_scripts()[0].name);
EXPECT_EQ("update path", runnable_scripts()[0].path);
EXPECT_EQ("update name 2", runnable_scripts()[1].name);
EXPECT_EQ("update path 2", runnable_scripts()[1].path);
}
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -216,6 +216,19 @@ message ActionsResponseProto { ...@@ -216,6 +216,19 @@ message ActionsResponseProto {
// Actions to be performed in order. // Actions to be performed in order.
// Should stop processing as soon as an action fails. // Should stop processing as soon as an action fails.
repeated ActionProto actions = 3; repeated ActionProto actions = 3;
// List of scripts to update.
//
// The client is expected to update the cache of scripts with this new
// information. No action is needed when this field is not set. If the field
// is set with an empty list of scripts, then no script is eligible to run
// anymore.
//
// Note: This is an intermediate solution and the logic associated with this
// field will eventually be absorbed into the supports script response from
// the backend.
message UpdateScriptListProto { repeated SupportedScriptProto scripts = 1; }
optional UpdateScriptListProto update_script_list = 5;
} }
// An action could be performed. // An action could be performed.
......
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