Make SizeBounds constexpr like gfx::Size.

This also adds a move constructor (since there's no obvious reason not
to, and Optional has them) and copy and move assignment (since
constructors and assignments should be defined together per style guide).

This also makes the comparison operators non-members since this is
preferred when feasible (e.g. makes implicit conversion for comparisons
apply equally to both sides).

Bug: none
Change-Id: I5b593f078063b8f9502dae232ed28e01ae41cd5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026688
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: Dana Fried <dfried@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736272}

ui/views/layout/animating_layout_manager_unittest.cc

@@ -4186,7 +4186,7 @@ TEST_F(AnimatingLayoutManagerRealtimeTest, TestAnimateStretch) {
 
 TEST_F(AnimatingLayoutManagerRealtimeTest, TestConstrainedSpaceStopsAnimation) {
   constexpr gfx::Insets kChildMargins(5);
-  static const SizeBounds kSizeBounds(45, base::nullopt);
+  constexpr SizeBounds kSizeBounds(45, base::nullopt);
   layout()->SetShouldAnimateBounds(true);
   layout()->SetAnimationDuration(kMinimumAnimationTime);
   auto* const flex_layout =
@@ -4230,7 +4230,7 @@ TEST_F(AnimatingLayoutManagerRealtimeTest, TestConstrainedSpaceStopsAnimation) {
 
 TEST_F(AnimatingLayoutManagerRealtimeTest, TestConstrainedSpaceDoesNotRestart) {
   constexpr gfx::Insets kChildMargins(5);
-  static const SizeBounds kSizeBounds(45, base::nullopt);
+  constexpr SizeBounds kSizeBounds(45, base::nullopt);
   layout()->SetShouldAnimateBounds(true);
   layout()->SetAnimationDuration(kMinimumAnimationTime);
   auto* const flex_layout =
@@ -4278,7 +4278,7 @@ TEST_F(AnimatingLayoutManagerRealtimeTest, TestConstrainedSpaceDoesNotRestart) {
 TEST_F(AnimatingLayoutManagerRealtimeTest,
        TestConstrainedSpaceRestartedAnimationSucceeds) {
   constexpr gfx::Insets kChildMargins(5);
-  static const SizeBounds kSizeBounds(45, base::nullopt);
+  constexpr SizeBounds kSizeBounds(45, base::nullopt);
   layout()->SetShouldAnimateBounds(true);
   layout()->SetAnimationDuration(kMinimumAnimationTime);
   auto* const flex_layout =
@@ -4287,7 +4287,7 @@ TEST_F(AnimatingLayoutManagerRealtimeTest,
   flex_layout->SetCollapseMargins(true);
   flex_layout->SetCrossAxisAlignment(LayoutAlignment::kStart);
   flex_layout->SetDefault(kMarginsKey, kChildMargins);
-  InitRootView(std::move(kSizeBounds));
+  InitRootView(kSizeBounds);
   child(0)->SetProperty(kFlexBehaviorKey, FlexSpecification::ForSizeRule(
                                               MinimumFlexSizeRule::kScaleToZero,
                                               MaximumFlexSizeRule::kPreferred));

ui/views/layout/layout_types.cc

@@ -7,23 +7,10 @@
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 
-#include "ui/gfx/geometry/size.h"
-
 namespace views {
 
 // SizeBounds ------------------------------------------------------------------
 
-SizeBounds::SizeBounds() = default;
-
-SizeBounds::SizeBounds(base::Optional<int> width, base::Optional<int> height)
-    : width_(std::move(width)), height_(std::move(height)) {}
-
-SizeBounds::SizeBounds(const SizeBounds& other)
-    : width_(other.width()), height_(other.height()) {}
-
-SizeBounds::SizeBounds(const gfx::Size& other)
-    : width_(other.width()), height_(other.height()) {}
-
 void SizeBounds::Enlarge(int width, int height) {
   if (width_)
     width_ = std::max(0, *width_ + width);
@@ -31,18 +18,6 @@ void SizeBounds::Enlarge(int width, int height) {
     height_ = std::max(0, *height_ + height);
 }
 
-bool SizeBounds::operator==(const SizeBounds& other) const {
-  return std::tie(width_, height_) == std::tie(other.width_, other.height_);
-}
-
-bool SizeBounds::operator!=(const SizeBounds& other) const {
-  return !(*this == other);
-}
-
-bool SizeBounds::operator<(const SizeBounds& other) const {
-  return std::tie(height_, width_) < std::tie(other.height_, other.width_);
-}
-
 std::string SizeBounds::ToString() const {
   return base::StrCat({width_ ? base::NumberToString(*width_) : "_", " x ",
                        height_ ? base::NumberToString(*height_) : "_"});

ui/views/layout/layout_types.h

@@ -7,14 +7,12 @@
 
 #include <ostream>
 #include <string>
+#include <tuple>
 
 #include "base/optional.h"
+#include "ui/gfx/geometry/size.h"
 #include "ui/views/views_export.h"
 
-namespace gfx {
-class Size;
-}  // namespace gfx
-
 namespace views {
 
 // Whether a layout is oriented horizontally or vertically.
@@ -27,31 +25,64 @@ enum class LayoutOrientation {
 // preferred size of a layout pursuant to a maximum available size.
 class VIEWS_EXPORT SizeBounds {
  public:
-  SizeBounds();
-  SizeBounds(base::Optional<int> width, base::Optional<int> height);
-  explicit SizeBounds(const gfx::Size& size);
-  SizeBounds(const SizeBounds& other);
-
-  const base::Optional<int>& width() const { return width_; }
-  void set_width(base::Optional<int> width) { width_ = std::move(width); }
-
-  const base::Optional<int>& height() const { return height_; }
-  void set_height(base::Optional<int> height) { height_ = std::move(height); }
+  // Method definitions below to avoid "complex constructor" warning.  Marked
+  // explicitly inline because Clang currently doesn't realize that "constexpr"
+  // explicitly means "inline" and thus should count as "intentionally inlined
+  // and thus shouldn't be warned about".
+  // TODO(crbug.com/1045568): Remove "inline" if Clang's isInlineSpecified()
+  // learns about constexpr.
+  // TODO(crbug.com/1045570): Put method bodies here if complex constructor
+  // heuristic learns to peer into types to discover that e.g. Optional is not
+  // complex.
+  inline constexpr SizeBounds();
+  inline constexpr SizeBounds(base::Optional<int> width,
+                              base::Optional<int> height);
+  inline constexpr explicit SizeBounds(const gfx::Size& size);
+  inline constexpr SizeBounds(const SizeBounds&);
+  inline constexpr SizeBounds(SizeBounds&&);
+  SizeBounds& operator=(const SizeBounds&) = default;
+  SizeBounds& operator=(SizeBounds&&) = default;
+  ~SizeBounds() = default;
+
+  constexpr const base::Optional<int>& width() const { return width_; }
+  constexpr void set_width(base::Optional<int> width) {
+    width_ = std::move(width);
+  }
+
+  constexpr const base::Optional<int>& height() const { return height_; }
+  constexpr void set_height(base::Optional<int> height) {
+    height_ = std::move(height);
+  }
 
   // Enlarges (or shrinks, if negative) each upper bound that is present by the
   // specified amounts.
   void Enlarge(int width, int height);
 
-  bool operator==(const SizeBounds& other) const;
-  bool operator!=(const SizeBounds& other) const;
-  bool operator<(const SizeBounds& other) const;
-
   std::string ToString() const;
 
  private:
   base::Optional<int> width_;
   base::Optional<int> height_;
 };
+constexpr SizeBounds::SizeBounds() = default;
+constexpr SizeBounds::SizeBounds(base::Optional<int> width,
+                                 base::Optional<int> height)
+    : width_(std::move(width)), height_(std::move(height)) {}
+constexpr SizeBounds::SizeBounds(const gfx::Size& size)
+    : width_(size.width()), height_(size.height()) {}
+constexpr SizeBounds::SizeBounds(const SizeBounds&) = default;
+constexpr SizeBounds::SizeBounds(SizeBounds&&) = default;
+constexpr bool operator==(const SizeBounds& lhs, const SizeBounds& rhs) {
+  return std::tie(lhs.width(), lhs.height()) ==
+         std::tie(rhs.width(), rhs.height());
+}
+constexpr bool operator!=(const SizeBounds& lhs, const SizeBounds& rhs) {
+  return !(lhs == rhs);
+}
+constexpr bool operator<(const SizeBounds& lhs, const SizeBounds& rhs) {
+  return std::tie(lhs.height(), lhs.width()) <
+         std::tie(rhs.height(), rhs.width());
+}
 
 // These are declared here for use in gtest-based unit tests but is defined in
 // the views_test_support target. Depend on that to use this in your unit test.