Commit 1bcd4df3 authored by Mathias Carlen's avatar Mathias Carlen Committed by Commit Bot

[Autofill Assistant] Report autofill scripts as real direct actions.

Before this patch we only provided listAction and performAction as
_pure_ direct actions. Via listActions more actions would be exposed and
available to be performed via performAction.

This patch also exposes dynamic actions as reported by listAction as
_pure_ direct actions so that an external direct action user can perform
them directly instead of the indirecation through performAction.

R=szermatt@google.com

Bug: b/138833619
Change-Id: If8d0f09d28bd8fa8f9d1681570cfc3dbfbbb5c18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866830
Commit-Queue: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: default avatarStephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708149}
parent e3da69de
...@@ -23,6 +23,8 @@ import java.util.Set; ...@@ -23,6 +23,8 @@ import java.util.Set;
* A handler that provides Autofill Assistant actions for a specific activity. * A handler that provides Autofill Assistant actions for a specific activity.
*/ */
class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandler { class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandler {
private static final String[] EMPTY_ARRAY = new String[0];
private final Context mContext; private final Context mContext;
private final BottomSheetController mBottomSheetController; private final BottomSheetController mBottomSheetController;
private final ScrimView mScrimView; private final ScrimView mScrimView;
...@@ -52,6 +54,15 @@ class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandl ...@@ -52,6 +54,15 @@ class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandl
client.listDirectActions(userName, experimentIds, toArgumentMap(arguments), callback); client.listDirectActions(userName, experimentIds, toArgumentMap(arguments), callback);
} }
@Override
public String[] getActions() {
AutofillAssistantClient client = getOrCreateClient();
if (client == null) {
return EMPTY_ARRAY;
}
return client.getDirectActions();
}
@Override @Override
public void performOnboarding(String experimentIds, Callback<Boolean> callback) { public void performOnboarding(String experimentIds, Callback<Boolean> callback) {
AssistantOnboardingCoordinator coordinator = new AssistantOnboardingCoordinator( AssistantOnboardingCoordinator coordinator = new AssistantOnboardingCoordinator(
...@@ -73,11 +84,6 @@ class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandl ...@@ -73,11 +84,6 @@ class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandl
Callback<AssistantOnboardingCoordinator> afterOnboarding = (onboardingCoordinator) -> { Callback<AssistantOnboardingCoordinator> afterOnboarding = (onboardingCoordinator) -> {
Map<String, String> argumentMap = toArgumentMap(arguments); Map<String, String> argumentMap = toArgumentMap(arguments);
if (name.isEmpty()) {
callback.onResult(client.start(/* initialUrl= */ "", argumentMap, experimentIds,
Bundle.EMPTY, onboardingCoordinator));
return;
}
callback.onResult(client.performDirectAction( callback.onResult(client.performDirectAction(
name, experimentIds, argumentMap, onboardingCoordinator)); name, experimentIds, argumentMap, onboardingCoordinator));
}; };
......
...@@ -159,6 +159,15 @@ class AutofillAssistantClient { ...@@ -159,6 +159,15 @@ class AutofillAssistantClient {
arguments.values().toArray(new String[arguments.size()]), callback); arguments.values().toArray(new String[arguments.size()]), callback);
} }
/** Lists available direct actions. */
public String[] getDirectActions() {
if (mNativeClientAndroid == 0) {
return null;
}
return AutofillAssistantClientJni.get().getDirectActions(
mNativeClientAndroid, AutofillAssistantClient.this);
}
/** /**
* Performs a direct action. * Performs a direct action.
* *
...@@ -357,6 +366,8 @@ class AutofillAssistantClient { ...@@ -357,6 +366,8 @@ class AutofillAssistantClient {
void listDirectActions(long nativeClientAndroid, AutofillAssistantClient caller, void listDirectActions(long nativeClientAndroid, AutofillAssistantClient caller,
String experimentIds, String[] argumentNames, String[] argumentValues, String experimentIds, String[] argumentNames, String[] argumentValues,
Object callback); Object callback);
String[] getDirectActions(long nativeClientAndroid, AutofillAssistantClient caller);
boolean performDirectAction(long nativeClientAndroid, AutofillAssistantClient caller, boolean performDirectAction(long nativeClientAndroid, AutofillAssistantClient caller,
String actionId, String experimentId, String[] argumentNames, String actionId, String experimentId, String[] argumentNames,
String[] argumentValues, String[] argumentValues,
......
...@@ -84,7 +84,93 @@ public class AutofillAssistantDirectActionHandlerTest { ...@@ -84,7 +84,93 @@ public class AutofillAssistantDirectActionHandlerTest {
@Test @Test
@MediumTest @MediumTest
public void testReportAvailableDirectActions() { public void testReportOnboardingOnlyIfNotAccepted() throws Exception {
mModuleEntryProvider.setInstalled();
assertThat(listActions(), contains("onboarding"));
FakeDirectActionReporter reporter = new FakeDirectActionReporter();
mHandler.reportAvailableDirectActions(reporter);
assertEquals(1, reporter.mActions.size());
FakeDirectActionDefinition onboarding = reporter.mActions.get(0);
assertEquals("onboarding", onboarding.mId);
assertEquals(2, onboarding.mParameters.size());
assertEquals("name", onboarding.mParameters.get(0).mName);
assertEquals(Type.STRING, onboarding.mParameters.get(0).mType);
assertEquals("experiment_ids", onboarding.mParameters.get(1).mName);
assertEquals(Type.STRING, onboarding.mParameters.get(1).mType);
assertEquals(1, onboarding.mResults.size());
assertEquals("success", onboarding.mResults.get(0).mName);
assertEquals(Type.BOOLEAN, onboarding.mResults.get(0).mType);
}
@Test
@MediumTest
public void testReportAvailableDirectActions() throws Exception {
mModuleEntryProvider.setInstalled();
assertThat(listActions(), contains("onboarding"));
AutofillAssistantPreferencesUtil.setInitialPreferences(true);
// Ask again for a new list of actions.
listActions();
FakeDirectActionReporter reporter = new FakeDirectActionReporter();
mHandler.reportAvailableDirectActions(reporter);
assertEquals(4, reporter.mActions.size());
FakeDirectActionDefinition list = reporter.mActions.get(0);
assertEquals("list_assistant_actions", list.mId);
assertEquals(2, list.mParameters.size());
assertEquals("user_name", list.mParameters.get(0).mName);
assertEquals(Type.STRING, list.mParameters.get(0).mType);
assertEquals(false, list.mParameters.get(0).mRequired);
assertEquals("experiment_ids", list.mParameters.get(1).mName);
assertEquals(Type.STRING, list.mParameters.get(1).mType);
assertEquals(false, list.mParameters.get(1).mRequired);
assertEquals(1, list.mResults.size());
assertEquals("names", list.mResults.get(0).mName);
assertEquals(Type.STRING, list.mResults.get(0).mType);
FakeDirectActionDefinition perform = reporter.mActions.get(1);
assertEquals("perform_assistant_action", perform.mId);
assertEquals(2, perform.mParameters.size());
assertEquals("name", perform.mParameters.get(0).mName);
assertEquals(Type.STRING, perform.mParameters.get(0).mType);
assertEquals("experiment_ids", perform.mParameters.get(1).mName);
assertEquals(Type.STRING, perform.mParameters.get(1).mType);
assertEquals(1, perform.mResults.size());
assertEquals("success", perform.mResults.get(0).mName);
assertEquals(Type.BOOLEAN, perform.mResults.get(0).mType);
// Now we expect 2 dyamic actions "search" and "action2".
FakeDirectActionDefinition search = reporter.mActions.get(2);
assertEquals("search", search.mId);
assertEquals(1, search.mParameters.size());
assertEquals("experiment_ids", search.mParameters.get(0).mName);
assertEquals(Type.STRING, search.mParameters.get(0).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(3);
assertEquals("action2", action2.mId);
assertEquals(1, action2.mParameters.size());
assertEquals("experiment_ids", action2.mParameters.get(0).mName);
assertEquals(Type.STRING, action2.mParameters.get(0).mType);
assertEquals(1, action2.mResults.size());
assertEquals("success", action2.mResults.get(0).mName);
assertEquals(Type.BOOLEAN, action2.mResults.get(0).mType);
}
@Test
@MediumTest
public void testReportAvailableAutofillAssistantActions() throws Exception {
mModuleEntryProvider.setInstalled();
assertThat(listActions(), contains("onboarding"));
AutofillAssistantPreferencesUtil.setInitialPreferences(true);
FakeDirectActionReporter reporter = new FakeDirectActionReporter(); FakeDirectActionReporter reporter = new FakeDirectActionReporter();
mHandler.reportAvailableDirectActions(reporter); mHandler.reportAvailableDirectActions(reporter);
......
...@@ -4,8 +4,18 @@ ...@@ -4,8 +4,18 @@
package org.chromium.chrome.browser.autofill_assistant; package org.chromium.chrome.browser.autofill_assistant;
import android.content.Context;
import android.os.Bundle;
import androidx.annotation.NonNull;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
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.Map;
/** /**
* Implementation of {@link AutofillAssistantModuleEntryProvider} that can be manipulated to * Implementation of {@link AutofillAssistantModuleEntryProvider} that can be manipulated to
...@@ -15,6 +25,41 @@ class TestingAutofillAssistantModuleEntryProvider extends AutofillAssistantModul ...@@ -15,6 +25,41 @@ class TestingAutofillAssistantModuleEntryProvider extends AutofillAssistantModul
private boolean mNotInstalled; private boolean mNotInstalled;
private boolean mCannotInstall; private boolean mCannotInstall;
/*
* Mock action handler. We only override returning dynamic actions.
*
* TODO(crbug/806868): Inject a service also for the DirectAction path and get rid of this
* mock.
*/
static class MockAutofillAssistantActionHandler extends AutofillAssistantActionHandlerImpl {
public MockAutofillAssistantActionHandler(Context context,
BottomSheetController bottomSheetController, ScrimView scrimView,
GetCurrentTab getCurrentTab) {
super(context, bottomSheetController, scrimView, getCurrentTab);
}
@Override
public String[] getActions() {
return new String[] {"search", "action2"};
}
}
/** Mock module entry. */
static class MockAutofillAssistantModuleEntry implements AutofillAssistantModuleEntry {
@Override
public void start(@NonNull Tab tab, @NonNull WebContents webContents,
boolean skipOnboarding, String initialUrl, Map<String, String> parameters,
String experimentIds, Bundle intentExtras) {}
@Override
public AutofillAssistantActionHandler createActionHandler(Context context,
BottomSheetController bottomSheetController, ScrimView scrimView,
GetCurrentTab getCurrentTab) {
return new MockAutofillAssistantActionHandler(
context, bottomSheetController, scrimView, getCurrentTab);
}
}
/** The module is already installed. This is the default state. */ /** The module is already installed. This is the default state. */
public void setInstalled() { public void setInstalled() {
mNotInstalled = false; mNotInstalled = false;
...@@ -36,7 +81,7 @@ class TestingAutofillAssistantModuleEntryProvider extends AutofillAssistantModul ...@@ -36,7 +81,7 @@ class TestingAutofillAssistantModuleEntryProvider extends AutofillAssistantModul
@Override @Override
public AutofillAssistantModuleEntry getModuleEntryIfInstalled() { public AutofillAssistantModuleEntry getModuleEntryIfInstalled() {
if (mNotInstalled) return null; if (mNotInstalled) return null;
return super.getModuleEntryIfInstalled(); return new MockAutofillAssistantModuleEntry();
} }
@Override @Override
......
...@@ -28,6 +28,17 @@ public interface AutofillAssistantActionHandler { ...@@ -28,6 +28,17 @@ public interface AutofillAssistantActionHandler {
void listActions(String userName, String experimentIds, Bundle arguments, void listActions(String userName, String experimentIds, Bundle arguments,
Callback<Set<String>> callback); Callback<Set<String>> callback);
/**
* Returns the available AA actions to be reported to the direct actions framework.
*
* <p>This method simply returns the list of actions known to AA. An empty string array means
* 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.
*/
String[] getActions();
/** Performs onboarding and returns the result to the callback. */ /** Performs onboarding and returns the result to the callback. */
void performOnboarding(String experimentIds, Callback<Boolean> callback); void performOnboarding(String experimentIds, Callback<Boolean> callback);
......
...@@ -18,6 +18,7 @@ import org.chromium.chrome.browser.tab.Tab; ...@@ -18,6 +18,7 @@ import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.widget.ScrimView; import org.chromium.chrome.browser.widget.ScrimView;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController; import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Set; import java.util.Set;
...@@ -26,10 +27,13 @@ import java.util.Set; ...@@ -26,10 +27,13 @@ import java.util.Set;
* through direct actions. The actual implementation is in the module. * through direct actions. The actual implementation is in the module.
*/ */
public class AutofillAssistantDirectActionHandler implements DirectActionHandler { public class AutofillAssistantDirectActionHandler implements DirectActionHandler {
// TODO(b/138833619): Rename this action into something that shows that it is starting the
// controller / dynamic actions stack.
private static final String LIST_AA_ACTIONS = "list_assistant_actions"; private static final String LIST_AA_ACTIONS = "list_assistant_actions";
private static final String LIST_AA_ACTIONS_RESULT = "names"; private static final String LIST_AA_ACTIONS_RESULT = "names";
// TODO(b/138833619): Remove this action entirely.
private static final String PERFORM_AA_ACTION = "perform_assistant_action"; private static final String PERFORM_AA_ACTION = "perform_assistant_action";
private static final String PERFORM_AA_ACTION_RESULT = "success"; private static final String AA_ACTION_RESULT = "success";
private static final String ACTION_NAME = "name"; private static final String ACTION_NAME = "name";
private static final String EXPERIMENT_IDS = "experiment_ids"; private static final String EXPERIMENT_IDS = "experiment_ids";
private static final String ONBOARDING_ACTION = "onboarding"; private static final String ONBOARDING_ACTION = "onboarding";
...@@ -62,7 +66,17 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -62,7 +66,17 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
@Override @Override
public void reportAvailableDirectActions(DirectActionReporter reporter) { public void reportAvailableDirectActions(DirectActionReporter reporter) {
if (!AutofillAssistantPreferencesUtil.isAutofillAssistantSwitchOn()) return; if (!AutofillAssistantPreferencesUtil.isAutofillAssistantSwitchOn()) {
return;
}
if (!AutofillAssistantPreferencesUtil.isAutofillOnboardingAccepted()) {
reporter.addDirectAction(ONBOARDING_ACTION)
.withParameter(ACTION_NAME, Type.STRING, /* required= */ false)
.withParameter(EXPERIMENT_IDS, Type.STRING, /* required= */ false)
.withResult(AA_ACTION_RESULT, Type.BOOLEAN);
return;
}
reporter.addDirectAction(LIST_AA_ACTIONS) reporter.addDirectAction(LIST_AA_ACTIONS)
.withParameter(USER_NAME, Type.STRING, /* required= */ false) .withParameter(USER_NAME, Type.STRING, /* required= */ false)
...@@ -72,7 +86,16 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -72,7 +86,16 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
reporter.addDirectAction(PERFORM_AA_ACTION) reporter.addDirectAction(PERFORM_AA_ACTION)
.withParameter(ACTION_NAME, Type.STRING, /* required= */ false) .withParameter(ACTION_NAME, Type.STRING, /* required= */ false)
.withParameter(EXPERIMENT_IDS, Type.STRING, /* required= */ false) .withParameter(EXPERIMENT_IDS, Type.STRING, /* required= */ false)
.withResult(PERFORM_AA_ACTION_RESULT, Type.BOOLEAN); .withResult(AA_ACTION_RESULT, Type.BOOLEAN);
// Additionally report if there are dynamic actions.
if (mDelegate != null) {
for (String action : mDelegate.getActions()) {
reporter.addDirectAction(action)
.withParameter(EXPERIMENT_IDS, Type.STRING, /* required= */ false)
.withResult(AA_ACTION_RESULT, Type.BOOLEAN);
}
}
} }
@Override @Override
...@@ -86,9 +109,20 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -86,9 +109,20 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
performAction(arguments, callback); performAction(arguments, callback);
return true; return true;
} }
// Only handle and perform the action if it is known to the controller.
if (isActionAvailable(actionId)) {
arguments.putString("name", actionId);
performAction(arguments, callback);
return true;
}
return false; return false;
} }
private boolean isActionAvailable(String actionId) {
if (mDelegate == null) return false;
return Arrays.asList(mDelegate.getActions()).contains(actionId);
}
private void listActions(Bundle arguments, Callback<Bundle> bundleCallback) { private void listActions(Bundle arguments, Callback<Bundle> bundleCallback) {
Callback<Set<String>> namesCallback = (names) -> { Callback<Set<String>> namesCallback = (names) -> {
Bundle bundle = new Bundle(); Bundle bundle = new Bundle();
...@@ -124,7 +158,7 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -124,7 +158,7 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
private void performAction(Bundle arguments, Callback<Bundle> bundleCallback) { private void performAction(Bundle arguments, Callback<Bundle> bundleCallback) {
Callback<Boolean> booleanCallback = (result) -> { Callback<Boolean> booleanCallback = (result) -> {
Bundle bundle = new Bundle(); Bundle bundle = new Bundle();
bundle.putBoolean(PERFORM_AA_ACTION_RESULT, result); bundle.putBoolean(AA_ACTION_RESULT, result);
bundleCallback.onResult(bundle); bundleCallback.onResult(bundle);
}; };
......
...@@ -229,10 +229,16 @@ void ClientAndroid::ListDirectActions( ...@@ -229,10 +229,16 @@ void ClientAndroid::ListDirectActions(
weak_ptr_factory_.GetWeakPtr(), scoped_jcallback)); weak_ptr_factory_.GetWeakPtr(), scoped_jcallback));
} }
void ClientAndroid::OnListDirectActions( base::android::ScopedJavaLocalRef<jobjectArray>
const base::android::JavaRef<jobject>& jcallback) { ClientAndroid::GetDirectActionsAsJavaArrayOfStrings(JNIEnv* env) const {
// Using a set here helps remove duplicates. // Using a set here helps remove duplicates.
std::set<std::string> names; std::set<std::string> names;
if (!controller_) {
return base::android::ToJavaArrayOfStrings(
env, std::vector<std::string>(names.begin(), names.end()));
}
for (const UserAction& user_action : controller_->GetUserActions()) { for (const UserAction& user_action : controller_->GetUserActions()) {
if (!user_action.enabled()) if (!user_action.enabled())
continue; continue;
...@@ -246,11 +252,21 @@ void ClientAndroid::OnListDirectActions( ...@@ -246,11 +252,21 @@ void ClientAndroid::OnListDirectActions(
if (ui_controller_android_) if (ui_controller_android_)
names.insert(kCancelActionName); names.insert(kCancelActionName);
return base::android::ToJavaArrayOfStrings(
env, std::vector<std::string>(names.begin(), names.end()));
}
base::android::ScopedJavaLocalRef<jobjectArray> ClientAndroid::GetDirectActions(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller) {
return GetDirectActionsAsJavaArrayOfStrings(env);
}
void ClientAndroid::OnListDirectActions(
const base::android::JavaRef<jobject>& jcallback) {
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
Java_AutofillAssistantClient_sendDirectActionList( Java_AutofillAssistantClient_sendDirectActionList(
env, java_object_, jcallback, env, java_object_, jcallback, GetDirectActionsAsJavaArrayOfStrings(env));
base::android::ToJavaArrayOfStrings(
env, std::vector<std::string>(names.begin(), names.end())));
} }
bool ClientAndroid::PerformDirectAction( bool ClientAndroid::PerformDirectAction(
......
...@@ -72,6 +72,10 @@ class ClientAndroid : public Client, ...@@ -72,6 +72,10 @@ class ClientAndroid : public Client,
const base::android::JavaParamRef<jobjectArray>& jargument_values, const base::android::JavaParamRef<jobjectArray>& jargument_values,
const base::android::JavaParamRef<jobject>& jcallback); const base::android::JavaParamRef<jobject>& jcallback);
base::android::ScopedJavaLocalRef<jobjectArray> GetDirectActions(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller);
bool PerformDirectAction( bool PerformDirectAction(
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller, const base::android::JavaParamRef<jobject>& jcaller,
...@@ -110,6 +114,9 @@ class ClientAndroid : public Client, ...@@ -110,6 +114,9 @@ class ClientAndroid : public Client,
bool NeedsUI(); bool NeedsUI();
void OnListDirectActions(const base::android::JavaRef<jobject>& jcallback); void OnListDirectActions(const base::android::JavaRef<jobject>& jcallback);
base::android::ScopedJavaLocalRef<jobjectArray>
GetDirectActionsAsJavaArrayOfStrings(JNIEnv* env) const;
// Returns the index of a direct action with that name, to pass to // Returns the index of a direct action with that name, to pass to
// UiDelegate::PerformUserAction() or -1 if not found. // UiDelegate::PerformUserAction() or -1 if not found.
int FindDirectAction(const std::string& action_name); int FindDirectAction(const std::string& action_name);
......
...@@ -20,4 +20,4 @@ const base::Feature kAutofillAssistantDirectActions{ ...@@ -20,4 +20,4 @@ const base::Feature kAutofillAssistantDirectActions{
"AutofillAssistantDirectActions", base::FEATURE_DISABLED_BY_DEFAULT}; "AutofillAssistantDirectActions", base::FEATURE_DISABLED_BY_DEFAULT};
} // namespace features } // namespace features
} // namespace autofill_assistant } // namespace autofill_assistant
\ No newline at end of file
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