Commit f83e73d6 authored by Clemens Arbesser's avatar Clemens Arbesser Committed by Commit Bot

[Autofill Assistant] Added support for implicit interactions.

Implicit interactions are interactions that are defined as part of a
view definition, and not explicitly in the interactions. They are useful
because they reduce the complexity of the proto. Furthermore, they allow
the client to hide certain implementation details from the proto, as
will be required in an upcoming change for radio buttons.

As a first proof-of-concept, this CL changes how the text input view
works. Before this CL, the model identifier in the proto was only used
for the output of the view (i.e., java->native). The input of the view
had to be explicitly defined via an interaction. Now, this is done
implicitly, such that the text input view will effectively sync its
contents with a specific model identifier (no explicit interaction
necessary).

Bug: b/145043394
Change-Id: Ibc506fbbd8d0a60927429638e41d3a8f4ee9934a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2120580
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: default avatarMarian Fechete <marianfe@google.com>
Cr-Commit-Position: refs/heads/master@{#753659}
parent e2354e62
...@@ -1524,16 +1524,6 @@ public class AutofillAssistantGenericUiTest { ...@@ -1524,16 +1524,6 @@ public class AutofillAssistantGenericUiTest {
.build()); .build());
List<InteractionProto> interactions = new ArrayList<>(); List<InteractionProto> interactions = new ArrayList<>();
interactions.add(
(InteractionProto) InteractionProto.newBuilder()
.setTriggerEvent(EventProto.newBuilder().setOnValueChanged(
OnModelValueChangedEventProto.newBuilder().setModelIdentifier(
"text_value")))
.addCallbacks(CallbackProto.newBuilder().setSetText(
SetTextProto.newBuilder()
.setViewIdentifier("text_view")
.setModelIdentifier("text_value")))
.build());
interactions.add( interactions.add(
(InteractionProto) InteractionProto.newBuilder() (InteractionProto) InteractionProto.newBuilder()
.setTriggerEvent(EventProto.newBuilder().setOnValueChanged( .setTriggerEvent(EventProto.newBuilder().setOnValueChanged(
......
...@@ -18,7 +18,7 @@ namespace autofill_assistant { ...@@ -18,7 +18,7 @@ namespace autofill_assistant {
namespace { namespace {
// Forward declaration to allow recursive calls. // Forward declarations to allow recursive calls.
base::android::ScopedJavaGlobalRef<jobject> CreateJavaView( base::android::ScopedJavaGlobalRef<jobject> CreateJavaView(
JNIEnv* env, JNIEnv* env,
const base::android::ScopedJavaLocalRef<jobject>& jcontext, const base::android::ScopedJavaLocalRef<jobject>& jcontext,
...@@ -26,6 +26,12 @@ base::android::ScopedJavaGlobalRef<jobject> CreateJavaView( ...@@ -26,6 +26,12 @@ base::android::ScopedJavaGlobalRef<jobject> CreateJavaView(
const ViewProto& proto, const ViewProto& proto,
std::map<std::string, base::android::ScopedJavaGlobalRef<jobject>>* views); std::map<std::string, base::android::ScopedJavaGlobalRef<jobject>>* views);
// Creates all implicit interactions for the views defined in |proto|. Returns
// true on success, false on failure.
bool CreateImplicitInteractionsForView(
const ViewProto& proto,
InteractionHandlerAndroid* interaction_handler);
base::android::ScopedJavaLocalRef<jobject> CreateJavaDrawable( base::android::ScopedJavaLocalRef<jobject> CreateJavaDrawable(
JNIEnv* env, JNIEnv* env,
const base::android::ScopedJavaLocalRef<jobject>& jcontext, const base::android::ScopedJavaLocalRef<jobject>& jcontext,
...@@ -210,9 +216,10 @@ base::android::ScopedJavaGlobalRef<jobject> CreateJavaView( ...@@ -210,9 +216,10 @@ base::android::ScopedJavaGlobalRef<jobject> CreateJavaView(
proto.vertical_expander_view(), views); proto.vertical_expander_view(), views);
break; break;
} }
case ViewProto::kTextInputView: case ViewProto::kTextInputView: {
if (proto.text_input_view().model_identifier().empty()) { if (proto.text_input_view().model_identifier().empty()) {
VLOG(1) << "Failed to create text input view: model_identifier not set"; VLOG(1) << "Failed to create text input view '" << proto.identifier()
<< "': model_identifier not set";
return nullptr; return nullptr;
} }
jview = Java_AssistantViewFactory_createTextInputView( jview = Java_AssistantViewFactory_createTextInputView(
...@@ -223,6 +230,7 @@ base::android::ScopedJavaGlobalRef<jobject> CreateJavaView( ...@@ -223,6 +230,7 @@ base::android::ScopedJavaGlobalRef<jobject> CreateJavaView(
base::android::ConvertUTF8ToJavaString( base::android::ConvertUTF8ToJavaString(
env, proto.text_input_view().model_identifier())); env, proto.text_input_view().model_identifier()));
break; break;
}
case ViewProto::VIEW_NOT_SET: case ViewProto::VIEW_NOT_SET:
NOTREACHED(); NOTREACHED();
return nullptr; return nullptr;
...@@ -277,6 +285,72 @@ base::android::ScopedJavaGlobalRef<jobject> CreateJavaView( ...@@ -277,6 +285,72 @@ base::android::ScopedJavaGlobalRef<jobject> CreateJavaView(
return jview_global_ref; return jview_global_ref;
} }
bool CreateImplicitInteractionsForView(
const ViewProto& proto,
InteractionHandlerAndroid* interaction_handler) {
switch (proto.view_case()) {
case ViewProto::kTextInputView: {
// Auto-update the text of the view whenever the corresponding value in
// the model changes.
InteractionProto implicit_set_text_interaction;
implicit_set_text_interaction.mutable_trigger_event()
->mutable_on_value_changed()
->set_model_identifier(proto.text_input_view().model_identifier());
SetTextProto set_text_callback;
set_text_callback.set_model_identifier(
proto.text_input_view().model_identifier());
set_text_callback.set_view_identifier(proto.identifier());
*implicit_set_text_interaction.add_callbacks()->mutable_set_text() =
set_text_callback;
if (!interaction_handler->AddInteractionsFromProto(
implicit_set_text_interaction)) {
VLOG(1) << "Failed to create implicit SetText interaction for "
<< proto.identifier();
return false;
}
break;
}
case ViewProto::kVerticalExpanderView:
if (proto.vertical_expander_view().has_title_view() &&
!CreateImplicitInteractionsForView(
proto.vertical_expander_view().title_view(),
interaction_handler)) {
return false;
}
if (proto.vertical_expander_view().has_collapsed_view() &&
!CreateImplicitInteractionsForView(
proto.vertical_expander_view().collapsed_view(),
interaction_handler)) {
return false;
}
if (proto.vertical_expander_view().has_expanded_view() &&
!CreateImplicitInteractionsForView(
proto.vertical_expander_view().expanded_view(),
interaction_handler)) {
return false;
}
break;
case ViewProto::kViewContainer:
for (const auto& child : proto.view_container().views()) {
if (!CreateImplicitInteractionsForView(child, interaction_handler)) {
return false;
}
}
break;
case ViewProto::kTextView:
case ViewProto::kDividerView:
case ViewProto::kImageView:
// Nothing to do, no implicit interactions necessary.
break;
case ViewProto::VIEW_NOT_SET:
NOTREACHED();
return false;
}
return true;
}
} // namespace } // namespace
GenericUiControllerAndroid::GenericUiControllerAndroid( GenericUiControllerAndroid::GenericUiControllerAndroid(
...@@ -303,9 +377,9 @@ GenericUiControllerAndroid::CreateFromProto( ...@@ -303,9 +377,9 @@ GenericUiControllerAndroid::CreateFromProto(
UserModel* user_model, UserModel* user_model,
BasicInteractions* basic_interactions) { BasicInteractions* basic_interactions) {
// Create view layout. // Create view layout.
JNIEnv* env = base::android::AttachCurrentThread();
auto views = std::make_unique< auto views = std::make_unique<
std::map<std::string, base::android::ScopedJavaGlobalRef<jobject>>>(); std::map<std::string, base::android::ScopedJavaGlobalRef<jobject>>>();
JNIEnv* env = base::android::AttachCurrentThread();
auto jroot_view = auto jroot_view =
proto.has_root_view() proto.has_root_view()
? CreateJavaView(env, ? CreateJavaView(env,
...@@ -313,14 +387,23 @@ GenericUiControllerAndroid::CreateFromProto( ...@@ -313,14 +387,23 @@ GenericUiControllerAndroid::CreateFromProto(
jdelegate, proto.root_view(), views.get()) jdelegate, proto.root_view(), views.get())
: nullptr; : nullptr;
// Create proto interactions (i.e., native -> java). // Create implicit interactions.
auto interaction_handler = std::make_unique<InteractionHandlerAndroid>( auto interaction_handler = std::make_unique<InteractionHandlerAndroid>(
event_handler, user_model, basic_interactions, views.get(), jcontext, event_handler, user_model, basic_interactions, views.get(), jcontext,
jdelegate); jdelegate);
if (!interaction_handler->AddInteractionsFromProto(proto.interactions())) { if (proto.has_root_view() &&
!CreateImplicitInteractionsForView(proto.root_view(),
interaction_handler.get())) {
return nullptr; return nullptr;
} }
// Create proto interactions (i.e., native -> java).
for (const auto& interaction : proto.interactions().interactions()) {
if (!interaction_handler->AddInteractionsFromProto(interaction)) {
return nullptr;
}
}
// Create java listeners (i.e., java -> native). // Create java listeners (i.e., java -> native).
if (!android_events::CreateJavaListenersFromProto(env, views.get(), jdelegate, if (!android_events::CreateJavaListenersFromProto(env, views.get(), jdelegate,
proto.interactions())) { proto.interactions())) {
......
...@@ -36,7 +36,6 @@ void TryRunConditionalCallback( ...@@ -36,7 +36,6 @@ void TryRunConditionalCallback(
base::Optional<EventHandler::EventKey> CreateEventKeyFromProto( base::Optional<EventHandler::EventKey> CreateEventKeyFromProto(
const EventProto& proto, const EventProto& proto,
JNIEnv* env,
std::map<std::string, base::android::ScopedJavaGlobalRef<jobject>>* views, std::map<std::string, base::android::ScopedJavaGlobalRef<jobject>>* views,
base::android::ScopedJavaGlobalRef<jobject> jdelegate) { base::android::ScopedJavaGlobalRef<jobject> jdelegate) {
switch (proto.kind_case()) { switch (proto.kind_case()) {
...@@ -243,39 +242,33 @@ void InteractionHandlerAndroid::StopListening() { ...@@ -243,39 +242,33 @@ void InteractionHandlerAndroid::StopListening() {
} }
bool InteractionHandlerAndroid::AddInteractionsFromProto( bool InteractionHandlerAndroid::AddInteractionsFromProto(
const InteractionsProto& proto) { const InteractionProto& proto) {
if (is_listening_) { if (is_listening_) {
NOTREACHED() << "Interactions can not be added while listening to events!"; NOTREACHED() << "Interactions can not be added while listening to events!";
return false; return false;
} }
JNIEnv* env = base::android::AttachCurrentThread(); auto key = CreateEventKeyFromProto(proto.trigger_event(), views_, jdelegate_);
for (const auto& interaction_proto : proto.interactions()) { if (!key) {
auto key = CreateEventKeyFromProto(interaction_proto.trigger_event(), env, VLOG(1) << "Invalid trigger event for interaction";
views_, jdelegate_); return false;
if (!key) { }
VLOG(1) << "Invalid trigger event for interaction";
for (const auto& callback_proto : proto.callbacks()) {
auto callback = CreateInteractionCallbackFromProto(
callback_proto, user_model_, basic_interactions_, views_, jcontext_,
jdelegate_);
if (!callback) {
VLOG(1) << "Invalid callback for interaction";
return false; return false;
} }
// Wrap callback in condition handler if necessary.
for (const auto& callback_proto : interaction_proto.callbacks()) { if (callback_proto.has_condition_model_identifier()) {
auto callback = CreateInteractionCallbackFromProto( callback = base::Optional<InteractionHandlerAndroid::InteractionCallback>(
callback_proto, user_model_, basic_interactions_, views_, jcontext_, base::BindRepeating(
jdelegate_); &TryRunConditionalCallback, basic_interactions_->GetWeakPtr(),
if (!callback) { callback_proto.condition_model_identifier(), *callback));
VLOG(1) << "Invalid callback for interaction";
return false;
}
// Wrap callback in condition handler if necessary.
if (callback_proto.has_condition_model_identifier()) {
callback =
base::Optional<InteractionHandlerAndroid::InteractionCallback>(
base::BindRepeating(&TryRunConditionalCallback,
basic_interactions_->GetWeakPtr(),
callback_proto.condition_model_identifier(),
*callback));
}
AddInteraction(*key, *callback);
} }
AddInteraction(*key, *callback);
} }
return true; return true;
} }
......
...@@ -46,9 +46,9 @@ class InteractionHandlerAndroid : public EventHandler::Observer { ...@@ -46,9 +46,9 @@ class InteractionHandlerAndroid : public EventHandler::Observer {
void StartListening(); void StartListening();
void StopListening(); void StopListening();
// Creates callbacks for each interaction in |proto|. Returns false if |proto| // Creates interaction callbacks as specified by |proto|. Returns false if
// is invalid. // |proto| is invalid.
bool AddInteractionsFromProto(const InteractionsProto& proto); bool AddInteractionsFromProto(const InteractionProto& proto);
// Overrides autofill_assistant::EventHandler::Observer. // Overrides autofill_assistant::EventHandler::Observer.
void OnEvent(const EventHandler::EventKey& key) override; void OnEvent(const EventHandler::EventKey& key) override;
......
...@@ -226,7 +226,8 @@ message TextInputViewProto { ...@@ -226,7 +226,8 @@ message TextInputViewProto {
// Note that the actual impact is version-dependent and not all hints may be // Note that the actual impact is version-dependent and not all hints may be
// supported, in which case a regular text input field will be displayed. // supported, in which case a regular text input field will be displayed.
optional InputTypeHint type = 2; optional InputTypeHint type = 2;
// The model identifier to write to. // Both input and output. The model identifier containing this view's text.
// Should point to a single string value.
optional string model_identifier = 3; optional string model_identifier = 3;
} }
......
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