[AA Direct Actions] Provide direct action args to NGA.

Before this patch required and optional arguments part of a direct
action / AA script were not relayed through the direct actions layer.
This patch properly adds these args according to the proto definition
provided by the backend and forwards those args to the direct action
android stack.

R=szermatt@chromium.org

Bug: b/138833619
Change-Id: I964addd2822c81ccd01353f42085889bafa21ae0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1895332
Commit-Queue: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712105}

chrome/android/features/autofill_assistant/BUILD.gn

@@ -79,6 +79,7 @@ android_library("java") {
     "java/src/org/chromium/chrome/browser/autofill_assistant/AssistantTextUtils.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiController.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantClient.java",
+    "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantDirectActionImpl.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantModuleEntryImpl.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantActionHandlerImpl.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantServiceInjector.java",
@@ -154,6 +155,7 @@ generate_jni("jni_headers") {
     "java/src/org/chromium/chrome/browser/autofill_assistant/AssistantDimension.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AssistantModel.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantClient.java",
+    "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantDirectActionImpl.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiController.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/details/AssistantDetails.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/details/AssistantDetailsModel.java",

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantActionHandlerImpl.java

@@ -15,15 +15,15 @@ import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.widget.ScrimView;
 import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
 
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 /**
  * A handler that provides Autofill Assistant actions for a specific activity.
  */
 class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandler {
-    private static final String[] EMPTY_ARRAY = new String[0];
-
     private final Context mContext;
     private final BottomSheetController mBottomSheetController;
     private final ScrimView mScrimView;
@@ -62,10 +62,10 @@ class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandl
     }
 
     @Override
-    public String[] getActions() {
+    public List<AutofillAssistantDirectAction> getActions() {
         AutofillAssistantClient client = getOrCreateClient();
         if (client == null) {
-            return EMPTY_ARRAY;
+            return Collections.emptyList();
         }
         return client.getDirectActions();
     }

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantClient.java

@@ -22,6 +22,8 @@ import org.chromium.components.signin.AccountManagerFacade;
 import org.chromium.components.signin.OAuth2TokenService;
 import org.chromium.content_public.browser.WebContents;
 
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -169,12 +171,13 @@ class AutofillAssistantClient {
     }
 
     /** Lists available direct actions. */
-    public String[] getDirectActions() {
+    public List<AutofillAssistantDirectAction> getDirectActions() {
         if (mNativeClientAndroid == 0) {
-            return null;
+            return Collections.emptyList();
         }
-        return AutofillAssistantClientJni.get().getDirectActions(
+        AutofillAssistantDirectAction[] actions = AutofillAssistantClientJni.get().getDirectActions(
                 mNativeClientAndroid, AutofillAssistantClient.this);
+        return Arrays.asList(actions);
     }
 
     /**
@@ -373,7 +376,8 @@ class AutofillAssistantClient {
                 Object callback);
         boolean hasRunFirstCheck(long nativeClientAndroid, AutofillAssistantClient caller);
 
-        String[] getDirectActions(long nativeClientAndroid, AutofillAssistantClient caller);
+        AutofillAssistantDirectAction[] getDirectActions(
+                long nativeClientAndroid, AutofillAssistantClient caller);
 
         boolean performDirectAction(long nativeClientAndroid, AutofillAssistantClient caller,
                 String actionId, String experimentId, String[] argumentNames,

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantDirectActionImpl.java

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.autofill_assistant;
+
+import org.chromium.base.annotations.CalledByNative;
+import org.chromium.base.annotations.JNINamespace;
+
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * A container that corresponds to the DirectActionProto used to store action realated data like
+ * arguments.
+ */
+@JNINamespace("autofill_assistant")
+class AutofillAssistantDirectActionImpl implements AutofillAssistantDirectAction {
+    /* List of direct actions with the given names. */
+    private final List<String> mNames;
+
+    /* List of required argument names for this action. */
+    private final List<String> mRequiredArguments;
+
+    /* List of optional argument names for this action. */
+    private final List<String> mOptionalArguments;
+
+    @CalledByNative
+    public AutofillAssistantDirectActionImpl(String[] names, String[] required, String[] optional) {
+        mNames = Arrays.asList(names);
+        mRequiredArguments = Arrays.asList(required);
+        mOptionalArguments = Arrays.asList(optional);
+    }
+
+    @Override
+    public List<String> getNames() {
+        return mNames;
+    }
+
+    @Override
+    public List<String> getRequiredArguments() {
+        return mRequiredArguments;
+    }
+
+    @Override
+    public List<String> getOptionalArguments() {
+        return mOptionalArguments;
+    }
+}

chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantDirectActionHandlerTest.java

@@ -134,28 +134,47 @@ public class AutofillAssistantDirectActionHandlerTest {
         reportAvailableDirectActions(mHandler, reporter);
 
         // Now that the AA stack is up, the fetdch_website_actions should no longer show up.
-        assertEquals(2, reporter.mActions.size());
+        assertEquals(3, reporter.mActions.size());
 
-        // Now we expect 2 dyamic actions "search" and "action2".
+        // Now we expect 3 dyamic actions "search", "action2" and "action2_alias".
         FakeDirectActionDefinition search = reporter.mActions.get(0);
         assertEquals("search", search.mId);
-        assertEquals(2, search.mParameters.size());
+        assertEquals(3, search.mParameters.size());
         assertEquals("experiment_ids", search.mParameters.get(0).mName);
         assertEquals(Type.STRING, search.mParameters.get(0).mType);
         assertEquals("SEARCH_QUERY", search.mParameters.get(1).mName);
         assertEquals(Type.STRING, search.mParameters.get(1).mType);
+        assertEquals("arg2", search.mParameters.get(2).mName);
+        assertEquals(Type.STRING, search.mParameters.get(2).mType);
         assertEquals(1, search.mResults.size());
         assertEquals("success", search.mResults.get(0).mName);
         assertEquals(Type.BOOLEAN, search.mResults.get(0).mType);
 
         FakeDirectActionDefinition action2 = reporter.mActions.get(1);
         assertEquals("action2", action2.mId);
-        assertEquals(1, action2.mParameters.size());
+        assertEquals(3, action2.mParameters.size());
         assertEquals("experiment_ids", action2.mParameters.get(0).mName);
         assertEquals(Type.STRING, action2.mParameters.get(0).mType);
+        assertEquals("SEARCH_QUERY", action2.mParameters.get(1).mName);
+        assertEquals(Type.STRING, action2.mParameters.get(1).mType);
+        assertEquals("arg2", action2.mParameters.get(2).mName);
+        assertEquals(Type.STRING, action2.mParameters.get(2).mType);
         assertEquals(1, action2.mResults.size());
         assertEquals("success", action2.mResults.get(0).mName);
         assertEquals(Type.BOOLEAN, action2.mResults.get(0).mType);
+
+        FakeDirectActionDefinition action2Alias = reporter.mActions.get(2);
+        assertEquals("action2_alias", action2Alias.mId);
+        assertEquals(3, action2Alias.mParameters.size());
+        assertEquals("experiment_ids", action2Alias.mParameters.get(0).mName);
+        assertEquals(Type.STRING, action2Alias.mParameters.get(0).mType);
+        assertEquals("SEARCH_QUERY", action2Alias.mParameters.get(1).mName);
+        assertEquals(Type.STRING, action2Alias.mParameters.get(1).mType);
+        assertEquals("arg2", action2Alias.mParameters.get(2).mName);
+        assertEquals(Type.STRING, action2Alias.mParameters.get(2).mType);
+        assertEquals(1, action2Alias.mResults.size());
+        assertEquals("success", action2Alias.mResults.get(0).mName);
+        assertEquals(Type.BOOLEAN, action2Alias.mResults.get(0).mType);
     }
 
     @Test

chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/TestingAutofillAssistantModuleEntryProvider.java

@@ -15,6 +15,9 @@ import org.chromium.chrome.browser.widget.ScrimView;
 import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
 import org.chromium.content_public.browser.WebContents;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -39,8 +42,15 @@ class TestingAutofillAssistantModuleEntryProvider extends AutofillAssistantModul
         }
 
         @Override
-        public String[] getActions() {
-            return new String[] {"search", "action2"};
+        public List<AutofillAssistantDirectAction> getActions() {
+            String[] search = new String[] {"search"};
+            String[] required = new String[] {"SEARCH_QUERY"};
+            String[] optional = new String[] {"arg2"};
+            String[] action2 = new String[] {"action2", "action2_alias"};
+            AutofillAssistantDirectAction[] actions = new AutofillAssistantDirectActionImpl[] {
+                    new AutofillAssistantDirectActionImpl(search, required, optional),
+                    new AutofillAssistantDirectActionImpl(action2, required, optional)};
+            return new ArrayList<>(Arrays.asList(actions));
         }
     }
 

chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantActionHandler.java

@@ -8,6 +8,8 @@ import android.os.Bundle;
 
 import org.chromium.base.Callback;
 
+import java.util.List;
+
 /**
  * Interface that provides implementation for AA actions, triggered by direct actions.
  */
@@ -46,9 +48,9 @@ public interface AutofillAssistantActionHandler {
      * either that the controller has not yet been started or there are no actions available for the
      * current website.
      *
-     * @return Array of strings with the names of known actions.
+     * @return Array of actions containing the names and arguments of known actions.
      */
-    String[] getActions();
+    List<AutofillAssistantDirectAction> getActions();
 
     /** Performs onboarding and returns the result to the callback. */
     void performOnboarding(String experimentIds, Callback<Boolean> callback);

chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantDirectAction.java

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.autofill_assistant;
+
+import java.util.List;
+
+/**
+ * Interface for direct action containers.
+ */
+interface AutofillAssistantDirectAction {
+    /** Returns a list of name and aliases for this direct action. */
+    List<String> getNames();
+
+    /** Returns the list of required argument names. */
+    List<String> getRequiredArguments();
+
+    /** Returns the list of optional argument names. */
+    List<String> getOptionalArguments();
+}

chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantDirectActionHandler.java

@@ -19,8 +19,6 @@ import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.widget.ScrimView;
 import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
 
-import java.util.Arrays;
-
 /**
  * A handler that provides just enough functionality to allow on-demand loading of the module
  * through direct actions. The actual implementation is in the module.
@@ -73,20 +71,24 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
                     .withParameter(USER_NAME, Type.STRING, /* required= */ false)
                     .withParameter(EXPERIMENT_IDS, Type.STRING, /* required= */ false)
                     .withResult(FETCH_WEBSITE_ACTIONS_RESULT, Type.BOOLEAN);
-        }
-
-        // Additionally report if there are dynamic actions.
-        if (mDelegate != null) {
-            for (String action : mDelegate.getActions()) {
-                Definition definition =
-                        reporter.addDirectAction(action)
-                                .withParameter(EXPERIMENT_IDS, Type.STRING, /* required= */ false)
-                                .withResult(AA_ACTION_RESULT, Type.BOOLEAN);
-
-                // TODO(b/138833619): For testing purposes only. Remove this when script
-                // parameters are properly supported.
-                if (action.equals("search")) {
-                    definition.withParameter("SEARCH_QUERY", Type.STRING, /* required= */ true);
+        } else {
+            // Otherwise we are already done fetching scripts and can just return the ones we know
+            // about.
+            for (AutofillAssistantDirectAction action : mDelegate.getActions()) {
+                for (String name : action.getNames()) {
+                    Definition definition = reporter.addDirectAction(name)
+                                                    .withParameter(EXPERIMENT_IDS, Type.STRING,
+                                                            /* required= */ false)
+                                                    .withResult(AA_ACTION_RESULT, Type.BOOLEAN);
+
+                    // TODO(b/138833619): Support non-string arguments. Requires updating the proto
+                    // definition.
+                    for (String required : action.getRequiredArguments()) {
+                        definition.withParameter(required, Type.STRING, /* required= */ true);
+                    }
+                    for (String optional : action.getOptionalArguments()) {
+                        definition.withParameter(optional, Type.STRING, /* required= */ false);
+                    }
                 }
             }
         }
@@ -95,7 +97,8 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
     @Override
     public boolean performDirectAction(
             String actionId, Bundle arguments, Callback<Bundle> callback) {
-        if (actionId.equals(FETCH_WEBSITE_ACTIONS)) {
+        if (actionId.equals(FETCH_WEBSITE_ACTIONS)
+                && AutofillAssistantPreferencesUtil.isAutofillOnboardingAccepted()) {
             fetchWebsiteActions(arguments, callback);
             return true;
         }
@@ -110,7 +113,10 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
 
     private boolean isActionAvailable(String actionId) {
         if (mDelegate == null) return false;
-        return Arrays.asList(mDelegate.getActions()).contains(actionId);
+        for (AutofillAssistantDirectAction action : mDelegate.getActions()) {
+            if (action.getNames().contains(actionId)) return true;
+        }
+        return false;
     }
 
     private void fetchWebsiteActions(Bundle arguments, Callback<Bundle> bundleCallback) {
@@ -174,7 +180,7 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
             }
 
             Callback<Boolean> successCallback = (success) -> {
-                booleanCallback.onResult(success && delegate.getActions().length != 0);
+                booleanCallback.onResult(success && !delegate.getActions().isEmpty());
             };
             delegate.performAction(name, experimentIds, arguments, successCallback);
         });

chrome/android/features/autofill_assistant/public/java_sources.gni

@@ -9,6 +9,7 @@ public_autofill_assistant_java_sources = [
   "//chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantModuleEntryProvider.java",
   "//chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantPreferencesUtil.java",
   "//chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantDirectActionHandler.java",
+  "//chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantDirectAction.java",
   "//chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantActionHandler.java",
   "//chrome/android/features/autofill_assistant/public/java/src/org/chromium/chrome/browser/autofill_assistant/GetCurrentTab.java",
 ]

chrome/browser/android/autofill_assistant/client_android.cc

@@ -18,6 +18,7 @@
 #include "base/task/post_task.h"
 #include "base/time/default_tick_clock.h"
 #include "chrome/android/features/autofill_assistant/jni_headers/AutofillAssistantClient_jni.h"
+#include "chrome/android/features/autofill_assistant/jni_headers/AutofillAssistantDirectActionImpl_jni.h"
 #include "chrome/browser/android/chrome_feature_list.h"
 #include "chrome/browser/autofill/android/personal_data_manager_android.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
@@ -262,10 +263,63 @@ ClientAndroid::GetDirectActionsAsJavaArrayOfStrings(JNIEnv* env) const {
       env, std::vector<std::string>(names.begin(), names.end()));
 }
 
+base::android::ScopedJavaLocalRef<jobject>
+ClientAndroid::ToJavaAutofillAssistantDirectAction(
+    JNIEnv* env,
+    const DirectAction& direct_action) const {
+  std::set<std::string> names;
+  for (const std::string& name : direct_action.names)
+    names.insert(name);
+  auto jnames = base::android::ToJavaArrayOfStrings(
+      env, std::vector<std::string>(names.begin(), names.end()));
+
+  std::vector<std::string> required_arguments;
+  for (const std::string& arg : direct_action.required_arguments)
+    required_arguments.emplace_back(arg);
+  auto jrequired_arguments =
+      base::android::ToJavaArrayOfStrings(env, required_arguments);
+
+  std::vector<std::string> optional_arguments;
+  for (const std::string& arg : direct_action.optional_arguments)
+    optional_arguments.emplace_back(arg);
+  auto joptional_arguments =
+      base::android::ToJavaArrayOfStrings(env, std::move(optional_arguments));
+
+  return Java_AutofillAssistantDirectActionImpl_Constructor(
+      env, jnames, jrequired_arguments, joptional_arguments);
+}
+
 base::android::ScopedJavaLocalRef<jobjectArray> ClientAndroid::GetDirectActions(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& jcaller) {
-  return GetDirectActionsAsJavaArrayOfStrings(env);
+  DCHECK(controller_ != nullptr);
+  size_t actions_count = 0;
+  for (const UserAction& user_action : controller_->GetUserActions()) {
+    if (!user_action.enabled())
+      continue;
+    ++actions_count;
+  }
+
+  // Prepare the java array to hold the direct actions.
+  base::android::ScopedJavaLocalRef<jclass> directaction_array_class =
+      base::android::GetClass(env,
+                              "org/chromium/chrome/browser/autofill_assistant/"
+                              "AutofillAssistantDirectActionImpl");
+
+  jobjectArray joa = env->NewObjectArray(
+      actions_count, directaction_array_class.obj(), nullptr);
+  jni_generator::CheckException(env);
+
+  actions_count = 0;
+  for (const UserAction& user_action : controller_->GetUserActions()) {
+    if (!user_action.enabled())
+      continue;
+
+    auto jdirect_action =
+        ToJavaAutofillAssistantDirectAction(env, user_action.direct_action());
+    env->SetObjectArrayElement(joa, actions_count++, jdirect_action.obj());
+  }
+  return base::android::ScopedJavaLocalRef<jobjectArray>(env, joa);
 }
 
 void ClientAndroid::OnFetchWebsiteActions(

chrome/browser/android/autofill_assistant/client_android.h

@@ -121,6 +121,10 @@ class ClientAndroid : public Client,
   base::android::ScopedJavaLocalRef<jobjectArray>
   GetDirectActionsAsJavaArrayOfStrings(JNIEnv* env) const;
 
+  base::android::ScopedJavaLocalRef<jobject>
+  ToJavaAutofillAssistantDirectAction(JNIEnv* env,
+                                      const DirectAction& direct_action) const;
+
   // Returns the index of a direct action with that name, to pass to
   // UiDelegate::PerformUserAction() or -1 if not found.
   int FindDirectAction(const std::string& action_name);