Apply menu boundary changes to QuickAnswer views

Previously the menu boundary changes were not applied if the quick
answer views (QuickAnswerView or NotificationView) were still pending,
causing the quick answer view to show up too low.

Bug: b/168936614
Tests: ash_unittests --gtest_filter="QuickAnswer*" and manually deployed
Change-Id: I1f81cbed4d9c5e8e718d556e2763ec9727c5fe36
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2439538
Commit-Queue: Jeroen Dhollander <jeroendh@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Yue Li <updowndota@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Auto-Submit: Jeroen Dhollander <jeroendh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813298}

ash/quick_answers/quick_answers_controller_impl.cc

@@ -14,7 +14,6 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
-// TODO(yanxiao):Add a unit test for QuickAnswersControllerImpl.
 namespace {
 using chromeos::quick_answers::Context;
 using chromeos::quick_answers::IntentType;
@@ -211,8 +210,6 @@ void QuickAnswersControllerImpl::OnQuickAnswerClick() {
 
 void QuickAnswersControllerImpl::UpdateQuickAnswersAnchorBounds(
     const gfx::Rect& anchor_bounds) {
-  if (visibility_ != QuickAnswersVisibility::kVisible)
-    return;
   anchor_bounds_ = anchor_bounds;
   quick_answers_ui_controller_->UpdateQuickAnswersBounds(anchor_bounds);
 }

ash/quick_answers/quick_answers_controller_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "ash/quick_answers/quick_answers_ui_controller.h"
 #include "ash/quick_answers/ui/quick_answers_view.h"
+#include "ash/quick_answers/ui/user_consent_view.h"
 #include "ash/test/ash_test_base.h"
 #include "base/test/scoped_feature_list.h"
 #include "chromeos/components/quick_answers/quick_answers_client.h"
@@ -21,6 +22,12 @@ constexpr gfx::Rect kDefaultAnchorBoundsInScreen =
     gfx::Rect(gfx::Point(500, 250), gfx::Size(80, 140));
 constexpr char kDefaultTitle[] = "default_title";
 
+gfx::Rect BoundsWithXPosition(int x) {
+  constexpr int kAnyValue = 100;
+  return gfx::Rect(x, /*y=*/kAnyValue, /*width=*/kAnyValue,
+                   /*height=*/kAnyValue);
+}
+
 }  // namespace
 
 class QuickAnswersControllerTest : public AshTestBase {
@@ -54,7 +61,9 @@ class QuickAnswersControllerTest : public AshTestBase {
         QuickAnswersController::Get());
   }
 
-  void ShowQuickAnswers(bool set_visibility = true) {
+  // Show the quick answer or notification view (depending on the notification
+  // consent status).
+  void ShowView(bool set_visibility = true) {
     // To show the quick answers view, its visibility must be set to 'pending'
     // first.
     if (set_visibility)
@@ -63,6 +72,35 @@ class QuickAnswersControllerTest : public AshTestBase {
                                         kDefaultTitle, {});
   }
 
+  void ShowNotificationView() {
+    // We can only show the notification view if the consent has not been
+    // granted, so we add a sanity check here.
+    EXPECT_TRUE(consent_controller()->ShouldShowConsent())
+        << "Can not show notification view as the user consent has already "
+           "been given.";
+    ShowView();
+  }
+
+  void ShowQuickAnswersView() {
+    // Grant the user consent so the quick answers view is shown.
+    AcceptConsent();
+    ShowView();
+  }
+
+  const views::View* GetQuickAnswersView() {
+    return ui_controller()->quick_answers_view_for_testing();
+  }
+
+  const views::View* GetNotificationView() {
+    return ui_controller()->notification_view_for_testing();
+  }
+
+  void AcceptConsent() {
+    consent_controller()->StartConsent();
+    consent_controller()->AcceptConsent(
+        chromeos::quick_answers::ConsentInteractionType::kAccept);
+  }
+
   void DismissQuickAnswers() {
     controller()->DismissQuickAnswers(/*is_active=*/true);
   }
@@ -82,7 +120,7 @@ class QuickAnswersControllerTest : public AshTestBase {
 
 TEST_F(QuickAnswersControllerTest, ShouldNotShowWhenFeatureNotEligible) {
   controller()->OnEligibilityChanged(false);
-  ShowQuickAnswers();
+  ShowView();
 
   // The feature is not eligible, nothing should be shown.
   EXPECT_FALSE(ui_controller()->is_showing_user_consent_view());
@@ -91,7 +129,7 @@ TEST_F(QuickAnswersControllerTest, ShouldNotShowWhenFeatureNotEligible) {
 
 TEST_F(QuickAnswersControllerTest, ShouldNotShowWhenClosed) {
   controller()->SetVisibilityForTesting(QuickAnswersVisibility::kClosed);
-  ShowQuickAnswers(/*set_visibility=*/false);
+  ShowView(/*set_visibility=*/false);
 
   // The UI is closed and session is inactive, nothing should be shown.
   EXPECT_FALSE(ui_controller()->is_showing_user_consent_view());
@@ -101,7 +139,7 @@ TEST_F(QuickAnswersControllerTest, ShouldNotShowWhenClosed) {
 
 TEST_F(QuickAnswersControllerTest,
        ShouldShowPendingQueryAfterUserAcceptsConsent) {
-  ShowQuickAnswers();
+  ShowView();
   // 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());
@@ -116,10 +154,8 @@ TEST_F(QuickAnswersControllerTest,
 }
 
 TEST_F(QuickAnswersControllerTest, UserConsentAlreadyAccepted) {
-  consent_controller()->StartConsent();
-  consent_controller()->AcceptConsent(
-      chromeos::quick_answers::ConsentInteractionType::kAccept);
-  ShowQuickAnswers();
+  AcceptConsent();
+  ShowView();
 
   // With user consent already accepted, only the quick answers view should
   // show.
@@ -132,7 +168,7 @@ TEST_F(QuickAnswersControllerTest,
        ShouldShowQuickAnswersIfUserIgnoresConsentViewThreeTimes) {
   // Show and dismiss user consent window the first 3 times
   for (int i = 0; i < 3; i++) {
-    ShowQuickAnswers();
+    ShowView();
     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());
@@ -140,13 +176,13 @@ TEST_F(QuickAnswersControllerTest,
   }
 
   // The 4th time we should simply show the quick answer.
-  ShowQuickAnswers();
+  ShowView();
   EXPECT_FALSE(ui_controller()->is_showing_user_consent_view());
   EXPECT_TRUE(ui_controller()->is_showing_quick_answers_view());
 }
 
 TEST_F(QuickAnswersControllerTest, DismissUserConsentView) {
-  ShowQuickAnswers();
+  ShowNotificationView();
   EXPECT_TRUE(ui_controller()->is_showing_user_consent_view());
 
   DismissQuickAnswers();
@@ -156,10 +192,7 @@ TEST_F(QuickAnswersControllerTest, DismissUserConsentView) {
 }
 
 TEST_F(QuickAnswersControllerTest, DismissQuickAnswersView) {
-  consent_controller()->StartConsent();
-  consent_controller()->AcceptConsent(
-      chromeos::quick_answers::ConsentInteractionType::kAccept);
-  ShowQuickAnswers();
+  ShowQuickAnswersView();
   EXPECT_TRUE(ui_controller()->is_showing_quick_answers_view());
 
   controller()->DismissQuickAnswers(true);
@@ -167,4 +200,28 @@ TEST_F(QuickAnswersControllerTest, DismissQuickAnswersView) {
   EXPECT_EQ(controller()->visibility(), QuickAnswersVisibility::kClosed);
 }
 
+TEST_F(QuickAnswersControllerTest,
+       ShouldUpdateQuickAnswersViewBoundsWhenMenuBoundsChange) {
+  ShowQuickAnswersView();
+
+  controller()->UpdateQuickAnswersAnchorBounds(BoundsWithXPosition(123));
+
+  // We only check the 'x' position as that is guaranteed to be identical
+  // between the view and the menu.
+  const views::View* quick_answers_view = GetQuickAnswersView();
+  EXPECT_EQ(123, quick_answers_view->GetBoundsInScreen().x());
+}
+
+TEST_F(QuickAnswersControllerTest,
+       ShouldUpdateNotificationViewBoundsWhenMenuBoundsChange) {
+  ShowNotificationView();
+
+  controller()->UpdateQuickAnswersAnchorBounds(BoundsWithXPosition(123));
+
+  // We only check the 'x' position as that is guaranteed to be identical
+  // between the view and the menu.
+  const views::View* notification_view = GetNotificationView();
+  EXPECT_EQ(123, notification_view->GetBoundsInScreen().x());
+}
+
 }  // namespace ash

ash/quick_answers/quick_answers_ui_controller.h

@@ -91,6 +91,13 @@ class ASH_EXPORT QuickAnswersUiController {
   // Invoked when user clicks the Dogfood button on Quick-Answers related views.
   void OnDogfoodButtonPressed();
 
+  const QuickAnswersView* quick_answers_view_for_testing() const {
+    return quick_answers_view_;
+  }
+  const quick_answers::UserConsentView* notification_view_for_testing() const {
+    return user_consent_view_;
+  }
+
  private:
   QuickAnswersControllerImpl* controller_ = nullptr;