libui: Fix clamping of input values in ui::Size

A previous change to fix a compiler warning broke the clamped-conversion
of floating point input values to the int32_t values used internally.
As a result, calling setWidth() or steHeight() with any floating point
value resulted in a value of INT32_MIN being set,

The existing Size_test unit test would have caught this issue, but it
would have had to have been manually run since it was not configured to
run automatically. No one noticed it was broken for a while since it is
relatively new and the existing callers were not setting a floating
point value.

This change:

1) Fixes clamping of floating point values to work again, as verified
   by the existing test.

2) Helps enforce that the header remains conversion-warning free by
   enabling a few conversion warnings (as errors) when building the
   test.

3) Adds a TEST_MAPPING file, which will run Size_test as part of the
   presubmit runs (effectively continuously), as well as including the
   test in the device-test suite in the Android.bp file.

4) Adds a PrintTo() function for more clearly printing out a ui::Size
   value when using the Google Test framework.

Bug: 149495759
Test: atest Size_test
Change-Id: I8f18f20e6b5a3ea54eb383486a1f7222a1a2a544

4 files changed, 37 insertions, 11 deletions

diff --git a/libs/ui/include/ui/Size.h b/libs/ui/include/ui/Size.h
index d9b713df4c..c2cda17a6f 100644
--- a/libs/ui/include/ui/Size.h
+++ b/libs/ui/include/ui/Size.h
@@ -19,6 +19,7 @@
 #include <algorithm>
 #include <cstdint>
 #include <limits>
+#include <ostream>
 #include <type_traits>
 #include <utility>
 
@@ -113,13 +114,12 @@ struct Size {
                     std::numeric_limits<Size::remove_cv_reference_t<ToType>>::is_bounded &&
                             std::numeric_limits<Size::remove_cv_reference_t<FromType>>::is_bounded,
                     FromType&&>::type v) {
-        static constexpr auto toHighest = std::numeric_limits<remove_cv_reference_t<ToType>>::max();
-        static constexpr auto toLowest =
-                std::numeric_limits<remove_cv_reference_t<ToType>>::lowest();
-        static constexpr auto fromHighest =
-                std::numeric_limits<remove_cv_reference_t<FromType>>::max();
-        static constexpr auto fromLowest =
-                std::numeric_limits<remove_cv_reference_t<FromType>>::lowest();
+        using BareToType = remove_cv_reference_t<ToType>;
+        using BareFromType = remove_cv_reference_t<FromType>;
+        static constexpr auto toHighest = std::numeric_limits<BareToType>::max();
+        static constexpr auto toLowest = std::numeric_limits<BareToType>::lowest();
+        static constexpr auto fromHighest = std::numeric_limits<BareFromType>::max();
+        static constexpr auto fromLowest = std::numeric_limits<BareFromType>::lowest();
 
         // A clamp is needed if the range of FromType is not a subset of the range of ToType
         static constexpr bool isClampNeeded = (toLowest > fromLowest) || (toHighest < fromHighest);
@@ -129,10 +129,13 @@ struct Size {
             return static_cast<ToType>(v);
         }
 
-        // Otherwise we leverage implicit conversion to safely compare values of
-        // different types, to ensure we return a value clamped to the range of
-        // ToType.
-        return v < toLowest ? toLowest : (static_cast<ToType>(v) > toHighest ? toHighest : static_cast<ToType>(v));
+        // Otherwise we need to carefully compare the limits of ToType (casted
+        // for the comparisons to be warning free to FromType) while still
+        // ensuring we return a value clamped to the range of ToType.
+        return v < static_cast<const BareFromType>(toLowest)
+                ? toLowest
+                : (v > static_cast<const BareFromType>(toHighest) ? toHighest
+                                                                  : static_cast<ToType>(v));
     }
 };
 
@@ -154,5 +157,10 @@ inline bool operator<(const Size& lhs, const Size& rhs) {
     return lhs.height < rhs.height;
 }
 
+// Defining PrintTo helps with Google Tests.
+static inline void PrintTo(const Size& size, ::std::ostream* os) {
+    *os << "Size(" << size.width << ", " << size.height << ")";
+}
+
 } // namespace ui
 } // namespace android
diff --git a/libs/ui/tests/Android.bp b/libs/ui/tests/Android.bp
index ff55bc2303..605c5a9ba0 100644
--- a/libs/ui/tests/Android.bp
+++ b/libs/ui/tests/Android.bp
@@ -111,6 +111,7 @@ cc_test {
 
 cc_test {
     name: "Size_test",
+    test_suites: ["device-tests"],
     shared_libs: ["libui"],
     srcs: ["Size_test.cpp"],
     cflags: ["-Wall", "-Werror"],
diff --git a/libs/ui/tests/Size_test.cpp b/libs/ui/tests/Size_test.cpp
index 69e1ac8b27..40dc702a8b 100644
--- a/libs/ui/tests/Size_test.cpp
+++ b/libs/ui/tests/Size_test.cpp
@@ -19,8 +19,18 @@
 #include <cmath>
 #include <cstdlib>
 
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic error "-Wimplicit-int-float-conversion"
+#pragma clang diagnostic error "-Wconversion"
+#endif // __clang__
+
 #include <ui/Size.h>
 
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif // __clang__
+
 #include <gtest/gtest.h>
 
 namespace android {
diff --git a/libs/ui/tests/TEST_MAPPING b/libs/ui/tests/TEST_MAPPING
new file mode 100644
index 0000000000..7fcd7de319
--- /dev/null
+++ b/libs/ui/tests/TEST_MAPPING
@@ -0,0 +1,7 @@
+{
+  "presubmit": [
+    {
+      "name": "Size_test"
+    }
+  ]
+}