Stabilize empty rect handling in EnclosingRect.

ui/gfx/geometry/rect_conversions.cc

Index: ui/gfx/geometry/rect_conversions.cc
diff --git a/ui/gfx/geometry/rect_conversions.cc b/ui/gfx/geometry/rect_conversions.cc
index b4ef3d44ec9c4e434b93ae64531cde2f6197014e..ff125e97761d717289e0f2272b9ebd51389f26eb 100644
--- a/ui/gfx/geometry/rect_conversions.cc
+++ b/ui/gfx/geometry/rect_conversions.cc
@@ -17,18 +17,13 @@ Rect ToEnclosingRect(const RectF& rect) {
   int min_y = ToFlooredInt(rect.y());
   float max_x = rect.right();
   float max_y = rect.bottom();
-  int width =
-      rect.width() == 0
-          ? 0
-          : std::max(
-                ToCeiledInt(static_cast<double>(ToCeiledInt(max_x)) - min_x),
-                0);
+  int width = rect.size().width()

[danakj, 2017/03/23 20:12:06]

I think you can revert the changes in this file?

Why dropping the std::max()? That should be a separate CL if that's intentional.

[Peter Mayo, 2017/03/23 23:58:41]

std::max with 0 is irrelevant once Size_F can't be smaller than zero.

size.width() >=0 -> min_x <= max_x -> CeiledInt(max_x-min_x) >= 0

So, I think it should be part of this CL, since all the stuff less than kTrivial
-> 0.0

If it shouldn't, we should have some unit tests to demonstrate the edge case.

[danakj, 2017/03/24 14:55:35]

Ah, this was clamping but RectF already does so it's redundant. Okay.

rect.width() is a simpler way to write rect.size().width() tho, can you leave
that? same with rect.height()

+                  ? ToCeiledInt(static_cast<double>(ToCeiledInt(max_x)) - min_x)
+                  : 0;
   int height =
-      rect.height() == 0
-          ? 0
-          : std::max(
-                ToCeiledInt(static_cast<double>(ToCeiledInt(max_y)) - min_y),
-                0);
+      rect.size().height()
+          ? ToCeiledInt(static_cast<double>(ToCeiledInt(max_y)) - min_y)
+          : 0;
   return Rect(min_x, min_y, width, height);
 }
 
@@ -91,4 +86,3 @@ Rect ToFlooredRectDeprecated(const RectF& rect) {
 }
 
 }  // namespace gfx
-