Set query source in AssistantInteractionMetadata.

Query source should be set in |AssistantInteractionMetadata| so that we
have the most accurate information possible when setting the pending
query source in |AssistantInteractionController::OnInteractionStarted|.

This is a follow up to:
https://chromium-review.googlesource.com/c/chromium/src/+/1869811

Bug: None
Test: Built locally
Change-Id: I18280710fc48d623c303453ab860c0b0331f071a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1870989
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708262}

ash/assistant/assistant_interaction_controller.cc

@@ -408,7 +408,7 @@ void AssistantInteractionController::OnInteractionStarted(
     // pending query type will always be |kNull| here.
     if (model_.pending_query().type() == AssistantQueryType::kNull) {
       model_.SetPendingQuery(std::make_unique<AssistantTextQuery>(
-          metadata->query, AssistantQuerySource::kLibAssistantInitiated));
+          metadata->query, metadata->source));
     }
     model_.CommitPendingQuery();
     model_.SetMicState(MicState::kClosed);
@@ -878,7 +878,8 @@ void AssistantInteractionController::StartProactiveSuggestionsInteraction(
       description, AssistantQuerySource::kProactiveSuggestions));
 
   OnInteractionStarted(AssistantInteractionMetadata::New(
-      AssistantInteractionType::kText, /*query=*/description));
+      AssistantInteractionType::kText,
+      AssistantQuerySource::kProactiveSuggestions, /*query=*/description));
 
   OnHtmlResponse(proactive_suggestions->html(), /*fallback=*/std::string());
 
@@ -913,7 +914,7 @@ void AssistantInteractionController::StartTextInteraction(
   model_.SetPendingQuery(
       std::make_unique<AssistantTextQuery>(text, query_source));
 
-  assistant_->StartTextInteraction(text, allow_tts);
+  assistant_->StartTextInteraction(text, query_source, allow_tts);
 }
 
 void AssistantInteractionController::StartVoiceInteraction() {

ash/assistant/assistant_interaction_controller.h

@@ -44,6 +44,7 @@ class AssistantInteractionController
       chromeos::assistant::mojom::AssistantInteractionResolution;
   using AssistantInteractionType =
       chromeos::assistant::mojom::AssistantInteractionType;
+  using AssistantQuerySource = chromeos::assistant::mojom::AssistantQuerySource;
   using AssistantSuggestion = chromeos::assistant::mojom::AssistantSuggestion;
   using AssistantSuggestionPtr =
       chromeos::assistant::mojom::AssistantSuggestionPtr;

ash/assistant/model/assistant_query.h

@@ -9,6 +9,7 @@
 
 #include "base/component_export.h"
 #include "base/macros.h"
+#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 
 namespace ash {
 
@@ -21,26 +22,13 @@ enum class AssistantQueryType {
   kVoice,  // See AssistantVoiceQuery.
 };
 
-// Defines possible source of an Assistant query. These values are persisted
-// to logs. Entries should not be renumbered and numeric values should never
-// be reused. Only append to this enum is allowed if the possible source grows.
-enum class AssistantQuerySource {
-  kUnspecified = 0,
-  kDeepLink = 1,
-  kDialogPlateTextField = 2,
-  kStylus = 3,
-  kSuggestionChip = 4,
-  kVoiceInput = 5,
-  kProactiveSuggestions = 6,
-  kLibAssistantInitiated = 7,
-  kMaxValue = kLibAssistantInitiated
-};
-
 // AssistantQuery --------------------------------------------------------------
 
 // Base class for an Assistant query.
 class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantQuery {
  public:
+  using AssistantQuerySource = chromeos::assistant::mojom::AssistantQuerySource;
+
   virtual ~AssistantQuery() = default;
 
   // Returns the type for the query.
@@ -58,7 +46,6 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantQuery {
 
  private:
   const AssistantQueryType type_;
-
   const AssistantQuerySource source_;
 
   DISALLOW_COPY_AND_ASSIGN(AssistantQuery);

ash/assistant/test/test_assistant_service.cc

@@ -199,9 +199,11 @@ void TestAssistantService ::StartMetalayerInteraction(const gfx::Rect& region) {
   NOTIMPLEMENTED_LOG_ONCE();
 }
 
-void TestAssistantService ::StartTextInteraction(const std::string& query,
-                                                 bool allow_tts) {
-  StartInteraction(AssistantInteractionType::kText, query);
+void TestAssistantService ::StartTextInteraction(
+    const std::string& query,
+    chromeos::assistant::mojom::AssistantQuerySource source,
+    bool allow_tts) {
+  StartInteraction(AssistantInteractionType::kText, source, query);
   SendInteractionResponse();
 }
 
@@ -265,10 +267,11 @@ void TestAssistantService::CreateTimer(base::TimeDelta duration) {
 
 void TestAssistantService::StartInteraction(
     chromeos::assistant::mojom::AssistantInteractionType type,
+    chromeos::assistant::mojom::AssistantQuerySource source,
     const std::string& query) {
   for (auto& subscriber : interaction_subscribers_) {
     subscriber->OnInteractionStarted(
-        AssistantInteractionMetadata::New(type, query));
+        AssistantInteractionMetadata::New(type, source, query));
   }
 }
 

ash/assistant/test/test_assistant_service.h

@@ -81,7 +81,10 @@ class TestAssistantService : public chromeos::assistant::mojom::Assistant {
   void StartCachedScreenContextInteraction() override;
   void StartEditReminderInteraction(const std::string& client_id) override;
   void StartMetalayerInteraction(const gfx::Rect& region) override;
-  void StartTextInteraction(const std::string& query, bool allow_tts) override;
+  void StartTextInteraction(
+      const std::string& query,
+      chromeos::assistant::mojom::AssistantQuerySource source,
+      bool allow_tts) override;
   void StartVoiceInteraction() override;
   void StartWarmerWelcomeInteraction(int num_warmer_welcome_triggered,
                                      bool allow_tts) override;
@@ -106,7 +109,9 @@ class TestAssistantService : public chromeos::assistant::mojom::Assistant {
  private:
   void StartInteraction(
       chromeos::assistant::mojom::AssistantInteractionType type,
-      const std::string& query = {});
+      chromeos::assistant::mojom::AssistantQuerySource source =
+          chromeos::assistant::mojom::AssistantQuerySource::kUnspecified,
+      const std::string& query = std::string());
   void SendInteractionResponse();
   InteractionResponse PopInteractionResponse();
 

ash/assistant/util/BUILD.gn

@@ -26,6 +26,7 @@ component("util") {
     "//ash/assistant/model",
     "//base",
     "//base:i18n",
+    "//chromeos/services/assistant/public/mojom",
     "//net",
     "//ui/base",
     "//ui/gfx",

ash/assistant/util/histogram_util.cc

@@ -4,9 +4,9 @@
 
 #include "ash/assistant/util/histogram_util.h"
 
-#include "ash/assistant/model/assistant_query.h"
 #include "ash/assistant/model/assistant_ui_model.h"
 #include "base/metrics/histogram_macros.h"
+#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 
 namespace ash {
 namespace assistant {
@@ -30,7 +30,8 @@ void IncrementAssistantButtonClickCount(AssistantButtonId button_id) {
                             AssistantButtonId::kMaxValue);
 }
 
-void RecordAssistantQuerySource(AssistantQuerySource source) {
+void RecordAssistantQuerySource(
+    chromeos::assistant::mojom::AssistantQuerySource source) {
   UMA_HISTOGRAM_ENUMERATION("Assistant.QuerySource", source);
 }
 

ash/assistant/util/histogram_util.h

@@ -6,13 +6,13 @@
 #define ASH_ASSISTANT_UTIL_HISTOGRAM_UTIL_H_
 
 #include "base/component_export.h"
+#include "chromeos/services/assistant/public/mojom/assistant.mojom-forward.h"
 
 namespace ash {
 
 enum class AssistantButtonId;
 enum class AssistantEntryPoint;
 enum class AssistantExitPoint;
-enum class AssistantQuerySource;
 
 namespace assistant {
 namespace util {
@@ -35,7 +35,8 @@ void IncrementAssistantButtonClickCount(AssistantButtonId button_id);
 
 // Record the input source of each query (e.g. voice, typing).
 COMPONENT_EXPORT(ASSISTANT_UTIL)
-void RecordAssistantQuerySource(AssistantQuerySource source);
+void RecordAssistantQuerySource(
+    chromeos::assistant::mojom::AssistantQuerySource source);
 
 }  // namespace util
 }  // namespace assistant

chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.cc

@@ -1913,7 +1913,9 @@ class AssistantInteractionHelper
 
   void SendTextQuery(const std::string& query, bool allow_tts) {
     // Start text interaction with Assistant server.
-    assistant_->StartTextInteraction(query, allow_tts);
+    assistant_->StartTextInteraction(
+        query, chromeos::assistant::mojom::AssistantQuerySource::kUnspecified,
+        allow_tts);
 
     query_status_->SetKey("queryText", base::Value(query));
   }

chromeos/services/assistant/assistant_manager_service_impl.cc

@@ -447,8 +447,10 @@ void AssistantManagerServiceImpl::StartMetalayerInteraction(
                      /*assistant_tree=*/nullptr));
 }
 
-void AssistantManagerServiceImpl::StartTextInteraction(const std::string& query,
-                                                       bool allow_tts) {
+void AssistantManagerServiceImpl::StartTextInteraction(
+    const std::string& query,
+    mojom::AssistantQuerySource source,
+    bool allow_tts) {
   assistant_client::VoicelessOptions options;
   options.is_user_initiated = true;
 
@@ -462,7 +464,7 @@ void AssistantManagerServiceImpl::StartTextInteraction(const std::string& query,
   options.conversation_turn_id = base::NumberToString(next_interaction_id_++);
   pending_interactions_[options.conversation_turn_id] =
       mojom::AssistantInteractionMetadata::New(
-          /*type=*/mojom::AssistantInteractionType::kText, /*query=*/query);
+          mojom::AssistantInteractionType::kText, source, query);
 
   if (base::FeatureList::IsEnabled(
           assistant::features::kEnableTextQueriesWithClientDiscourseContext) &&
@@ -1259,6 +1261,7 @@ void AssistantManagerServiceImpl::OnConversationTurnStartedOnMainThread(
     metadata_ptr->type = metadata.is_mic_open
                              ? mojom::AssistantInteractionType::kVoice
                              : mojom::AssistantInteractionType::kText;
+    metadata_ptr->source = mojom::AssistantQuerySource::kLibAssistantInitiated;
   }
 
   for (auto& it : interaction_subscribers_)

chromeos/services/assistant/assistant_manager_service_impl.h

@@ -128,7 +128,9 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
   void StartCachedScreenContextInteraction() override;
   void StartEditReminderInteraction(const std::string& client_id) override;
   void StartMetalayerInteraction(const gfx::Rect& region) override;
-  void StartTextInteraction(const std::string& query, bool allow_tts) override;
+  void StartTextInteraction(const std::string& query,
+                            mojom::AssistantQuerySource source,
+                            bool allow_tts) override;
   void StartVoiceInteraction() override;
   void StartWarmerWelcomeInteraction(int num_warmer_welcome_triggered,
                                      bool allow_tts) override;

chromeos/services/assistant/fake_assistant_manager_service_impl.cc

@@ -67,6 +67,7 @@ void FakeAssistantManagerServiceImpl::StartMetalayerInteraction(
 
 void FakeAssistantManagerServiceImpl::StartTextInteraction(
     const std::string& query,
+    mojom::AssistantQuerySource source,
     bool allow_tts) {}
 
 void FakeAssistantManagerServiceImpl::StartVoiceInteraction() {}

chromeos/services/assistant/fake_assistant_manager_service_impl.h

@@ -49,7 +49,9 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) FakeAssistantManagerServiceImpl
   void StartCachedScreenContextInteraction() override;
   void StartEditReminderInteraction(const std::string& client_id) override;
   void StartMetalayerInteraction(const gfx::Rect& region) override;
-  void StartTextInteraction(const std::string& query, bool allow_tts) override;
+  void StartTextInteraction(const std::string& query,
+                            mojom::AssistantQuerySource source,
+                            bool allow_tts) override;
   void StartVoiceInteraction() override;
   void StartWarmerWelcomeInteraction(int num_warmer_welcome_triggered,
                                      bool allow_tts) override;

chromeos/services/assistant/public/mojom/assistant.mojom

@@ -80,7 +80,8 @@ interface Assistant {
   // result will contain TTS. Otherwise TTS will not be present in the
   // generated server response. Results will be returned through registered
   // |AssistantInteractionSubscriber|.
-  StartTextInteraction(string query, bool allow_tts);
+  StartTextInteraction(
+    string query, AssistantQuerySource source, bool allow_tts);
 
   // Starts a new Assistant voice interaction.
   StartVoiceInteraction();
@@ -141,6 +142,7 @@ interface Assistant {
 // Describes an Assistant interaction.
 struct AssistantInteractionMetadata {
   AssistantInteractionType type;
+  AssistantQuerySource source;
   string query;
 };
 
@@ -150,6 +152,20 @@ enum AssistantInteractionType {
   kVoice,
 };
 
+// Enumeration of possible Assistant query sources. These values are persisted
+// to logs. Entries should not be renumbered and numeric values should never
+// be reused. Append new values to the end.
+enum AssistantQuerySource {
+  kUnspecified = 0,
+  kDeepLink = 1,
+  kDialogPlateTextField = 2,
+  kStylus = 3,
+  kSuggestionChip = 4,
+  kVoiceInput = 5,
+  kProactiveSuggestions = 6,
+  kLibAssistantInitiated = 7,
+};
+
 // Subscribes to Assistant's interaction event. These events are server driven
 // in response to the user's direct interaction with the assistant. Responses
 // from the assistant may contain untrusted third-party content. Subscriber

chromeos/services/assistant/test_support/mock_assistant.h

@@ -28,7 +28,8 @@ class MockAssistant : public mojom::Assistant {
 
   MOCK_METHOD1(StartMetalayerInteraction, void(const gfx::Rect&));
 
-  MOCK_METHOD2(StartTextInteraction, void(const std::string&, bool));
+  MOCK_METHOD3(StartTextInteraction,
+               void(const std::string&, mojom::AssistantQuerySource, bool));
 
   MOCK_METHOD0(StartVoiceInteraction, void());