Views: Update FlexLayoutView with View builder support

This CL updates the FlexLayoutView with View builder support. It
also removes the use of enums for property ids and adds unittests.

Bug: None
Change-Id: If968a111eb8945f7150cd9df99a889f30156175a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2473238
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Reviewed-by: Wei Li <weili@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817689}

ui/views/BUILD.gn

@@ -1122,6 +1122,7 @@ test("views_unittests") {
     "layout/composite_layout_tests.cc",
     "layout/fill_layout_unittest.cc",
     "layout/flex_layout_unittest.cc",
+    "layout/flex_layout_view_unittest.cc",
     "layout/grid_layout_unittest.cc",
     "layout/layout_manager_base_unittest.cc",
     "layout/normalized_geometry_unittest.cc",

ui/views/layout/flex_layout_view.cc

@@ -6,47 +6,41 @@
 
 #include <memory>
 
-namespace {
-
-// An enum giving different RenderText properties unique keys for the
-// OnPropertyChanged call.
-enum LabelPropertyKey {
-  kOrientation = 1,
-  kMainAxisAlignment,
-  kCrossAxisAlignment,
-  kInteriorMargin,
-  kMinimumCrossAxisSize,
-  kCollapseMargins,
-  kIncludeHostInsetsInLayout,
-  kIgnoreMainAxisMargins,
-  kFlexAllocationOrder,
-};
-
-}  // namespace
-
 namespace views {
 
 FlexLayoutView::FlexLayoutView()
-    : layout_(SetLayoutManager(std::make_unique<FlexLayout>())) {}
+    : layout_(SetLayoutManager(std::make_unique<FlexLayout>())),
+      orientation_(layout_->orientation()),
+      main_axis_alignment_(layout_->main_axis_alignment()),
+      cross_axis_alignment_(layout_->cross_axis_alignment()),
+      interior_margin_(layout_->interior_margin()),
+      minimum_cross_axis_size_(layout_->minimum_cross_axis_size()),
+      collapse_margins_(layout_->collapse_margins()),
+      include_host_insets_in_layout_(layout_->include_host_insets_in_layout()),
+      ignore_default_main_axis_margins_(
+          layout_->ignore_default_main_axis_margins()),
+      flex_allocation_order_(layout_->flex_allocation_order()) {}
 
 FlexLayoutView::~FlexLayoutView() = default;
 
 void FlexLayoutView::SetOrientation(LayoutOrientation orientation) {
-  if (orientation == layout_->orientation())
+  if (orientation_ == orientation)
     return;
   layout_->SetOrientation(orientation);
-  OnPropertyChanged(&layout_ + kOrientation, kPropertyEffectsLayout);
+  orientation_ = orientation;
+  OnPropertyChanged(&orientation_, kPropertyEffectsLayout);
 }
 
 LayoutOrientation FlexLayoutView::GetOrientation() const {
-  return layout_->orientation();
+  return orientation_;
 }
 
 void FlexLayoutView::SetMainAxisAlignment(LayoutAlignment main_axis_alignment) {
-  if (main_axis_alignment == layout_->main_axis_alignment())
+  if (main_axis_alignment_ == main_axis_alignment)
     return;
   layout_->SetMainAxisAlignment(main_axis_alignment);
-  OnPropertyChanged(&layout_ + kMainAxisAlignment, kPropertyEffectsLayout);
+  main_axis_alignment_ = main_axis_alignment;
+  OnPropertyChanged(&main_axis_alignment_, kPropertyEffectsLayout);
 }
 
 LayoutAlignment FlexLayoutView::GetMainAxisAlignment() const {
@@ -55,86 +49,91 @@ LayoutAlignment FlexLayoutView::GetMainAxisAlignment() const {
 
 void FlexLayoutView::SetCrossAxisAlignment(
     LayoutAlignment cross_axis_alignment) {
-  if (cross_axis_alignment == layout_->cross_axis_alignment())
+  if (cross_axis_alignment_ == cross_axis_alignment)
     return;
   layout_->SetCrossAxisAlignment(cross_axis_alignment);
-  OnPropertyChanged(&layout_ + kCrossAxisAlignment, kPropertyEffectsLayout);
+  cross_axis_alignment_ = cross_axis_alignment;
+  OnPropertyChanged(&cross_axis_alignment_, kPropertyEffectsLayout);
 }
 
 LayoutAlignment FlexLayoutView::GetCrossAxisAlignment() const {
-  return layout_->cross_axis_alignment();
+  return cross_axis_alignment_;
 }
 
 void FlexLayoutView::SetInteriorMargin(const gfx::Insets& interior_margin) {
-  if (interior_margin == layout_->interior_margin())
+  if (interior_margin_ == interior_margin)
     return;
   layout_->SetInteriorMargin(interior_margin);
-  OnPropertyChanged(&layout_ + kInteriorMargin, kPropertyEffectsLayout);
+  interior_margin_ = interior_margin;
+  OnPropertyChanged(&interior_margin_, kPropertyEffectsLayout);
 }
 
 const gfx::Insets& FlexLayoutView::GetInteriorMargin() const {
-  return layout_->interior_margin();
+  return interior_margin_;
 }
 
 void FlexLayoutView::SetMinimumCrossAxisSize(int size) {
-  if (size == layout_->minimum_cross_axis_size())
+  if (minimum_cross_axis_size_ == size)
     return;
   layout_->SetMinimumCrossAxisSize(size);
-  OnPropertyChanged(&layout_ + kMinimumCrossAxisSize, kPropertyEffectsLayout);
+  minimum_cross_axis_size_ = size;
+  OnPropertyChanged(&minimum_cross_axis_size_, kPropertyEffectsLayout);
 }
 
 int FlexLayoutView::GetMinimumCrossAxisSize() const {
-  return layout_->minimum_cross_axis_size();
+  return minimum_cross_axis_size_;
 }
 
 void FlexLayoutView::SetCollapseMargins(bool collapse_margins) {
-  if (collapse_margins == layout_->collapse_margins())
+  if (collapse_margins_ == collapse_margins)
     return;
   layout_->SetCollapseMargins(collapse_margins);
-  OnPropertyChanged(&layout_ + kCollapseMargins, kPropertyEffectsLayout);
+  collapse_margins_ = collapse_margins;
+  OnPropertyChanged(&collapse_margins_, kPropertyEffectsLayout);
 }
 
 bool FlexLayoutView::GetCollapseMargins() const {
-  return layout_->collapse_margins();
+  return collapse_margins_;
 }
 
 void FlexLayoutView::SetIncludeHostInsetsInLayout(
     bool include_host_insets_in_layout) {
-  if (include_host_insets_in_layout == layout_->include_host_insets_in_layout())
+  if (include_host_insets_in_layout_ == include_host_insets_in_layout)
     return;
   layout_->SetIncludeHostInsetsInLayout(include_host_insets_in_layout);
-  OnPropertyChanged(&layout_ + kIncludeHostInsetsInLayout,
-                    kPropertyEffectsLayout);
+  include_host_insets_in_layout_ = include_host_insets_in_layout;
+  OnPropertyChanged(&include_host_insets_in_layout_, kPropertyEffectsLayout);
 }
 
 bool FlexLayoutView::GetIncludeHostInsetsInLayout() const {
-  return layout_->include_host_insets_in_layout();
+  return include_host_insets_in_layout_;
 }
 
 void FlexLayoutView::SetIgnoreDefaultMainAxisMargins(
     bool ignore_default_main_axis_margins) {
-  if (ignore_default_main_axis_margins ==
-      layout_->ignore_default_main_axis_margins()) {
+  if (ignore_default_main_axis_margins == ignore_default_main_axis_margins_) {
     return;
   }
   layout_->SetIgnoreDefaultMainAxisMargins(ignore_default_main_axis_margins);
-  OnPropertyChanged(&layout_ + kIgnoreMainAxisMargins, kPropertyEffectsLayout);
+  ignore_default_main_axis_margins_ = ignore_default_main_axis_margins;
+  OnPropertyChanged(&ignore_default_main_axis_margins_, kPropertyEffectsLayout);
 }
 
 bool FlexLayoutView::GetIgnoreDefaultMainAxisMargins() const {
-  return layout_->ignore_default_main_axis_margins();
+  return ignore_default_main_axis_margins_;
 }
 
 void FlexLayoutView::SetFlexAllocationOrder(
     FlexAllocationOrder flex_allocation_order) {
-  if (flex_allocation_order == layout_->flex_allocation_order())
+  if (flex_allocation_order_ == flex_allocation_order)
     return;
   layout_->SetFlexAllocationOrder(flex_allocation_order);
-  OnPropertyChanged(&layout_ + kFlexAllocationOrder, kPropertyEffectsLayout);
+  flex_allocation_order_ = flex_allocation_order;
+  OnPropertyChanged(&flex_allocation_order_, kPropertyEffectsLayout);
 }
 
 FlexAllocationOrder FlexLayoutView::GetFlexAllocationOrder() const {
-  return layout_->flex_allocation_order();
+  return flex_allocation_order_;
 }
 
 FlexRule FlexLayoutView::GetDefaultFlexRule() const {

ui/views/layout/flex_layout_view.h

@@ -7,6 +7,7 @@
 
 #include "ui/views/layout/flex_layout.h"
 #include "ui/views/layout/flex_layout_types.h"
+#include "ui/views/metadata/view_factory.h"
 #include "ui/views/view.h"
 #include "ui/views/views_export.h"
 
@@ -54,18 +55,41 @@ class VIEWS_EXPORT FlexLayoutView : public View {
   template <class T, class U>
   void SetDefault(const ui::ClassProperty<T>* key, U&& value) {
     layout_->SetDefault(key, value);
+    InvalidateLayout();
   }
 
   // Copies and uses |value| as the default value for layout property |key|.
   template <class T, class U>
   void SetDefault(const ui::ClassProperty<T>* key, const U& value) {
     layout_->SetDefault(key, value);
+    InvalidateLayout();
   }
 
  private:
   FlexLayout* layout_;
+  LayoutOrientation orientation_;
+  LayoutAlignment main_axis_alignment_;
+  LayoutAlignment cross_axis_alignment_;
+  gfx::Insets interior_margin_;
+  int minimum_cross_axis_size_;
+  bool collapse_margins_;
+  bool include_host_insets_in_layout_;
+  bool ignore_default_main_axis_margins_;
+  FlexAllocationOrder flex_allocation_order_;
 };
 
+BEGIN_VIEW_BUILDER(VIEWS_EXPORT, FlexLayoutView, View)
+VIEW_BUILDER_PROPERTY(LayoutOrientation, Orientation)
+VIEW_BUILDER_PROPERTY(LayoutAlignment, MainAxisAlignment)
+VIEW_BUILDER_PROPERTY(LayoutAlignment, CrossAxisAlignment)
+VIEW_BUILDER_PROPERTY(const gfx::Insets, InteriorMargin)
+VIEW_BUILDER_PROPERTY(int, MinimumCrossAxisSize)
+VIEW_BUILDER_PROPERTY(bool, CollapseMargins)
+VIEW_BUILDER_PROPERTY(bool, IncludeHostInsetsInLayout)
+VIEW_BUILDER_PROPERTY(bool, IgnoreDefaultMainAxisMargins)
+VIEW_BUILDER_PROPERTY(FlexAllocationOrder, FlexAllocationOrder)
+END_VIEW_BUILDER(VIEWS_EXPORT, FlexLayoutView)
+
 }  // namespace views
 
 #endif  // UI_VIEWS_LAYOUT_FLEX_LAYOUT_VIEW_H_

ui/views/layout/flex_layout_view_unittest.cc

+// Copyright 2020 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 "ui/views/layout/flex_layout_view.h"
+
+#include <memory>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/geometry/insets.h"
+#include "ui/views/layout/flex_layout_types.h"
+#include "ui/views/view_test_api.h"
+
+namespace views {
+
+class FlexLayoutViewTest : public testing::Test {
+ public:
+  void SetUp() override { host_ = std::make_unique<FlexLayoutView>(); }
+
+  FlexLayoutView* host() { return host_.get(); }
+
+ private:
+  std::unique_ptr<FlexLayoutView> host_;
+};
+
+TEST_F(FlexLayoutViewTest, LayoutInvalidationWhenPropertyChanged) {
+  ViewTestApi view_test_api(host());
+  auto reset_layout = [&]() {
+    EXPECT_TRUE(view_test_api.needs_layout());
+    // Call layout() to set layout to a valid state.
+    host()->Layout();
+  };
+
+  // Ensure host() starts with a valid layout.
+  host()->Layout();
+
+  EXPECT_NE(LayoutOrientation::kVertical, host()->GetOrientation());
+  host()->SetOrientation(LayoutOrientation::kVertical);
+  reset_layout();
+
+  EXPECT_NE(LayoutAlignment::kEnd, host()->GetMainAxisAlignment());
+  host()->SetMainAxisAlignment(LayoutAlignment::kEnd);
+  reset_layout();
+
+  EXPECT_NE(LayoutAlignment::kEnd, host()->GetCrossAxisAlignment());
+  host()->SetCrossAxisAlignment(LayoutAlignment::kEnd);
+  reset_layout();
+
+  constexpr gfx::Insets interior_margin(10, 10);
+  EXPECT_NE(interior_margin, host()->GetInteriorMargin());
+  host()->SetInteriorMargin(interior_margin);
+  reset_layout();
+
+  constexpr int min_cross_axis_size = 10;
+  EXPECT_NE(min_cross_axis_size, host()->GetMinimumCrossAxisSize());
+  host()->SetMinimumCrossAxisSize(min_cross_axis_size);
+  reset_layout();
+
+  constexpr bool collapse_margins = true;
+  EXPECT_NE(collapse_margins, host()->GetCollapseMargins());
+  host()->SetCollapseMargins(collapse_margins);
+  reset_layout();
+
+  constexpr bool include_host_insets_in_layout = true;
+  EXPECT_NE(include_host_insets_in_layout,
+            host()->GetIncludeHostInsetsInLayout());
+  host()->SetIncludeHostInsetsInLayout(include_host_insets_in_layout);
+  reset_layout();
+
+  constexpr bool ignore_default_main_axis_margins = true;
+  EXPECT_NE(ignore_default_main_axis_margins,
+            host()->GetIgnoreDefaultMainAxisMargins());
+  host()->SetIgnoreDefaultMainAxisMargins(ignore_default_main_axis_margins);
+  reset_layout();
+
+  constexpr FlexAllocationOrder flex_allocation_order =
+      FlexAllocationOrder::kReverse;
+  EXPECT_NE(flex_allocation_order, host()->GetFlexAllocationOrder());
+  host()->SetFlexAllocationOrder(flex_allocation_order);
+  reset_layout();
+}
+
+TEST_F(FlexLayoutViewTest, NoLayoutInvalidationWhenPropertyUnchanged) {
+  ViewTestApi view_test_api(host());
+
+  // Ensure view starts with a valid layout.
+  host()->Layout();
+  host()->SetOrientation(host()->GetOrientation());
+  host()->SetMainAxisAlignment(host()->GetMainAxisAlignment());
+  host()->SetCrossAxisAlignment(host()->GetCrossAxisAlignment());
+  host()->SetInteriorMargin(host()->GetInteriorMargin());
+  host()->SetMinimumCrossAxisSize(host()->GetMinimumCrossAxisSize());
+  host()->SetCollapseMargins(host()->GetCollapseMargins());
+  host()->SetIncludeHostInsetsInLayout(host()->GetIncludeHostInsetsInLayout());
+  host()->SetIgnoreDefaultMainAxisMargins(
+      host()->GetIgnoreDefaultMainAxisMargins());
+  host()->SetFlexAllocationOrder(host()->GetFlexAllocationOrder());
+  EXPECT_FALSE(view_test_api.needs_layout());
+}
+
+}  // namespace views