[Autofill Assistant] Added optional output for ShowListPopup.

This allows ShowListPopup to optionally also write the names of the
selected item(s), not just the indices. This is important for several
reasons. First, it allows the generic_ui to directly use those names for
other interactions, which currently is not possible because of the lack
of an interaction which extracts values by index. Second, in many cases
backend may not be interested in the selected indices at all. Matching
indices back to items is tedious and unnecessary.

Also moved the corresponding test from the CollectUserData action to the
dedicated ShowGenericUi action.

Bug: b/145043394
Change-Id: Iec9b3ff7ed00f2daba3fffbc125eba9b87741a23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2093439Reviewed-by: Sandro Maggi <sandromaggi@google.com>
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#749152}

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/generic_ui/AssistantGenericUiDelegate.java

@@ -4,6 +4,8 @@
 
 package org.chromium.chrome.browser.autofill_assistant.generic_ui;
 
+import android.support.annotation.Nullable;
+
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
@@ -28,11 +30,13 @@ public class AssistantGenericUiDelegate {
                 mNativeAssistantGenericUiDelegate, AssistantGenericUiDelegate.this, identifier);
     }
 
-    void onListPopupSelectionChanged(String identifier, AssistantValue value) {
+    void onListPopupSelectionChanged(String selectedIndicesIdentifier,
+            AssistantValue selectedIndices, @Nullable String selectedNamesIdentifier,
+            @Nullable AssistantValue selectedNames) {
         assert mNativeAssistantGenericUiDelegate != 0;
         AssistantGenericUiDelegateJni.get().onListPopupSelectionChanged(
-                mNativeAssistantGenericUiDelegate, AssistantGenericUiDelegate.this, identifier,
-                value);
+                mNativeAssistantGenericUiDelegate, AssistantGenericUiDelegate.this,
+                selectedIndicesIdentifier, selectedIndices, selectedNamesIdentifier, selectedNames);
     }
 
     void onCalendarPopupDateChanged(String identifier, AssistantValue value) {
@@ -52,7 +56,9 @@ public class AssistantGenericUiDelegate {
         void onViewClicked(long nativeAssistantGenericUiDelegate, AssistantGenericUiDelegate caller,
                 String identifier);
         void onListPopupSelectionChanged(long nativeAssistantGenericUiDelegate,
-                AssistantGenericUiDelegate caller, String identifier, AssistantValue value);
+                AssistantGenericUiDelegate caller, String selectedIndicesIdentifier,
+                AssistantValue selectedIndices, @Nullable String selectedNamesIdentifier,
+                @Nullable AssistantValue selectedNames);
         void onCalendarPopupDateChanged(long nativeAssistantGenericUiDelegate,
                 AssistantGenericUiDelegate caller, String identifier, AssistantValue value);
     }

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/generic_ui/AssistantViewInteractions.java

@@ -34,18 +34,26 @@ public class AssistantViewInteractions {
     @CalledByNative
     private static void showListPopup(Context context, String[] itemNames,
             @PopupItemType int[] itemTypes, int[] selectedItems, boolean multiple,
-            String identifier, AssistantGenericUiDelegate delegate) {
+            String selectedIndicesIdentifier, @Nullable String selectedNamesIdentifier,
+            AssistantGenericUiDelegate delegate) {
         assert (itemNames.length == itemTypes.length);
         List<SelectPopupItem> popupItems = new ArrayList<>();
         for (int i = 0; i < itemNames.length; i++) {
             popupItems.add(new SelectPopupItem(itemNames[i], itemTypes[i]));
         }
 
-        SelectPopupDialog dialog = new SelectPopupDialog(context,
-                (indices)
-                        -> delegate.onListPopupSelectionChanged(
-                                identifier, new AssistantValue(indices)),
-                popupItems, multiple, selectedItems);
+        SelectPopupDialog dialog = new SelectPopupDialog(context, (indices) -> {
+            AssistantValue selectedNamesValue = null;
+            if (selectedNamesIdentifier != null) {
+                String[] selectedNames = new String[indices != null ? indices.length : 0];
+                for (int i = 0; i < selectedNames.length; ++i) {
+                    selectedNames[i] = itemNames[indices[i]];
+                }
+                selectedNamesValue = new AssistantValue(selectedNames);
+            }
+            delegate.onListPopupSelectionChanged(selectedIndicesIdentifier,
+                    new AssistantValue(indices), selectedNamesIdentifier, selectedNamesValue);
+        }, popupItems, multiple, selectedItems);
         dialog.show();
     }
 

chrome/browser/android/autofill_assistant/assistant_collect_user_data_delegate.cc

@@ -20,19 +20,6 @@
 using base::android::AttachCurrentThread;
 using base::android::JavaParamRef;
 
-namespace {
-// Converts a java string to native. Returns an empty string if input is null.
-std::string SafeConvertJavaStringToNative(
-    JNIEnv* env,
-    const base::android::JavaParamRef<jstring>& jstring) {
-  std::string native_string;
-  if (jstring) {
-    base::android::ConvertJavaStringToUTF8(env, jstring, &native_string);
-  }
-  return native_string;
-}
-}  // namespace
-
 namespace autofill_assistant {
 
 AssistantCollectUserDataDelegate::AssistantCollectUserDataDelegate(
@@ -59,9 +46,14 @@ void AssistantCollectUserDataDelegate::OnContactInfoChanged(
     return;
   }
 
-  std::string name = SafeConvertJavaStringToNative(env, jpayer_name);
-  std::string phone = SafeConvertJavaStringToNative(env, jpayer_phone);
-  std::string email = SafeConvertJavaStringToNative(env, jpayer_email);
+  std::string name = ui_controller_android_utils::SafeConvertJavaStringToNative(
+      env, jpayer_name);
+  std::string phone =
+      ui_controller_android_utils::SafeConvertJavaStringToNative(env,
+                                                                 jpayer_phone);
+  std::string email =
+      ui_controller_android_utils::SafeConvertJavaStringToNative(env,
+                                                                 jpayer_email);
 
   auto contact_profile = std::make_unique<autofill::AutofillProfile>();
   contact_profile->SetRawInfo(autofill::ServerFieldType::NAME_FULL,
@@ -139,7 +131,9 @@ void AssistantCollectUserDataDelegate::OnLoginChoiceChanged(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& jcaller,
     const base::android::JavaParamRef<jstring>& jidentifier) {
-  std::string identifier = SafeConvertJavaStringToNative(env, jidentifier);
+  std::string identifier =
+      ui_controller_android_utils::SafeConvertJavaStringToNative(env,
+                                                                 jidentifier);
   ui_controller_->OnLoginChoiceChanged(identifier);
 }
 
@@ -205,7 +199,7 @@ void AssistantCollectUserDataDelegate::OnKeyValueChanged(
     const base::android::JavaParamRef<jstring>& jkey,
     const base::android::JavaParamRef<jobject>& jvalue) {
   ui_controller_->OnKeyValueChanged(
-      SafeConvertJavaStringToNative(env, jkey),
+      ui_controller_android_utils::SafeConvertJavaStringToNative(env, jkey),
       ui_controller_android_utils::ToNativeValue(env, jvalue));
 }
 

chrome/browser/android/autofill_assistant/assistant_generic_ui_delegate.cc

@@ -42,19 +42,20 @@ void AssistantGenericUiDelegate::OnViewClicked(
 void AssistantGenericUiDelegate::OnListPopupSelectionChanged(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& jcaller,
-    const base::android::JavaParamRef<jstring>& jmodel_identifier,
-    const base::android::JavaParamRef<jobject>& jvalue) {
-  std::string identifier;
-  if (jmodel_identifier) {
-    base::android::ConvertJavaStringToUTF8(env, jmodel_identifier, &identifier);
-  }
-
-  ValueProto value;
-  if (jvalue) {
-    value = ui_controller_android_utils::ToNativeValue(env, jvalue);
+    const base::android::JavaParamRef<jstring>& jindices_model_identifier,
+    const base::android::JavaParamRef<jobject>& jindicies_value,
+    const base::android::JavaParamRef<jstring>& jnames_model_identifier,
+    const base::android::JavaParamRef<jobject>& jnames_value) {
+  ui_controller_->OnValueChanged(
+      ui_controller_android_utils::SafeConvertJavaStringToNative(
+          env, jindices_model_identifier),
+      ui_controller_android_utils::ToNativeValue(env, jindicies_value));
+  if (jnames_model_identifier) {
+    ui_controller_->OnValueChanged(
+        ui_controller_android_utils::SafeConvertJavaStringToNative(
+            env, jnames_model_identifier),
+        ui_controller_android_utils::ToNativeValue(env, jnames_value));
   }
-
-  ui_controller_->OnValueChanged(identifier, value);
 }
 
 // TODO(b/145043394): refactor delegate methods into a single SetValue() where

chrome/browser/android/autofill_assistant/assistant_generic_ui_delegate.h

@@ -23,14 +23,17 @@ class AssistantGenericUiDelegate {
       const base::android::JavaParamRef<jobject>& jcaller,
       const base::android::JavaParamRef<jstring>& jview_identifier);
 
-  // The selection in a list popup has changed. |jmodel_identifier| is the model
-  // identifier that the new selection indices should be written to. |jvalue| is
-  // a Java array of integers containing the newly selected indices.
+  // The selection in a list popup has changed. |jindices_model_identifier| is
+  // the model identifier that |jindicies_value| should be written to.
+  // |jnames_model_identifier| is the model identifier that |jnames_value|
+  // should be written to, if specified.
   void OnListPopupSelectionChanged(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& jcaller,
-      const base::android::JavaParamRef<jstring>& jmodel_identifier,
-      const base::android::JavaParamRef<jobject>& jvalue);
+      const base::android::JavaParamRef<jstring>& jindices_model_identifier,
+      const base::android::JavaParamRef<jobject>& jindicies_value,
+      const base::android::JavaParamRef<jstring>& jnames_model_identifier,
+      const base::android::JavaParamRef<jobject>& jnames_value);
 
   // The date in a calendar popup has changed. |jmodel_identifier| is the model
   // identifier that the new date should be written to. |jvalue| is a Java

chrome/browser/android/autofill_assistant/interaction_handler_android.cc

@@ -129,9 +129,6 @@ void ShowListPopup(base::WeakPtr<UserModel> user_model,
   }
 
   JNIEnv* env = base::android::AttachCurrentThread();
-  auto jidentifier = base::android::ConvertUTF8ToJavaString(
-      env, proto.selected_item_indices_model_identifier());
-
   std::vector<std::string> item_names_vec;
   std::copy(item_names->strings().values().begin(),
             item_names->strings().values().end(),
@@ -151,7 +148,14 @@ void ShowListPopup(base::WeakPtr<UserModel> user_model,
       env, jcontext, base::android::ToJavaArrayOfStrings(env, item_names_vec),
       base::android::ToJavaIntArray(env, item_types_vec),
       base::android::ToJavaIntArray(env, selected_indices_vec),
-      proto.allow_multiselect(), jidentifier, jdelegate);
+      proto.allow_multiselect(),
+      base::android::ConvertUTF8ToJavaString(
+          env, proto.selected_item_indices_model_identifier()),
+      proto.selected_item_names_model_identifier().empty()
+          ? nullptr
+          : base::android::ConvertUTF8ToJavaString(
+                env, proto.selected_item_names_model_identifier()),
+      jdelegate);
 }
 
 void ShowCalendarPopup(base::WeakPtr<UserModel> user_model,

chrome/browser/android/autofill_assistant/ui_controller_android_utils.cc

@@ -146,6 +146,9 @@ base::android::ScopedJavaLocalRef<jobject> ToJavaValue(
 ValueProto ToNativeValue(JNIEnv* env,
                          const base::android::JavaParamRef<jobject>& jvalue) {
   ValueProto proto;
+  if (!jvalue) {
+    return proto;
+  }
   auto jints = Java_AssistantValue_getIntegers(env, jvalue);
   if (jints) {
     auto* mutable_ints = proto.mutable_ints();
@@ -261,6 +264,16 @@ void ShowJavaInfoPopup(JNIEnv* env,
   Java_AssistantInfoPopup_show(env, jinfo_popup, jcontext);
 }
 
+std::string SafeConvertJavaStringToNative(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jstring>& jstring) {
+  std::string native_string;
+  if (jstring) {
+    base::android::ConvertJavaStringToUTF8(env, jstring, &native_string);
+  }
+  return native_string;
+}
+
 }  // namespace ui_controller_android_utils
 
 }  // namespace autofill_assistant

chrome/browser/android/autofill_assistant/ui_controller_android_utils.h

@@ -51,7 +51,8 @@ int GetPixelSizeOrDefault(
 base::android::ScopedJavaLocalRef<jobject> ToJavaValue(JNIEnv* env,
                                                        const ValueProto& proto);
 
-// Returns the native equivalent of |jvalue|.
+// Returns the native equivalent of |jvalue|. Returns an empty ValueProto if
+// |jvalue| is null.
 ValueProto ToNativeValue(JNIEnv* env,
                          const base::android::JavaParamRef<jobject>& jvalue);
 
@@ -65,6 +66,11 @@ void ShowJavaInfoPopup(JNIEnv* env,
                        base::android::ScopedJavaLocalRef<jobject> jinfo_popup,
                        base::android::ScopedJavaLocalRef<jobject> jcontext);
 
+// Converts a java string to native. Returns an empty string if input is null.
+std::string SafeConvertJavaStringToNative(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jstring>& jstring);
+
 }  // namespace ui_controller_android_utils
 
 }  //  namespace autofill_assistant

components/autofill_assistant/browser/interactions.proto

@@ -134,6 +134,9 @@ message ShowListPopupProto {
   optional string selected_item_indices_model_identifier = 3;
   // Whether to allow the selection of multiple items or not.
   optional bool allow_multiselect = 4;
+  // Optional output model identifier to store the names of the selected items
+  // in a StringList.
+  optional string selected_item_names_model_identifier = 5;
 }
 
 // Sets the list of available user actions. User actions are either direct