Commit 0ec00ebe authored by Mathias Carlen's avatar Mathias Carlen Committed by Commit Bot

[Autofill Assistant] Wrong proto field fix.

Before this patch the client and server protos were not in sync. The
reason why this worked is that the proto messages used had the same
fields and the same ids. This patch introduces a proper Result message
that also contains the navigation_ended field used to signal to the
backend that the prompt action was terminated due to a reload or
navigation of the underlying website.

Bug: b/152724265
Change-Id: I8247dddc058fda95d5a1948d5b9e5405eceaaa95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2215055
Commit-Queue: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#773179}
parent d7fb20bf
...@@ -221,8 +221,8 @@ void PromptAction::OnSuggestionChosen(int choice_index) { ...@@ -221,8 +221,8 @@ void PromptAction::OnSuggestionChosen(int choice_index) {
} }
DCHECK(choice_index >= 0 && choice_index <= proto_.prompt().choices_size()); DCHECK(choice_index >= 0 && choice_index <= proto_.prompt().choices_size());
*processed_action_proto_->mutable_prompt_choice() = processed_action_proto_->mutable_prompt_choice()->set_server_payload(
proto_.prompt().choices(choice_index); proto_.prompt().choices(choice_index).server_payload());
EndAction(ClientStatus(ACTION_APPLIED)); EndAction(ClientStatus(ACTION_APPLIED));
} }
......
...@@ -172,9 +172,9 @@ TEST_F(PromptActionTest, SelectButtons) { ...@@ -172,9 +172,9 @@ TEST_F(PromptActionTest, SelectButtons) {
Run(Pointee(AllOf( Run(Pointee(AllOf(
Property(&ProcessedActionProto::status, ACTION_APPLIED), Property(&ProcessedActionProto::status, ACTION_APPLIED),
Property(&ProcessedActionProto::prompt_choice, Property(&ProcessedActionProto::prompt_choice,
Property(&PromptProto::Choice::navigation_ended, false)), Property(&PromptProto::Result::navigation_ended, false)),
Property(&ProcessedActionProto::prompt_choice, Property(&ProcessedActionProto::prompt_choice,
Property(&PromptProto::Choice::server_payload, "ok")))))); Property(&PromptProto::Result::server_payload, "ok"))))));
EXPECT_TRUE((*user_actions_)[0].HasCallback()); EXPECT_TRUE((*user_actions_)[0].HasCallback());
(*user_actions_)[0].Call(TriggerContext::CreateEmpty()); (*user_actions_)[0].Call(TriggerContext::CreateEmpty());
} }
...@@ -275,7 +275,7 @@ TEST_F(PromptActionTest, AutoSelectWhenElementExists) { ...@@ -275,7 +275,7 @@ TEST_F(PromptActionTest, AutoSelectWhenElementExists) {
callback_, callback_,
Run(Pointee(AllOf(Property(&ProcessedActionProto::status, ACTION_APPLIED), Run(Pointee(AllOf(Property(&ProcessedActionProto::status, ACTION_APPLIED),
Property(&ProcessedActionProto::prompt_choice, Property(&ProcessedActionProto::prompt_choice,
Property(&PromptProto::Choice::server_payload, Property(&PromptProto::Result::server_payload,
"auto-select")))))); "auto-select"))))));
task_env_.FastForwardBy(base::TimeDelta::FromSeconds(1)); task_env_.FastForwardBy(base::TimeDelta::FromSeconds(1));
} }
...@@ -303,7 +303,7 @@ TEST_F(PromptActionTest, AutoSelectWithButton) { ...@@ -303,7 +303,7 @@ TEST_F(PromptActionTest, AutoSelectWithButton) {
callback_, callback_,
Run(Pointee(AllOf(Property(&ProcessedActionProto::status, ACTION_APPLIED), Run(Pointee(AllOf(Property(&ProcessedActionProto::status, ACTION_APPLIED),
Property(&ProcessedActionProto::prompt_choice, Property(&ProcessedActionProto::prompt_choice,
Property(&PromptProto::Choice::server_payload, Property(&PromptProto::Result::server_payload,
"auto-select")))))); "auto-select"))))));
task_env_.FastForwardBy(base::TimeDelta::FromSeconds(1)); task_env_.FastForwardBy(base::TimeDelta::FromSeconds(1));
} }
...@@ -419,7 +419,7 @@ TEST_F(PromptActionTest, ForwardInterruptFailure) { ...@@ -419,7 +419,7 @@ TEST_F(PromptActionTest, ForwardInterruptFailure) {
Pointee(Property(&ProcessedActionProto::status, INTERRUPT_FAILED)), Pointee(Property(&ProcessedActionProto::status, INTERRUPT_FAILED)),
Pointee( Pointee(
Property(&ProcessedActionProto::prompt_choice, Property(&ProcessedActionProto::prompt_choice,
Property(&PromptProto::Choice::server_payload, "")))))); Property(&PromptProto::Result::server_payload, ""))))));
ASSERT_TRUE(fake_wait_for_dom_done_); ASSERT_TRUE(fake_wait_for_dom_done_);
std::move(fake_wait_for_dom_done_).Run(ClientStatus(INTERRUPT_FAILED)); std::move(fake_wait_for_dom_done_).Run(ClientStatus(INTERRUPT_FAILED));
} }
...@@ -446,7 +446,7 @@ TEST_F(PromptActionTest, EndActionOnNavigation) { ...@@ -446,7 +446,7 @@ TEST_F(PromptActionTest, EndActionOnNavigation) {
Run(Pointee(AllOf( Run(Pointee(AllOf(
Property(&ProcessedActionProto::status, ACTION_APPLIED), Property(&ProcessedActionProto::status, ACTION_APPLIED),
Property(&ProcessedActionProto::prompt_choice, Property(&ProcessedActionProto::prompt_choice,
Property(&PromptProto::Choice::navigation_ended, true)))))); Property(&PromptProto::Result::navigation_ended, true))))));
action.ProcessAction(callback_.Get()); action.ProcessAction(callback_.Get());
} }
......
...@@ -528,7 +528,7 @@ message ProcessedActionProto { ...@@ -528,7 +528,7 @@ message ProcessedActionProto {
optional ProcessedActionStatusDetailsProto status_details = 19; optional ProcessedActionStatusDetailsProto status_details = 19;
oneof result_data { oneof result_data {
PromptProto.Choice prompt_choice = 5; PromptProto.Result prompt_choice = 5;
string html_source = 12; string html_source = 12;
// Should be set as a result of CollectUserDataAction. // Should be set as a result of CollectUserDataAction.
CollectUserDataResultProto collect_user_data_result = 15; CollectUserDataResultProto collect_user_data_result = 15;
...@@ -1216,12 +1216,7 @@ message PromptProto { ...@@ -1216,12 +1216,7 @@ message PromptProto {
// be transmitted as-is by the client without interpreting. // be transmitted as-is by the client without interpreting.
optional bytes server_payload = 5; optional bytes server_payload = 5;
// This field is only used when crafting a response Choice for the server reserved 4, 6, 8, 13, 14, 17;
// when the |end_on_navigation| option is set. It means there was a
// navigation event while in prompt mode that ended the action.
optional bool navigation_ended = 17;
reserved 4, 6, 8, 13, 14;
} }
repeated Choice choices = 4; repeated Choice choices = 4;
...@@ -1239,13 +1234,28 @@ message PromptProto { ...@@ -1239,13 +1234,28 @@ message PromptProto {
optional bool browse_mode = 7; optional bool browse_mode = 7;
// When set to true, end prompt on navigation events happening during a prompt // When set to true, end prompt on navigation events happening during a prompt
// action. The result sent back to the server will contain a Choice marked as // action. The result sent back to the server in
// |navigation_ended|. // ProcessedActionProto.prompt_choice will have |navigation_ended| set to
// true.
optional bool end_on_navigation = 8; optional bool end_on_navigation = 8;
// A list of domains and subdomains to allow users to navigate to when in // A list of domains and subdomains to allow users to navigate to when in
// browse mode. // browse mode.
repeated string browse_domains_whitelist = 9; repeated string browse_domains_whitelist = 9;
// Result to pass to ProcessedActionProto.
message Result {
// This field is only used when crafting a response Choice for the server
// when the |end_on_navigation| option is set. It means there was a
// navigation event while in prompt mode that ended the action.
optional bool navigation_ended = 2;
// Server payload originally found in Choice.server_payload. This should be
// transmitted as-is by the client without interpreting.
optional bytes server_payload = 5;
reserved 1;
}
} }
message ContactDetailsProto { message ContactDetailsProto {
......
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