Replace gfx::ClampToInt with base::saturated_cast.

This found a bug in saturated_cast that was covered by ClampToInt tests
so it fixes that.

If you have a floating point value of MAX_INT, then comparing an
integer against it for equality with promote it to a float and
it will compare true. However if you cast the float to an int, its
actually outside the bounds of integer, so you can end up with a
negative int as a result. Added unittests to check this for
saturated_cast.

Review URL: https://codereview.chromium.org/1164063005

Cr-Commit-Position: refs/heads/master@{#333092}

base/numerics/safe_conversions_impl.h

@@ -148,10 +148,10 @@ struct DstRangeRelationToSrcRangeImpl<Dst,
                                       NUMERIC_RANGE_NOT_CONTAINED> {
   static RangeConstraint Check(Src value) {
     return std::numeric_limits<Dst>::is_iec559
-               ? GetRangeConstraint(value <= std::numeric_limits<Dst>::max(),
-                                    value >= -std::numeric_limits<Dst>::max())
-               : GetRangeConstraint(value <= std::numeric_limits<Dst>::max(),
-                                    value >= std::numeric_limits<Dst>::min());
+               ? GetRangeConstraint((value < std::numeric_limits<Dst>::max()),
+                                    (value > -std::numeric_limits<Dst>::max()))
+               : GetRangeConstraint((value < std::numeric_limits<Dst>::max()),
+                                    (value > std::numeric_limits<Dst>::min()));
   }
 };
 
@@ -163,7 +163,7 @@ struct DstRangeRelationToSrcRangeImpl<Dst,
                                       INTEGER_REPRESENTATION_UNSIGNED,
                                       NUMERIC_RANGE_NOT_CONTAINED> {
   static RangeConstraint Check(Src value) {
-    return GetRangeConstraint(value <= std::numeric_limits<Dst>::max(), true);
+    return GetRangeConstraint(value < std::numeric_limits<Dst>::max(), true);
   }
 };
 
@@ -178,7 +178,7 @@ struct DstRangeRelationToSrcRangeImpl<Dst,
     return sizeof(Dst) > sizeof(Src)
                ? RANGE_VALID
                : GetRangeConstraint(
-                     value <= static_cast<Src>(std::numeric_limits<Dst>::max()),
+                     value < static_cast<Src>(std::numeric_limits<Dst>::max()),
                      true);
   }
 };
@@ -195,7 +195,7 @@ struct DstRangeRelationToSrcRangeImpl<Dst,
     return (MaxExponent<Dst>::value >= MaxExponent<Src>::value)
                ? GetRangeConstraint(true, value >= static_cast<Src>(0))
                : GetRangeConstraint(
-                     value <= static_cast<Src>(std::numeric_limits<Dst>::max()),
+                     value < static_cast<Src>(std::numeric_limits<Dst>::max()),
                      value >= static_cast<Src>(0));
   }
 };

base/numerics/safe_numerics_unittest.cc

@@ -562,6 +562,8 @@ TEST(SafeNumerics, CastTests) {
   double double_small = 1.0;
   double double_large = numeric_limits<double>::max();
   double double_infinity = numeric_limits<float>::infinity();
+  double double_large_int = numeric_limits<int>::max();
+  double double_small_int = numeric_limits<int>::min();
 
   // Just test that the casts compile, since the other tests cover logic.
   EXPECT_EQ(0, checked_cast<int>(static_cast<size_t>(0)));
@@ -594,5 +596,7 @@ TEST(SafeNumerics, CastTests) {
   EXPECT_EQ(saturated_cast<int>(double_large), numeric_limits<int>::max());
   EXPECT_EQ(saturated_cast<float>(double_large), double_infinity);
   EXPECT_EQ(saturated_cast<float>(-double_large), -double_infinity);
+  EXPECT_EQ(numeric_limits<int>::min(), saturated_cast<int>(double_small_int));
+  EXPECT_EQ(numeric_limits<int>::max(), saturated_cast<int>(double_large_int));
 }
 

chrome/browser/ui/views/extensions/bookmark_app_bubble_view.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/views/extensions/bookmark_app_bubble_view.h"
 
+#include "base/numerics/safe_conversions.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -15,7 +16,6 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/events/keycodes/keyboard_codes.h"
-#include "ui/gfx/geometry/safe_integer_conversions.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_source.h"
 #include "ui/views/controls/button/checkbox.h"
@@ -47,7 +47,7 @@ class WebAppInfoImageSource : public gfx::ImageSkiaSource {
 
  private:
   gfx::ImageSkiaRep GetImageForScale(float scale) override {
-    int size = gfx::ClampToInt(dip_size_ * scale);
+    int size = base::saturated_cast<int>(dip_size_ * scale);
     for (const auto& icon_info : info_.icons) {
       if (icon_info.width == size) {
         return gfx::ImageSkiaRep(icon_info.data, scale);

ui/gfx/canvas_skia.cc

@@ -7,10 +7,10 @@
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/numerics/safe_conversions.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/geometry/rect.h"
-#include "ui/gfx/geometry/safe_integer_conversions.h"
 #include "ui/gfx/range/range.h"
 #include "ui/gfx/render_text.h"
 #include "ui/gfx/shadow_value.h"
@@ -141,7 +141,7 @@ void Canvas::SizeStringFloat(const base::string16& text,
     std::vector<base::string16> strings;
     ElideRectangleText(text, font_list, *width, INT_MAX, wrap_behavior,
                        &strings);
-    Rect rect(ClampToInt(*width), INT_MAX);
+    Rect rect(base::saturated_cast<int>(*width), INT_MAX);
     scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
     UpdateRenderText(rect, base::string16(), font_list, flags, 0,
                      render_text.get());
@@ -161,7 +161,8 @@ void Canvas::SizeStringFloat(const base::string16& text,
     *height = h;
   } else {
     scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
-    Rect rect(ClampToInt(*width), ClampToInt(*height));
+    Rect rect(base::saturated_cast<int>(*width),
+              base::saturated_cast<int>(*height));
     base::string16 adjusted_text = text;
     StripAcceleratorChars(flags, &adjusted_text);
     UpdateRenderText(rect, adjusted_text, font_list, flags, 0,

ui/gfx/geometry/rect_unittest.cc

@@ -497,11 +497,6 @@ TEST(RectTest, ToEnclosedRect) {
       std::numeric_limits<int>::max() },
     { 20000.5f, 20000.5f, 0.5f, 0.5f,
       20001, 20001, 0, 0 },
-    { std::numeric_limits<float>::quiet_NaN(),
-      std::numeric_limits<float>::quiet_NaN(),
-      std::numeric_limits<float>::quiet_NaN(),
-      std::numeric_limits<float>::quiet_NaN(),
-      0, 0, 0, 0 }
   };
 
   for (size_t i = 0; i < arraysize(tests); ++i) {
@@ -549,11 +544,6 @@ TEST(RectTest, ToEnclosingRect) {
       std::numeric_limits<int>::max() },
     { 20000.5f, 20000.5f, 0.5f, 0.5f,
       20000, 20000, 1, 1 },
-    { std::numeric_limits<float>::quiet_NaN(),
-      std::numeric_limits<float>::quiet_NaN(),
-      std::numeric_limits<float>::quiet_NaN(),
-      std::numeric_limits<float>::quiet_NaN(),
-      0, 0, 0, 0 }
   };
 
   for (size_t i = 0; i < arraysize(tests); ++i) {

ui/gfx/geometry/safe_integer_conversions.h

@@ -8,34 +8,25 @@
 #include <cmath>
 #include <limits>
 
+#include "base/numerics/safe_conversions.h"
 #include "ui/gfx/gfx_export.h"
 
 namespace gfx {
 
-inline int ClampToInt(float value) {
-  if (value != value)
-    return 0; // no int NaN.
-  if (value >= std::numeric_limits<int>::max())
-    return std::numeric_limits<int>::max();
-  if (value <= std::numeric_limits<int>::min())
-    return std::numeric_limits<int>::min();
-  return static_cast<int>(value);
-}
-
 inline int ToFlooredInt(float value) {
-  return ClampToInt(std::floor(value));
+  return base::saturated_cast<int>(std::floor(value));
 }
 
 inline int ToCeiledInt(float value) {
-  return ClampToInt(std::ceil(value));
+  return base::saturated_cast<int>(std::ceil(value));
 }
 
 inline int ToFlooredInt(double value) {
-  return ClampToInt(std::floor(value));
+  return base::saturated_cast<int>(std::floor(value));
 }
 
 inline int ToCeiledInt(double value) {
-  return ClampToInt(std::ceil(value));
+  return base::saturated_cast<int>(std::ceil(value));
 }
 
 inline int ToRoundedInt(float value) {
@@ -44,7 +35,7 @@ inline int ToRoundedInt(float value) {
     rounded = std::floor(value + 0.5f);
   else
     rounded = std::ceil(value - 0.5f);
-  return ClampToInt(rounded);
+  return base::saturated_cast<int>(rounded);
 }
 
 inline int ToRoundedInt(double value) {
@@ -53,7 +44,7 @@ inline int ToRoundedInt(double value) {
     rounded = std::floor(value + 0.5);
   else
     rounded = std::ceil(value - 0.5);
-  return ClampToInt(rounded);
+  return base::saturated_cast<int>(rounded);
 }
 
 inline bool IsExpressibleAsInt(float value) {

ui/gfx/geometry/safe_integer_conversions_unittest.cc

@@ -10,32 +10,7 @@
 
 namespace gfx {
 
-TEST(SafeIntegerConversions, ClampToInt) {
-  EXPECT_EQ(0, ClampToInt(std::numeric_limits<float>::quiet_NaN()));
-
-  float max = std::numeric_limits<int>::max();
-  float min = std::numeric_limits<int>::min();
-  float infinity = std::numeric_limits<float>::infinity();
-
-  int int_max = std::numeric_limits<int>::max();
-  int int_min = std::numeric_limits<int>::min();
-
-  EXPECT_EQ(int_max, ClampToInt(infinity));
-  EXPECT_EQ(int_max, ClampToInt(max));
-  EXPECT_EQ(int_max, ClampToInt(max + 100));
-
-  EXPECT_EQ(-100, ClampToInt(-100.5f));
-  EXPECT_EQ(0, ClampToInt(0));
-  EXPECT_EQ(100, ClampToInt(100.5f));
-
-  EXPECT_EQ(int_min, ClampToInt(-infinity));
-  EXPECT_EQ(int_min, ClampToInt(min));
-  EXPECT_EQ(int_min, ClampToInt(min - 100));
-}
-
 TEST(SafeIntegerConversions, ToFlooredInt) {
-  EXPECT_EQ(0, ToFlooredInt(std::numeric_limits<float>::quiet_NaN()));
-
   float max = std::numeric_limits<int>::max();
   float min = std::numeric_limits<int>::min();
   float infinity = std::numeric_limits<float>::infinity();
@@ -57,8 +32,6 @@ TEST(SafeIntegerConversions, ToFlooredInt) {
 }
 
 TEST(SafeIntegerConversions, ToCeiledInt) {
-  EXPECT_EQ(0, ToCeiledInt(std::numeric_limits<float>::quiet_NaN()));
-
   float max = std::numeric_limits<int>::max();
   float min = std::numeric_limits<int>::min();
   float infinity = std::numeric_limits<float>::infinity();
@@ -80,8 +53,6 @@ TEST(SafeIntegerConversions, ToCeiledInt) {
 }
 
 TEST(SafeIntegerConversions, ToRoundedInt) {
-  EXPECT_EQ(0, ToRoundedInt(std::numeric_limits<float>::quiet_NaN()));
-
   float max = std::numeric_limits<int>::max();
   float min = std::numeric_limits<int>::min();
   float infinity = std::numeric_limits<float>::infinity();

ui/gfx/transform_unittest.cc

@@ -448,10 +448,6 @@ TEST(XFormTest, ConcatTranslate2D) {
     { 0, 0, 10.0f, 20.0f, 10, 20},
     { 0, 0, -10.0f, -20.0f, 0, 0},
     { 0, 0, -10.0f, -20.0f, -10, -20},
-    { 0, 0,
-      std::numeric_limits<float>::quiet_NaN(),
-      std::numeric_limits<float>::quiet_NaN(),
-      10, 20},
   };
 
   Transform xform;
@@ -481,7 +477,6 @@ TEST(XFormTest, ConcatScale2D) {
     { 1, .1f, 1},
     { 1, 100.0f, 100},
     { 1, -1.0f, -100},
-    { 1, std::numeric_limits<float>::quiet_NaN(), 1}
   };
 
   Transform xform;
@@ -513,7 +508,6 @@ TEST(XFormTest, ConcatRotate2D) {
     { 1, 0, 90.0f, 0, 1},
     { 1, 0, 360.0f, 0, 1},
     { 1, 0, 0.0f, 0, 1},
-    { 1, 0, std::numeric_limits<float>::quiet_NaN(), 1, 0}
   };
 
   Transform xform;
@@ -541,10 +535,6 @@ TEST(XFormTest, SetTranslate2D) {
     { 0, 0, 10.0f, 20.0f, 10, 20},
     { 10, 20, 10.0f, 20.0f, 20, 40},
     { 10, 20, 0.0f, 0.0f, 10, 20},
-    { 0, 0,
-      std::numeric_limits<float>::quiet_NaN(),
-      std::numeric_limits<float>::quiet_NaN(),
-      0, 0}
   };
 
   for (size_t i = 0; i < arraysize(test_cases); ++i) {
@@ -597,7 +587,6 @@ TEST(XFormTest, SetScale2D) {
     { 1, 1.0f, 1},
     { 1, 0.0f, 0},
     { 0, 10.0f, 0},
-    { 1, std::numeric_limits<float>::quiet_NaN(), 0},
   };
 
   for (size_t i = 0; i < arraysize(test_cases); ++i) {