Commit 45ff50d4 authored by Clemens Arbesser's avatar Clemens Arbesser Committed by Commit Bot

[Autofill Assistant] Allow GenericUi to request UserData.

This bridges the gap between legacy UserData and the new UserModel. For
now, only additional values can be requested, but this can be easily
extended to apply to the selected card/profile etc. as well.

All values requested in this manner will be client-side only and never
leave the device.

Bug: b/145043394
Change-Id: Iaf3fe8af7a24418eabe36393b5428752a1d20aae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2308417Reviewed-by: default avatarMarian Fechete <marianfe@google.com>
Reviewed-by: default avatarLuca Hunkeler <hluca@google.com>
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#790430}
parent 63c487f5
...@@ -145,6 +145,22 @@ void ShowGenericUiAction::OnViewInflationFinished(const ClientStatus& status) { ...@@ -145,6 +145,22 @@ void ShowGenericUiAction::OnViewInflationFinished(const ClientStatus& status) {
// Note: it is important to write autofill profiles etc. to the model AFTER // Note: it is important to write autofill profiles etc. to the model AFTER
// the UI has been inflated, otherwise the UI won't get change notifications // the UI has been inflated, otherwise the UI won't get change notifications
// for them. // for them.
for (const auto& additional_value :
proto_.show_generic_ui().request_user_data().additional_values()) {
if (!delegate_->GetUserData()->has_additional_value(
additional_value.source_identifier())) {
EndAction(ClientStatus(PRECONDITION_FAILED));
return;
}
}
for (const auto& additional_value :
proto_.show_generic_ui().request_user_data().additional_values()) {
ValueProto value = *delegate_->GetUserData()->additional_value(
additional_value.source_identifier());
value.set_is_client_side_only(true);
delegate_->GetUserModel()->SetValue(additional_value.model_identifier(),
value);
}
if (proto_.show_generic_ui().has_request_login_options()) { if (proto_.show_generic_ui().has_request_login_options()) {
auto login_options = auto login_options =
proto_.show_generic_ui().request_login_options().login_options(); proto_.show_generic_ui().request_login_options().login_options();
......
...@@ -57,6 +57,8 @@ class ShowGenericUiActionTest : public content::RenderViewHostTestHarness { ...@@ -57,6 +57,8 @@ class ShowGenericUiActionTest : public content::RenderViewHostTestHarness {
ON_CALL(mock_action_delegate_, ClearGenericUi()).WillByDefault(Return()); ON_CALL(mock_action_delegate_, ClearGenericUi()).WillByDefault(Return());
ON_CALL(mock_action_delegate_, GetUserModel()) ON_CALL(mock_action_delegate_, GetUserModel())
.WillByDefault(Return(&user_model_)); .WillByDefault(Return(&user_model_));
ON_CALL(mock_action_delegate_, GetUserData())
.WillByDefault(Return(&user_data_));
ON_CALL(mock_action_delegate_, GetPersonalDataManager) ON_CALL(mock_action_delegate_, GetPersonalDataManager)
.WillByDefault(Return(&mock_personal_data_manager_)); .WillByDefault(Return(&mock_personal_data_manager_));
ON_CALL(mock_action_delegate_, GetWebsiteLoginManager) ON_CALL(mock_action_delegate_, GetWebsiteLoginManager)
...@@ -83,6 +85,7 @@ class ShowGenericUiActionTest : public content::RenderViewHostTestHarness { ...@@ -83,6 +85,7 @@ class ShowGenericUiActionTest : public content::RenderViewHostTestHarness {
return action; return action;
} }
UserData user_data_;
UserModel user_model_; UserModel user_model_;
MockPersonalDataManager mock_personal_data_manager_; MockPersonalDataManager mock_personal_data_manager_;
MockWebsiteLoginManager mock_website_login_manager_; MockWebsiteLoginManager mock_website_login_manager_;
...@@ -520,6 +523,44 @@ TEST_F(ShowGenericUiActionTest, BreakingNavigationBeforeUiIsSet) { ...@@ -520,6 +523,44 @@ TEST_F(ShowGenericUiActionTest, BreakingNavigationBeforeUiIsSet) {
Run(); Run();
} }
TEST_F(ShowGenericUiActionTest, RequestUserDataFailsOnMissingValues) {
auto* request_user_data = proto_.mutable_request_user_data();
auto* additional_value = request_user_data->add_additional_values();
additional_value->set_source_identifier("client_memory");
additional_value->set_model_identifier("target");
EXPECT_CALL(callback_, Run(Pointee(Property(&ProcessedActionProto::status,
PRECONDITION_FAILED))));
Run();
}
TEST_F(ShowGenericUiActionTest, RequestUserData) {
auto* request_user_data = proto_.mutable_request_user_data();
auto* additional_value = request_user_data->add_additional_values();
additional_value->set_source_identifier("client_memory_1");
additional_value->set_model_identifier("target_1");
additional_value = request_user_data->add_additional_values();
additional_value->set_source_identifier("client_memory_2");
additional_value->set_model_identifier("target_2");
user_data_.additional_values_["client_memory_1"] =
SimpleValue(std::string("value_1"));
user_data_.additional_values_["client_memory_2"] = SimpleValue(123);
EXPECT_CALL(
callback_,
Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
Run();
auto expected_value_1 = SimpleValue(std::string("value_1"));
expected_value_1.set_is_client_side_only(true);
auto expected_value_2 = SimpleValue(123);
expected_value_2.set_is_client_side_only(true);
EXPECT_EQ(*user_model_.GetValue("target_1"), expected_value_1);
EXPECT_EQ(*user_model_.GetValue("target_2"), expected_value_2);
}
// TODO(b/161652848): Add test coverage for element checks and interrupts. // TODO(b/161652848): Add test coverage for element checks and interrupts.
} // namespace } // namespace
......
...@@ -1321,6 +1321,16 @@ message ShowGenericUiProto { ...@@ -1321,6 +1321,16 @@ message ShowGenericUiProto {
repeated LoginOption login_options = 1; repeated LoginOption login_options = 1;
optional string model_identifier = 2; optional string model_identifier = 2;
} }
message RequestUserData {
message AdditionalValue {
// The client memory identifier (from |UserData|).
optional string source_identifier = 1;
// The model identifier to write the value to (to |UserModel|).
optional string model_identifier = 2;
}
// Additional values to write from |UserData| to |UserModel|.
repeated AdditionalValue additional_values = 1;
}
message Result { message Result {
// The model containing the values for all keys specified in // The model containing the values for all keys specified in
// |output_model_identifiers|. // |output_model_identifiers|.
...@@ -1365,6 +1375,11 @@ message ShowGenericUiProto { ...@@ -1365,6 +1375,11 @@ message ShowGenericUiProto {
// If true, run scripts flagged with |interrupt=true| as soon as their // If true, run scripts flagged with |interrupt=true| as soon as their
// preconditions match, then go back to the parent action. // preconditions match, then go back to the parent action.
optional bool allow_interrupt = 8; optional bool allow_interrupt = 8;
// If specified, will write the requested values from |UserData| to
// |UserModel|. Will fail the action with PRECONDITION_FAILED if any of the
// requested values is missing. Note that all values will have
// |is_client_side_only| set to true.
optional RequestUserData request_user_data = 9;
} }
// Allow choosing one or more possibility. If FocusElement was called just // Allow choosing one or more possibility. If FocusElement was called just
......
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