Add unittests for QuickAnswer Consent

If the quick answer consent view is discarded 3 times, we no longer show it
and start showing the quick answers.
This was previously not tested.

Bug: b/169362551
Tests: ash_unittests --gtest_filter="QuickAnswers*"
Change-Id: I7352b45a724980ac169e0d250f06e7b46a7c2440
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2436316Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Yue Li <updowndota@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Jeroen Dhollander <jeroendh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812296}

ash/quick_answers/quick_answers_controller_impl.cc

@@ -45,6 +45,13 @@ base::string16 IntentTypeToString(IntentType intent_type) {
       return base::string16();
   }
 }
+
+// Returns if the request has already been processed (by the text annotator).
+bool IsProcessedRequest(const QuickAnswersRequest& request) {
+  return (request.preprocessed_output.intent_info.intent_type !=
+          chromeos::quick_answers::IntentType::kUnknown);
+}
+
 }  // namespace
 
 namespace ash {
@@ -87,13 +94,26 @@ void QuickAnswersControllerImpl::MaybeShowQuickAnswers(
     // Send the request for preprocessing. Only shows quick answers view if the
     // predicted intent is not |kUnknown| at |OnRequestPreprocessFinish|.
     quick_answers_client_->SendRequestForPreprocessing(request);
-  } else if (!MaybeShowUserConsent(base::string16(), base::string16())) {
-    // Text annotator is not enabled and consent view is not showing, shows
-    // quick answers view with placeholder and send the request.
+  } else {
+    HandleQuickAnswerRequest(request);
+  }
+}
+
+void QuickAnswersControllerImpl::HandleQuickAnswerRequest(
+    const chromeos::quick_answers::QuickAnswersRequest& request) {
+  if (ShouldShowUserConsent()) {
+    ShowUserConsent(
+        IntentTypeToString(request.preprocessed_output.intent_info.intent_type),
+        base::UTF8ToUTF16(request.preprocessed_output.intent_info.intent_text));
+  } else {
     visibility_ = QuickAnswersVisibility::kVisible;
-    quick_answers_ui_controller_->CreateQuickAnswersView(anchor_bounds, title_,
+    quick_answers_ui_controller_->CreateQuickAnswersView(anchor_bounds_, title_,
                                                          query_);
-    quick_answers_client_->SendRequest(request);
+
+    if (IsProcessedRequest(request))
+      quick_answers_client_->FetchQuickAnswers(request);
+    else
+      quick_answers_client_->SendRequest(request);
   }
 }
 
@@ -172,15 +192,7 @@ void QuickAnswersControllerImpl::OnRequestPreprocessFinished(
   query_ = processed_request.preprocessed_output.query;
   title_ = processed_request.preprocessed_output.intent_info.intent_text;
 
-  if (!MaybeShowUserConsent(IntentTypeToString(intent_type),
-                            base::UTF8ToUTF16(title_))) {
-    if (!quick_answers_ui_controller_->is_showing_quick_answers_view()) {
-      visibility_ = QuickAnswersVisibility::kVisible;
-      quick_answers_ui_controller_->CreateQuickAnswersView(anchor_bounds_,
-                                                           title_, query_);
-    }
-    quick_answers_client_->FetchQuickAnswers(processed_request);
-  }
+  HandleQuickAnswerRequest(processed_request);
 }
 
 void QuickAnswersControllerImpl::OnRetryQuickAnswersRequest() {
@@ -237,19 +249,19 @@ void QuickAnswersControllerImpl::MaybeDismissQuickAnswersConsent() {
   quick_answers_ui_controller_->CloseUserConsentView();
 }
 
-bool QuickAnswersControllerImpl::MaybeShowUserConsent(
+bool QuickAnswersControllerImpl::ShouldShowUserConsent() const {
+  return consent_controller_->ShouldShowConsent();
+}
+
+void QuickAnswersControllerImpl::ShowUserConsent(
     const base::string16& intent_type,
     const base::string16& intent_text) {
-  if (consent_controller_->ShouldShowConsent()) {
-    // Show user-consent notice informing user about the feature if required.
-    if (!quick_answers_ui_controller_->is_showing_user_consent_view()) {
-      quick_answers_ui_controller_->CreateUserConsentView(
-          anchor_bounds_, intent_type, intent_text);
-      consent_controller_->StartConsent();
-    }
-    return true;
+  // Show user-consent notice informing user about the feature if required.
+  if (!quick_answers_ui_controller_->is_showing_user_consent_view()) {
+    quick_answers_ui_controller_->CreateUserConsentView(
+        anchor_bounds_, intent_type, intent_text);
+    consent_controller_->StartConsent();
   }
-  return false;
 }
 
 QuickAnswersRequest QuickAnswersControllerImpl::BuildRequest() {

ash/quick_answers/quick_answers_controller_impl.h

@@ -110,9 +110,13 @@ class ASH_EXPORT QuickAnswersControllerImpl
  private:
   void MaybeDismissQuickAnswersConsent();
 
-  // Return true if the user consent view is showing.
-  bool MaybeShowUserConsent(const base::string16& intent_type,
-                            const base::string16& intent_text);
+  void HandleQuickAnswerRequest(
+      const chromeos::quick_answers::QuickAnswersRequest& request);
+
+  bool ShouldShowUserConsent() const;
+  // Show the user consent view. Does nothing if the view is already visible.
+  void ShowUserConsent(const base::string16& intent_type,
+                       const base::string16& intent_text);
 
   chromeos::quick_answers::QuickAnswersRequest BuildRequest();
 

ash/quick_answers/quick_answers_controller_unittest.cc

@@ -44,6 +44,9 @@ class QuickAnswersControllerTest : public AshTestBase {
         std::make_unique<chromeos::quick_answers::QuickAnswersClient>(
             &test_url_loader_factory_, ash::AssistantState::Get(),
             controller()->GetQuickAnswersDelegate()));
+
+    controller()->OnEligibilityChanged(true);
+    controller()->SetVisibilityForTesting(QuickAnswersVisibility::kPending);
   }
 
   QuickAnswersControllerImpl* controller() {
@@ -51,6 +54,19 @@ class QuickAnswersControllerTest : public AshTestBase {
         QuickAnswersController::Get());
   }
 
+  void ShowQuickAnswers(bool set_visibility = true) {
+    // To show the quick answers view, its visibility must be set to 'pending'
+    // first.
+    if (set_visibility)
+      controller()->SetPendingShowQuickAnswers();
+    controller()->MaybeShowQuickAnswers(kDefaultAnchorBoundsInScreen,
+                                        kDefaultTitle, {});
+  }
+
+  void DismissQuickAnswers() {
+    controller()->DismissQuickAnswers(/*is_active=*/true);
+  }
+
   QuickAnswersUiController* ui_controller() {
     return controller()->quick_answers_ui_controller();
   }
@@ -66,9 +82,7 @@ class QuickAnswersControllerTest : public AshTestBase {
 
 TEST_F(QuickAnswersControllerTest, ShouldNotShowWhenFeatureNotEligible) {
   controller()->OnEligibilityChanged(false);
-  controller()->SetVisibilityForTesting(QuickAnswersVisibility::kPending);
-  controller()->MaybeShowQuickAnswers(kDefaultAnchorBoundsInScreen,
-                                      kDefaultTitle, {});
+  ShowQuickAnswers();
 
   // The feature is not eligible, nothing should be shown.
   EXPECT_FALSE(ui_controller()->is_showing_user_consent_view());
@@ -76,10 +90,8 @@ TEST_F(QuickAnswersControllerTest, ShouldNotShowWhenFeatureNotEligible) {
 }
 
 TEST_F(QuickAnswersControllerTest, ShouldNotShowWhenClosed) {
-  controller()->OnEligibilityChanged(true);
   controller()->SetVisibilityForTesting(QuickAnswersVisibility::kClosed);
-  controller()->MaybeShowQuickAnswers(kDefaultAnchorBoundsInScreen,
-                                      kDefaultTitle, {});
+  ShowQuickAnswers(/*set_visibility=*/false);
 
   // The UI is closed and session is inactive, nothing should be shown.
   EXPECT_FALSE(ui_controller()->is_showing_user_consent_view());
@@ -87,11 +99,9 @@ TEST_F(QuickAnswersControllerTest, ShouldNotShowWhenClosed) {
   EXPECT_EQ(controller()->visibility(), QuickAnswersVisibility::kClosed);
 }
 
-TEST_F(QuickAnswersControllerTest, AcceptUserConsent) {
-  controller()->OnEligibilityChanged(true);
-  controller()->SetVisibilityForTesting(QuickAnswersVisibility::kPending);
-  controller()->MaybeShowQuickAnswers(kDefaultAnchorBoundsInScreen,
-                                      kDefaultTitle, {});
+TEST_F(QuickAnswersControllerTest,
+       ShouldShowPendingQueryAfterUserAcceptsConsent) {
+  ShowQuickAnswers();
   // Without user consent, only the user consent view should show.
   EXPECT_TRUE(ui_controller()->is_showing_user_consent_view());
   EXPECT_FALSE(ui_controller()->is_showing_quick_answers_view());
@@ -105,14 +115,11 @@ TEST_F(QuickAnswersControllerTest, AcceptUserConsent) {
   EXPECT_EQ(controller()->visibility(), QuickAnswersVisibility::kVisible);
 }
 
-TEST_F(QuickAnswersControllerTest, UserConsentAlreadyAccpeted) {
+TEST_F(QuickAnswersControllerTest, UserConsentAlreadyAccepted) {
   consent_controller()->StartConsent();
-  controller()->OnEligibilityChanged(true);
-  controller()->SetVisibilityForTesting(QuickAnswersVisibility::kPending);
   consent_controller()->AcceptConsent(
       chromeos::quick_answers::ConsentInteractionType::kAccept);
-  controller()->MaybeShowQuickAnswers(kDefaultAnchorBoundsInScreen,
-                                      kDefaultTitle, {});
+  ShowQuickAnswers();
 
   // With user consent already accepted, only the quick answers view should
   // show.
@@ -121,26 +128,38 @@ TEST_F(QuickAnswersControllerTest, UserConsentAlreadyAccpeted) {
   EXPECT_EQ(controller()->visibility(), QuickAnswersVisibility::kVisible);
 }
 
+TEST_F(QuickAnswersControllerTest,
+       ShouldShowQuickAnswersIfUserIgnoresConsentViewThreeTimes) {
+  // Show and dismiss user consent window the first 3 times
+  for (int i = 0; i < 3; i++) {
+    ShowQuickAnswers();
+    EXPECT_TRUE(ui_controller()->is_showing_user_consent_view())
+        << "Consent view not shown the " << (i + 1) << " time";
+    EXPECT_FALSE(ui_controller()->is_showing_quick_answers_view());
+    DismissQuickAnswers();
+  }
+
+  // The 4th time we should simply show the quick answer.
+  ShowQuickAnswers();
+  EXPECT_FALSE(ui_controller()->is_showing_user_consent_view());
+  EXPECT_TRUE(ui_controller()->is_showing_quick_answers_view());
+}
+
 TEST_F(QuickAnswersControllerTest, DismissUserConsentView) {
-  controller()->OnEligibilityChanged(true);
-  controller()->SetVisibilityForTesting(QuickAnswersVisibility::kPending);
-  controller()->MaybeShowQuickAnswers(kDefaultAnchorBoundsInScreen,
-                                      kDefaultTitle, {});
+  ShowQuickAnswers();
   EXPECT_TRUE(ui_controller()->is_showing_user_consent_view());
 
-  controller()->DismissQuickAnswers(true);
+  DismissQuickAnswers();
+
   EXPECT_FALSE(ui_controller()->is_showing_user_consent_view());
   EXPECT_EQ(controller()->visibility(), QuickAnswersVisibility::kClosed);
 }
 
 TEST_F(QuickAnswersControllerTest, DismissQuickAnswersView) {
   consent_controller()->StartConsent();
-  controller()->OnEligibilityChanged(true);
-  controller()->SetVisibilityForTesting(QuickAnswersVisibility::kPending);
   consent_controller()->AcceptConsent(
       chromeos::quick_answers::ConsentInteractionType::kAccept);
-  controller()->MaybeShowQuickAnswers(kDefaultAnchorBoundsInScreen,
-                                      kDefaultTitle, {});
+  ShowQuickAnswers();
   EXPECT_TRUE(ui_controller()->is_showing_quick_answers_view());
 
   controller()->DismissQuickAnswers(true);