Revert "Views: Add BoxLayoutView"

This reverts commit 635cbc1a.

Reason for revert: suspect causing views_unittests failure on Linux ChromiumOS MSan Tests
---
BoxLayoutViewTest.NoLayoutInvalidationWhenPropertyUnchanged
BoxLayoutViewTest.LayoutInvalidationWhenPropertyChanged
---

First test failure: https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/20798

Sample log: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8867505219215733920/+/steps/views_unittests/0/logs/Deterministic_failure:_BoxLayoutViewTest.LayoutInvalidationWhenPropertyChanged__status_CRASH_/0
---
 [ RUN      ] BoxLayoutViewTest.LayoutInvalidationWhenPropertyChanged
 ==11171==WARNING: MemorySanitizer: use-of-uninitialized-value
     #0 0x5570eca375a3 in testing::AssertionResult testing::internal::CmpHelperNE<int, int>(char const*, char const*, int const&, int const&) ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:1638
     #1 0x5570eca375a3 in ?? ??:0
     #2 0x5570ed05abfe in views::BoxLayoutViewTest_LayoutInvalidationWhenPropertyChanged_Test::TestBody() ./../../ui/views/layout/box_layout_view_unittest.cc:78
---

Original change's description:
> Views: Add BoxLayoutView
>
> This CL adds class member getters and setters to the BoxLayout class
> where needed for the BoxLayoutView properties. Adds a TODO to ensure
> layout is invalidated when these members are set.
>
> Bug: None
> Change-Id: I13d73c75ca93f9de391b46c7241682ee7ced9a89
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2434561
> Reviewed-by: Wei Li <weili@chromium.org>
> Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#813351}

TBR=weili@chromium.org,tluk@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: None
Change-Id: I4683ac2a66b75c98da6adf20a80636fb6486aeef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2448573Reviewed-by: Takashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#813619}

ui/views/BUILD.gn

@@ -189,7 +189,6 @@ component("views") {
     "input_event_activation_protector.h",
     "layout/animating_layout_manager.h",
     "layout/box_layout.h",
-    "layout/box_layout_view.h",
     "layout/fill_layout.h",
     "layout/flex_layout.h",
     "layout/flex_layout_types.h",
@@ -393,7 +392,6 @@ component("views") {
     "input_event_activation_protector.cc",
     "layout/animating_layout_manager.cc",
     "layout/box_layout.cc",
-    "layout/box_layout_view.cc",
     "layout/fill_layout.cc",
     "layout/flex_layout.cc",
     "layout/flex_layout_types.cc",
@@ -1101,7 +1099,6 @@ test("views_unittests") {
     "focus/focus_traversal_unittest.cc",
     "layout/animating_layout_manager_unittest.cc",
     "layout/box_layout_unittest.cc",
-    "layout/box_layout_view_unittest.cc",
     "layout/composite_layout_tests.cc",
     "layout/fill_layout_unittest.cc",
     "layout/flex_layout_unittest.cc",

ui/views/layout/box_layout.cc

@@ -99,28 +99,6 @@ BoxLayout::BoxLayout(BoxLayout::Orientation orientation,
 
 BoxLayout::~BoxLayout() = default;
 
-void BoxLayout::SetOrientation(Orientation orientation) {
-  if (orientation_ != orientation) {
-    orientation_ = orientation;
-    InvalidateLayout();
-  }
-}
-
-BoxLayout::Orientation BoxLayout::GetOrientation() const {
-  return orientation_;
-}
-
-void BoxLayout::SetCollapseMarginsSpacing(bool collapse_margins_spacing) {
-  if (collapse_margins_spacing != collapse_margins_spacing_) {
-    collapse_margins_spacing_ = collapse_margins_spacing;
-    InvalidateLayout();
-  }
-}
-
-bool BoxLayout::GetCollapseMarginsSpacing() const {
-  return collapse_margins_spacing_;
-}
-
 void BoxLayout::SetFlexForView(const View* view,
                                int flex_weight,
                                bool use_min_size) {
@@ -142,10 +120,6 @@ void BoxLayout::SetDefaultFlex(int default_flex) {
   default_flex_ = default_flex;
 }
 
-int BoxLayout::GetDefaultFlex() const {
-  return default_flex_;
-}
-
 void BoxLayout::Layout(View* host) {
   DCHECK_EQ(host_, host);
   gfx::Rect child_area(host->GetContentsBounds());

ui/views/layout/box_layout.h

@@ -7,6 +7,8 @@
 
 #include <map>
 
+#include "base/compiler_specific.h"
+#include "base/macros.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/views/layout/layout_manager.h"
 #include "ui/views/view.h"
@@ -104,48 +106,31 @@ class VIEWS_EXPORT BoxLayout : public LayoutManager {
   // SSSSSSSSSSSSSSSSSSSS
   // --------------------
   //
-  explicit BoxLayout(Orientation orientation = Orientation::kHorizontal,
+  explicit BoxLayout(Orientation orientation,
                      const gfx::Insets& inside_border_insets = gfx::Insets(),
                      int between_child_spacing = 0,
                      bool collapse_margins_spacing = false);
   ~BoxLayout() override;
 
-  void SetOrientation(Orientation orientation);
-  Orientation GetOrientation() const;
-
-  // TODO(tluk): These class member setters should likely be calling
-  // LayoutManager::InvalidateLayout() .
   void set_main_axis_alignment(MainAxisAlignment main_axis_alignment) {
     main_axis_alignment_ = main_axis_alignment;
   }
-  MainAxisAlignment main_axis_alignment() const { return main_axis_alignment_; }
 
   void set_cross_axis_alignment(CrossAxisAlignment cross_axis_alignment) {
     cross_axis_alignment_ = cross_axis_alignment;
   }
-  CrossAxisAlignment cross_axis_alignment() const {
-    return cross_axis_alignment_;
-  }
 
   void set_inside_border_insets(const gfx::Insets& insets) {
     inside_border_insets_ = insets;
   }
-  const gfx::Insets& inside_border_insets() const {
-    return inside_border_insets_;
-  }
 
   void set_minimum_cross_axis_size(int size) {
     minimum_cross_axis_size_ = size;
   }
-  int minimum_cross_axis_size() const { return minimum_cross_axis_size_; }
 
   void set_between_child_spacing(int spacing) {
     between_child_spacing_ = spacing;
   }
-  int between_child_spacing() const { return between_child_spacing_; }
-
-  void SetCollapseMarginsSpacing(bool collapse_margins_spacing);
-  bool GetCollapseMarginsSpacing() const;
 
   // Sets the flex weight for the given |view|. Using the preferred size as
   // the basis, free space along the main axis is distributed to views in the
@@ -167,7 +152,6 @@ class VIEWS_EXPORT BoxLayout : public LayoutManager {
 
   // Sets the flex for views to use when none is specified.
   void SetDefaultFlex(int default_flex);
-  int GetDefaultFlex() const;
 
   // Overridden from views::LayoutManager:
   void Installed(View* host) override;
@@ -320,7 +304,7 @@ class VIEWS_EXPORT BoxLayout : public LayoutManager {
   // Return the last visible view in the host or nullptr if none are visible.
   View* LastVisibleView() const;
 
-  Orientation orientation_;
+  const Orientation orientation_;
 
   // Spacing between child views and host view border.
   gfx::Insets inside_border_insets_;
@@ -346,10 +330,12 @@ class VIEWS_EXPORT BoxLayout : public LayoutManager {
   int minimum_cross_axis_size_ = 0;
 
   // Adjacent view margins and spacing should be collapsed.
-  bool collapse_margins_spacing_;
+  const bool collapse_margins_spacing_;
 
   // The view that this BoxLayout is managing the layout for.
   views::View* host_ = nullptr;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(BoxLayout);
 };
 
 }  // namespace views

ui/views/layout/box_layout_view.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/box_layout_view.h"
-
-#include <memory>
-
-#include "ui/gfx/geometry/insets.h"
-#include "ui/views/layout/layout_manager.h"
-
-namespace views {
-
-BoxLayoutView::BoxLayoutView()
-    : layout_(SetLayoutManager(std::make_unique<BoxLayout>())),
-      orientation_(layout_->GetOrientation()),
-      main_axis_alignment_(layout_->main_axis_alignment()),
-      cross_axis_alignment_(layout_->cross_axis_alignment()),
-      inside_border_insets_(layout_->inside_border_insets()),
-      minimum_cross_axis_size_(layout_->minimum_cross_axis_size()),
-      between_child_spacing_(layout_->between_child_spacing()),
-      collapse_margins_spacing_(layout_->GetCollapseMarginsSpacing()) {}
-
-void BoxLayoutView::SetOrientation(BoxLayout::Orientation orientation) {
-  if (orientation_ == orientation)
-    return;
-  layout_->SetOrientation(orientation);
-  orientation_ = orientation;
-  OnPropertyChanged(&orientation_, kPropertyEffectsLayout);
-}
-
-BoxLayout::Orientation BoxLayoutView::GetOrientation() const {
-  return orientation_;
-}
-
-void BoxLayoutView::SetMainAxisAlignment(
-    BoxLayout::MainAxisAlignment main_axis_alignment) {
-  if (main_axis_alignment_ == main_axis_alignment)
-    return;
-  layout_->set_main_axis_alignment(main_axis_alignment);
-  main_axis_alignment_ = main_axis_alignment;
-  OnPropertyChanged(&main_axis_alignment_, kPropertyEffectsLayout);
-}
-
-BoxLayout::MainAxisAlignment BoxLayoutView::GetMainAxisAlignment() const {
-  return main_axis_alignment_;
-}
-
-void BoxLayoutView::SetCrossAxisAlignment(
-    BoxLayout::CrossAxisAlignment cross_axis_alignment) {
-  if (cross_axis_alignment_ == cross_axis_alignment)
-    return;
-  layout_->set_cross_axis_alignment(cross_axis_alignment);
-  cross_axis_alignment_ = cross_axis_alignment;
-  OnPropertyChanged(&cross_axis_alignment_, kPropertyEffectsLayout);
-}
-
-BoxLayout::CrossAxisAlignment BoxLayoutView::GetCrossAxisAlignment() const {
-  return cross_axis_alignment_;
-}
-
-void BoxLayoutView::SetInsideBorderInsets(const gfx::Insets& insets) {
-  if (inside_border_insets_ == insets)
-    return;
-  layout_->set_inside_border_insets(insets);
-  inside_border_insets_ = insets;
-  OnPropertyChanged(&inside_border_insets_, kPropertyEffectsLayout);
-}
-
-const gfx::Insets& BoxLayoutView::GetInsideBorderInsets() const {
-  return inside_border_insets_;
-}
-
-void BoxLayoutView::SetMinimumCrossAxisSize(int size) {
-  if (minimum_cross_axis_size_ == size)
-    return;
-  layout_->set_minimum_cross_axis_size(size);
-  minimum_cross_axis_size_ = size;
-  OnPropertyChanged(&minimum_cross_axis_size_, kPropertyEffectsLayout);
-}
-
-int BoxLayoutView::GetMinimumCrossAxisSize() const {
-  return minimum_cross_axis_size_;
-}
-
-void BoxLayoutView::SetBetweenChildSpacing(int spacing) {
-  if (between_child_spacing_ == spacing)
-    return;
-  layout_->set_between_child_spacing(spacing);
-  between_child_spacing_ = spacing;
-  OnPropertyChanged(&between_child_spacing_, kPropertyEffectsLayout);
-}
-
-int BoxLayoutView::GetBetweenChildSpacing() const {
-  return between_child_spacing_;
-}
-
-void BoxLayoutView::SetCollapseMarginsSpacing(bool collapse_margins_spacing) {
-  if (collapse_margins_spacing_ == collapse_margins_spacing)
-    return;
-  layout_->SetCollapseMarginsSpacing(collapse_margins_spacing);
-  collapse_margins_spacing_ = collapse_margins_spacing;
-  OnPropertyChanged(&collapse_margins_spacing_, kPropertyEffectsLayout);
-}
-
-bool BoxLayoutView::GetCollapseMarginsSpacing() const {
-  return collapse_margins_spacing_;
-}
-
-void BoxLayoutView::SetDefaultFlex(int default_flex) {
-  if (default_flex_ == default_flex)
-    return;
-  layout_->SetDefaultFlex(default_flex);
-  default_flex_ = default_flex;
-  OnPropertyChanged(&default_flex_, kPropertyEffectsLayout);
-}
-
-int BoxLayoutView::GetDefaultFlex() const {
-  return default_flex_;
-}
-
-void BoxLayoutView::SetFlexForView(const View* view,
-                                   int flex,
-                                   bool use_min_size) {
-  layout_->SetFlexForView(view, flex, use_min_size);
-  InvalidateLayout();
-}
-
-void BoxLayoutView::ClearFlexForView(const View* view) {
-  layout_->ClearFlexForView(view);
-  InvalidateLayout();
-}
-
-}  // namespace views

ui/views/layout/box_layout_view.h

-// 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.
-
-#ifndef UI_VIEWS_LAYOUT_BOX_LAYOUT_VIEW_H_
-#define UI_VIEWS_LAYOUT_BOX_LAYOUT_VIEW_H_
-
-#include "ui/views/layout/box_layout.h"
-#include "ui/views/view.h"
-
-namespace gfx {
-class Insets;
-}  // namespace gfx
-
-namespace views {
-
-class VIEWS_EXPORT BoxLayoutView : public View {
- public:
-  BoxLayoutView();
-  BoxLayoutView(BoxLayoutView&) = delete;
-  BoxLayoutView& operator=(BoxLayoutView&) = delete;
-  ~BoxLayoutView() override = default;
-
-  void SetOrientation(BoxLayout::Orientation orientation);
-  BoxLayout::Orientation GetOrientation() const;
-
-  void SetMainAxisAlignment(BoxLayout::MainAxisAlignment main_axis_alignment);
-  BoxLayout::MainAxisAlignment GetMainAxisAlignment() const;
-
-  void SetCrossAxisAlignment(
-      BoxLayout::CrossAxisAlignment cross_axis_alignment);
-  BoxLayout::CrossAxisAlignment GetCrossAxisAlignment() const;
-
-  void SetInsideBorderInsets(const gfx::Insets& insets);
-  const gfx::Insets& GetInsideBorderInsets() const;
-
-  void SetMinimumCrossAxisSize(int size);
-  int GetMinimumCrossAxisSize() const;
-
-  void SetBetweenChildSpacing(int spacing);
-  int GetBetweenChildSpacing() const;
-
-  void SetCollapseMarginsSpacing(bool collapse_margins_spacing);
-  bool GetCollapseMarginsSpacing() const;
-
-  void SetDefaultFlex(int default_flex);
-  int GetDefaultFlex() const;
-
-  void SetFlexForView(const View* view, int flex, bool use_min_size = false);
-  void ClearFlexForView(const View* view);
-
- private:
-  BoxLayout* const layout_;
-
-  // TODO(tluk): Merge these with the values in BoxLayout after transition to
-  // layout views is complete.
-  BoxLayout::Orientation orientation_;
-  BoxLayout::MainAxisAlignment main_axis_alignment_;
-  BoxLayout::CrossAxisAlignment cross_axis_alignment_;
-  gfx::Insets inside_border_insets_;
-  int minimum_cross_axis_size_;
-  int between_child_spacing_;
-  int collapse_margins_spacing_;
-  int default_flex_;
-};
-
-}  // namespace views
-
-#endif  // UI_VIEWS_LAYOUT_BOX_LAYOUT_VIEW_H_

ui/views/layout/box_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/box_layout_view.h"
-
-#include <memory>
-
-#include "testing/gtest/include/gtest/gtest.h"
-#include "ui/gfx/geometry/insets.h"
-#include "ui/views/view_test_api.h"
-
-namespace views {
-
-class BoxLayoutViewTest : public testing::Test {
- public:
-  void SetUp() override { host_ = std::make_unique<BoxLayoutView>(); }
-
-  BoxLayoutView* host() { return host_.get(); }
-
- private:
-  std::unique_ptr<BoxLayoutView> host_;
-};
-
-TEST_F(BoxLayoutViewTest, 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_FALSE(view_test_api.needs_layout());
-  EXPECT_NE(BoxLayout::Orientation::kVertical, host()->GetOrientation());
-  host()->SetOrientation(BoxLayout::Orientation::kVertical);
-  reset_layout();
-
-  EXPECT_FALSE(view_test_api.needs_layout());
-  EXPECT_NE(BoxLayout::MainAxisAlignment::kEnd, host()->GetMainAxisAlignment());
-  host()->SetMainAxisAlignment(BoxLayout::MainAxisAlignment::kEnd);
-  reset_layout();
-
-  EXPECT_FALSE(view_test_api.needs_layout());
-  EXPECT_NE(BoxLayout::CrossAxisAlignment::kEnd,
-            host()->GetCrossAxisAlignment());
-  host()->SetCrossAxisAlignment(BoxLayout::CrossAxisAlignment::kEnd);
-  reset_layout();
-
-  EXPECT_FALSE(view_test_api.needs_layout());
-  constexpr gfx::Insets inside_border_insets(10, 10);
-  EXPECT_NE(inside_border_insets, host()->GetInsideBorderInsets());
-  host()->SetInsideBorderInsets(inside_border_insets);
-  reset_layout();
-
-  EXPECT_FALSE(view_test_api.needs_layout());
-  constexpr int minimum_cross_axis_size = 10;
-  EXPECT_NE(minimum_cross_axis_size, host()->GetMinimumCrossAxisSize());
-  host()->SetMinimumCrossAxisSize(minimum_cross_axis_size);
-  reset_layout();
-
-  EXPECT_FALSE(view_test_api.needs_layout());
-  constexpr int between_child_spacing = 10;
-  EXPECT_NE(between_child_spacing, host()->GetBetweenChildSpacing());
-  host()->SetBetweenChildSpacing(between_child_spacing);
-  reset_layout();
-
-  EXPECT_FALSE(view_test_api.needs_layout());
-  constexpr bool collapse_margins_spacing = true;
-  EXPECT_NE(collapse_margins_spacing, host()->GetCollapseMarginsSpacing());
-  host()->SetCollapseMarginsSpacing(collapse_margins_spacing);
-  reset_layout();
-
-  EXPECT_FALSE(view_test_api.needs_layout());
-  constexpr int default_flex = 10;
-  EXPECT_NE(default_flex, host()->GetDefaultFlex());
-  host()->SetDefaultFlex(default_flex);
-  reset_layout();
-}
-
-TEST_F(BoxLayoutViewTest, NoLayoutInvalidationWhenPropertyUnchanged) {
-  ViewTestApi view_test_api(host());
-
-  // Ensure view starts with a valid layout.
-  host()->Layout();
-  EXPECT_FALSE(view_test_api.needs_layout());
-  host()->SetOrientation(host()->GetOrientation());
-  host()->SetMainAxisAlignment(host()->GetMainAxisAlignment());
-  host()->SetCrossAxisAlignment(host()->GetCrossAxisAlignment());
-  host()->SetInsideBorderInsets(host()->GetInsideBorderInsets());
-  host()->SetMinimumCrossAxisSize(host()->GetMinimumCrossAxisSize());
-  host()->SetBetweenChildSpacing(host()->GetBetweenChildSpacing());
-  host()->SetCollapseMarginsSpacing(host()->GetCollapseMarginsSpacing());
-  host()->SetDefaultFlex(host()->GetDefaultFlex());
-  EXPECT_FALSE(view_test_api.needs_layout());
-}
-
-}  // namespace views