Anchor Assistant to correct display.

Previously, we always anchored to the primary display.

Now, we'll check for where new windows will be opened. We match a
display against the bounds for that window and anchor there.

Bug: b:111445270
Change-Id: Ice37359fe61937d452d7a908a609d1b60d913a37
Reviewed-on: https://chromium-review.googlesource.com/1150920
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582247}

ash/BUILD.gn

@@ -1692,6 +1692,7 @@ test("ash_unittests") {
     "ash_service_unittest.cc",
     "assistant/assistant_controller_unittest.cc",
     "assistant/assistant_screen_context_controller_unittest.cc",
+    "assistant/ui/assistant_container_view_unittest.cc",
     "assistant/util/deep_link_util_unittest.cc",
     "autoclick/autoclick_unittest.cc",
     "cursor_unittest.cc",

ash/assistant/assistant_ui_controller.cc

@@ -283,4 +283,8 @@ void AssistantUiController::UpdateUiMode(
                                     : AssistantUiMode::kMainUi);
 }
 
+AssistantContainerView* AssistantUiController::GetViewForTest() {
+  return container_view_;
+}
+
 }  // namespace ash

ash/assistant/assistant_ui_controller.h

@@ -100,6 +100,8 @@ class ASH_EXPORT AssistantUiController
   void HideUi(AssistantSource source);
   void ToggleUi(AssistantSource source);
 
+  AssistantContainerView* GetViewForTest();
+
  private:
   // Updates UI mode to |ui_mode| if specified. Otherwise UI mode is updated on
   // the basis of interaction/widget visibility state.

ash/assistant/ui/assistant_container_view.cc

@@ -12,6 +12,7 @@
 #include "ash/assistant/ui/assistant_main_view.h"
 #include "ash/assistant/ui/assistant_mini_view.h"
 #include "ash/assistant/ui/assistant_web_view.h"
+#include "ash/shell.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
@@ -190,14 +191,14 @@ void AssistantContainerView::RequestFocus() {
     assistant_main_view_->RequestFocus();
 }
 
+// TODO(dmblack): Handle dynamic re-anchoring due to shelf repositioning, etc.
 void AssistantContainerView::SetAnchor() {
-  // TODO(dmblack): Handle multiple displays, dynamic shelf repositioning, and
-  // any other corner cases.
-  // Anchors to bottom center of primary display's work area.
-  display::Display primary_display =
-      display::Screen::GetScreen()->GetPrimaryDisplay();
+  // Anchor to the display matching where new windows will be opened.
+  display::Display display = display::Screen::GetScreen()->GetDisplayMatching(
+      Shell::Get()->GetRootWindowForNewWindows()->GetBoundsInScreen());
 
-  gfx::Rect work_area = primary_display.work_area();
+  // Anchor to the bottom center of the work area.
+  gfx::Rect work_area = display.work_area();
   gfx::Rect anchor = gfx::Rect(work_area.x(), work_area.bottom() - kMarginDip,
                                work_area.width(), 0);
 

ash/assistant/ui/assistant_container_view_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/assistant/ui/assistant_container_view.h"
+
+#include "ash/assistant/assistant_controller.h"
+#include "ash/assistant/assistant_ui_controller.h"
+#include "ash/shell.h"
+#include "ash/test/ash_test_base.h"
+#include "ash/voice_interaction/voice_interaction_controller.h"
+#include "base/macros.h"
+#include "base/test/scoped_feature_list.h"
+#include "chromeos/chromeos_switches.h"
+#include "chromeos/services/assistant/test_support/mock_assistant.h"
+#include "ui/compositor/scoped_animation_duration_scale_mode.h"
+
+namespace ash {
+
+namespace {
+
+class AssistantContainerViewTest : public AshTestBase {
+ public:
+  AssistantContainerViewTest() = default;
+  ~AssistantContainerViewTest() override = default;
+
+  void SetUp() override {
+    // Enable Assistant feature.
+    scoped_feature_list_.InitAndEnableFeature(
+        chromeos::switches::kAssistantFeature);
+    ASSERT_TRUE(chromeos::switches::IsAssistantEnabled());
+
+    AshTestBase::SetUp();
+
+    // Enable Assistant in settings.
+    Shell::Get()->voice_interaction_controller()->NotifySettingsEnabled(true);
+
+    // Cache controller.
+    controller_ = Shell::Get()->assistant_controller();
+    DCHECK(controller_);
+
+    // Cache UI controller.
+    ui_controller_ = controller_->ui_controller();
+    DCHECK(ui_controller_);
+
+    SetUpMocks();
+
+    // After mocks are set up our Assistant service is ready for use. Indicate
+    // this by changing status from NOT_READY to STOPPED.
+    Shell::Get()->voice_interaction_controller()->NotifyStatusChanged(
+        mojom::VoiceInteractionState::STOPPED);
+  }
+
+  AssistantUiController* ui_controller() { return ui_controller_; }
+
+ private:
+  void SetUpMocks() {
+    // Mock the Assistant service.
+    assistant_ = std::make_unique<chromeos::assistant::MockAssistant>();
+    assistant_binding_ =
+        std::make_unique<mojo::Binding<chromeos::assistant::mojom::Assistant>>(
+            assistant_.get());
+    chromeos::assistant::mojom::AssistantPtr assistant;
+    assistant_binding_->Bind(mojo::MakeRequest(&assistant));
+    controller_->SetAssistant(std::move(assistant));
+
+    // Mock any screen context requests by immediately invoking callback.
+    ON_CALL(*assistant_, DoRequestScreenContext(testing::_, testing::_))
+        .WillByDefault(testing::Invoke(
+            [](const gfx::Rect& rect, base::OnceClosure* callback) {
+              std::move(*callback).Run();
+            }));
+  }
+
+  base::test::ScopedFeatureList scoped_feature_list_;
+
+  std::unique_ptr<chromeos::assistant::MockAssistant> assistant_;
+
+  std::unique_ptr<mojo::Binding<chromeos::assistant::mojom::Assistant>>
+      assistant_binding_;
+
+  AssistantController* controller_ = nullptr;
+  AssistantUiController* ui_controller_ = nullptr;
+
+  DISALLOW_COPY_AND_ASSIGN(AssistantContainerViewTest);
+};
+
+}  // namespace
+
+TEST_F(AssistantContainerViewTest, InitialAnchoring) {
+  // Guarantee short but non-zero duration for animations.
+  ui::ScopedAnimationDurationScaleMode scoped_animation_duration(
+      ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
+
+  // Show Assistant UI and grab a reference to our view under test.
+  ui_controller()->ShowUi(AssistantSource::kUnspecified);
+  AssistantContainerView* view = ui_controller()->GetViewForTest();
+
+  // We expect the view to appear in the work area where new windows will open.
+  gfx::Rect expected_work_area =
+      display::Screen::GetScreen()
+          ->GetDisplayMatching(
+              Shell::Get()->GetRootWindowForNewWindows()->GetBoundsInScreen())
+          .work_area();
+
+  // We expect a bottom margin.
+  constexpr int bottom_margin = 8;
+
+  // We expect the view to be horizontally centered and bottom aligned.
+  gfx::Rect expected_bounds = gfx::Rect(expected_work_area);
+  expected_bounds.ClampToCenteredSize(view->size());
+  expected_bounds.set_y(expected_work_area.bottom() - view->height() -
+                        bottom_margin);
+
+  ASSERT_EQ(expected_bounds, view->GetBoundsInScreen());
+}
+
+}  // namespace ash

chromeos/services/assistant/test_support/mock_assistant.h

@@ -45,7 +45,17 @@ class MockAssistant : public mojom::Assistant {
   MOCK_METHOD1(DismissNotification,
                void(chromeos::assistant::mojom::AssistantNotificationPtr));
 
-  MOCK_METHOD2(RequestScreenContext, void(const gfx::Rect&, base::OnceClosure));
+  // Mock DoRequestScreenContext in lieu of RequestScreenContext.
+  MOCK_METHOD2(DoRequestScreenContext,
+               void(const gfx::Rect&, base::OnceClosure*));
+
+  // Note: We can't mock RequestScreenContext directly because of the move
+  // semantics required around base::OnceClosure. Instead, we route calls to a
+  // mockable delegate method, DoRequestScreenContext.
+  void RequestScreenContext(const gfx::Rect& region,
+                            base::OnceClosure callback) override {
+    DoRequestScreenContext(region, &callback);
+  }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockAssistant);