[Autofill Assistant] Sending onboarding flag to server.

Before this change the server had no knowledge of whether or not the
onboarding was shown to the user in this run.
To be able to log this, a new flag is now sent to the server in the
requests for getting scripts.

Adding unit tests to check new behaviour.

Bug: b/142846407
Change-Id: Ic77bbc70c8add122beaff80a47e3cf7dca4454d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866631
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707763}

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

@@ -49,6 +49,8 @@ class AssistantOnboardingCoordinator {
     private AssistantBottomSheetContent mContent;
     private boolean mAnimate = true;
 
+    private boolean mOnboardingShown;
+
     AssistantOnboardingCoordinator(String experimentIds, Context context,
             BottomSheetController controller, @Nullable Tab tab) {
         mExperimentIds = experimentIds;
@@ -69,6 +71,7 @@ class AssistantOnboardingCoordinator {
      */
     void show(Callback<Boolean> callback) {
         AutofillAssistantMetrics.recordOnBoarding(OnBoarding.OB_SHOWN);
+        mOnboardingShown = true;
 
         if (mTab != null) {
             // If there's a tab, cover it with an overlay.
@@ -127,6 +130,14 @@ class AssistantOnboardingCoordinator {
         mAnimate = false;
     }
 
+    /**
+     * Returns {@code true} if the onboarding has been shown at the beginning when this
+     * autofill assistant flow got triggered.
+     */
+    boolean getOnboardingShown() {
+        return mOnboardingShown;
+    }
+
     /**
      * Set the content of the bottom sheet to be the Autofill Assistant onboarding.
      */

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

@@ -112,6 +112,8 @@ class AutofillAssistantClient {
                 AutofillAssistantClient.this, initialUrl, experimentIds,
                 parameters.keySet().toArray(new String[parameters.size()]),
                 parameters.values().toArray(new String[parameters.size()]), onboardingCoordinator,
+                /* onboardingShown= */
+                onboardingCoordinator != null && onboardingCoordinator.getOnboardingShown(),
                 AutofillAssistantServiceInjector.getServiceToInject());
     }
 
@@ -344,7 +346,8 @@ class AutofillAssistantClient {
         AutofillAssistantClient fromWebContents(WebContents webContents);
         boolean start(long nativeClientAndroid, AutofillAssistantClient caller, String initialUrl,
                 String experimentIds, String[] parameterNames, String[] parameterValues,
-                @Nullable AssistantOnboardingCoordinator onboardingCoordinator, long nativeService);
+                @Nullable AssistantOnboardingCoordinator onboardingCoordinator,
+                boolean onboardingShown, long nativeService);
         void onAccessToken(long nativeClientAndroid, AutofillAssistantClient caller,
                 boolean success, String accessToken);
         String getPrimaryAccountName(long nativeClientAndroid, AutofillAssistantClient caller);

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

@@ -157,6 +157,17 @@ public class AssistantOnboardingCoordinatorTest {
                 instanceOf(AssistantBottomSheetContent.class));
     }
 
+    @Test
+    @MediumTest
+    @DisableIf.Build(sdk_is_greater_than = 22) // TODO(crbug/991938): re-enable
+    public void testShownFlag() throws Exception {
+        AssistantOnboardingCoordinator coordinator = createCoordinator(/* tab= */ null);
+        assertFalse(coordinator.getOnboardingShown());
+
+        showOnboardingAndWait(coordinator, mCallback);
+        assertTrue(coordinator.getOnboardingShown());
+    }
+
     /** Trigger onboarding and wait until it is fully displayed. */
     private void showOnboardingAndWait(
             AssistantOnboardingCoordinator coordinator, Callback<Boolean> callback) {

chrome/browser/android/autofill_assistant/client_android.cc

@@ -131,6 +131,7 @@ bool ClientAndroid::Start(JNIEnv* env,
                           const JavaParamRef<jobjectArray>& parameter_names,
                           const JavaParamRef<jobjectArray>& parameter_values,
                           const JavaParamRef<jobject>& jonboarding_coordinator,
+                          jboolean jonboarding_shown,
                           jlong jservice) {
   // When Start() is called, AA_START should have been measured. From now on,
   // the client is responsible for keeping track of dropouts, so that for each
@@ -151,6 +152,7 @@ bool ClientAndroid::Start(JNIEnv* env,
   std::unique_ptr<TriggerContextImpl> trigger_context = CreateTriggerContext(
       env, jexperiment_ids, parameter_names, parameter_values);
   trigger_context->SetCCT(true);
+  trigger_context->SetOnboardingShown(jonboarding_shown);
 
   GURL initial_url(base::android::ConvertJavaStringToUTF8(env, jinitial_url));
   return controller_->Start(initial_url, std::move(trigger_context));

chrome/browser/android/autofill_assistant/client_android.h

@@ -47,6 +47,7 @@ class ClientAndroid : public Client,
       const base::android::JavaParamRef<jobjectArray>& parameter_names,
       const base::android::JavaParamRef<jobjectArray>& parameter_values,
       const base::android::JavaParamRef<jobject>& jonboarding_coordinator,
+      jboolean jonboarding_shown,
       jlong jservice);
   void DestroyUI(JNIEnv* env,
                  const base::android::JavaParamRef<jobject>& jcaller);

components/autofill_assistant/browser/protocol_utils.cc

@@ -52,6 +52,9 @@ void FillClientContext(const ClientContextProto& client_context,
   if (trigger_context.is_cct()) {
     proto->set_is_cct(true);
   }
+  if (trigger_context.is_onboarding_shown()) {
+    proto->set_is_onboarding_shown(true);
+  }
   if (trigger_context.is_direct_action()) {
     proto->set_is_direct_action(true);
   }

components/autofill_assistant/browser/protocol_utils_unittest.cc

@@ -98,6 +98,7 @@ TEST(ProtocolUtilsTest, CreateInitialScriptActionsRequest) {
   AssertClientContext(request.client_context());
   EXPECT_THAT(request.client_context().experiment_ids(), Eq("1,2,3"));
   EXPECT_TRUE(request.client_context().is_cct());
+  EXPECT_FALSE(request.client_context().is_onboarding_shown());
   EXPECT_FALSE(request.client_context().is_direct_action());
 
   const InitialScriptActionsRequestProto& initial = request.initial_request();
@@ -112,6 +113,41 @@ TEST(ProtocolUtilsTest, CreateInitialScriptActionsRequest) {
   EXPECT_EQ("script_payload", request.script_payload());
 }
 
+TEST(ProtocolUtilsTest, TestCreateInitialScriptActionsRequestFlags) {
+  std::map<std::string, std::string> parameters;
+
+  ScriptActionRequestProto request;
+
+  // With flags.
+  TriggerContextImpl trigger_context_flags(parameters, std::string());
+  trigger_context_flags.SetCCT(true);
+  trigger_context_flags.SetOnboardingShown(true);
+  trigger_context_flags.SetDirectAction(true);
+
+  EXPECT_TRUE(
+      request.ParseFromString(ProtocolUtils::CreateInitialScriptActionsRequest(
+          "script_path", GURL("http://example.com/"), trigger_context_flags,
+          "global_payload", "script_payload", CreateClientContextProto())));
+
+  AssertClientContext(request.client_context());
+  EXPECT_TRUE(request.client_context().is_cct());
+  EXPECT_TRUE(request.client_context().is_onboarding_shown());
+  EXPECT_TRUE(request.client_context().is_direct_action());
+
+  // Without flags.
+  TriggerContextImpl trigger_context_no_flags(parameters, std::string());
+
+  EXPECT_TRUE(
+      request.ParseFromString(ProtocolUtils::CreateInitialScriptActionsRequest(
+          "script_path", GURL("http://example.com/"), trigger_context_no_flags,
+          "global_payload", "script_payload", CreateClientContextProto())));
+
+  AssertClientContext(request.client_context());
+  EXPECT_FALSE(request.client_context().is_cct());
+  EXPECT_FALSE(request.client_context().is_onboarding_shown());
+  EXPECT_FALSE(request.client_context().is_direct_action());
+}
+
 TEST(ProtocolUtilsTest, CreateNextScriptActionsRequest) {
   std::map<std::string, std::string> parameters;
   parameters["a"] = "b";
@@ -131,6 +167,44 @@ TEST(ProtocolUtilsTest, CreateNextScriptActionsRequest) {
   EXPECT_EQ(1, request.next_request().processed_actions().size());
 }
 
+TEST(ProtocolUtilsTest, TestCreateNextScriptActionsRequestFlags) {
+  std::map<std::string, std::string> parameters;
+
+  std::vector<ProcessedActionProto> processed_actions;
+  processed_actions.emplace_back(ProcessedActionProto());
+
+  ScriptActionRequestProto request;
+
+  // With flags.
+  TriggerContextImpl trigger_context_flags(parameters, std::string());
+  trigger_context_flags.SetCCT(true);
+  trigger_context_flags.SetOnboardingShown(true);
+  trigger_context_flags.SetDirectAction(true);
+
+  EXPECT_TRUE(
+      request.ParseFromString(ProtocolUtils::CreateNextScriptActionsRequest(
+          trigger_context_flags, "global_payload", "script_payload",
+          processed_actions, CreateClientContextProto())));
+
+  AssertClientContext(request.client_context());
+  EXPECT_TRUE(request.client_context().is_cct());
+  EXPECT_TRUE(request.client_context().is_onboarding_shown());
+  EXPECT_TRUE(request.client_context().is_direct_action());
+
+  // Without flags.
+  TriggerContextImpl trigger_context_no_flags(parameters, std::string());
+
+  EXPECT_TRUE(
+      request.ParseFromString(ProtocolUtils::CreateNextScriptActionsRequest(
+          trigger_context_no_flags, "global_payload", "script_payload",
+          processed_actions, CreateClientContextProto())));
+
+  AssertClientContext(request.client_context());
+  EXPECT_FALSE(request.client_context().is_cct());
+  EXPECT_FALSE(request.client_context().is_onboarding_shown());
+  EXPECT_FALSE(request.client_context().is_direct_action());
+}
+
 TEST(ProtocolUtilsTest, CreateGetScriptsRequest) {
   std::map<std::string, std::string> parameters;
   parameters["a"] = "b";
@@ -146,6 +220,7 @@ TEST(ProtocolUtilsTest, CreateGetScriptsRequest) {
   AssertClientContext(request.client_context());
   EXPECT_THAT(request.client_context().experiment_ids(), Eq("1,2,3"));
   EXPECT_FALSE(request.client_context().is_cct());
+  EXPECT_FALSE(request.client_context().is_onboarding_shown());
   EXPECT_TRUE(request.client_context().is_direct_action());
 
   EXPECT_EQ("http://example.com/", request.url());
@@ -156,6 +231,38 @@ TEST(ProtocolUtilsTest, CreateGetScriptsRequest) {
   EXPECT_EQ("d", request.script_parameters(1).value());
 }
 
+TEST(ProtocolUtilsTest, TestCreateGetScriptsRequestFlags) {
+  std::map<std::string, std::string> parameters;
+  SupportsScriptRequestProto request;
+
+  // With flags.
+  TriggerContextImpl trigger_context_flags(parameters, std::string());
+  trigger_context_flags.SetCCT(true);
+  trigger_context_flags.SetOnboardingShown(true);
+  trigger_context_flags.SetDirectAction(true);
+
+  EXPECT_TRUE(request.ParseFromString(ProtocolUtils::CreateGetScriptsRequest(
+      GURL("http://example.com/"), trigger_context_flags,
+      CreateClientContextProto())));
+
+  AssertClientContext(request.client_context());
+  EXPECT_TRUE(request.client_context().is_cct());
+  EXPECT_TRUE(request.client_context().is_onboarding_shown());
+  EXPECT_TRUE(request.client_context().is_direct_action());
+
+  // Without flags.
+  TriggerContextImpl trigger_context_no_flags(parameters, std::string());
+
+  EXPECT_TRUE(request.ParseFromString(ProtocolUtils::CreateGetScriptsRequest(
+      GURL("http://example.com/"), trigger_context_no_flags,
+      CreateClientContextProto())));
+
+  AssertClientContext(request.client_context());
+  EXPECT_FALSE(request.client_context().is_cct());
+  EXPECT_FALSE(request.client_context().is_onboarding_shown());
+  EXPECT_FALSE(request.client_context().is_direct_action());
+}
+
 TEST(ProtocolUtilsTest, AddScriptIgnoreInvalid) {
   SupportedScriptProto script_proto;
   std::vector<std::unique_ptr<Script>> scripts;

components/autofill_assistant/browser/service.proto

@@ -42,6 +42,9 @@ message ClientContextProto {
   // Assistant.
   optional bool is_cct = 8;
 
+  // True if the onboarding screen was shown to the user.
+  optional bool is_onboarding_shown = 10;
+
   // True if the script was triggered by a direct action.
   optional bool is_direct_action = 9;
 }

components/autofill_assistant/browser/trigger_context.cc

@@ -62,6 +62,10 @@ bool TriggerContextImpl::is_cct() const {
   return cct_;
 }
 
+bool TriggerContextImpl::is_onboarding_shown() const {
+  return onboarding_shown_;
+}
+
 bool TriggerContextImpl::is_direct_action() const {
   return direct_action_;
 }
@@ -112,6 +116,14 @@ bool MergedTriggerContext::is_cct() const {
   return false;
 }
 
+bool MergedTriggerContext::is_onboarding_shown() const {
+  for (const TriggerContext* context : contexts_) {
+    if (context->is_onboarding_shown())
+      return true;
+  }
+  return false;
+}
+
 bool MergedTriggerContext::is_direct_action() const {
   for (const TriggerContext* context : contexts_) {
     if (context->is_direct_action())

components/autofill_assistant/browser/trigger_context.h

@@ -53,6 +53,10 @@ class TriggerContext {
   // Java.
   virtual bool is_cct() const = 0;
 
+  // Returns true if the onboarding was shown at the beginning when this
+  // autofill assistant flow got triggered.
+  virtual bool is_onboarding_shown() const = 0;
+
   // Returns true if the current action was triggered by a direct action.
   virtual bool is_direct_action() const = 0;
 };
@@ -76,10 +80,12 @@ class TriggerContextImpl : public TriggerContext {
       const std::string& name) const override;
 
   void SetCCT(bool value) { cct_ = value; }
+  void SetOnboardingShown(bool value) { onboarding_shown_ = value; }
   void SetDirectAction(bool value) { direct_action_ = value; }
 
   std::string experiment_ids() const override;
   bool is_cct() const override;
+  bool is_onboarding_shown() const override;
   bool is_direct_action() const override;
 
  private:
@@ -93,6 +99,8 @@ class TriggerContextImpl : public TriggerContext {
   bool cct_ = false;
 
   bool direct_action_ = false;
+
+  bool onboarding_shown_ = false;
 };
 
 // Merges several TriggerContexts together.
@@ -110,6 +118,7 @@ class MergedTriggerContext : public TriggerContext {
       const std::string& name) const override;
   std::string experiment_ids() const override;
   bool is_cct() const override;
+  bool is_onboarding_shown() const override;
   bool is_direct_action() const override;
 
  private:

components/autofill_assistant/browser/trigger_context_unittest.cc

@@ -102,6 +102,28 @@ TEST(TriggerContextText, MergeCCT) {
   EXPECT_TRUE(one_with_cct->is_cct());
 }
 
+TEST(TriggerContextTest, OnboardingShown) {
+  TriggerContextImpl context;
+
+  EXPECT_FALSE(context.is_onboarding_shown());
+  context.SetOnboardingShown(true);
+  EXPECT_TRUE(context.is_onboarding_shown());
+}
+
+TEST(TriggerContextTest, MergeOnboardingShown) {
+  auto empty = TriggerContext::CreateEmpty();
+
+  auto all_empty = TriggerContext::Merge({empty.get(), empty.get()});
+  EXPECT_FALSE(all_empty->is_onboarding_shown());
+
+  TriggerContextImpl onboarding_context;
+  onboarding_context.SetOnboardingShown(true);
+  auto one_with_onboarding =
+      TriggerContext::Merge({empty.get(), &onboarding_context, empty.get()});
+
+  EXPECT_TRUE(one_with_onboarding->is_onboarding_shown());
+}
+
 TEST(TriggerContextText, DirectAction) {
   TriggerContextImpl context;