Adjust gfx::Rect's width and height to avoid integer overflow

ui/gfx/geometry/rect.cc

Index: ui/gfx/geometry/rect.cc
diff --git a/ui/gfx/geometry/rect.cc b/ui/gfx/geometry/rect.cc
index e7a3aa2aac93f14eb96c4832a130f4bbeafa9253..4c64b9770bd85a5a854928cf4981e06633efd7e7 100644
--- a/ui/gfx/geometry/rect.cc
+++ b/ui/gfx/geometry/rect.cc
@@ -142,10 +142,19 @@ void Rect::Union(const Rect& rect) {
 
   int rx = std::min(x(), rect.x());
   int ry = std::min(y(), rect.y());
-  int rr = std::max(right(), rect.right());
-  int rb = std::max(bottom(), rect.bottom());
 
-  SetRect(rx, ry, rr - rx, rb - ry);
+  // This is a temporary fix to avoid integer overflow, gfx::Rect is not
+  // supposed to have instances whose right or bottom is greater than maximum
+  // integer. This hack should be removed if root cause is fixed.

[danakj, 2016/08/23 17:47:56]

I'm not sure what this comment means, there are like infinity root causes. Rect
should simply protect against being invalid.

[sunxd, 2016/08/23 19:08:24]

I'll remove this comment.

+  int64_t right = static_cast<int64_t>(x()) + width();

[danakj, 2016/08/23 17:47:56]

why not just use right()? Does the rect already have an invalid right()? Can you
fix the place that is causing that in this class too?

[sunxd, 2016/08/23 19:08:24]

It does not. Is there any difference between right() and bottom() that makes the
invalid value come with one but not with the other?

[danakj, 2016/08/23 19:25:12]

No, I mean it just depends on the inputs. Neither should be allowed to happen in
this class.

+  int64_t rect_right = static_cast<int64_t>(rect.x()) + rect.width();

[danakj, 2016/08/23 17:47:56]

What performs better? int64_ts with (properly) saturating downcast or
CheckedNumeric? Why did you choose int64?

[sunxd, 2016/08/23 19:08:24]

Hmm actually I have no idea which has a better performance. I basically just
copied the solution in PictureLayerImpl::SafeIntersectRects which was landed to
fix a related clusterfuzz bug.

I'll switch to CheckedNumeric if that's faster.

[danakj, 2016/08/23 19:25:12]

You could do a simple test to figure out what's better ala cc_perftests.

+  int64_t bottom = static_cast<int64_t>(y()) + height();
+  int64_t rect_bottom = static_cast<int64_t>(rect.y()) + rect.height();
+
+  int64_t rr = std::max(right, rect_right);
+  int64_t rb = std::max(bottom, rect_bottom);
+
+  SetRect(rx, ry, static_cast<int>(rr - rx), static_cast<int>(rb - ry));

[danakj, 2016/08/23 17:47:56]

Does static cast really do what you want?

http://stackoverflow.com/questions/5990576/static-cast-with-bounded-types

Can you demonstrate that this is doing what you expect with tests, to show that
we're not going to get a right/bottom() that overflow?

[sunxd, 2016/08/23 19:08:24]

Working on tests.

 }
 
 void Rect::Subtract(const Rect& rect) {