[Autofill Assistant] Filter fetch_website_actions.

Before this patch, all potential direct actions were reported
unconditionally. The fetch_website_actions direct action is only
required to bring up the AA stack and fetch/check for scripts once.
This patch ties this filtering to the controller being up and ready and
the first round of scripts has been checked for matching preconditions.

This will give external callers a better signal as to when AA is up to
date and when it needs to be setup first before being able to execute
actions.

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

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

@@ -10,6 +10,7 @@ import android.os.Bundle;
 import androidx.annotation.Nullable;
 
 import org.chromium.base.Callback;
+import org.chromium.base.ThreadUtils;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.widget.ScrimView;
 import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
@@ -52,6 +53,14 @@ class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandl
         client.fetchWebsiteActions(userName, experimentIds, toArgumentMap(arguments), callback);
     }
 
+    @Override
+    public boolean hasRunFirstCheck() {
+        if (!AutofillAssistantPreferencesUtil.isAutofillOnboardingAccepted()) return false;
+        AutofillAssistantClient client = getOrCreateClient();
+        if (client == null) return false;
+        return client.hasRunFirstCheck();
+    }
+
     @Override
     public String[] getActions() {
         AutofillAssistantClient client = getOrCreateClient();
@@ -108,6 +117,7 @@ class AutofillAssistantActionHandlerImpl implements AutofillAssistantActionHandl
      */
     @Nullable
     private AutofillAssistantClient getOrCreateClient() {
+        ThreadUtils.assertOnUiThread();
         Tab tab = mGetCurrentTab.get();
 
         if (tab == null || tab.getWebContents() == null) return null;

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

@@ -13,6 +13,7 @@ import androidx.annotation.Nullable;
 
 import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
@@ -156,6 +157,17 @@ class AutofillAssistantClient {
                 arguments.values().toArray(new String[arguments.size()]), callback);
     }
 
+    /** Return true if the controller exists and is in tracking mode. */
+    public boolean hasRunFirstCheck() {
+        if (mNativeClientAndroid == 0) {
+            return false;
+        }
+
+        ThreadUtils.assertOnUiThread();
+        return AutofillAssistantClientJni.get().hasRunFirstCheck(
+                mNativeClientAndroid, AutofillAssistantClient.this);
+    }
+
     /** Lists available direct actions. */
     public String[] getDirectActions() {
         if (mNativeClientAndroid == 0) {
@@ -359,6 +371,8 @@ class AutofillAssistantClient {
         void fetchWebsiteActions(long nativeClientAndroid, AutofillAssistantClient caller,
                 String experimentIds, String[] argumentNames, String[] argumentValues,
                 Object callback);
+        boolean hasRunFirstCheck(long nativeClientAndroid, AutofillAssistantClient caller);
+
         String[] getDirectActions(long nativeClientAndroid, AutofillAssistantClient caller);
 
         boolean performDirectAction(long nativeClientAndroid, AutofillAssistantClient caller,

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

@@ -85,7 +85,7 @@ public class AutofillAssistantDirectActionHandlerTest {
         mModuleEntryProvider.setInstalled();
 
         FakeDirectActionReporter reporter = new FakeDirectActionReporter();
-        mHandler.reportAvailableDirectActions(reporter);
+        reportAvailableDirectActions(mHandler, reporter);
 
         assertEquals(1, reporter.mActions.size());
 
@@ -110,7 +110,7 @@ public class AutofillAssistantDirectActionHandlerTest {
         // Start the autofill assistant stack.
 
         FakeDirectActionReporter reporter = new FakeDirectActionReporter();
-        mHandler.reportAvailableDirectActions(reporter);
+        reportAvailableDirectActions(mHandler, reporter);
 
         assertEquals(2, reporter.mActions.size());
 
@@ -142,16 +142,15 @@ public class AutofillAssistantDirectActionHandlerTest {
         fetchWebsiteActions();
         // Reset the reported actions.
         reporter = new FakeDirectActionReporter();
-        mHandler.reportAvailableDirectActions(reporter);
+        reportAvailableDirectActions(mHandler, reporter);
 
-        assertEquals(4, reporter.mActions.size());
+        // Now that the AA stack is up, the fetdch_website_actions should no longer show up.
+        assertEquals(3, reporter.mActions.size());
 
-        // The previous two actions are still reported for now.
         assertEquals("perform_assistant_action", reporter.mActions.get(0).mId);
-        assertEquals("fetch_website_actions", reporter.mActions.get(1).mId);
 
         // Now we expect 2 dyamic actions "search" and "action2".
-        FakeDirectActionDefinition search = reporter.mActions.get(2);
+        FakeDirectActionDefinition search = reporter.mActions.get(1);
         assertEquals("search", search.mId);
         assertEquals(1, search.mParameters.size());
         assertEquals("experiment_ids", search.mParameters.get(0).mName);
@@ -160,7 +159,7 @@ public class AutofillAssistantDirectActionHandlerTest {
         assertEquals("success", search.mResults.get(0).mName);
         assertEquals(Type.BOOLEAN, search.mResults.get(0).mType);
 
-        FakeDirectActionDefinition action2 = reporter.mActions.get(3);
+        FakeDirectActionDefinition action2 = reporter.mActions.get(2);
         assertEquals("action2", action2.mId);
         assertEquals(1, action2.mParameters.size());
         assertEquals("experiment_ids", action2.mParameters.get(0).mName);
@@ -177,7 +176,7 @@ public class AutofillAssistantDirectActionHandlerTest {
         AutofillAssistantPreferencesUtil.setInitialPreferences(true);
 
         FakeDirectActionReporter reporter = new FakeDirectActionReporter();
-        mHandler.reportAvailableDirectActions(reporter);
+        reportAvailableDirectActions(mHandler, reporter);
 
         assertEquals(2, reporter.mActions.size());
 
@@ -258,9 +257,9 @@ public class AutofillAssistantDirectActionHandlerTest {
         assertEquals(Boolean.TRUE, onboardingCallback.waitForResult("accept onboarding"));
     }
 
-    private boolean isOnboardingReported() {
+    private boolean isOnboardingReported() throws Exception {
         FakeDirectActionReporter reporter = new FakeDirectActionReporter();
-        mHandler.reportAvailableDirectActions(reporter);
+        reportAvailableDirectActions(mHandler, reporter);
 
         for (FakeDirectActionDefinition definition : reporter.mActions) {
             if (definition.mId.equals("onboarding")) {
@@ -283,6 +282,18 @@ public class AutofillAssistantDirectActionHandlerTest {
         return callback.waitForResult("fetch_website_actions").getBoolean("success", false);
     }
 
+    /**
+     * When reporting direct actions involves web_contents in the controller, it needs to run on the
+     * UI thread.
+     */
+    private void reportAvailableDirectActions(
+            DirectActionHandler handler, DirectActionReporter reporter) throws Exception {
+        assertTrue(TestThreadUtils.runOnUiThreadBlocking(() -> {
+            handler.reportAvailableDirectActions(reporter);
+            return true;
+        }));
+    }
+
     /** Calls perform_assistant_action and returns the result. */
     private Boolean performAction(String name, Bundle arguments) throws Exception {
         return performActionAsync(name, arguments).waitForResult("perform_assistant_action");

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

@@ -16,16 +16,29 @@ public interface AutofillAssistantActionHandler {
      * Start fetching potential actions for websites.
      *
      * <p>This method starts AA on the current tab, if necessary, and waits for the first results.
+     * The method hasRunFirstCheck will return true when that's done. Note that {@code callback}
+     * will receive true if fetching actions was successful and the resulting set is not empty.
      * <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 experimentIds comma-separated set of experiment ids. Might be empty
      * @param arguments extra arguments to include into the RPC. Might be empty.
-     * @param callback callback to report success or failure to.
+     * @param callback callback to report when actions are available.
      */
     void fetchWebsiteActions(
             String userName, String experimentIds, Bundle arguments, Callback<Boolean> callback);
 
+    /**
+     * Returns if autofill assistant has fetched actions for the current URL.
+     *
+     * <p>This method queries the underlying Autofill Assistant stack to find out if it has been
+     * started and whether the first round of fetching scripts and checking availability has been
+     * completed.
+     *
+     * @return Whether scripts have been fetched and preconditions checked.
+     */
+    boolean hasRunFirstCheck();
+
     /**
      * Returns the available AA actions to be reported to the direct actions framework.
      *

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

@@ -10,6 +10,7 @@ import android.os.Bundle;
 import androidx.annotation.Nullable;
 
 import org.chromium.base.Callback;
+import org.chromium.base.ThreadUtils;
 import org.chromium.chrome.browser.directactions.DirectActionHandler;
 import org.chromium.chrome.browser.directactions.DirectActionReporter;
 import org.chromium.chrome.browser.directactions.DirectActionReporter.Type;
@@ -72,12 +73,13 @@ public class AutofillAssistantDirectActionHandler implements DirectActionHandler
                 .withParameter(EXPERIMENT_IDS, Type.STRING, /* required= */ false)
                 .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);
+        ThreadUtils.assertOnUiThread();
+        if (mDelegate == null || (mDelegate != null && !mDelegate.hasRunFirstCheck())) {
+            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.
         if (mDelegate != null) {

chrome/browser/android/autofill_assistant/client_android.cc

@@ -229,6 +229,12 @@ void ClientAndroid::FetchWebsiteActions(
                      weak_ptr_factory_.GetWeakPtr(), scoped_jcallback));
 }
 
+bool ClientAndroid::HasRunFirstCheck(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& jcaller) const {
+  return controller_ != nullptr && controller_->HasRunFirstCheck();
+}
+
 base::android::ScopedJavaLocalRef<jobjectArray>
 ClientAndroid::GetDirectActionsAsJavaArrayOfStrings(JNIEnv* env) const {
   // Using a set here helps remove duplicates.

chrome/browser/android/autofill_assistant/client_android.h

@@ -72,6 +72,10 @@ class ClientAndroid : public Client,
       const base::android::JavaParamRef<jobjectArray>& jargument_values,
       const base::android::JavaParamRef<jobject>& jcallback);
 
+  bool HasRunFirstCheck(
+      JNIEnv* env,
+      const base::android::JavaParamRef<jobject>& jcaller) const;
+
   base::android::ScopedJavaLocalRef<jobjectArray> GetDirectActions(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& jcaller);

components/autofill_assistant/browser/controller.cc

@@ -873,6 +873,10 @@ void Controller::Track(std::unique_ptr<TriggerContext> trigger_context,
   }
 }
 
+bool Controller::HasRunFirstCheck() const {
+  return tracking_ && has_run_first_check_;
+}
+
 bool Controller::Start(const GURL& deeplink_url,
                        std::unique_ptr<TriggerContext> trigger_context) {
   if (state_ != AutofillAssistantState::INACTIVE &&
@@ -1195,6 +1199,7 @@ void Controller::PerformDelayedShutdownIfNecessary() {
   if (delayed_shutdown_reason_ && script_domain_ != GetCurrentURL().host()) {
     Metrics::DropOutReason reason = delayed_shutdown_reason_.value();
     delayed_shutdown_reason_ = base::nullopt;
+    tracking_ = false;
     client_->Shutdown(reason);
   }
 }

components/autofill_assistant/browser/controller.h

@@ -72,6 +72,10 @@ class Controller : public ScriptExecutorDelegate,
   void Track(std::unique_ptr<TriggerContext> trigger_context,
              base::OnceCallback<void()> on_first_check_done);
 
+  // Returns true if we are in tracking mode and the first round of script
+  // checks has been completed.
+  bool HasRunFirstCheck() const;
+
   // Called when autofill assistant should start.
   //
   // This shows a UI, containing a progress bar, and executes the first

components/autofill_assistant/browser/controller_unittest.cc

@@ -1230,6 +1230,7 @@ TEST_F(ControllerTest, TrackReportsFirstSetOfScripts) {
                          base::Unretained(controller_.get()),
                          base::Unretained(&first_check_done)));
   EXPECT_FALSE(first_check_done);
+  EXPECT_FALSE(controller_->HasRunFirstCheck());
 
   ASSERT_TRUE(get_scripts_callback);
 
@@ -1240,6 +1241,7 @@ TEST_F(ControllerTest, TrackReportsFirstSetOfScripts) {
   std::move(get_scripts_callback).Run(true, response_str);
 
   EXPECT_TRUE(first_check_done);
+  EXPECT_TRUE(controller_->HasRunFirstCheck());
 }
 
 TEST_F(ControllerTest, TrackReportsNoScripts) {