Commit 9d71f4dc authored by Clemens Arbesser's avatar Clemens Arbesser Committed by Commit Bot

[Autofill Assistant] Add support for mandatory fields to generic UI.

This CL allows the CollectUserData action to surface generic UI which
contains mandatory sections. Since currently chips can't be directly
controlled by the generic UI framework, this is done with change in the
action itself. In the future, this workaround will be removed.

This CL also adds logical operations to the set of supported
interactions.

Bug: b/145043394
Change-Id: I269d44620a2b194dc3218feebfd16f38d592ecdd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036100
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: default avatarSandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#742147}
parent 6b9eb390
......@@ -28,6 +28,132 @@ void SetValue(base::WeakPtr<UserModel> user_model,
user_model->SetValue(identifier, value);
}
void ComputeValueBooleanAnd(base::WeakPtr<UserModel> user_model,
const BooleanAndProto& proto,
const std::string& result_model_identifier,
const ValueProto& ignored) {
if (!user_model) {
return;
}
auto values = user_model->GetValues(proto.model_identifiers());
if (!values.has_value()) {
DVLOG(2) << "Failed to find values in user model";
return;
}
if (!AreAllValuesOfType(*values, ValueProto::kBooleans) ||
!AreAllValuesOfSize(*values, 1)) {
DVLOG(2) << "All values must be 'boolean' and contain exactly 1 value each";
return;
}
bool result = true;
for (const auto& value : *values) {
result &= value.booleans().values(0);
}
user_model->SetValue(result_model_identifier, SimpleValue(result));
}
void ComputeValueBooleanOr(base::WeakPtr<UserModel> user_model,
const BooleanOrProto& proto,
const std::string& result_model_identifier,
const ValueProto& ignored) {
if (!user_model) {
return;
}
auto values = user_model->GetValues(proto.model_identifiers());
if (!values.has_value()) {
DVLOG(2) << "Failed to find values in user model";
return;
}
if (!AreAllValuesOfType(*values, ValueProto::kBooleans) ||
!AreAllValuesOfSize(*values, 1)) {
DVLOG(2) << "All values must be 'boolean' and contain exactly 1 value each";
return;
}
bool result = true;
for (const auto& value : *values) {
result |= value.booleans().values(0);
}
user_model->SetValue(result_model_identifier, SimpleValue(result));
}
void ComputeValueBooleanNot(base::WeakPtr<UserModel> user_model,
const BooleanNotProto& proto,
const std::string& result_model_identifier,
const ValueProto& ignored) {
if (!user_model) {
return;
}
auto value = user_model->GetValue(proto.model_identifier());
if (!value.has_value()) {
DVLOG(2) << "Error evaluating " << __func__ << ": "
<< proto.model_identifier() << " not found in model";
return;
}
if (value->booleans().values().size() != 1) {
DVLOG(2) << "Error evaluating " << __func__
<< ": expected single boolean, but got " << *value;
return;
}
user_model->SetValue(result_model_identifier,
SimpleValue(!value->booleans().values(0)));
}
base::Optional<InteractionHandlerAndroid::InteractionCallback>
CreateComputeValueCallbackForProto(base::WeakPtr<UserModel> user_model,
const ComputeValueProto& proto) {
if (proto.result_model_identifier().empty()) {
DVLOG(1) << "Error creating ComputeValue interaction: "
"result_model_identifier empty";
return base::nullopt;
}
switch (proto.kind_case()) {
case ComputeValueProto::kBooleanAnd:
if (proto.boolean_and().model_identifiers().size() == 0) {
DVLOG(1) << "Error creating ComputeValue::BooleanAnd interaction: no "
"model_identifiers specified";
return base::nullopt;
}
return base::Optional<InteractionHandlerAndroid::InteractionCallback>(
base::BindRepeating(&ComputeValueBooleanAnd, user_model,
proto.boolean_and(),
proto.result_model_identifier()));
case ComputeValueProto::kBooleanOr:
if (proto.boolean_or().model_identifiers().size() == 0) {
DVLOG(1) << "Error creating ComputeValue::BooleanOr interaction: no "
"model_identifiers specified";
return base::nullopt;
}
return base::Optional<InteractionHandlerAndroid::InteractionCallback>(
base::BindRepeating(&ComputeValueBooleanOr, user_model,
proto.boolean_or(),
proto.result_model_identifier()));
case ComputeValueProto::kBooleanNot:
if (proto.boolean_not().model_identifier().empty()) {
DVLOG(1) << "Error creating ComputeValue::BooleanNot interaction: "
"model_identifier not specified";
return base::nullopt;
}
return base::Optional<InteractionHandlerAndroid::InteractionCallback>(
base::BindRepeating(&ComputeValueBooleanNot, user_model,
proto.boolean_not(),
proto.result_model_identifier()));
case ComputeValueProto::KIND_NOT_SET:
DVLOG(1) << "Error creating ComputeValue interaction: kind not set";
return base::nullopt;
}
return base::nullopt;
}
void ShowInfoPopup(const InfoPopupProto& proto,
base::android::ScopedJavaGlobalRef<jobject> jcontext,
const ValueProto& ignored) {
......@@ -197,6 +323,9 @@ CreateInteractionCallbackFromProto(
return base::Optional<InteractionHandlerAndroid::InteractionCallback>(
base::BindRepeating(&ShowListPopup, user_model->GetWeakPtr(),
proto.show_list_popup(), jcontext, jdelegate));
case CallbackProto::kComputeValue:
return CreateComputeValueCallbackForProto(user_model->GetWeakPtr(),
proto.compute_value());
case CallbackProto::KIND_NOT_SET:
DVLOG(1) << "Error creating interaction: kind not set";
return base::nullopt;
......
......@@ -26,7 +26,6 @@
#include "components/autofill_assistant/browser/metrics.h"
#include "components/autofill_assistant/browser/service.pb.h"
#include "components/autofill_assistant/browser/user_data_util.h"
#include "components/autofill_assistant/browser/user_model.h"
#include "components/autofill_assistant/browser/website_login_fetcher_impl.h"
#include "components/strings/grit/components_strings.h"
#include "content/public/browser/web_contents.h"
......@@ -288,6 +287,34 @@ bool IsValidUserFormSection(
return true;
}
bool IsValidUserModel(const autofill_assistant::UserModel& user_model,
const autofill_assistant::CollectUserDataOptions&
collect_user_data_options) {
if (!collect_user_data_options.additional_model_identifier_to_check
.has_value()) {
return true;
}
auto valid = user_model.GetValue(
*collect_user_data_options.additional_model_identifier_to_check);
if (!valid.has_value()) {
VLOG(1) << "Error evaluating validity of user model: '"
<< *collect_user_data_options.additional_model_identifier_to_check
<< "' not found in user model.";
return false;
}
if (valid->kind_case() != autofill_assistant::ValueProto::kBooleans ||
valid->booleans().values().size() != 1) {
VLOG(1) << "Error evaluating validity of user model: expected single "
"boolean, but got "
<< *valid;
return false;
}
return valid->booleans().values(0);
}
// Merges |model_a| and |model_b| into a new model.
// TODO(arbesser): deal with overlapping keys.
autofill_assistant::ModelProto MergeModelProtos(
......@@ -950,6 +977,10 @@ bool CollectUserDataAction::CreateOptionsFromProto() {
collect_user_data_options_->generic_user_interface_appended =
collect_user_data.generic_user_interface_appended();
}
if (collect_user_data.has_additional_model_identifier_to_check()) {
collect_user_data_options_->additional_model_identifier_to_check =
collect_user_data.additional_model_identifier_to_check();
}
// TODO(crbug.com/806868): Maybe we could refactor this to make the confirm
// chip and direct_action part of the additional_actions.
......@@ -1062,6 +1093,7 @@ bool CollectUserDataAction::CheckInitialAutofillDataComplete(
// static
bool CollectUserDataAction::IsUserDataComplete(
const UserData& user_data,
const UserModel& user_model,
const CollectUserDataOptions& options) {
auto* selected_profile =
user_data.selected_address(options.contact_details_name);
......@@ -1080,7 +1112,8 @@ bool CollectUserDataAction::IsUserDataComplete(
user_data.date_time_range_end_date_,
user_data.date_time_range_end_timeslot_,
options) &&
AreAdditionalSectionsComplete(user_data.additional_values_, options);
AreAdditionalSectionsComplete(user_data.additional_values_, options) &&
IsValidUserModel(user_model, options);
}
// TODO(b/148448649): Move to dedicated helper namespace.
......
......@@ -17,6 +17,7 @@
#include "components/autofill/core/common/password_form.h"
#include "components/autofill_assistant/browser/actions/action.h"
#include "components/autofill_assistant/browser/user_data.h"
#include "components/autofill_assistant/browser/user_model.h"
#include "components/autofill_assistant/browser/website_login_fetcher.h"
namespace autofill_assistant {
......@@ -35,6 +36,7 @@ class CollectUserDataAction : public Action,
static bool IsUserDataComplete(
const UserData& user_data,
const UserModel& user_model,
const CollectUserDataOptions& collect_user_data_options);
// Ensures that |end| is > |start| by modifying either |start| or |end|,
......
......@@ -1311,7 +1311,7 @@ void Controller::UpdateCollectUserDataActions() {
}
bool confirm_button_enabled = CollectUserDataAction::IsUserDataComplete(
*user_data_, *collect_user_data_options_);
*user_data_, user_model_, *collect_user_data_options_);
UserAction confirm(collect_user_data_options_->confirm_action);
confirm.SetEnabled(confirm_button_enabled);
......@@ -1621,6 +1621,14 @@ void Controller::OnValueChanged(const std::string& identifier,
const ValueProto& new_value) {
event_handler_.DispatchEvent({EventProto::kOnValueChanged, identifier},
new_value);
// TODO(b/145043394) Remove this once chips are part of generic UI.
if (collect_user_data_options_ != nullptr &&
collect_user_data_options_->additional_model_identifier_to_check
.has_value() &&
identifier ==
*collect_user_data_options_->additional_model_identifier_to_check) {
UpdateCollectUserDataActions();
}
}
void Controller::OnTouchableAreaChanged(
......
......@@ -31,6 +31,7 @@ message CallbackProto {
SetModelValueProto set_value = 1;
ShowInfoPopupProto show_info_popup = 2;
ShowListPopupProto show_list_popup = 3;
ComputeValueProto compute_value = 4;
}
}
......@@ -61,6 +62,41 @@ message SetModelValueProto {
optional string model_identifier = 1;
}
// Computes a value and stores the result to |result_model_identifier|.
message ComputeValueProto {
oneof kind {
// Computes the logical AND of the specified model identifiers.
BooleanAndProto boolean_and = 2;
// Computes the logical OR of the specified model identifiers.
BooleanOrProto boolean_or = 3;
// Computes the logical NOT of the specified model identifiers.
BooleanNotProto boolean_not = 4;
}
// The model identifier to write the result to.
optional string result_model_identifier = 1;
}
// Performs a logical AND on all specified values.
message BooleanAndProto {
// The model identifiers to logically AND. All identifiers must point to
// boolean ValueProtos.
repeated string model_identifiers = 1;
}
// Performs a logical OR on all specified values.
message BooleanOrProto {
// The model identifiers to logically OR. All identifiers must point to
// boolean ValueProtos.
repeated string model_identifiers = 1;
}
// Performs a logical NOT on the specified value.
message BooleanNotProto {
// The model identifier to logically invert. Must point to a boolean value.
optional string model_identifier = 1;
}
// Displays a standard info popup.
message ShowInfoPopupProto {
optional InfoPopupProto info_popup = 1;
......
......@@ -1495,7 +1495,7 @@ message PopupListSectionProto {
// Asks to provide the data used by UseAddressAction and
// UseCreditCardAction.
// Next: 26
// Next: 27
message CollectUserDataProto {
enum TermsAndConditionsState {
// No choice has been made yet.
......@@ -1574,6 +1574,9 @@ message CollectUserDataProto {
optional GenericUserInterfaceProto generic_user_interface_appended = 25;
// Text to be shown in a separate info section.
optional string info_section_text = 24;
// Optional. If specified, the continue button will only be enabled if the
// boolean at this location is true (and everything else is valid too).
optional string additional_model_identifier_to_check = 26;
}
// Stop Autofill Assistant.
......
......@@ -193,6 +193,7 @@ struct CollectUserDataOptions {
std::vector<UserFormSectionProto> additional_appended_sections;
base::Optional<GenericUserInterfaceProto> generic_user_interface_prepended;
base::Optional<GenericUserInterfaceProto> generic_user_interface_appended;
base::Optional<std::string> additional_model_identifier_to_check;
base::OnceCallback<void(UserData*, const UserModel*)> confirm_callback;
base::OnceCallback<void(int)> additional_actions_callback;
......
......@@ -33,7 +33,8 @@ void UserModel::SetValue(const std::string& identifier,
}
}
base::Optional<ValueProto> UserModel::GetValue(const std::string& identifier) {
base::Optional<ValueProto> UserModel::GetValue(
const std::string& identifier) const {
auto it = values_.find(identifier);
if (it != values_.end()) {
return it->second;
......
......@@ -7,6 +7,7 @@
#include <map>
#include <string>
#include <vector>
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
......@@ -46,7 +47,24 @@ class UserModel {
bool force_notification = false);
// Returns the value for |identifier| or nullopt if there is no such value.
base::Optional<ValueProto> GetValue(const std::string& identifier);
base::Optional<ValueProto> GetValue(const std::string& identifier) const;
// Returns all specified values in a new std::vector. Returns nullopt if any
// of the requested values was not found. |model_identifiers| must be
// a std::string iterable.
template <class T>
base::Optional<std::vector<ValueProto>> GetValues(
const T& model_identifiers) {
std::vector<ValueProto> values;
for (const auto& identifier : model_identifiers) {
auto value = GetValue(identifier);
if (!value.has_value()) {
return base::nullopt;
}
values.emplace_back(*value);
}
return values;
}
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
......
......@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "components/autofill_assistant/browser/value_util.h"
#include <algorithm>
namespace autofill_assistant {
......@@ -114,4 +115,104 @@ std::ostream& operator<<(std::ostream& out,
return out;
}
// Convenience constructors.
ValueProto SimpleValue(bool b) {
ValueProto value;
value.mutable_booleans()->add_values(b);
return value;
}
ValueProto SimpleValue(const std::string& s) {
ValueProto value;
value.mutable_strings()->add_values(s);
return value;
}
ValueProto SimpleValue(int i) {
ValueProto value;
value.mutable_ints()->add_values(i);
return value;
}
bool AreAllValuesOfType(const std::vector<ValueProto>& values,
ValueProto::KindCase target_type) {
if (values.empty()) {
return false;
}
for (const auto& value : values) {
if (value.kind_case() != target_type) {
return false;
}
}
return true;
}
bool AreAllValuesOfSize(const std::vector<ValueProto>& values,
int target_size) {
if (values.empty()) {
return false;
}
for (const auto& value : values) {
switch (value.kind_case()) {
case ValueProto::kStrings:
if (value.strings().values_size() != target_size)
return false;
break;
case ValueProto::kBooleans:
if (value.booleans().values_size() != target_size)
return false;
break;
case ValueProto::kInts:
if (value.ints().values_size() != target_size)
return false;
break;
case ValueProto::KIND_NOT_SET:
if (target_size != 0) {
return false;
}
break;
}
}
return true;
}
base::Optional<ValueProto> CombineValues(
const std::vector<ValueProto>& values) {
if (values.empty()) {
return base::nullopt;
}
auto shared_type = values[0].kind_case();
if (!AreAllValuesOfType(values, shared_type)) {
return base::nullopt;
}
if (shared_type == ValueProto::KIND_NOT_SET) {
return ValueProto();
}
ValueProto result;
for (const auto& value : values) {
switch (shared_type) {
case ValueProto::kStrings:
std::for_each(
value.strings().values().begin(), value.strings().values().end(),
[&](auto& s) { result.mutable_strings()->add_values(s); });
break;
case ValueProto::kBooleans:
std::for_each(
value.booleans().values().begin(), value.booleans().values().end(),
[&](auto& b) { result.mutable_booleans()->add_values(b); });
break;
case ValueProto::kInts:
std::for_each(
value.ints().values().begin(), value.ints().values().end(),
[&](const auto& i) { result.mutable_ints()->add_values(i); });
break;
case ValueProto::KIND_NOT_SET:
NOTREACHED();
return base::nullopt;
}
}
return result;
}
} // namespace autofill_assistant
......@@ -6,6 +6,9 @@
#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_VALUE_UTIL_H_
#include <ostream>
#include <string>
#include <vector>
#include "base/optional.h"
#include "components/autofill_assistant/browser/model.pb.h"
namespace autofill_assistant {
......@@ -23,6 +26,23 @@ std::ostream& operator<<(std::ostream& out, const ValueProto& value);
std::ostream& operator<<(std::ostream& out,
const ModelProto::ModelValue& value);
// Convenience constructors.
ValueProto SimpleValue(bool value);
ValueProto SimpleValue(const std::string& value);
ValueProto SimpleValue(int value);
// Returns true if all |values| share the specified |target_type|.
bool AreAllValuesOfType(const std::vector<ValueProto>& values,
ValueProto::KindCase target_type);
// Returns true if all |values| share the specified |target_size|.
bool AreAllValuesOfSize(const std::vector<ValueProto>& values, int target_size);
// Combines all specified |values| in a single ValueProto where the individual
// value lists are appended after each other. Returns nullopt if |values| do not
// share the same type.
base::Optional<ValueProto> CombineValues(const std::vector<ValueProto>& values);
} // namespace autofill_assistant
#endif // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_VALUE_UTIL_H_
......@@ -120,5 +120,91 @@ TEST_F(ValueUtilTest, BoolComparison) {
EXPECT_TRUE(value_a == value_b);
}
TEST_F(ValueUtilTest, AreAllValuesOfType) {
ValueProto value_a;
ValueProto value_b;
ValueProto value_c;
EXPECT_TRUE(AreAllValuesOfType({value_a, value_b, value_c},
ValueProto::KIND_NOT_SET));
EXPECT_FALSE(
AreAllValuesOfType({value_a, value_b, value_c}, ValueProto::kStrings));
EXPECT_FALSE(
AreAllValuesOfType({value_a, value_b, value_c}, ValueProto::kBooleans));
EXPECT_FALSE(
AreAllValuesOfType({value_a, value_b, value_c}, ValueProto::kInts));
value_a = SimpleValue(std::string(""));
value_b = SimpleValue(std::string("non-empty"));
EXPECT_TRUE(AreAllValuesOfType({value_a, value_b}, ValueProto::kStrings));
EXPECT_FALSE(
AreAllValuesOfType({value_a, value_b, value_c}, ValueProto::kStrings));
value_c = CreateStringValue();
EXPECT_TRUE(
AreAllValuesOfType({value_a, value_b, value_c}, ValueProto::kStrings));
}
TEST_F(ValueUtilTest, AreAllValuesOfSize) {
// Not-set values have size 0.
ValueProto value_a;
ValueProto value_b;
ValueProto value_c;
EXPECT_TRUE(AreAllValuesOfSize({value_a, value_b, value_c}, 0));
value_a = SimpleValue(std::string(""));
value_b = SimpleValue(std::string("non-empty"));
EXPECT_TRUE(AreAllValuesOfSize({value_a, value_b}, 1));
value_c = SimpleValue(std::string("another"));
EXPECT_TRUE(AreAllValuesOfSize({value_a, value_b, value_c}, 1));
value_c.mutable_strings()->add_values(std::string("second value"));
EXPECT_FALSE(AreAllValuesOfSize({value_a, value_b, value_c}, 1));
value_a.mutable_strings()->add_values(std::string(""));
value_b.mutable_strings()->add_values(std::string("test"));
EXPECT_TRUE(AreAllValuesOfSize({value_a, value_b, value_c}, 2));
}
TEST_F(ValueUtilTest, CombineValues) {
ValueProto value_a;
ValueProto value_b;
ValueProto value_c;
EXPECT_THAT(CombineValues({value_a, value_b, value_c}), ValueProto());
value_a = SimpleValue(1);
EXPECT_THAT(CombineValues({value_a, value_b, value_c}), base::nullopt);
value_a.mutable_strings()->add_values("First");
value_a.mutable_strings()->add_values("Second");
value_b.mutable_strings();
value_c.mutable_strings()->add_values("Third");
ValueProto expected;
expected.mutable_strings()->add_values("First");
expected.mutable_strings()->add_values("Second");
expected.mutable_strings()->add_values("Third");
EXPECT_THAT(CombineValues({value_a, value_b, value_c}), expected);
value_a = ValueProto();
value_a.mutable_ints();
value_b = SimpleValue(1);
value_c = SimpleValue(2);
value_c.mutable_ints()->add_values(3);
expected.mutable_ints()->add_values(1);
expected.mutable_ints()->add_values(2);
expected.mutable_ints()->add_values(3);
EXPECT_THAT(CombineValues({value_a, value_b, value_c}), expected);
value_a = SimpleValue(false);
value_b = SimpleValue(true);
value_b.mutable_booleans()->add_values(false);
value_c = ValueProto();
value_c.mutable_booleans();
expected.mutable_booleans()->add_values(false);
expected.mutable_booleans()->add_values(true);
expected.mutable_booleans()->add_values(false);
EXPECT_THAT(CombineValues({value_a, value_b, value_c}), expected);
}
} // namespace value_util
} // namespace autofill_assistant
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