Misc. cleanup found while fixing bug 1012136.

A whole slew of unrelated small changes:

* Use =default more
* Shorten code
* Better use of idioms like GetMainAxis(), ClampToRange(), StrCat(), etc.
* Attempt to improve comments (clearer, briefer, etc.)
* Change some {} constructions to constructor syntax.  Improves compliance
  with
  https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++-dos-and-donts.md#variable-initialization
  and makes the type in question clearer.
* More use of pass-by-value-and-move where callee always copies.
* Reorder declarations to match definitions
* Eliminate default arg to AllocateFlexSpace(), which just decreased
  clarity slightly
* Remove unnecessary conditionals
* Eliminate else after return
* Declare variables as close to first use as possible
* Implement some equality checks with std::tie() (not required style,
  but mentioned in totw/43 and tends to reduce maintenance hassle when
  more fields are added; is also more consistent with how we already used
  it for some inequalities of these types)
* Fix badly-copy-and-pasted param names
* Remove unreachable code that also defeats warn-if-unhandled-enum-added

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

chrome/browser/ui/views/layout/interpolating_layout_manager.cc

@@ -8,9 +8,10 @@
 #include <utility>
 
 #include "ui/gfx/animation/tween.h"
+#include "ui/views/layout/normalized_geometry.h"
 #include "ui/views/view.h"
 
-InterpolatingLayoutManager::InterpolatingLayoutManager() {}
+InterpolatingLayoutManager::InterpolatingLayoutManager() = default;
 InterpolatingLayoutManager::~InterpolatingLayoutManager() = default;
 
 InterpolatingLayoutManager& InterpolatingLayoutManager::SetOrientation(
@@ -55,9 +56,7 @@ InterpolatingLayoutManager::GetInterpolation(
   LayoutInterpolation result;
 
   const base::Optional<int> dimension =
-      orientation_ == views::LayoutOrientation::kHorizontal
-          ? size_bounds.width()
-          : size_bounds.height();
+      views::GetMainAxis(orientation_, size_bounds);
 
   // Find the larger layout that overlaps the target size.
   auto match = dimension ? embedded_layouts_.upper_bound({*dimension, 0})
@@ -77,7 +76,7 @@ InterpolatingLayoutManager::GetInterpolation(
         << " but there is no smaller layout to interpolate with.";
     result.second = (--match)->second;
     result.percent_second =
-        float{first_span.end() - *dimension} / float{first_span.length()};
+        float{first_span.end() - *dimension} / first_span.length();
   }
 
   return result;
@@ -85,8 +84,8 @@ InterpolatingLayoutManager::GetInterpolation(
 
 views::ProposedLayout InterpolatingLayoutManager::CalculateProposedLayout(
     const views::SizeBounds& size_bounds) const {
-  // For interpolating layout we will never call this method except for fully-
-  // specified sizes.
+  // For interpolating layout we will never call this method for unbounded
+  // sizes.
   DCHECK(size_bounds.width());
   DCHECK(size_bounds.height());
   const gfx::Size size(*size_bounds.width(), *size_bounds.height());
@@ -150,7 +149,7 @@ int InterpolatingLayoutManager::GetPreferredHeightForWidth(
   // experience; if this doesn't work in practice we can look at other options.
 
   const LayoutInterpolation interpolation =
-      GetInterpolation({width, base::nullopt});
+      GetInterpolation(views::SizeBounds(width, base::nullopt));
   const int first =
       interpolation.first->GetPreferredHeightForWidth(host, width);
   if (!interpolation.second)

chrome/browser/ui/views/toolbar/browser_actions_container.cc

@@ -381,8 +381,8 @@ views::FlexRule BrowserActionsContainer::GetFlexRule() {
                                       : browser_actions->GetResizeAreaWidth();
             // The ceiling on the value is the lesser of the preferred and
             // available size.
-            width = std::max(min_width, std::min(preferred_size.width(),
-                                                 *maximum_size.width()));
+            width = base::ClampToRange(preferred_size.width(), min_width,
+                                       *maximum_size.width());
           } else {
             // When not animating or resizing, the desired width should always
             // be based on the number of icons that can be displayed.

ui/views/layout/animating_layout_manager.cc

@@ -25,11 +25,8 @@ namespace {
 // Returns false if either the width or height of |bounds| is specified and is
 // smaller than the corresponding element of |size|.
 bool CanFitInBounds(const gfx::Size& size, const SizeBounds& bounds) {
-  if (bounds.width() && *bounds.width() < size.width())
-    return false;
-  if (bounds.height() && *bounds.height() < size.height())
-    return false;
-  return true;
+  return (!bounds.width() || (*bounds.width() >= size.width())) &&
+         (!bounds.height() || (*bounds.height() >= size.height()));
 }
 
 // Returns the ChildLayout data for the child view in the proposed layout, or
@@ -991,9 +988,9 @@ ChildLayout AnimatingLayoutManager::CalculateSlideFade(
 }
 
 SizeBounds AnimatingLayoutManager::GetAvailableHostSize() const {
-  if (!host_view() || !host_view()->parent())
-    return SizeBounds();
-  return host_view()->parent()->GetAvailableSize(host_view());
+  DCHECK(host_view());
+  const auto* const parent = host_view()->parent();
+  return parent ? parent->GetAvailableSize(host_view()) : SizeBounds();
 }
 
 // Returns the space in which to calculate the target layout.
@@ -1006,16 +1003,17 @@ gfx::Size AnimatingLayoutManager::GetAvailableTargetLayoutSize() {
   const gfx::Size preferred_size =
       target_layout_manager()->GetPreferredSize(host_view());
   if (!bounds.width() || *bounds.width() > preferred_size.width()) {
-    return {preferred_size.width(),
-            bounds.height()
-                ? std::min(preferred_size.height(), *bounds.height())
-                : preferred_size.height()};
+    return gfx::Size(preferred_size.width(),
+                     bounds.height()
+                         ? std::min(preferred_size.height(), *bounds.height())
+                         : preferred_size.height());
   }
 
   const int height = target_layout_manager()->GetPreferredHeightForWidth(
       host_view(), *bounds.width());
-  return {*bounds.width(),
-          bounds.height() ? std::min(height, *bounds.height()) : height};
+  return gfx::Size(*bounds.width(), bounds.height()
+                                        ? std::min(height, *bounds.height())
+                                        : height);
 }
 
 // static
@@ -1034,8 +1032,8 @@ gfx::Size AnimatingLayoutManager::DefaultFlexRuleImpl(
   if (CanFitInBounds(preferred_size, size_bounds))
     return preferred_size;
 
-  const LayoutOrientation orientation = animating_layout->orientation();
-  const base::Optional<int> bounds_main = GetMainAxis(orientation, size_bounds);
+  const base::Optional<int> bounds_main =
+      GetMainAxis(animating_layout->orientation(), size_bounds);
 
   // Special case - if we're being asked for a zero-size layout we'll return the
   // minimum size of the layout. This is because we're being probed for how
@@ -1060,22 +1058,24 @@ gfx::Size AnimatingLayoutManager::DefaultFlexRuleImpl(
   // provided.
   gfx::Size size;
   if (size_bounds.width() && size_bounds.height()) {
-    // If both width and height are specified, query the preferred layout in
-    // that space and return its size.
-    size = {*size_bounds.width(), *size_bounds.height()};
+    // Both width and height are specified.  Constraining the width may change
+    // the desired height, so we can't just blindly return the minimum in both
+    // dimensions.  Instead, query the target layout in the constrained space
+    // and return its size.
+    size = gfx::Size(*size_bounds.width(), *size_bounds.height());
   } else if (size_bounds.width()) {
-    // If only the width is specified and we are still constrained, use the
-    // height-for-width calculation.
+    // The width is specified and too small.  Use the height-for-width
+    // calculation.
     // TODO(dfried): This should be rare, but it is also inefficient. See if we
-    // can't add an alternative to GetPreferredHeightForWidth that actually
+    // can't add an alternative to GetPreferredHeightForWidth() that actually
     // calculates the layout in this space so we don't have to do it twice.
     const int height =
         target_layout->GetPreferredHeightForWidth(view, *size_bounds.width());
-    size = {*size_bounds.width(), height};
+    size = gfx::Size(*size_bounds.width(), height);
   } else {
-    // We now know that only the height is constrained and it's too small.
-    // Fortunately the height of a layout can't (shouldn't?) affect its width.
-    size = {target_preferred.width(), *size_bounds.height()};
+    // The height is specified and too small.  Fortunately the height of a
+    // layout can't (shouldn't?) affect its width.
+    size = gfx::Size(target_preferred.width(), *size_bounds.height());
   }
 
   return target_layout->GetProposedLayout(size).host_size;

ui/views/layout/animating_layout_manager_unittest.cc

@@ -2714,8 +2714,9 @@ constexpr base::TimeDelta kMinimumAnimationTime =
 class ImmediateLayoutManager : public LayoutManagerBase {
  public:
   explicit ImmediateLayoutManager(bool use_preferred_size,
-                                  const SizeBounds& size_bounds = SizeBounds())
-      : use_preferred_size_(use_preferred_size), size_bounds_(size_bounds) {
+                                  SizeBounds size_bounds = SizeBounds())
+      : use_preferred_size_(use_preferred_size),
+        size_bounds_(std::move(size_bounds)) {
     DCHECK(use_preferred_size_ || size_bounds == SizeBounds());
   }
 
@@ -3519,7 +3520,7 @@ TEST_F(AnimatingLayoutManagerFlexRuleTest, ReturnsPreferredSize) {
   InitLayout(LayoutOrientation::kHorizontal, kScaleToMinimumSnapToZero,
              gfx::Size(5, 5), false);
   EXPECT_EQ(flex_layout()->GetPreferredSize(view()),
-            flex_rule()->Run(view(), {}));
+            flex_rule()->Run(view(), SizeBounds()));
 }
 
 TEST_F(AnimatingLayoutManagerFlexRuleTest,
@@ -3527,8 +3528,8 @@ TEST_F(AnimatingLayoutManagerFlexRuleTest,
   InitLayout(LayoutOrientation::kVertical, kScaleToMinimumSnapToZero,
              gfx::Size(5, 5), true);
   const gfx::Size preferred = flex_layout()->GetPreferredSize(view());
-  const gfx::Size result =
-      flex_rule()->Run(view(), {preferred.width() + 5, base::nullopt});
+  const gfx::Size result = flex_rule()->Run(
+      view(), SizeBounds(preferred.width() + 5, base::nullopt));
   EXPECT_EQ(preferred, result);
   EXPECT_EQ(3U, GetVisibleChildCount(result));
 }
@@ -3543,7 +3544,8 @@ TEST_F(AnimatingLayoutManagerFlexRuleTest,
   const int height_for_width =
       flex_layout()->GetPreferredHeightForWidth(view(), width);
   DCHECK_GT(height_for_width, preferred.height());
-  const gfx::Size result = flex_rule()->Run(view(), {width, base::nullopt});
+  const gfx::Size result =
+      flex_rule()->Run(view(), SizeBounds(width, base::nullopt));
   EXPECT_EQ(gfx::Size(width, height_for_width), result);
   EXPECT_EQ(3U, GetVisibleChildCount(result));
 }
@@ -3553,7 +3555,7 @@ TEST_F(AnimatingLayoutManagerFlexRuleTest, HorizontalBounded_FlexToSize) {
              gfx::Size(5, 5), false);
 
   const gfx::Size preferred = flex_layout()->GetPreferredSize(view());
-  const gfx::Size actual{preferred.width() - 5, preferred.height()};
+  const gfx::Size actual(preferred.width() - 5, preferred.height());
   const ProposedLayout layout = flex_layout()->GetProposedLayout(actual);
   DCHECK_LT(layout.host_size.width(), preferred.width());
   const gfx::Size result = flex_rule()->Run(view(), SizeBounds(actual));
@@ -3565,7 +3567,7 @@ TEST_F(AnimatingLayoutManagerFlexRuleTest, HorizontalBounded_DropOut) {
   InitLayout(LayoutOrientation::kHorizontal, kDropOut, {}, false);
 
   const gfx::Size preferred = flex_layout()->GetPreferredSize(view());
-  const gfx::Size actual{preferred.width() - 5, preferred.height()};
+  const gfx::Size actual(preferred.width() - 5, preferred.height());
   const ProposedLayout layout = flex_layout()->GetProposedLayout(actual);
   DCHECK_LT(layout.host_size.width(), actual.width());
   const gfx::Size result = flex_rule()->Run(view(), SizeBounds(actual));
@@ -3578,7 +3580,7 @@ TEST_F(AnimatingLayoutManagerFlexRuleTest, VerticalBounded_FlexToSize) {
              gfx::Size(5, 5), false);
 
   const gfx::Size preferred = flex_layout()->GetPreferredSize(view());
-  const gfx::Size actual{preferred.width(), preferred.height() - 5};
+  const gfx::Size actual(preferred.width(), preferred.height() - 5);
   const ProposedLayout layout = flex_layout()->GetProposedLayout(actual);
   DCHECK_LT(layout.host_size.height(), preferred.height());
   const gfx::Size result = flex_rule()->Run(view(), SizeBounds(actual));
@@ -3590,7 +3592,7 @@ TEST_F(AnimatingLayoutManagerFlexRuleTest, VerticalBounded_DropOut) {
   InitLayout(LayoutOrientation::kVertical, kDropOut, {}, false);
 
   const gfx::Size preferred = flex_layout()->GetPreferredSize(view());
-  const gfx::Size actual{preferred.width(), preferred.height() - 5};
+  const gfx::Size actual(preferred.width(), preferred.height() - 5);
   const ProposedLayout layout = flex_layout()->GetProposedLayout(actual);
   DCHECK_LT(layout.host_size.height(), actual.height());
   const gfx::Size result = flex_rule()->Run(view(), SizeBounds(actual));
@@ -3603,7 +3605,7 @@ TEST_F(AnimatingLayoutManagerFlexRuleTest, HorizontalDoubleBounded_DropOut) {
              gfx::Size(10, 5), true);
 
   const gfx::Size preferred = flex_layout()->GetPreferredSize(view());
-  const gfx::Size actual{preferred.width() - 5, preferred.height() - 5};
+  const gfx::Size actual(preferred.width() - 5, preferred.height() - 5);
   const ProposedLayout layout = flex_layout()->GetProposedLayout(actual);
   DCHECK_LT(layout.host_size.width(), preferred.width());
   DCHECK_LT(layout.host_size.height(), preferred.height());
@@ -3617,7 +3619,7 @@ TEST_F(AnimatingLayoutManagerFlexRuleTest, VerticalDoubleBounded_DropOut) {
              gfx::Size(5, 10), true);
 
   const gfx::Size preferred = flex_layout()->GetPreferredSize(view());
-  const gfx::Size actual{preferred.width() - 5, preferred.height() - 5};
+  const gfx::Size actual(preferred.width() - 5, preferred.height() - 5);
   const ProposedLayout layout = flex_layout()->GetProposedLayout(actual);
   DCHECK_LT(layout.host_size.width(), preferred.width());
   DCHECK_LT(layout.host_size.height(), preferred.height());
@@ -4093,9 +4095,9 @@ class AnimatingLayoutManagerRealtimeTest
   bool UseContainerTestApi() const override { return false; }
 
  protected:
-  void InitRootView(const SizeBounds& bounds = SizeBounds()) {
+  void InitRootView(SizeBounds bounds = SizeBounds()) {
     root_view()->SetLayoutManager(std::make_unique<ImmediateLayoutManager>(
-        layout()->should_animate_bounds(), bounds));
+        layout()->should_animate_bounds(), std::move(bounds)));
     layout()->EnableAnimationForTesting();
   }
 
@@ -4285,7 +4287,7 @@ TEST_F(AnimatingLayoutManagerRealtimeTest,
   flex_layout->SetCollapseMargins(true);
   flex_layout->SetCrossAxisAlignment(LayoutAlignment::kStart);
   flex_layout->SetDefault(kMarginsKey, kChildMargins);
-  InitRootView(kSizeBounds);
+  InitRootView(std::move(kSizeBounds));
   child(0)->SetProperty(kFlexBehaviorKey, FlexSpecification::ForSizeRule(
                                               MinimumFlexSizeRule::kScaleToZero,
                                               MaximumFlexSizeRule::kPreferred));

ui/views/layout/flex_layout.cc

@@ -353,20 +353,16 @@ ProposedLayout FlexLayout::CalculateProposedLayout(
   UpdateLayoutFromChildren(bounds, &data, &child_spacing);
 
   if (bounds.main().has_value()) {
+    // Flex up to preferred size.
     CalculateNonFlexAvailableSpace(&data,
                                    *bounds.main() - data.total_size.main(),
                                    child_spacing, order_to_view_index);
+    FlexOrderToViewIndexMap expandable_views;
+    AllocateFlexSpace(bounds, order_to_view_index, &data, &child_spacing,
+                      &expandable_views);
 
-    if (!order_to_view_index.empty()) {
-      // Flex up to preferred size.
-      FlexOrderToViewIndexMap expandable_views;
-      AllocateFlexSpace(bounds, order_to_view_index, &data, &child_spacing,
-                        &expandable_views);
-
-      // Flex views that can exceed their preferred size.
-      if (!expandable_views.empty())
-        AllocateFlexSpace(bounds, expandable_views, &data, &child_spacing);
-    }
+    // Flex up to maximum size.
+    AllocateFlexSpace(bounds, expandable_views, &data, &child_spacing, nullptr);
   }
 
   // Calculate the size of the host view.
@@ -391,6 +387,7 @@ void FlexLayout::InitializeChildData(
   for (View* child : host_view()->children()) {
     if (!IsChildIncludedInLayout(child))
       continue;
+
     const size_t view_index = data->num_children();
     data->layout.child_layouts.emplace_back(ChildLayout{child});
     ChildLayout& child_layout = data->layout.child_layouts.back();
@@ -412,33 +409,32 @@ void FlexLayout::InitializeChildData(
     // the necessity to alter a view's height in a vertical layout if the width
     // is bounded. In the common case this will be equivalent to calling
     // GetPreferredSize().
-    base::Optional<int> available_cross =
+    const base::Optional<int> available_cross =
         GetAvailableCrossAxisSize(*data, view_index, bounds);
 
+    const NormalizedSize default_size = Normalize(
+        orientation(), flex_child.flex.rule().Run(child, SizeBounds()));
+
     // In vertical layouts it's important to consider height-for-width type
-    // calculations when evaluating the base/preferred size of the child view.
+    // calculations.
     if (orientation() == LayoutOrientation::kVertical) {
-      flex_child.preferred_size = Normalize(
-          orientation(),
-          flex_child.flex.rule().Run(
-              child,
-              Denormalize(orientation(), {base::nullopt, available_cross})));
-
-      // In non-stretch environments, we don't want the cross-axis preferred
-      // size to exceed the default preferred, or the cross-axis to shrink below
-      // the default preferred.
-      if (cross_axis_alignment() != LayoutAlignment::kStretch) {
-        const NormalizedSize default_preferred =
-            Normalize(orientation(), flex_child.flex.rule().Run(child, {}));
-        flex_child.preferred_size.set_main(std::max(
-            flex_child.preferred_size.main(), default_preferred.main()));
-        flex_child.preferred_size.set_cross(std::min(
-            flex_child.preferred_size.cross(), default_preferred.cross()));
+      const NormalizedSize stretch_size =
+          Normalize(orientation(),
+                    flex_child.flex.rule().Run(
+                        child, SizeBounds(available_cross, base::nullopt)));
+      if (cross_axis_alignment() == LayoutAlignment::kStretch) {
+        flex_child.preferred_size = stretch_size;
+      } else {
+        // In non-stretch environments, we don't want the cross-axis size to
+        // exceed the default, or the main-axis size to shrink below the
+        // default.
+        flex_child.preferred_size = NormalizedSize(
+            std::max(default_size.main(), stretch_size.main()),
+            std::min(default_size.cross(), stretch_size.cross()));
       }
     } else {
       // Just use the default preferred size.
-      flex_child.preferred_size =
-          Normalize(orientation(), flex_child.flex.rule().Run(child, {}));
+      flex_child.preferred_size = default_size;
     }
 
     // gfx::Size calculation depends on whether flex is allowed.
@@ -478,8 +474,8 @@ void FlexLayout::CalculateChildBounds(const SizeBounds& size_bounds,
       Normalize(orientation(), size_bounds);
   const NormalizedSize normalized_host_size =
       Normalize(orientation(), data->layout.host_size);
-  int available_main = (normalized_bounds.main() ? *normalized_bounds.main()
-                                                 : normalized_host_size.main());
+  int available_main = normalized_bounds.main() ? *normalized_bounds.main()
+                                                : normalized_host_size.main();
   available_main = std::max(0, available_main - data->host_insets.main_size());
   const int excess_main = available_main - data->total_size.main();
   NormalizedPoint start(data->host_insets.main_leading(),
@@ -507,8 +503,8 @@ void FlexLayout::CalculateChildBounds(const SizeBounds& size_bounds,
       actual.Offset(start.main(), start.cross());
       if (actual.size_main() > flex_child.preferred_size.main() &&
           flex_child.flex.alignment() != LayoutAlignment::kStretch) {
-        Span container{actual.origin_main(), actual.size_main()};
-        Span new_main{0, flex_child.preferred_size.main()};
+        Span container(actual.origin_main(), actual.size_main());
+        Span new_main(0, flex_child.preferred_size.main());
         new_main.Align(container, flex_child.flex.alignment());
         actual.set_origin_main(new_main.start());
         actual.set_size_main(new_main.length());
@@ -790,16 +786,14 @@ void FlexLayout::AllocateFlexSpace(
         continue;
 
       // Offer a share of the remaining space to the view.
-      int flex_amount;
-      if (flex_child.flex.weight() > 0) {
-        const int flex_weight = flex_child.flex.weight();
+      int flex_amount = remaining;
+      const int flex_weight = flex_child.flex.weight();
+      if (flex_weight > 0) {
         // Round up so we give slightly greater weight to earlier views.
         flex_amount =
-            int{std::ceil((float{remaining} * flex_weight) / flex_total)};
-        flex_total -= flex_weight;
-      } else {
-        flex_amount = remaining;
+            gfx::ToCeiledInt(remaining * flex_weight / float{flex_total});
       }
+      flex_total -= flex_weight;
 
       // Offer the modified flex space to the child view and see how large it
       // wants to be (or if it wants to be visible at that size at all).
@@ -807,11 +801,11 @@ void FlexLayout::AllocateFlexSpace(
           flex_amount + old_size - margin_delta,
           GetCrossAxis(orientation(), child_layout.available_size));
 
-      NormalizedSize new_size = Normalize(
+      NormalizedSize desired_size = Normalize(
           orientation(),
           flex_child.flex.rule().Run(child_layout.child_view,
                                      Denormalize(orientation(), available)));
-      if (new_size.main() <= 0)
+      if (desired_size.main() <= 0)
         continue;
 
       // Limit the expansion of views past their preferred size in the first
@@ -819,18 +813,17 @@ void FlexLayout::AllocateFlexSpace(
       // them to |expandable_views| so that the remaining space can be allocated
       // later.
       if (expandable_views &&
-          new_size.main() > flex_child.preferred_size.main()) {
+          desired_size.main() > flex_child.preferred_size.main()) {
         (*expandable_views)[flex_order].push_back(view_index);
-        new_size.set_main(flex_child.preferred_size.main());
+        desired_size.set_main(flex_child.preferred_size.main());
       }
 
-      // If the amount of space claimed increases (but is still within
-      // bounds set by our flex rule) we can make the control visible and
-      // claim the additional space.
-      const int to_deduct = (new_size.main() - old_size) + margin_delta;
+      // If the desired size increases (but is still within bounds), we can make
+      // the control visible and allocate the additional space.
+      const int to_deduct = (desired_size.main() - old_size) + margin_delta;
       DCHECK_GE(to_deduct, 0);
       if (to_deduct > 0 && to_deduct <= remaining) {
-        flex_child.current_size = new_size;
+        flex_child.current_size = desired_size;
         child_layout.visible = true;
         remaining -= to_deduct;
         if (!child_spacing->HasViewIndex(view_index))

ui/views/layout/flex_layout.h

@@ -145,6 +145,25 @@ class VIEWS_EXPORT FlexLayout : public LayoutManagerBase {
   // See FlexSpecification::order().
   using FlexOrderToViewIndexMap = std::map<int, std::vector<size_t>>;
 
+  // Fills out the child entries for |data| and generates some initial size
+  // and visibility data, and stores off information about which views can
+  // expand in |flex_order_to_index|.
+  void InitializeChildData(const NormalizedSizeBounds& bounds,
+                           FlexLayoutData* data,
+                           FlexOrderToViewIndexMap* flex_order_to_index) const;
+
+  // Caclulates the child bounds (in screen coordinates) for each visible child
+  // in the layout.
+  void CalculateChildBounds(const SizeBounds& size_bounds,
+                            FlexLayoutData* data) const;
+
+  // Calculates available space for non-flex views.
+  void CalculateNonFlexAvailableSpace(
+      FlexLayoutData* data,
+      int available_space,
+      const ChildViewSpacing& child_spacing,
+      const FlexOrderToViewIndexMap& flex_views) const;
+
   // Returns the combined margins across the cross axis of the host view, for a
   // particular child view.
   Inset1D GetCrossAxisMargins(const FlexLayoutData& layout,
@@ -185,31 +204,11 @@ class VIEWS_EXPORT FlexLayout : public LayoutManagerBase {
   //
   // Typically, this method will be called once with |expandable_views| set and
   // then again with it null to allocate the remaining space.
-  void AllocateFlexSpace(
-      const NormalizedSizeBounds& bounds,
-      const FlexOrderToViewIndexMap& order_to_index,
-      FlexLayoutData* data,
-      ChildViewSpacing* child_spacing,
-      FlexOrderToViewIndexMap* expandable_views = nullptr) const;
-
-  // Fills out the child entries for |data| and generates some initial size
-  // and visibility data, and stores off information about which views can
-  // expand in |flex_order_to_index|.
-  void InitializeChildData(const NormalizedSizeBounds& bounds,
-                           FlexLayoutData* data,
-                           FlexOrderToViewIndexMap* flex_order_to_index) const;
-
-  // Caclulates the child bounds (in screen coordinates) for each visible child
-  // in the layout.
-  void CalculateChildBounds(const SizeBounds& size_bounds,
-                            FlexLayoutData* data) const;
-
-  // Calculates available space for non-flex views.
-  void CalculateNonFlexAvailableSpace(
-      FlexLayoutData* data,
-      int available_space,
-      const ChildViewSpacing& child_spacing,
-      const FlexOrderToViewIndexMap& flex_views) const;
+  void AllocateFlexSpace(const NormalizedSizeBounds& bounds,
+                         const FlexOrderToViewIndexMap& order_to_index,
+                         FlexLayoutData* data,
+                         ChildViewSpacing* child_spacing,
+                         FlexOrderToViewIndexMap* expandable_views) const;
 
   // Gets the default value for a particular layout property, which will be used
   // if the property is not set on a child view being laid out (e.g.

ui/views/layout/flex_layout_types.cc

@@ -38,7 +38,8 @@ int InterpolateSize(MinimumFlexSizeRule minimum_size_rule,
       case MinimumFlexSizeRule::kPreferredSnapToZero:
         return 0;
     }
-  } else if (available_size < preferred_size) {
+  }
+  if (available_size < preferred_size) {
     switch (minimum_size_rule) {
       case MinimumFlexSizeRule::kPreferred:
         return preferred_size;
@@ -51,13 +52,12 @@ int InterpolateSize(MinimumFlexSizeRule minimum_size_rule,
       case MinimumFlexSizeRule::kPreferredSnapToZero:
         return 0;
     }
-  } else {
-    switch (maximum_size_rule) {
-      case MaximumFlexSizeRule::kPreferred:
-        return preferred_size;
-      case MaximumFlexSizeRule::kUnbounded:
-        return available_size;
-    }
+  }
+  switch (maximum_size_rule) {
+    case MaximumFlexSizeRule::kPreferred:
+      return preferred_size;
+    case MaximumFlexSizeRule::kUnbounded:
+      return available_size;
   }
 }
 
@@ -90,8 +90,7 @@ gfx::Size GetPreferredSize(MinimumFlexSizeRule minimum_size_rule,
   LazyMinimumSize min(view);
   gfx::Size preferred = view->GetPreferredSize();
 
-  int width, height;
-
+  int width;
   if (!maximum_size.width()) {
     // Not having a maximum size is different from having a large available
     // size; a view can't grow infinitely, so we go with its preferred size.
@@ -127,6 +126,7 @@ gfx::Size GetPreferredSize(MinimumFlexSizeRule minimum_size_rule,
     }
   }
 
+  int height;
   if (!maximum_size.height()) {
     // Not having a maximum size is different from having a large available
     // size; a view can't grow infinitely, so we go with its preferred size.
@@ -212,7 +212,8 @@ void Inset1D::Expand(int leading, int trailing) {
 }
 
 bool Inset1D::operator==(const Inset1D& other) const {
-  return leading_ == other.leading_ && trailing_ == other.trailing_;
+  return std::tie(leading_, trailing_) ==
+         std::tie(other.leading_, other.trailing_);
 }
 
 bool Inset1D::operator!=(const Inset1D& other) const {
@@ -292,7 +293,7 @@ void Span::Align(const Span& container,
 }
 
 bool Span::operator==(const Span& other) const {
-  return start_ == other.start_ && length_ == other.length_;
+  return std::tie(start_, length_) == std::tie(other.start_, other.length_);
 }
 
 bool Span::operator!=(const Span& other) const {

ui/views/layout/layout_manager_base_unittest.cc

@@ -646,8 +646,8 @@ class LayoutManagerBaseAvailableSizeTest : public testing::Test {
 };
 
 TEST_F(LayoutManagerBaseAvailableSizeTest, ReturnsCorrectValues) {
-  const SizeBounds kChild1Bounds{3, 7};
-  const SizeBounds kChild2Bounds{11, 13};
+  const SizeBounds kChild1Bounds(3, 7);
+  const SizeBounds kChild2Bounds(11, 13);
   View* const child1 = view()->AddChildView(std::make_unique<View>());
   View* const child2 = view()->AddChildView(std::make_unique<View>());
   View not_a_child;

ui/views/layout/layout_types.cc

@@ -4,9 +4,9 @@
 
 #include "ui/views/layout/layout_types.h"
 
-#include <algorithm>
+#include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
 
-#include "base/strings/stringprintf.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace views {
@@ -15,9 +15,8 @@ namespace views {
 
 SizeBounds::SizeBounds() = default;
 
-SizeBounds::SizeBounds(const base::Optional<int>& width,
-                       const base::Optional<int>& height)
-    : width_(width), height_(height) {}
+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()) {}
@@ -33,7 +32,7 @@ void SizeBounds::Enlarge(int width, int height) {
 }
 
 bool SizeBounds::operator==(const SizeBounds& other) const {
-  return width_ == other.width_ && height_ == other.height_;
+  return std::tie(width_, height_) == std::tie(other.width_, other.height_);
 }
 
 bool SizeBounds::operator!=(const SizeBounds& other) const {
@@ -45,17 +44,8 @@ bool SizeBounds::operator<(const SizeBounds& other) const {
 }
 
 std::string SizeBounds::ToString() const {
-  std::ostringstream oss;
-  if (width().has_value())
-    oss << *width();
-  else
-    oss << "_";
-  oss << " x ";
-  if (height().has_value())
-    oss << *height();
-  else
-    oss << "_";
-  return oss.str();
+  return base::StrCat({width_ ? base::NumberToString(*width_) : "_", " x ",
+                       height_ ? base::NumberToString(*height_) : "_"});
 }
 
 }  // namespace views

ui/views/layout/layout_types.h

@@ -28,16 +28,15 @@ enum class LayoutOrientation {
 class VIEWS_EXPORT SizeBounds {
  public:
   SizeBounds();
-  SizeBounds(const base::Optional<int>& width,
-             const base::Optional<int>& height);
+  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(const base::Optional<int>& width) { width_ = width; }
+  void set_width(base::Optional<int> width) { width_ = std::move(width); }
 
   const base::Optional<int>& height() const { return height_; }
-  void set_height(const base::Optional<int>& height) { height_ = height; }
+  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.

ui/views/layout/normalized_geometry.cc

@@ -8,6 +8,8 @@
 #include <tuple>
 
 #include "base/numerics/ranges.h"
+#include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/geometry/point.h"
@@ -30,7 +32,7 @@ void NormalizedPoint::Offset(int delta_main, int delta_cross) {
 }
 
 bool NormalizedPoint::operator==(const NormalizedPoint& other) const {
-  return main_ == other.main_ && cross_ == other.cross_;
+  return std::tie(main_, cross_) == std::tie(other.main_, other.cross_);
 }
 
 bool NormalizedPoint::operator!=(const NormalizedPoint& other) const {
@@ -76,7 +78,7 @@ void NormalizedSize::SetToMin(const NormalizedSize& other) {
 }
 
 bool NormalizedSize::operator==(const NormalizedSize& other) const {
-  return main_ == other.main_ && cross_ == other.cross_;
+  return std::tie(main_, cross_) == std::tie(other.main_, other.cross_);
 }
 
 bool NormalizedSize::operator!=(const NormalizedSize& other) const {
@@ -94,7 +96,7 @@ std::string NormalizedSize::ToString() const {
 // NormalizedInsets ------------------------------------------------------------
 
 bool NormalizedInsets::operator==(const NormalizedInsets& other) const {
-  return main_ == other.main_ && cross_ == other.cross_;
+  return std::tie(main_, cross_) == std::tie(other.main_, other.cross_);
 }
 
 bool NormalizedInsets::operator!=(const NormalizedInsets& other) const {
@@ -106,18 +108,17 @@ bool NormalizedInsets::operator<(const NormalizedInsets& other) const {
 }
 
 std::string NormalizedInsets::ToString() const {
-  return base::StringPrintf("main: [%s], cross: [%s]",
-                            main().ToString().c_str(),
-                            cross().ToString().c_str());
+  return base::StrCat(
+      {"main: [", main_.ToString(), "], cross: [", cross_.ToString(), "]"});
 }
 
 // NormalizedSizeBounds --------------------------------------------------------
 
 NormalizedSizeBounds::NormalizedSizeBounds() = default;
 
-NormalizedSizeBounds::NormalizedSizeBounds(const base::Optional<int>& main,
-                                           const base::Optional<int>& cross)
-    : main_(main), cross_(cross) {}
+NormalizedSizeBounds::NormalizedSizeBounds(base::Optional<int> main,
+                                           base::Optional<int> cross)
+    : main_(std::move(main)), cross_(std::move(cross)) {}
 
 NormalizedSizeBounds::NormalizedSizeBounds(const NormalizedSizeBounds& other)
     : main_(other.main()), cross_(other.cross()) {}
@@ -137,7 +138,7 @@ void NormalizedSizeBounds::Inset(const NormalizedInsets& insets) {
 }
 
 bool NormalizedSizeBounds::operator==(const NormalizedSizeBounds& other) const {
-  return main_ == other.main_ && cross_ == other.cross_;
+  return std::tie(main_, cross_) == std::tie(other.main_, other.cross_);
 }
 
 bool NormalizedSizeBounds::operator!=(const NormalizedSizeBounds& other) const {
@@ -149,17 +150,8 @@ bool NormalizedSizeBounds::operator<(const NormalizedSizeBounds& other) const {
 }
 
 std::string NormalizedSizeBounds::ToString() const {
-  std::ostringstream oss;
-  if (main().has_value())
-    oss << *main();
-  else
-    oss << "_";
-  oss << " x ";
-  if (cross().has_value())
-    oss << *cross();
-  else
-    oss << "_";
-  return oss.str();
+  return base::StrCat({main_ ? base::NumberToString(*main_) : "_", " x ",
+                       cross_ ? base::NumberToString(*cross_) : "_"});
 }
 
 // NormalizedRect --------------------------------------------------------------
@@ -238,7 +230,7 @@ void NormalizedRect::Offset(int main, int cross) {
 }
 
 bool NormalizedRect::operator==(const NormalizedRect& other) const {
-  return origin_ == other.origin_ && size_ == other.size_;
+  return std::tie(origin_, size_) == std::tie(other.origin_, other.size_);
 }
 
 bool NormalizedRect::operator!=(const NormalizedRect& other) const {
@@ -250,8 +242,7 @@ bool NormalizedRect::operator<(const NormalizedRect& other) const {
 }
 
 std::string NormalizedRect::ToString() const {
-  return base::StringPrintf("(%s) [%s]", origin().ToString().c_str(),
-                            size().ToString().c_str());
+  return base::StrCat({"(", origin_.ToString(), ") [", size_.ToString(), "]"});
 }
 
 // Normalization and Denormalization -------------------------------------------
@@ -263,9 +254,6 @@ NormalizedPoint Normalize(LayoutOrientation orientation,
       return NormalizedPoint(point.x(), point.y());
     case LayoutOrientation::kVertical:
       return NormalizedPoint(point.y(), point.x());
-    default:
-      DCHECK(false);
-      return NormalizedPoint(point.x(), point.y());
   }
 }
 
@@ -276,9 +264,6 @@ gfx::Point Denormalize(LayoutOrientation orientation,
       return gfx::Point(point.main(), point.cross());
     case LayoutOrientation::kVertical:
       return gfx::Point(point.cross(), point.main());
-    default:
-      DCHECK(false);
-      return gfx::Point(point.main(), point.cross());
   }
 }
 
@@ -288,9 +273,6 @@ NormalizedSize Normalize(LayoutOrientation orientation, const gfx::Size& size) {
       return NormalizedSize(size.width(), size.height());
     case LayoutOrientation::kVertical:
       return NormalizedSize(size.height(), size.width());
-    default:
-      DCHECK(false);
-      return NormalizedSize(size.width(), size.height());
   }
 }
 
@@ -301,9 +283,6 @@ gfx::Size Denormalize(LayoutOrientation orientation,
       return gfx::Size(size.main(), size.cross());
     case LayoutOrientation::kVertical:
       return gfx::Size(size.cross(), size.main());
-    default:
-      DCHECK(false);
-      return gfx::Size(size.main(), size.cross());
   }
 }
 
@@ -314,9 +293,6 @@ NormalizedSizeBounds Normalize(LayoutOrientation orientation,
       return NormalizedSizeBounds(bounds.width(), bounds.height());
     case LayoutOrientation::kVertical:
       return NormalizedSizeBounds(bounds.height(), bounds.width());
-    default:
-      DCHECK(false);
-      return NormalizedSizeBounds(bounds.width(), bounds.height());
   }
 }
 
@@ -327,9 +303,6 @@ SizeBounds Denormalize(LayoutOrientation orientation,
       return SizeBounds(bounds.main(), bounds.cross());
     case LayoutOrientation::kVertical:
       return SizeBounds(bounds.cross(), bounds.main());
-    default:
-      DCHECK(false);
-      return SizeBounds(bounds.main(), bounds.cross());
   }
 }
 
@@ -342,10 +315,6 @@ NormalizedInsets Normalize(LayoutOrientation orientation,
     case LayoutOrientation::kVertical:
       return NormalizedInsets(insets.top(), insets.left(), insets.bottom(),
                               insets.right());
-    default:
-      DCHECK(false);
-      return NormalizedInsets(insets.left(), insets.top(), insets.right(),
-                              insets.bottom());
   }
 }
 
@@ -358,10 +327,6 @@ gfx::Insets Denormalize(LayoutOrientation orientation,
     case LayoutOrientation::kVertical:
       return gfx::Insets(insets.main_leading(), insets.cross_leading(),
                          insets.main_trailing(), insets.cross_trailing());
-    default:
-      DCHECK(false);
-      return gfx::Insets(insets.cross_leading(), insets.main_leading(),
-                         insets.cross_trailing(), insets.main_trailing());
   }
 }
 
@@ -442,10 +407,10 @@ void SetMainAxis(SizeBounds* size,
                  base::Optional<int> main) {
   switch (orientation) {
     case LayoutOrientation::kHorizontal:
-      size->set_width(main);
+      size->set_width(std::move(main));
       break;
     case LayoutOrientation::kVertical:
-      size->set_height(main);
+      size->set_height(std::move(main));
       break;
   }
 }
@@ -455,10 +420,10 @@ void SetCrossAxis(SizeBounds* size,
                   base::Optional<int> cross) {
   switch (orientation) {
     case LayoutOrientation::kHorizontal:
-      size->set_height(cross);
+      size->set_height(std::move(cross));
       break;
     case LayoutOrientation::kVertical:
-      size->set_width(cross);
+      size->set_width(std::move(cross));
       break;
   }
 }

ui/views/layout/normalized_geometry.h

@@ -147,16 +147,15 @@ class VIEWS_EXPORT NormalizedInsets {
 class VIEWS_EXPORT NormalizedSizeBounds {
  public:
   NormalizedSizeBounds();
-  NormalizedSizeBounds(const base::Optional<int>& main,
-                       const base::Optional<int>& cross);
+  NormalizedSizeBounds(base::Optional<int> main, base::Optional<int> cross);
   explicit NormalizedSizeBounds(const NormalizedSize& size);
   NormalizedSizeBounds(const NormalizedSizeBounds& size_bounds);
 
   const base::Optional<int>& main() const { return main_; }
-  void set_main(const base::Optional<int>& main) { main_ = main; }
+  void set_main(base::Optional<int> main) { main_ = std::move(main); }
 
   const base::Optional<int>& cross() const { return cross_; }
-  void set_cross(const base::Optional<int>& cross) { cross_ = cross; }
+  void set_cross(base::Optional<int> cross) { cross_ = std::move(cross); }
 
   void Expand(int main, int cross);
   void Inset(const NormalizedInsets& insets);
@@ -259,19 +258,19 @@ gfx::Point VIEWS_EXPORT Denormalize(LayoutOrientation orientation,
                                     const NormalizedPoint& point);
 
 NormalizedSize VIEWS_EXPORT Normalize(LayoutOrientation orientation,
-                                      const gfx::Size& point);
+                                      const gfx::Size& size);
 gfx::Size VIEWS_EXPORT Denormalize(LayoutOrientation orientation,
-                                   const NormalizedSize& point);
+                                   const NormalizedSize& size);
 
 NormalizedSizeBounds VIEWS_EXPORT Normalize(LayoutOrientation orientation,
-                                            const SizeBounds& point);
+                                            const SizeBounds& bounds);
 SizeBounds VIEWS_EXPORT Denormalize(LayoutOrientation orientation,
-                                    const NormalizedSizeBounds& point);
+                                    const NormalizedSizeBounds& bounds);
 
 NormalizedInsets VIEWS_EXPORT Normalize(LayoutOrientation orientation,
-                                        const gfx::Insets& point);
+                                        const gfx::Insets& insets);
 gfx::Insets VIEWS_EXPORT Denormalize(LayoutOrientation orientation,
-                                     const NormalizedInsets& point);
+                                     const NormalizedInsets& insets);
 
 NormalizedRect VIEWS_EXPORT Normalize(LayoutOrientation orientation,
                                       const gfx::Rect& rect);

ui/views/layout/normalized_geometry_unittest.cc

@@ -47,15 +47,15 @@ TEST(NormalizedRectTest, Inset_Negative) {
 }
 
 TEST(NormalizedGeometryTest, GetMainAxis_Size) {
-  gfx::Size size{1, 2};
+  gfx::Size size(1, 2);
 
   EXPECT_EQ(1, GetMainAxis(LayoutOrientation::kHorizontal, size));
   EXPECT_EQ(2, GetMainAxis(LayoutOrientation::kVertical, size));
 }
 
 TEST(NormalizedGeometryTest, GetMainAxis_SizeBounds) {
-  SizeBounds size{1, 2};
-  SizeBounds size2{};
+  SizeBounds size(1, 2);
+  SizeBounds size2;
 
   EXPECT_EQ(base::Optional<int>(1),
             GetMainAxis(LayoutOrientation::kHorizontal, size));
@@ -68,7 +68,7 @@ TEST(NormalizedGeometryTest, GetMainAxis_SizeBounds) {
 }
 
 TEST(NormalizedGeometryTest, GetCrossAxis_Size) {
-  gfx::Size size{1, 2};
+  gfx::Size size(1, 2);
 
   EXPECT_EQ(2, GetCrossAxis(LayoutOrientation::kHorizontal, size));
   EXPECT_EQ(1, GetCrossAxis(LayoutOrientation::kVertical, size));
@@ -76,7 +76,7 @@ TEST(NormalizedGeometryTest, GetCrossAxis_Size) {
 
 TEST(NormalizedGeometryTest, GetCrossAxis_SizeBounds) {
   SizeBounds size{1, 2};
-  SizeBounds size2{};
+  SizeBounds size2;
 
   EXPECT_EQ(base::Optional<int>(2),
             GetCrossAxis(LayoutOrientation::kHorizontal, size));
@@ -89,7 +89,7 @@ TEST(NormalizedGeometryTest, GetCrossAxis_SizeBounds) {
 }
 
 TEST(NormalizedGeometryTest, SetMainAxis_Size) {
-  gfx::Size size{1, 2};
+  gfx::Size size(1, 2);
 
   SetMainAxis(&size, LayoutOrientation::kHorizontal, 3);
   SetMainAxis(&size, LayoutOrientation::kVertical, 4);
@@ -97,7 +97,7 @@ TEST(NormalizedGeometryTest, SetMainAxis_Size) {
 }
 
 TEST(NormalizedGeometryTest, SetMainAxis_SizeBounds) {
-  SizeBounds size{1, 2};
+  SizeBounds size(1, 2);
 
   SetMainAxis(&size, LayoutOrientation::kHorizontal, 3);
   SetMainAxis(&size, LayoutOrientation::kVertical, 4);
@@ -111,7 +111,7 @@ TEST(NormalizedGeometryTest, SetMainAxis_SizeBounds) {
 }
 
 TEST(NormalizedGeometryTest, SetCrossAxis_Size) {
-  gfx::Size size{1, 2};
+  gfx::Size size(1, 2);
 
   SetCrossAxis(&size, LayoutOrientation::kHorizontal, 3);
   SetCrossAxis(&size, LayoutOrientation::kVertical, 4);
@@ -119,7 +119,7 @@ TEST(NormalizedGeometryTest, SetCrossAxis_Size) {
 }
 
 TEST(NormalizedGeometryTest, SetCrossAxis_SizeBounds) {
-  SizeBounds size{1, 2};
+  SizeBounds size(1, 2);
 
   SetCrossAxis(&size, LayoutOrientation::kHorizontal, 3);
   SetCrossAxis(&size, LayoutOrientation::kVertical, 4);

ui/views/layout/proposed_layout.cc

@@ -13,13 +13,11 @@ namespace views {
 namespace {
 
 base::Optional<int> OptionalValueBetween(double value,
-                                         base::Optional<int> start,
-                                         base::Optional<int> target) {
-  if (start.has_value() != target.has_value())
-    return target;
-  if (start)
-    return gfx::Tween::IntValueBetween(value, *start, *target);
-  return base::nullopt;
+                                         const base::Optional<int>& start,
+                                         const base::Optional<int>& target) {
+  return (start.has_value() && target.has_value())
+             ? gfx::Tween::IntValueBetween(value, *start, *target)
+             : target;
 }
 
 SizeBounds SizeBoundsBetween(double value,