Fix impression logging for Quick Answers

Currently, an active impression is approximated by whenever the menu or
the Quick Answer is dismissed without clicking on a menu-item. This
indicates user could be actively looking for Quick-Answers instead of
any other menu operation.

This is currently buggy because DismissQuickAnswers() overcounts the
metric even when a Quick-Answer was not shown to the user (because the
User-Consent view was shown instead).

This CL ensures that the metric is not overcounted by fixing that, and
also ensures this change will not lead to under-counting by routing the
view-close operations via DismissQuickAnswers() where logging happens.

Bug: b:158265647
Test: Tested on Chrome OS VM.
Change-Id: Ieed9356fbb64650b521c893a44e83caa49ea831e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2233342Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Abhijeet Singh <siabhijeet@google.com>
Cr-Commit-Position: refs/heads/master@{#775799}

ash/quick_answers/quick_answers_controller_impl.cc

@@ -88,10 +88,10 @@ void QuickAnswersControllerImpl::MaybeShowQuickAnswers(
 
 void QuickAnswersControllerImpl::DismissQuickAnswers(bool is_active) {
   MaybeDismissQuickAnswersConsent();
-  quick_answers_ui_controller_->CloseQuickAnswersView();
+  bool closed = quick_answers_ui_controller_->CloseQuickAnswersView();
   quick_answers_client_->OnQuickAnswersDismissed(
       quick_answer_ ? quick_answer_->result_type : ResultType::kNoResult,
-      is_active);
+      is_active && closed);
 }
 
 chromeos::quick_answers::QuickAnswersDelegate*

ash/quick_answers/quick_answers_ui_controller.cc

@@ -44,18 +44,22 @@ void QuickAnswersUiController::CreateQuickAnswersView(
 }
 
 void QuickAnswersUiController::OnQuickAnswersViewPressed() {
-  CloseQuickAnswersView();
+  // Route dismissal through |controller_| for logging impressions.
+  controller_->DismissQuickAnswers(/*is_active=*/true);
+
   ash::AssistantInteractionController::Get()->StartTextInteraction(
       query_, /*allow_tts=*/false,
       chromeos::assistant::mojom::AssistantQuerySource::kQuickAnswers);
   controller_->OnQuickAnswerClick();
 }
 
-void QuickAnswersUiController::CloseQuickAnswersView() {
+bool QuickAnswersUiController::CloseQuickAnswersView() {
   if (quick_answers_view_) {
     quick_answers_view_->GetWidget()->Close();
     quick_answers_view_ = nullptr;
+    return true;
   }
+  return false;
 }
 
 void QuickAnswersUiController::OnRetryLabelPressed() {
@@ -101,11 +105,13 @@ void QuickAnswersUiController::CreateUserConsentView(
   user_consent_view_->GetWidget()->ShowInactive();
 }
 
-void QuickAnswersUiController::CloseUserConsentView() {
+bool QuickAnswersUiController::CloseUserConsentView() {
   if (user_consent_view_) {
     user_consent_view_->GetWidget()->Close();
     user_consent_view_ = nullptr;
+    return true;
   }
+  return false;
 }
 
 void QuickAnswersUiController::OnConsentGrantedButtonPressed() {
@@ -118,11 +124,9 @@ void QuickAnswersUiController::OnManageSettingsButtonPressed() {
 }
 
 void QuickAnswersUiController::OnDogfoodButtonPressed() {
-  // Close Quick-Answers related views and open the Dogfood link.
-  if (quick_answers_view_)
-    CloseQuickAnswersView();
-  if (user_consent_view_)
-    CloseUserConsentView();
+  // Route dismissal through |controller_| for logging impressions.
+  controller_->DismissQuickAnswers(/*is_active=*/true);
+
   controller_->OpenQuickAnswersDogfoodLink();
 }
 

ash/quick_answers/quick_answers_ui_controller.h

@@ -39,7 +39,8 @@ class ASH_EXPORT QuickAnswersUiController {
                               const std::string& title,
                               const std::string& query);
 
-  void CloseQuickAnswersView();
+  // Returns true if there was a QuickAnswersView to close.
+  bool CloseQuickAnswersView();
 
   void OnQuickAnswersViewPressed();
 
@@ -61,7 +62,8 @@ class ASH_EXPORT QuickAnswersUiController {
   // anchor.
   void CreateUserConsentView(const gfx::Rect& anchor_bounds);
 
-  void CloseUserConsentView();
+  // Returns true if there was a UserConsentView to close.
+  bool CloseUserConsentView();
 
   // Invoked when user clicks the consent button to grant consent for using
   // Quick Answers.