[Autofill Assistant] Make the definition of the PromptProto nicer.

This change updates the protocol used to control the PromptAction to
make it more powerful and more in-line with the current protocol:

 - the suggestion and element existence results are merged into one list
 of choice.

 - the string result is replaced by an opaque server_payload result,
 which allows the server to include whatever data it wants.

To make that work, this changes forwards the data proto as an opaque
byte array between layers instead of a single UTF8 string that was used
for both message and result.

Bug: 1343266
Change-Id: I601c31504bd231ef9904bced92fc350a939b6f9d
Reviewed-on: https://chromium-review.googlesource.com/c/1346461
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610576}

chrome/android/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiController.java

@@ -23,7 +23,6 @@ import org.chromium.content_public.browser.WebContents;
 import org.chromium.payments.mojom.PaymentOptions;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collections;
 import java.util.Date;
@@ -137,8 +136,8 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
     }
 
     @Override
-    public void onSuggestionSelected(String suggestion) {
-        nativeOnSuggestionSelected(mUiControllerAndroid, suggestion);
+    public void onChoice(byte[] serverPayload) {
+        nativeOnChoice(mUiControllerAndroid, serverPayload);
     }
 
     @Override
@@ -231,13 +230,17 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
     }
 
     @CalledByNative
-    private void onChoose(String[] suggestions) {
-        // An empty suggestion list is supported. Selection can still be forced. onForceChoose
-        // should be a no-op in this case.
-        if (suggestions.length == 0) return;
-
-        mUiDelegateHolder.performUiOperation(
-                uiDelegate -> uiDelegate.showSuggestions(Arrays.asList(suggestions)));
+    private void onChoose(String[] names, byte[][] serverPayloads) {
+        // An empty choice list is supported, as selection can still be forced. onForceChoose should
+        // be a no-op in this case.
+        if (names.length == 0) return;
+
+        List<AutofillAssistantUiDelegate.Choice> choices = new ArrayList<>();
+        assert (names.length == serverPayloads.length);
+        for (int i = 0; i < names.length; i++) {
+            choices.add(new AutofillAssistantUiDelegate.Choice(names[i], serverPayloads[i]));
+        }
+        mUiDelegateHolder.performUiOperation(uiDelegate -> uiDelegate.showChoices(choices));
     }
 
     @CalledByNative
@@ -470,8 +473,7 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
             long nativeUiControllerAndroid, float distanceXRatio, float distanceYRatio);
     private native void nativeUpdateTouchableArea(long nativeUiControllerAndroid);
     private native void nativeOnScriptSelected(long nativeUiControllerAndroid, String scriptPath);
-    private native void nativeOnSuggestionSelected(
-            long nativeUiControllerAndroid, String selection);
+    private native void nativeOnChoice(long nativeUiControllerAndroid, byte[] serverPayload);
     private native void nativeOnAddressSelected(long nativeUiControllerAndroid, String guid);
     private native void nativeOnCardSelected(long nativeUiControllerAndroid, String guid);
     private native void nativeOnShowDetails(long nativeUiControllerAndroid, boolean canContinue);

chrome/android/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiDelegate.java

@@ -132,8 +132,8 @@ class AutofillAssistantUiDelegate {
          */
         void onScriptSelected(String scriptPath);
 
-        /** Called when a suggestion has been selected. */
-        void onSuggestionSelected(String suggestion);
+        /** Called when a choice has been selected, with the result of {@link Choice#getData()}. */
+        void onChoice(byte[] serverPayload);
 
         /**
          * Called when an address has been selected.
@@ -214,6 +214,27 @@ class AutofillAssistantUiDelegate {
         }
     }
 
+    /** A choice to pass to {@link AutofillAssistantUiDelegate#onChoose}. */
+    static class Choice {
+        private final String mName;
+        private final byte[] mServerPayload;
+
+        /** Returns the localized name to display. */
+        String getName() {
+            return mName;
+        }
+
+        /** Returns the server payload associated with this choice, to pass to the callback. */
+        byte[] getServerPayload() {
+            return mServerPayload;
+        }
+
+        Choice(String name, byte[] serverPayload) {
+            mName = name;
+            mServerPayload = serverPayload;
+        }
+    }
+
     // Names borrowed from :
     // - https://guidelines.googleplex.com/googlematerial/components/chips.html
     // - https://guidelines.googleplex.com/googlematerial/components/buttons.html
@@ -664,14 +685,14 @@ class AutofillAssistantUiDelegate {
         mTouchEventFilter.setPartialOverlay(enabled, boxes);
     }
 
-    /** Shows chip with the given suggestions. */
-    public void showSuggestions(List<String> suggestions) {
+    /** Shows chip with the given choices. */
+    public void showChoices(List<Choice> choices) {
         List<View> childViews = new ArrayList<>();
-        for (String suggestion : suggestions) {
-            TextView chipView = createChipView(suggestion, ChipStyle.CHIP_ASSISTIVE);
+        for (Choice choice : choices) {
+            TextView chipView = createChipView(choice.getName(), ChipStyle.CHIP_ASSISTIVE);
             chipView.setOnClickListener(unusedView -> {
                 clearCarousel();
-                mClient.onSuggestionSelected(suggestion);
+                mClient.onChoice(choice.getServerPayload());
             });
             childViews.add(chipView);
         }

chrome/browser/android/autofill_assistant/ui_controller_android.cc

@@ -180,16 +180,16 @@ void UiControllerAndroid::OnScriptSelected(
   ui_delegate_->OnScriptSelected(script_path);
 }
 
-void UiControllerAndroid::OnSuggestionSelected(
+void UiControllerAndroid::OnChoice(
     JNIEnv* env,
     const JavaParamRef<jobject>& jcaller,
-    const JavaParamRef<jstring>& jsuggestion) {
+    const JavaParamRef<jbyteArray>& jserver_payload) {
   if (!choice_callback_)  // possibly duplicate call
     return;
 
-  std::string suggestion;
-  base::android::ConvertJavaStringToUTF8(env, jsuggestion, &suggestion);
-  std::move(choice_callback_).Run(suggestion);
+  std::string server_payload;
+  base::android::JavaByteArrayToString(env, jserver_payload, &server_payload);
+  std::move(choice_callback_).Run(server_payload);
 }
 
 void UiControllerAndroid::OnAddressSelected(
@@ -307,15 +307,22 @@ UiControllerAndroid::OnRequestDebugContext(
 }
 
 void UiControllerAndroid::Choose(
-    const std::vector<std::string>& suggestions,
+    const std::vector<UiController::Choice>& choices,
     base::OnceCallback<void(const std::string&)> callback) {
   DCHECK(!choice_callback_);
   choice_callback_ = std::move(callback);
 
+  std::vector<std::string> names;
+  std::vector<std::string> server_payload;
+  for (const auto& choice : choices) {
+    names.emplace_back(choice.name);
+    server_payload.emplace_back(choice.server_payload);
+  }
   JNIEnv* env = AttachCurrentThread();
   Java_AutofillAssistantUiController_onChoose(
       env, java_autofill_assistant_ui_controller_,
-      base::android::ToJavaArrayOfStrings(env, suggestions));
+      base::android::ToJavaArrayOfStrings(env, names),
+      base::android::ToJavaArrayOfByteArray(env, server_payload));
 }
 
 void UiControllerAndroid::ForceChoose(const std::string& result) {

chrome/browser/android/autofill_assistant/ui_controller_android.h

@@ -44,7 +44,7 @@ class UiControllerAndroid : public UiController,
   void ShutdownGracefully() override;
   void CloseCustomTab() override;
   void UpdateScripts(const std::vector<ScriptHandle>& scripts) override;
-  void Choose(const std::vector<std::string>& suggestions,
+  void Choose(const std::vector<UiController::Choice>& choices,
               base::OnceCallback<void(const std::string&)> callback) override;
   void ForceChoose(const std::string& result) override;
   void ChooseAddress(
@@ -92,10 +92,9 @@ class UiControllerAndroid : public UiController,
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& jcaller,
       const base::android::JavaParamRef<jstring>& jscript_path);
-  void OnSuggestionSelected(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& jcaller,
-      const base::android::JavaParamRef<jstring>& jsuggestion);
+  void OnChoice(JNIEnv* env,
+                const base::android::JavaParamRef<jobject>& jcaller,
+                const base::android::JavaParamRef<jbyteArray>& jserver_payload);
   void OnAddressSelected(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& jcaller,

components/autofill_assistant/browser/actions/action_delegate.h

@@ -12,6 +12,7 @@
 #include "base/callback_forward.h"
 #include "components/autofill_assistant/browser/batch_element_checker.h"
 #include "components/autofill_assistant/browser/selector.h"
+#include "components/autofill_assistant/browser/ui_controller.h"
 #include "third_party/blink/public/mojom/payments/payment_request.mojom.h"
 
 class GURL;
@@ -79,7 +80,7 @@ class ActionDelegate {
   // allowing access to the touchable elements set previously, in the same
   // script.
   virtual void Choose(
-      const std::vector<std::string>& suggestions,
+      const std::vector<UiController::Choice>& choices,
       base::OnceCallback<void(const std::string&)> callback) = 0;
 
   // Cancels a choose action in progress and pass the given result to the

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -51,13 +51,13 @@ class MockActionDelegate : public ActionDelegate {
                void(const Selector& selector,
                     base::OnceCallback<void(bool)> callback));
 
-  void Choose(const std::vector<std::string>& suggestions,
+  void Choose(const std::vector<UiController::Choice>& choices,
               base::OnceCallback<void(const std::string&)> callback) override {
-    OnChoose(suggestions, callback);
+    OnChoose(choices, callback);
   }
 
   MOCK_METHOD2(OnChoose,
-               void(const std::vector<std::string>& suggestions,
+               void(const std::vector<UiController::Choice>& choice,
                     base::OnceCallback<void(const std::string&)>& callback));
 
   MOCK_METHOD1(ForceChoose, void(const std::string&));

components/autofill_assistant/browser/actions/prompt_action.cc

@@ -4,6 +4,7 @@
 
 #include "components/autofill_assistant/browser/actions/prompt_action.h"
 
+#include <algorithm>
 #include <memory>
 #include <utility>
 
@@ -23,35 +24,47 @@ PromptAction::~PromptAction() {}
 void PromptAction::InternalProcessAction(ActionDelegate* delegate,
                                          ProcessActionCallback callback) {
   delegate->ShowStatusMessage(proto_.prompt().message());
-  DCHECK_GT(proto_.prompt().suggestion_size(), 0);
 
   callback_ = std::move(callback);
-  delegate->Choose(ExtractVector(proto_.prompt().suggestion()),
-                   base::BindOnce(&PromptAction::OnSuggestionChosen,
-                                  weak_ptr_factory_.GetWeakPtr()));
+  std::vector<UiController::Choice> choices;
+  DCHECK_GT(proto_.prompt().choices_size(), 0);
+  for (const auto& choice_proto : proto_.prompt().choices()) {
+    if (choice_proto.name().empty())
+      continue;
 
-  if (proto_.prompt().auto_select_size() > 0) {
-    batch_element_checker_ = delegate->CreateBatchElementChecker();
-    for (const auto& auto_select : proto_.prompt().auto_select()) {
-      batch_element_checker_->AddElementCheck(
-          kExistenceCheck, ExtractSelector(auto_select.element()),
-          base::BindOnce(&PromptAction::OnElementExist, base::Unretained(this),
-                         auto_select.result()));
-    }
-    // Wait as long as necessary for one of the elements to show up. This is
-    // cancelled by OnSuggestionChosen()
-    batch_element_checker_->Run(
-        base::TimeDelta::Max(),
-        /* try_done= */
-        base::BindRepeating(&PromptAction::OnElementChecksDone,
-                            base::Unretained(this), base::Unretained(delegate)),
-        /* all_done= */ base::DoNothing());
+    choices.emplace_back();
+    auto& choice = choices.back();
+    choice.name = choice_proto.name();
+    choice_proto.SerializeToString(&choice.server_payload);
   }
+  delegate->Choose(choices, base::BindOnce(&PromptAction::OnSuggestionChosen,
+                                           weak_ptr_factory_.GetWeakPtr()));
+
+  batch_element_checker_ = delegate->CreateBatchElementChecker();
+  for (const auto& choice_proto : proto_.prompt().choices()) {
+    if (choice_proto.element_exists().selectors_size() == 0)
+      continue;
+
+    std::string payload;
+    choice_proto.SerializeToString(&payload);
+    batch_element_checker_->AddElementCheck(
+        kExistenceCheck, ExtractSelector(choice_proto.element_exists()),
+        base::BindOnce(&PromptAction::OnElementExist, base::Unretained(this),
+                       payload));
+  }
+  // Wait as long as necessary for one of the elements to show up. This is
+  // cancelled by OnSuggestionChosen()
+  batch_element_checker_->Run(
+      base::TimeDelta::Max(),
+      /* try_done= */
+      base::BindRepeating(&PromptAction::OnElementChecksDone,
+                          base::Unretained(this), base::Unretained(delegate)),
+      /* all_done= */ base::DoNothing());
 }
 
-void PromptAction::OnElementExist(const std::string& result, bool exists) {
+void PromptAction::OnElementExist(const std::string& payload, bool exists) {
   if (exists)
-    forced_result_ = result;
+    forced_payload_ = payload;
 
   // Calling ForceChoose is delayed until try_done, as it indirectly deletes
   // batch_element_checker_, which isn't supported from an element check
@@ -59,14 +72,20 @@ void PromptAction::OnElementExist(const std::string& result, bool exists) {
 }
 
 void PromptAction::OnElementChecksDone(ActionDelegate* delegate) {
-  if (!forced_result_.empty())
-    delegate->ForceChoose(forced_result_);
+  if (!forced_payload_.empty())
+    delegate->ForceChoose(forced_payload_);
 }
 
-void PromptAction::OnSuggestionChosen(const std::string& chosen) {
+void PromptAction::OnSuggestionChosen(const std::string& payload) {
   batch_element_checker_.reset();
-  UpdateProcessedAction(ACTION_APPLIED);
-  processed_action_proto_->mutable_prompt_result()->set_result(chosen);
+  PromptProto::Choice choice;
+  if (!choice.ParseFromString(payload)) {
+    DLOG(ERROR) << "Invalid result.";
+    UpdateProcessedAction(OTHER_ACTION_STATUS);
+  } else {
+    UpdateProcessedAction(ACTION_APPLIED);
+    std::swap(*processed_action_proto_->mutable_prompt_choice(), choice);
+  }
   std::move(callback_).Run(std::move(processed_action_proto_));
 }
 }  // namespace autofill_assistant

components/autofill_assistant/browser/actions/prompt_action.h

@@ -26,13 +26,13 @@ class PromptAction : public Action {
   void InternalProcessAction(ActionDelegate* delegate,
                              ProcessActionCallback callback) override;
 
-  void OnElementExist(const std::string& result, bool exists);
+  void OnElementExist(const std::string& payload, bool exists);
   void OnElementChecksDone(ActionDelegate* delegate);
-  void OnSuggestionChosen(const std::string& chosen);
+  void OnSuggestionChosen(const std::string& payload);
 
   ProcessActionCallback callback_;
 
-  std::string forced_result_;
+  std::string forced_payload_;
   std::unique_ptr<BatchElementChecker> batch_element_checker_;
   base::WeakPtrFactory<PromptAction> weak_ptr_factory_;
 

components/autofill_assistant/browser/mock_ui_controller.h

@@ -30,12 +30,12 @@ class MockUiController : public UiController {
   MOCK_METHOD0(CloseCustomTab, void());
   MOCK_METHOD1(UpdateScripts, void(const std::vector<ScriptHandle>& scripts));
 
-  void Choose(const std::vector<std::string>& suggestions,
+  void Choose(const std::vector<UiController::Choice>& choices,
               base::OnceCallback<void(const std::string&)> callback) override {
-    OnChoose(suggestions, callback);
+    OnChoose(choices, callback);
   }
   MOCK_METHOD2(OnChoose,
-               void(const std::vector<std::string>& suggestions,
+               void(const std::vector<UiController::Choice>& choices,
                     base::OnceCallback<void(const std::string&)>& callback));
 
   void ChooseAddress(

components/autofill_assistant/browser/script_executor.cc

@@ -124,7 +124,7 @@ void ScriptExecutor::GetPaymentInformation(
 }
 
 void ScriptExecutor::Choose(
-    const std::vector<std::string>& suggestions,
+    const std::vector<UiController::Choice>& choices,
     base::OnceCallback<void(const std::string&)> callback) {
   if (!touchable_elements_.empty()) {
     // Choose reproduces the end-of-script appearance and behavior during script
@@ -141,7 +141,7 @@ void ScriptExecutor::Choose(
     // ScriptExecutor::OnChosen
   }
   delegate_->GetUiController()->Choose(
-      suggestions,
+      choices,
       base::BindOnce(&ScriptExecutor::OnChosen, weak_ptr_factory_.GetWeakPtr(),
                      std::move(callback)));
 }
@@ -552,12 +552,10 @@ void ScriptExecutor::WaitWithInterrupts::RunCallback(
 
 void ScriptExecutor::OnChosen(
     base::OnceCallback<void(const std::string&)> callback,
-    const std::string& choice) {
-  // This simulates the beginning of a script and removes any touchable element
-  // area set by Choose().
+    const std::string& payload) {
   delegate_->GetUiController()->ShowOverlay();
   delegate_->ClearTouchableElementArea();
-  std::move(callback).Run(choice);
+  std::move(callback).Run(payload);
 }
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/script_executor.h

@@ -94,7 +94,7 @@ class ScriptExecutor : public ActionDelegate {
       base::OnceCallback<void(std::unique_ptr<PaymentInformation>)> callback,
       const std::string& title,
       const std::vector<std::string>& supported_basic_card_networks) override;
-  void Choose(const std::vector<std::string>& suggestions,
+  void Choose(const std::vector<UiController::Choice>& choice,
               base::OnceCallback<void(const std::string&)> callback) override;
   void ForceChoose(const std::string&) override;
   void ChooseAddress(

components/autofill_assistant/browser/service.proto

@@ -259,7 +259,7 @@ message ProcessedActionProto {
   optional ProcessedActionStatusProto status = 2;
 
   oneof result_data {
-    PromptProto.Result prompt_result = 5;
+    PromptProto.Choice prompt_choice = 5;
     string html_source = 12;
     // Should be set as a result of GetPaymentInformationAction.
     PaymentDetails payment_details = 15;
@@ -435,29 +435,33 @@ message NavigateProto {
   optional string url = 1;
 }
 
-// Allow the selection of one or more suggestions. If FocusElement was called
-// just before, allow interaction with the touchable element area, otherwise
-// don't allow any interactions.
+// Allow choosing one or more possibility. If FocusElement was called just
+// before, allow interaction with the touchable element area, otherwise don't
+// allow any interactions.
 message PromptProto {
-  message Result {
-    // One of the suggestions from PromptProto.suggestion or
-    // PromptProto.AutoSelect.result.
-    optional string result = 1;
-  }
-
-  // Localized text message to display.
+  // Display this message to the user.
   optional string message = 1;
 
-  // Localized suggestions to propose.
-  repeated string suggestion = 2;
+  // A choice that is made either directly by clicking on a chip or button, or
+  // implicitly by making a change on the website that is then detected by
+  // looking for the existence of an element.
+  //
+  // One of these protos must is transmitted as-is back to the server as part of
+  // ProcessedActionProto.
+  message Choice {
+    oneof type {
+      // A localized text message to display.
+      string name = 2;
+
+      // Auto-select if the given element exist.
+      ElementReferenceProto element_exists = 4;
+    }
 
-  // Auto-select the prompt with the given result once the given element exists.
-  message AutoSelect {
-    // Result to send to the server.
-    optional string result = 1;
-    optional ElementReferenceProto element = 2;
+    // Server payload originally sent by the server. This should
+    // be transmitted as-is by the client without interpreting.
+    optional bytes server_payload = 5;
   }
-  repeated AutoSelect auto_select = 3;
+  repeated Choice choices = 4;
 }
 
 message ContactDetailsProto {

components/autofill_assistant/browser/ui_controller.h

@@ -22,6 +22,15 @@ class DetailsProto;
 // Controller to control autofill assistant UI.
 class UiController {
  public:
+  // A choice, for Choose().
+  struct Choice {
+    // Localized string to display.
+    std::string name;
+
+    // Opaque data to send back to the callback. Not necessarily a UTF8 string.
+    std::string server_payload;
+  };
+
   virtual ~UiController() = default;
 
   // Set assistant UI delegate called by assistant UI controller.
@@ -52,15 +61,15 @@ class UiController {
   // Update the list of scripts in the UI.
   virtual void UpdateScripts(const std::vector<ScriptHandle>& scripts) = 0;
 
-  // Show UI to ask user to select one of the suggestions. Sends the selected
-  // suggestion to the callback.
+  // Show UI to ask user to make a choice. Sends the server_payload of the
+  // choice to the callback.
   virtual void Choose(
-      const std::vector<std::string>& suggestions,
+      const std::vector<Choice>& choices,
       base::OnceCallback<void(const std::string&)> callback) = 0;
 
   // Cancels a choose action in progress. Calls the registered callback, if any,
-  // with the given result.
-  virtual void ForceChoose(const std::string& choice) = 0;
+  // with the given server_payload.
+  virtual void ForceChoose(const std::string& server_payload) = 0;
 
   // Show UI to ask user to choose an address in personal data manager. GUID of
   // the chosen address will be returned through callback, otherwise empty