Use base::ClampAdd() in gfx::Insets::width(), height()

Change-Id: I7d6162c2dee31d1776d8ec36175ade1b360f0a96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1918787
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719629}

ui/gfx/geometry/insets.cc

@@ -15,8 +15,10 @@ std::string Insets::ToString() const {
 }
 
 Insets Insets::Offset(const gfx::Vector2d& vector) const {
-  return gfx::Insets(top() + vector.y(), left() + vector.x(),
-                     bottom() - vector.y(), right() - vector.x());
+  return gfx::Insets(base::ClampAdd(top(), vector.y()),
+                     base::ClampAdd(left(), vector.x()),
+                     base::ClampSub(bottom(), vector.y()),
+                     base::ClampSub(right(), vector.x()));
 }
 
 }  // namespace gfx

ui/gfx/geometry/insets.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/numerics/clamped_math.h"
 #include "ui/gfx/geometry/geometry_export.h"
 #include "ui/gfx/geometry/insets_f.h"
 
@@ -25,14 +26,20 @@ class GEOMETRY_EXPORT Insets {
  public:
   constexpr Insets() : top_(0), left_(0), bottom_(0), right_(0) {}
   constexpr explicit Insets(int all)
-      : top_(all), left_(all), bottom_(all), right_(all) {}
+      : top_(all),
+        left_(all),
+        bottom_(GetClampedValue(all, all)),
+        right_(GetClampedValue(all, all)) {}
   constexpr Insets(int vertical, int horizontal)
       : top_(vertical),
         left_(horizontal),
-        bottom_(vertical),
-        right_(horizontal) {}
+        bottom_(GetClampedValue(vertical, vertical)),
+        right_(GetClampedValue(horizontal, horizontal)) {}
   constexpr Insets(int top, int left, int bottom, int right)
-      : top_(top), left_(left), bottom_(bottom), right_(right) {}
+      : top_(top),
+        left_(left),
+        bottom_(GetClampedValue(top, bottom)),
+        right_(GetClampedValue(left, right)) {}
 
   constexpr int top() const { return top_; }
   constexpr int left() const { return left_; }
@@ -50,16 +57,22 @@ class GEOMETRY_EXPORT Insets {
   // Returns true if the insets are empty.
   bool IsEmpty() const { return width() == 0 && height() == 0; }
 
-  void set_top(int top) { top_ = top; }
-  void set_left(int left) { left_ = left; }
-  void set_bottom(int bottom) { bottom_ = bottom; }
-  void set_right(int right) { right_ = right; }
+  void set_top(int top) {
+    top_ = top;
+    bottom_ = GetClampedValue(top_, bottom_);
+  }
+  void set_left(int left) {
+    left_ = left;
+    right_ = GetClampedValue(left_, right_);
+  }
+  void set_bottom(int bottom) { bottom_ = GetClampedValue(top_, bottom); }
+  void set_right(int right) { right_ = GetClampedValue(left_, right); }
 
   void Set(int top, int left, int bottom, int right) {
     top_ = top;
     left_ = left;
-    bottom_ = bottom;
-    right_ = right;
+    bottom_ = GetClampedValue(top_, bottom);
+    right_ = GetClampedValue(left_, right);
   }
 
   bool operator==(const Insets& insets) const {
@@ -72,32 +85,32 @@ class GEOMETRY_EXPORT Insets {
   }
 
   void operator+=(const Insets& insets) {
-    top_ += insets.top_;
-    left_ += insets.left_;
-    bottom_ += insets.bottom_;
-    right_ += insets.right_;
+    top_ = base::ClampAdd(top_, insets.top_);
+    left_ = base::ClampAdd(left_, insets.left_);
+    bottom_ = GetClampedValue(top_, base::ClampAdd(bottom_, insets.bottom_));
+    right_ = GetClampedValue(left_, base::ClampAdd(right_, insets.right_));
   }
 
   void operator-=(const Insets& insets) {
-    top_ -= insets.top_;
-    left_ -= insets.left_;
-    bottom_ -= insets.bottom_;
-    right_ -= insets.right_;
+    top_ = base::ClampSub(top_, insets.top_);
+    left_ = base::ClampSub(left_, insets.left_);
+    bottom_ = GetClampedValue(top_, base::ClampSub(bottom_, insets.bottom_));
+    right_ = GetClampedValue(left_, base::ClampSub(right_, insets.right_));
   }
 
   Insets operator-() const {
-    return Insets(-top_, -left_, -bottom_, -right_);
+    return Insets(-base::MakeClampedNum(top_), -base::MakeClampedNum(left_),
+                  -base::MakeClampedNum(bottom_),
+                  -base::MakeClampedNum(right_));
   }
 
-  Insets Scale(float scale) const {
-    return Scale(scale, scale);
-  }
+  Insets Scale(float scale) const { return Scale(scale, scale); }
 
   Insets Scale(float x_scale, float y_scale) const {
-    return Insets(static_cast<int>(top() * y_scale),
-                  static_cast<int>(left() * x_scale),
-                  static_cast<int>(bottom() * y_scale),
-                  static_cast<int>(right() * x_scale));
+    return Insets(static_cast<int>(base::ClampMul(top(), y_scale)),
+                  static_cast<int>(base::ClampMul(left(), x_scale)),
+                  static_cast<int>(base::ClampMul(bottom(), y_scale)),
+                  static_cast<int>(base::ClampMul(right(), x_scale)));
   }
 
   // Adjusts the vertical and horizontal dimensions by the values described in
@@ -118,6 +131,42 @@ class GEOMETRY_EXPORT Insets {
   int left_;
   int bottom_;
   int right_;
+
+  // See ui/gfx/geometry/rect.h
+  // Returns true iff a+b would overflow max int.
+  static constexpr bool AddWouldOverflow(int a, int b) {
+    // In this function, GCC tries to make optimizations that would only work if
+    // max - a wouldn't overflow but it isn't smart enough to notice that a > 0.
+    // So cast everything to unsigned to avoid this.  As it is guaranteed that
+    // max - a and b are both already positive, the cast is a noop.
+    //
+    // This is intended to be: a > 0 && max - a < b
+    return a > 0 && b > 0 &&
+           static_cast<unsigned>(std::numeric_limits<int>::max() - a) <
+               static_cast<unsigned>(b);
+  }
+
+  // Returns true iff a+b would underflow min int.
+  static constexpr bool AddWouldUnderflow(int a, int b) {
+    return a < 0 && b < 0 && std::numeric_limits<int>::min() - a > b;
+  }
+
+  // Clamp the right/bottom to avoid integer over/underflow in width() and
+  // height(). This returns the right/bottom given a top_or_left and a
+  // bottom_or_right.
+  // TODO(enne): this should probably use base::ClampAdd, but that
+  // function is not a constexpr.
+  static constexpr int GetClampedValue(int top_or_left, int bottom_or_right) {
+    if (AddWouldOverflow(top_or_left, bottom_or_right)) {
+      return std::numeric_limits<int>::max() - top_or_left;
+    } else if (AddWouldUnderflow(top_or_left, bottom_or_right)) {
+      // If |top_or_left| and |bottom_or_right| are both negative,
+      // adds |top_or_left| to prevent underflow by subtracting it.
+      return std::numeric_limits<int>::min() - top_or_left;
+    } else {
+      return bottom_or_right;
+    }
+  }
 };
 
 inline Insets operator+(Insets lhs, const Insets& rhs) {

ui/gfx/geometry/insets_unittest.cc

@@ -161,3 +161,122 @@ TEST(InsetsTest, Offset) {
   EXPECT_EQ(inset_first, offset_first);
   EXPECT_EQ(inset_by_offset, inset_first);
 }
+
+TEST(InsetsTest, Scale) {
+  gfx::Insets test(10, 5);
+  test = test.Scale(2.f, 3.f);
+  EXPECT_EQ(gfx::Insets(30, 10), test);
+
+  test = gfx::Insets(7, 3);
+  test = test.Scale(-2.f, -3.f);
+  EXPECT_EQ(gfx::Insets(-21, -6), test);
+}
+
+TEST(InsetsTest, IntegerOverflow) {
+  constexpr int int_min = std::numeric_limits<int>::min();
+  constexpr int int_max = std::numeric_limits<int>::max();
+
+  gfx::Insets width_height_test(int_max);
+  EXPECT_EQ(int_max, width_height_test.width());
+  EXPECT_EQ(int_max, width_height_test.height());
+
+  gfx::Insets plus_test(int_max);
+  plus_test += gfx::Insets(int_max);
+  EXPECT_EQ(gfx::Insets(int_max), plus_test);
+
+  gfx::Insets negation_test = -gfx::Insets(int_min);
+  EXPECT_EQ(gfx::Insets(int_max), negation_test);
+
+  gfx::Insets scale_test(int_max);
+  scale_test = scale_test.Scale(2.f, 2.f);
+  EXPECT_EQ(gfx::Insets(int_max), scale_test);
+}
+
+TEST(InsetsTest, IntegerUnderflow) {
+  constexpr int int_min = std::numeric_limits<int>::min();
+  constexpr int int_max = std::numeric_limits<int>::max();
+
+  gfx::Insets width_height_test = gfx::Insets(int_min);
+  EXPECT_EQ(int_min, width_height_test.width());
+  EXPECT_EQ(int_min, width_height_test.height());
+
+  gfx::Insets minus_test(int_min);
+  minus_test -= gfx::Insets(int_max);
+  EXPECT_EQ(gfx::Insets(int_min), minus_test);
+
+  gfx::Insets scale_test = gfx::Insets(int_min);
+  scale_test = scale_test.Scale(2.f, 2.f);
+  EXPECT_EQ(gfx::Insets(int_min), scale_test);
+}
+
+TEST(InsetsTest, IntegerOverflowSetVariants) {
+  constexpr int int_max = std::numeric_limits<int>::max();
+
+  gfx::Insets set_test(20);
+  set_test.set_top(int_max);
+  EXPECT_EQ(int_max, set_test.top());
+  EXPECT_EQ(0, set_test.bottom());
+
+  set_test.set_left(int_max);
+  EXPECT_EQ(int_max, set_test.left());
+  EXPECT_EQ(0, set_test.right());
+
+  set_test = gfx::Insets(30);
+  set_test.set_bottom(int_max);
+  EXPECT_EQ(int_max - 30, set_test.bottom());
+  EXPECT_EQ(30, set_test.top());
+
+  set_test.set_right(int_max);
+  EXPECT_EQ(int_max - 30, set_test.right());
+  EXPECT_EQ(30, set_test.left());
+}
+
+TEST(InsetsTest, IntegerUnderflowSetVariants) {
+  constexpr int int_min = std::numeric_limits<int>::min();
+
+  gfx::Insets set_test(-20);
+  set_test.set_top(int_min);
+  EXPECT_EQ(int_min, set_test.top());
+  EXPECT_EQ(0, set_test.bottom());
+
+  set_test.set_left(int_min);
+  EXPECT_EQ(int_min, set_test.left());
+  EXPECT_EQ(0, set_test.right());
+
+  set_test = gfx::Insets(-30);
+  set_test.set_bottom(int_min);
+  EXPECT_EQ(int_min + 30, set_test.bottom());
+  EXPECT_EQ(-30, set_test.top());
+
+  set_test.set_right(int_min);
+  EXPECT_EQ(int_min + 30, set_test.right());
+  EXPECT_EQ(-30, set_test.left());
+}
+
+TEST(InsetsTest, IntegerOverflowSet) {
+  constexpr int int_max = std::numeric_limits<int>::max();
+
+  gfx::Insets set_all_test;
+  set_all_test.Set(10, 20, int_max, int_max);
+  EXPECT_EQ(gfx::Insets(10, 20, int_max - 10, int_max - 20), set_all_test);
+}
+
+TEST(InsetsTest, IntegerOverflowOffset) {
+  constexpr int int_max = std::numeric_limits<int>::max();
+
+  const gfx::Vector2d max_vector(int_max, int_max);
+  gfx::Insets insets(1, 2, 3, 4);
+  gfx::Insets offset_test = insets.Offset(max_vector);
+  EXPECT_EQ(gfx::Insets(int_max, int_max, 3 - int_max, 4 - int_max),
+            offset_test);
+}
+
+TEST(InsetsTest, IntegerUnderflowOffset) {
+  constexpr int int_min = std::numeric_limits<int>::min();
+
+  const gfx::Vector2d min_vector(int_min, int_min);
+  gfx::Insets insets(-10);
+  gfx::Insets offset_test = insets.Offset(min_vector);
+  EXPECT_EQ(gfx::Insets(int_min, int_min, -10 - int_min, -10 - int_min),
+            offset_test);
+}