ambient: layout manager in ambient_container_view

Stop manual layout of Ambient mode. Use layout manager to position
photo view and Ambient Assistant view.

BUG=b/163170162

Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Change-Id: I91b52e6e84e6a844ed13fe1c50d1e9389454e8ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2611024Reviewed-by: Tao Wu <wutao@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Jeffrey Young <cowmoo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842662}

ash/BUILD.gn

@@ -1969,6 +1969,7 @@ test("ash_unittests") {
     "ambient/ambient_photo_controller_unittest.cc",
     "ambient/autotest_ambient_api_unittest.cc",
     "ambient/model/ambient_backend_model_unittest.cc",
+    "ambient/ui/ambient_assistant_container_view_unittest.cc",
     "ambient/ui/ambient_container_view_unittest.cc",
     "ambient/ui/media_string_view_unittest.cc",
     "ambient/ui/photo_view_unittest.cc",

ash/ambient/ui/ambient_assistant_container_view.cc

@@ -24,8 +24,11 @@
 #include "ui/views/background.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
-#include "ui/views/layout/box_layout.h"
+#include "ui/views/layout/flex_layout.h"
+#include "ui/views/layout/flex_layout_types.h"
+#include "ui/views/layout/layout_types.h"
 #include "ui/views/metadata/metadata_impl_macros.h"
+#include "ui/views/view_class_properties.h"
 
 namespace ash {
 
@@ -34,6 +37,8 @@ namespace {
 // Appearance.
 constexpr int kAvatarImageSizeDip = 32;
 
+constexpr int kAssistantPreferredHeightDip = 128;
+
 // Greeting message.
 base::string16 GetGreetingMessage(const UserSession* user_session) {
   DCHECK(user_session);
@@ -80,25 +85,37 @@ void AmbientAssistantContainerView::OnUiVisibilityChanged(
 }
 
 void AmbientAssistantContainerView::InitLayout() {
-  SetPaintToLayer();
-  SetBackground(views::CreateSolidBackground(SK_ColorWHITE));
-
+  views::FlexLayout* outer_layout =
+      SetLayoutManager(std::make_unique<views::FlexLayout>());
+  outer_layout->SetOrientation(views::LayoutOrientation::kVertical);
+  outer_layout->SetMainAxisAlignment(views::LayoutAlignment::kStart);
+  outer_layout->SetCrossAxisAlignment(views::LayoutAlignment::kStretch);
+
+  auto* container = AddChildView(std::make_unique<views::View>());
+
+  // Set a placeholder value for width. |CrossAxisAlignment::kStretch| will
+  // expand the width to 100% of the parent.
+  container->SetPreferredSize(
+      {/*width=*/1, /*height=*/kAssistantPreferredHeightDip});
+  container->SetPaintToLayer();
+  container->SetBackground(views::CreateSolidBackground(SK_ColorWHITE));
+
+  views::FlexLayout* container_layout =
+      container->SetLayoutManager(std::make_unique<views::FlexLayout>());
   constexpr int kRightPaddingDip = 8;
-  views::BoxLayout* layout_manager =
-      SetLayoutManager(std::make_unique<views::BoxLayout>(
-          views::BoxLayout::Orientation::kHorizontal,
-          gfx::Insets(0, 0, 0, kRightPaddingDip)));
+  container_layout->SetInteriorMargin({0, 0, 0, kRightPaddingDip});
 
-  layout_manager->set_cross_axis_alignment(
-      views::BoxLayout::CrossAxisAlignment::kCenter);
+  container_layout->SetOrientation(views::LayoutOrientation::kHorizontal);
+  container_layout->SetMainAxisAlignment(views::LayoutAlignment::kStart);
+  container_layout->SetCrossAxisAlignment(views::LayoutAlignment::kCenter);
 
   // Mic button and input query view.
-  ambient_assistant_dialog_plate_ =
-      AddChildView(std::make_unique<AmbientAssistantDialogPlate>(delegate_));
+  ambient_assistant_dialog_plate_ = container->AddChildView(
+      std::make_unique<AmbientAssistantDialogPlate>(delegate_));
 
   // Response container view.
-  assistant_response_container_view_ =
-      AddChildView(std::make_unique<AssistantResponseContainerView>(delegate_));
+  assistant_response_container_view_ = container->AddChildView(
+      std::make_unique<AssistantResponseContainerView>(delegate_));
 
   // Greeting label.
   const UserSession* active_user_session =
@@ -106,7 +123,7 @@ void AmbientAssistantContainerView::InitLayout() {
   // TODO(meilinw): uses login user info instead as no active user session is
   // available on lock screen.
   if (active_user_session) {
-    greeting_label_ = AddChildView(std::make_unique<views::Label>(
+    greeting_label_ = container->AddChildView(std::make_unique<views::Label>(
         GetGreetingMessage(active_user_session)));
     greeting_label_->SetEnabledColor(kTextColorSecondary);
     greeting_label_->SetFontList(
@@ -118,12 +135,17 @@ void AmbientAssistantContainerView::InitLayout() {
   }
 
   // Spacer.
-  views::View* spacer = AddChildView(std::make_unique<views::View>());
-  // Sets the flex weight to be 1 so the spacer view can be resized.
-  layout_manager->SetFlexForView(spacer, 1);
+  views::View* spacer =
+      container->AddChildView(std::make_unique<views::View>());
+  // Allow the spacer to expand to push the avatar image to the end of the
+  // container.
+  spacer->SetProperty(
+      views::kFlexBehaviorKey,
+      views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToZero,
+                               views::MaximumFlexSizeRule::kUnbounded));
 
   // Rounded avatar image view.
-  avatar_view_ = AddChildView(std::make_unique<views::ImageView>());
+  avatar_view_ = container->AddChildView(std::make_unique<views::ImageView>());
   avatar_view_->SetImageSize(
       gfx::Size(kAvatarImageSizeDip, kAvatarImageSizeDip));
   avatar_view_->SetPreferredSize(

ash/ambient/ui/ambient_assistant_container_view_unittest.cc

+// Copyright 2021 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/ambient/ui/ambient_assistant_container_view.h"
+
+#include "ash/ambient/test/ambient_ash_test_base.h"
+#include "ash/ambient/ui/ambient_container_view.h"
+#include "ash/ambient/ui/ambient_view_ids.h"
+#include "base/test/scoped_feature_list.h"
+#include "chromeos/services/assistant/public/cpp/features.h"
+#include "ui/views/view.h"
+
+namespace ash {
+
+class AmbientAssistantContainerViewTest : public AmbientAshTestBase {
+ public:
+  AmbientAssistantContainerViewTest() : AmbientAshTestBase() {}
+  ~AmbientAssistantContainerViewTest() override = default;
+
+  // AmbientAshTestBase:
+  void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(
+        chromeos::assistant::features::kEnableAmbientAssistant);
+    AmbientAshTestBase::SetUp();
+    UpdateDisplay("800x600");
+    ShowAmbientScreen();
+  }
+
+  void TearDown() override {
+    CloseAmbientScreen();
+    AmbientAshTestBase::TearDown();
+  }
+
+  AmbientAssistantContainerView* GetAmbientAssistantContainerView() {
+    return static_cast<AmbientAssistantContainerView*>(
+        GetContainerView()->GetViewByID(
+            AmbientViewID::kAmbientAssistantContainerView));
+  }
+
+  views::View* GetInnerContainer() {
+    const auto& children = GetAmbientAssistantContainerView()->children();
+    EXPECT_EQ(children.size(), 1u);
+    return children.front();
+  }
+
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+TEST_F(AmbientAssistantContainerViewTest, LayoutAmbientContainerView) {
+  GetAmbientAssistantContainerView()->SetVisible(true);
+
+  auto* inner_container = GetInnerContainer();
+
+  // Should be 100% width and fixed height.
+  EXPECT_EQ(inner_container->GetBoundsInScreen(),
+            gfx::Rect(/*x=*/0, /*y=*/0, /*width=*/800, /*height=*/128));
+
+  UpdateDisplay("1920x1080");
+
+  // Should expand to still be 100% width.
+  EXPECT_EQ(inner_container->GetBoundsInScreen(),
+            gfx::Rect(/*x=*/0, /*y=*/0, /*width=*/1920, /*height=*/128));
+}
+
+}  // namespace ash

ash/ambient/ui/ambient_container_view.cc

@@ -17,6 +17,7 @@
 #include "chromeos/services/assistant/public/cpp/features.h"
 #include "ui/aura/window.h"
 #include "ui/views/background.h"
+#include "ui/views/layout/fill_layout.h"
 #include "ui/views/metadata/metadata_impl_macros.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
@@ -27,9 +28,6 @@ namespace {
 
 using chromeos::assistant::features::IsAmbientAssistantEnabled;
 
-// Appearance.
-constexpr int kAssistantPreferredHeightDip = 128;
-
 }  // namespace
 
 AmbientContainerView::AmbientContainerView(AmbientViewDelegate* delegate)
@@ -40,28 +38,12 @@ AmbientContainerView::AmbientContainerView(AmbientViewDelegate* delegate)
 
 AmbientContainerView::~AmbientContainerView() = default;
 
-gfx::Size AmbientContainerView::CalculatePreferredSize() const {
-  // TODO(b/139953389): Handle multiple displays.
-  return GetWidget()->GetNativeWindow()->GetRootWindow()->bounds().size();
-}
-
-void AmbientContainerView::Layout() {
-  // Layout child views first to have proper bounds set for children.
-  LayoutPhotoView();
-
-  // The assistant view may not exist if |kAmbientAssistant| feature is
-  // disabled.
-  if (ambient_assistant_container_view_)
-    LayoutAssistantView();
-
-  View::Layout();
-}
-
 void AmbientContainerView::Init() {
   // TODO(b/139954108): Choose a better dark mode theme color.
   SetBackground(views::CreateSolidBackground(SK_ColorBLACK));
   // Updates focus behavior to receive key press events.
   SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
+  SetLayoutManager(std::make_unique<views::FillLayout>());
 
   photo_view_ = AddChildView(std::make_unique<PhotoView>(delegate_));
 
@@ -72,18 +54,6 @@ void AmbientContainerView::Init() {
   }
 }
 
-void AmbientContainerView::LayoutPhotoView() {
-  // |photo_view_| should have the same size as the widget.
-  photo_view_->SetBoundsRect(GetLocalBounds());
-}
-
-void AmbientContainerView::LayoutAssistantView() {
-  int preferred_width = GetPreferredSize().width();
-  int preferred_height = kAssistantPreferredHeightDip;
-  ambient_assistant_container_view_->SetBoundsRect(
-      gfx::Rect(0, 0, preferred_width, preferred_height));
-}
-
 BEGIN_METADATA(AmbientContainerView, views::View)
 END_METADATA
 

ash/ambient/ui/ambient_container_view.h

@@ -26,26 +26,16 @@ class ASH_EXPORT AmbientContainerView : public views::View {
   explicit AmbientContainerView(AmbientViewDelegate* delegate);
   ~AmbientContainerView() override;
 
-  gfx::Size CalculatePreferredSize() const override;
-  void Layout() override;
-
  private:
   friend class AmbientAshTestBase;
 
   void Init();
 
-  // Layouts its child views.
-  // TODO(meilinw): Use LayoutManagers to lay out children instead of overriding
-  // Layout(). See b/163170162.
-  void LayoutPhotoView();
-  void LayoutAssistantView();
-
   AmbientViewDelegate* delegate_ = nullptr;
 
   // Owned by view hierarchy.
   PhotoView* photo_view_ = nullptr;
   AmbientAssistantContainerView* ambient_assistant_container_view_ = nullptr;
-
 };
 
 }  // namespace ash