Commit e5665d2d authored by Mathias Carlen's avatar Mathias Carlen Committed by Commit Bot

[Autofill Assistant] Rename list action to fetch.

Before this patch the direct action list_assistant_action was used to
start autofill assistant and to return available actions. That is
misleading and this patch disconnects that by renaming
list_assistant_action to fetch_website_actions and return a success
bool instead of a list of actions.

Out of scope for this patch: the next step will be to tie the
availability of the fetch_website_actions action to controller state
and to remove perform_assistant_action entirely.

Bug: b/138833619
Change-Id: I279e5bc2e6d13a48151d5c161483a2b9662dd940
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1873752
Commit-Queue: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: default avatarStephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709832}
parent 5fc55e2b
...@@ -14,10 +14,8 @@ import org.chromium.chrome.browser.tab.Tab; ...@@ -14,10 +14,8 @@ 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.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
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.
...@@ -39,19 +37,19 @@ class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandl ...@@ -39,19 +37,19 @@ class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandl
} }
@Override @Override
public void listActions(String userName, String experimentIds, Bundle arguments, public void fetchWebsiteActions(
Callback<Set<String>> callback) { String userName, String experimentIds, Bundle arguments, Callback<Boolean> callback) {
if (!AutofillAssistantPreferencesUtil.isAutofillOnboardingAccepted()) { if (!AutofillAssistantPreferencesUtil.isAutofillOnboardingAccepted()) {
callback.onResult(Collections.emptySet()); callback.onResult(false);
return; return;
} }
AutofillAssistantClient client = getOrCreateClient(); AutofillAssistantClient client = getOrCreateClient();
if (client == null) { if (client == null) {
callback.onResult(Collections.emptySet()); callback.onResult(false);
return; return;
} }
client.listDirectActions(userName, experimentIds, toArgumentMap(arguments), callback); client.fetchWebsiteActions(userName, experimentIds, toArgumentMap(arguments), callback);
} }
@Override @Override
......
...@@ -21,11 +21,8 @@ import org.chromium.components.signin.AccountManagerFacade; ...@@ -21,11 +21,8 @@ import org.chromium.components.signin.AccountManagerFacade;
import org.chromium.components.signin.OAuth2TokenService; import org.chromium.components.signin.OAuth2TokenService;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import java.util.Collections;
import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set;
/** /**
* An Autofill Assistant client, associated with a specific WebContents. * An Autofill Assistant client, associated with a specific WebContents.
...@@ -141,11 +138,11 @@ class AutofillAssistantClient { ...@@ -141,11 +138,11 @@ class AutofillAssistantClient {
mNativeClientAndroid, AutofillAssistantClient.this, otherWebContents); mNativeClientAndroid, AutofillAssistantClient.this, otherWebContents);
} }
/** Lists available direct actions. */ /** Starts the controller and fetching scripts for websites. */
public void listDirectActions(String userName, String experimentIds, public void fetchWebsiteActions(String userName, String experimentIds,
Map<String, String> arguments, Callback<Set<String>> callback) { Map<String, String> arguments, Callback<Boolean> callback) {
if (mNativeClientAndroid == 0) { if (mNativeClientAndroid == 0) {
callback.onResult(Collections.emptySet()); callback.onResult(false);
return; return;
} }
...@@ -153,7 +150,7 @@ class AutofillAssistantClient { ...@@ -153,7 +150,7 @@ class AutofillAssistantClient {
// The native side calls sendDirectActionList() on the callback once the controller has // The native side calls sendDirectActionList() on the callback once the controller has
// results. // results.
AutofillAssistantClientJni.get().listDirectActions(mNativeClientAndroid, AutofillAssistantClientJni.get().fetchWebsiteActions(mNativeClientAndroid,
AutofillAssistantClient.this, experimentIds, AutofillAssistantClient.this, experimentIds,
arguments.keySet().toArray(new String[arguments.size()]), arguments.keySet().toArray(new String[arguments.size()]),
arguments.values().toArray(new String[arguments.size()]), callback); arguments.values().toArray(new String[arguments.size()]), callback);
...@@ -184,7 +181,7 @@ class AutofillAssistantClient { ...@@ -184,7 +181,7 @@ class AutofillAssistantClient {
@Nullable AssistantOnboardingCoordinator onboardingCoordinator) { @Nullable AssistantOnboardingCoordinator onboardingCoordinator) {
if (mNativeClientAndroid == 0) return false; if (mNativeClientAndroid == 0) return false;
// Note that only listDirectActions can start AA, so only it needs // Note that only fetchWebsiteActions can start AA, so only it needs
// chooseAccountAsyncIfNecessary. // chooseAccountAsyncIfNecessary.
return AutofillAssistantClientJni.get().performDirectAction(mNativeClientAndroid, return AutofillAssistantClientJni.get().performDirectAction(mNativeClientAndroid,
AutofillAssistantClient.this, actionId, experimentIds, AutofillAssistantClient.this, actionId, experimentIds,
...@@ -337,12 +334,8 @@ class AutofillAssistantClient { ...@@ -337,12 +334,8 @@ class AutofillAssistantClient {
/** Adds a dynamic action to the given reporter. */ /** Adds a dynamic action to the given reporter. */
@CalledByNative @CalledByNative
private void sendDirectActionList(Callback<Set<String>> callback, String[] names) { private void onFetchWebsiteActions(Callback<Boolean> callback, boolean success) {
Set<String> set = new HashSet<>(); callback.onResult(success);
for (String name : names) {
set.add(name);
}
callback.onResult(set);
} }
@CalledByNative @CalledByNative
...@@ -363,7 +356,7 @@ class AutofillAssistantClient { ...@@ -363,7 +356,7 @@ class AutofillAssistantClient {
void destroyUI(long nativeClientAndroid, AutofillAssistantClient caller); void destroyUI(long nativeClientAndroid, AutofillAssistantClient caller);
void transferUITo( void transferUITo(
long nativeClientAndroid, AutofillAssistantClient caller, Object otherWebContents); long nativeClientAndroid, AutofillAssistantClient caller, Object otherWebContents);
void listDirectActions(long nativeClientAndroid, AutofillAssistantClient caller, void fetchWebsiteActions(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); String[] getDirectActions(long nativeClientAndroid, AutofillAssistantClient caller);
......
...@@ -8,25 +8,23 @@ import android.os.Bundle; ...@@ -8,25 +8,23 @@ import android.os.Bundle;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import java.util.Set;
/** /**
* Interface that provides implementation for AA actions, triggered by direct actions. * Interface that provides implementation for AA actions, triggered by direct actions.
*/ */
public interface AutofillAssistantActionHandler { public interface AutofillAssistantActionHandler {
/** /**
* Returns the names of available AA actions. * Start fetching potential actions for websites.
* *
* <p>This method starts AA on the current tab, if necessary, and waits for the first results. * <p>This method starts AA on the current tab, if necessary, and waits for the first results.
* Once AA is started, the results are reported immediately. * <p>This method needs to be called before getActions below will return anything.
* *
* @param userName name of the user to use when sending RPCs. Might be empty. * @param userName name of the user to use when sending RPCs. Might be empty.
* @param experimentIds comma-separated set of experiment ids. Might be empty * @param experimentIds comma-separated set of experiment ids. Might be empty
* @param arguments extra arguments to include into the RPC. Might be empty. * @param arguments extra arguments to include into the RPC. Might be empty.
* @param callback callback to report the result to * @param callback callback to report success or failure to.
*/ */
void listActions(String userName, String experimentIds, Bundle arguments, void fetchWebsiteActions(
Callback<Set<String>> callback); String userName, String experimentIds, Bundle arguments, Callback<Boolean> callback);
/** /**
* Returns the available AA actions to be reported to the direct actions framework. * Returns the available AA actions to be reported to the direct actions framework.
......
...@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.autofill_assistant; ...@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.autofill_assistant;
import android.content.Context; import android.content.Context;
import android.os.Bundle; import android.os.Bundle;
import android.text.TextUtils;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
...@@ -19,18 +18,14 @@ import org.chromium.chrome.browser.widget.ScrimView; ...@@ -19,18 +18,14 @@ 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.Arrays;
import java.util.Collections;
import java.util.Set;
/** /**
* A handler that provides just enough functionality to allow on-demand loading of the module * 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. * 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 private static final String FETCH_WEBSITE_ACTIONS = "fetch_website_actions";
// controller / dynamic actions stack. private static final String FETCH_WEBSITE_ACTIONS_RESULT = "success";
private static final String LIST_AA_ACTIONS = "list_assistant_actions";
private static final String LIST_AA_ACTIONS_RESULT = "names";
// TODO(b/138833619): Remove this action entirely. // 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 AA_ACTION_RESULT = "success"; private static final String AA_ACTION_RESULT = "success";
...@@ -39,12 +34,6 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -39,12 +34,6 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
private static final String ONBOARDING_ACTION = "onboarding"; private static final String ONBOARDING_ACTION = "onboarding";
private static final String USER_NAME = "user_name"; private static final String USER_NAME = "user_name";
/**
* Set of actions to report when AA is not available, because of preference or
* lack of DFM.
*/
private static final Set<String> FALLBACK_ACTION_SET = Collections.singleton(ONBOARDING_ACTION);
private final Context mContext; private final Context mContext;
private final BottomSheetController mBottomSheetController; private final BottomSheetController mBottomSheetController;
private final ScrimView mScrimView; private final ScrimView mScrimView;
...@@ -78,16 +67,18 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -78,16 +67,18 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
return; return;
} }
reporter.addDirectAction(LIST_AA_ACTIONS)
.withParameter(USER_NAME, Type.STRING, /* required= */ false)
.withParameter(EXPERIMENT_IDS, Type.STRING, /* required= */ false)
.withResult(LIST_AA_ACTIONS_RESULT, Type.STRING);
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(AA_ACTION_RESULT, Type.BOOLEAN); .withResult(AA_ACTION_RESULT, Type.BOOLEAN);
// TODO(b/138833619): Only report this action when the controller has not been started
// yet or if we navigated to another domain in the mean time.
reporter.addDirectAction(FETCH_WEBSITE_ACTIONS)
.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. // Additionally report if there are dynamic actions.
if (mDelegate != null) { if (mDelegate != null) {
for (String action : mDelegate.getActions()) { for (String action : mDelegate.getActions()) {
...@@ -101,8 +92,8 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -101,8 +92,8 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
@Override @Override
public boolean performDirectAction( public boolean performDirectAction(
String actionId, Bundle arguments, Callback<Bundle> callback) { String actionId, Bundle arguments, Callback<Bundle> callback) {
if (actionId.equals(LIST_AA_ACTIONS)) { if (actionId.equals(FETCH_WEBSITE_ACTIONS)) {
listActions(arguments, callback); fetchWebsiteActions(arguments, callback);
return true; return true;
} }
if (actionId.equals(PERFORM_AA_ACTION)) { if (actionId.equals(PERFORM_AA_ACTION)) {
...@@ -110,7 +101,7 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -110,7 +101,7 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
return true; return true;
} }
// Only handle and perform the action if it is known to the controller. // Only handle and perform the action if it is known to the controller.
if (isActionAvailable(actionId)) { if (isActionAvailable(actionId) || ONBOARDING_ACTION.equals(actionId)) {
arguments.putString("name", actionId); arguments.putString("name", actionId);
performAction(arguments, callback); performAction(arguments, callback);
return true; return true;
...@@ -123,20 +114,20 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -123,20 +114,20 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
return Arrays.asList(mDelegate.getActions()).contains(actionId); return Arrays.asList(mDelegate.getActions()).contains(actionId);
} }
private void listActions(Bundle arguments, Callback<Bundle> bundleCallback) { private void fetchWebsiteActions(Bundle arguments, Callback<Bundle> bundleCallback) {
Callback<Set<String>> namesCallback = (names) -> { Callback<Boolean> successCallback = (success) -> {
Bundle bundle = new Bundle(); Bundle bundle = new Bundle();
bundle.putString(LIST_AA_ACTIONS_RESULT, TextUtils.join(",", names)); bundle.putBoolean(FETCH_WEBSITE_ACTIONS_RESULT, success);
bundleCallback.onResult(bundle); bundleCallback.onResult(bundle);
}; };
if (!AutofillAssistantPreferencesUtil.isAutofillAssistantSwitchOn()) { if (!AutofillAssistantPreferencesUtil.isAutofillAssistantSwitchOn()) {
namesCallback.onResult(Collections.emptySet()); successCallback.onResult(false);
return; return;
} }
if (!AutofillAssistantPreferencesUtil.isAutofillOnboardingAccepted()) { if (!AutofillAssistantPreferencesUtil.isAutofillOnboardingAccepted()) {
namesCallback.onResult(FALLBACK_ACTION_SET); successCallback.onResult(false);
return; return;
} }
...@@ -148,10 +139,10 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -148,10 +139,10 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
getDelegate(/* installIfNecessary= */ false, (delegate) -> { getDelegate(/* installIfNecessary= */ false, (delegate) -> {
if (delegate == null) { if (delegate == null) {
namesCallback.onResult(FALLBACK_ACTION_SET); successCallback.onResult(false);
return; return;
} }
delegate.listActions(userName, experimentIds, arguments, namesCallback); delegate.fetchWebsiteActions(userName, experimentIds, arguments, successCallback);
}); });
} }
...@@ -182,7 +173,11 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler ...@@ -182,7 +173,11 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
delegate.performOnboarding(experimentIds, booleanCallback); delegate.performOnboarding(experimentIds, booleanCallback);
return; return;
} }
delegate.performAction(name, experimentIds, arguments, booleanCallback);
Callback<Boolean> successCallback = (success) -> {
booleanCallback.onResult(success && delegate.getActions().length != 0);
};
delegate.performAction(name, experimentIds, arguments, successCallback);
}); });
} }
......
...@@ -211,7 +211,7 @@ void ClientAndroid::OnAccessToken(JNIEnv* env, ...@@ -211,7 +211,7 @@ void ClientAndroid::OnAccessToken(JNIEnv* env,
} }
} }
void ClientAndroid::ListDirectActions( void ClientAndroid::FetchWebsiteActions(
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller, const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jstring>& jexperiment_ids, const base::android::JavaParamRef<jstring>& jexperiment_ids,
...@@ -225,7 +225,7 @@ void ClientAndroid::ListDirectActions( ...@@ -225,7 +225,7 @@ void ClientAndroid::ListDirectActions(
controller_->Track( controller_->Track(
CreateTriggerContext(env, jexperiment_ids, jargument_names, CreateTriggerContext(env, jexperiment_ids, jargument_names,
jargument_values), jargument_values),
base::BindOnce(&ClientAndroid::OnListDirectActions, base::BindOnce(&ClientAndroid::OnFetchWebsiteActions,
weak_ptr_factory_.GetWeakPtr(), scoped_jcallback)); weak_ptr_factory_.GetWeakPtr(), scoped_jcallback));
} }
...@@ -262,11 +262,11 @@ base::android::ScopedJavaLocalRef<jobjectArray> ClientAndroid::GetDirectActions( ...@@ -262,11 +262,11 @@ base::android::ScopedJavaLocalRef<jobjectArray> ClientAndroid::GetDirectActions(
return GetDirectActionsAsJavaArrayOfStrings(env); return GetDirectActionsAsJavaArrayOfStrings(env);
} }
void ClientAndroid::OnListDirectActions( void ClientAndroid::OnFetchWebsiteActions(
const base::android::JavaRef<jobject>& jcallback) { const base::android::JavaRef<jobject>& jcallback) {
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
Java_AutofillAssistantClient_sendDirectActionList( Java_AutofillAssistantClient_onFetchWebsiteActions(
env, java_object_, jcallback, GetDirectActionsAsJavaArrayOfStrings(env)); env, java_object_, jcallback, controller_ != nullptr);
} }
bool ClientAndroid::PerformDirectAction( bool ClientAndroid::PerformDirectAction(
...@@ -307,7 +307,7 @@ bool ClientAndroid::PerformDirectAction( ...@@ -307,7 +307,7 @@ bool ClientAndroid::PerformDirectAction(
int ClientAndroid::FindDirectAction(const std::string& action_name) { int ClientAndroid::FindDirectAction(const std::string& action_name) {
// It's too late to create a controller. This should have been done in // It's too late to create a controller. This should have been done in
// ListDirectActions. // FetchWebsiteActions.
if (!controller_) if (!controller_)
return -1; return -1;
......
...@@ -64,7 +64,7 @@ class ClientAndroid : public Client, ...@@ -64,7 +64,7 @@ class ClientAndroid : public Client,
jboolean success, jboolean success,
const base::android::JavaParamRef<jstring>& access_token); const base::android::JavaParamRef<jstring>& access_token);
void ListDirectActions( void FetchWebsiteActions(
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller, const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jstring>& jexperiment_ids, const base::android::JavaParamRef<jstring>& jexperiment_ids,
...@@ -112,7 +112,7 @@ class ClientAndroid : public Client, ...@@ -112,7 +112,7 @@ class ClientAndroid : public Client,
void AttachUI( void AttachUI(
const base::android::JavaParamRef<jobject>& jonboarding_coordinator); const base::android::JavaParamRef<jobject>& jonboarding_coordinator);
bool NeedsUI(); bool NeedsUI();
void OnListDirectActions(const base::android::JavaRef<jobject>& jcallback); void OnFetchWebsiteActions(const base::android::JavaRef<jobject>& jcallback);
base::android::ScopedJavaLocalRef<jobjectArray> base::android::ScopedJavaLocalRef<jobjectArray>
GetDirectActionsAsJavaArrayOfStrings(JNIEnv* env) const; GetDirectActionsAsJavaArrayOfStrings(JNIEnv* env) const;
......
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