ui: Remove gfx::Size::ClampToNonNegative, prevent negative sizes always.

ui: Remove gfx::Size::ClampToNonNegative, prevent negative sizes always.

This was added with the intention of using Size as a vector, replacing use of
IntSize. Since we have Vector2d now, negative sizes should not exist, so clamp
them in set_width/set_height and the constructor.

R=sky
BUG=160158
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=167062

[sky, 2012-11-09 17:03:55]

https://codereview.chromium.org/11365160/diff/2001/ui/gfx/rect_f.h
File ui/gfx/rect_f.h (left):

https://codereview.chromium.org/11365160/diff/2001/ui/gfx/rect_f.h#oldcode39
ui/gfx/rect_f.h:39: SizeF newSize = size().Scale(x_scale, y_scale);
newSize -> new_size

https://codereview.chromium.org/11365160/diff/2001/ui/gfx/size_base.h
File ui/gfx/size_base.h (right):

https://codereview.chromium.org/11365160/diff/2001/ui/gfx/size_base.h#newcode35
ui/gfx/size_base.h:35: void set_width(Type width) { width_ = width < 0 ? 0 :
width; }
Where we previously doing this? I thought previously we let through negatives.

[danakj, 2012-11-09 17:06:57]

https://codereview.chromium.org/11365160/diff/2001/ui/gfx/rect_f.h
File ui/gfx/rect_f.h (left):

https://codereview.chromium.org/11365160/diff/2001/ui/gfx/rect_f.h#oldcode39
ui/gfx/rect_f.h:39: SizeF newSize = size().Scale(x_scale, y_scale);
On 2012/11/09 17:03:55, sky wrote:
> newSize -> new_size

Done.

https://codereview.chromium.org/11365160/diff/2001/ui/gfx/size_base.h
File ui/gfx/size_base.h (right):

https://codereview.chromium.org/11365160/diff/2001/ui/gfx/size_base.h#newcode35
ui/gfx/size_base.h:35: void set_width(Type width) { width_ = width < 0 ? 0 :
width; }
On 2012/11/09 17:03:55, sky wrote:
> Where we previously doing this? I thought previously we let through negatives.

There were DCHECKs previously.

[danakj, 2012-11-09 17:08:52]

> There were DCHECKs previously.

https://chromiumcodereview.appspot.com/10996037/diff/9010/ui/gfx/size_base_im...

[sky, 2012-11-09 17:11:41]

Should we put back the DCHECKs then?

On Fri, Nov 9, 2012 at 9:08 AM,  <danakj@chromium.org> wrote:
>> There were DCHECKs previously.
>
>
>
https://chromiumcodereview.appspot.com/10996037/diff/9010/ui/gfx/size_base_im...
>
> https://codereview.chromium.org/11365160/

[danakj, 2012-11-09 18:28:35]

On Fri, Nov 9, 2012 at 12:11 PM, Scott Violet <sky@chromium.org> wrote:
> Should we put back the DCHECKs then?

That was before we had Scale. I guess negative scale is always wrong
though? Okay I'll return them.

[danakj, 2012-11-09 18:33:10]

I've added DCHECKs. The downside is you can't test code that is going to DCHECK,
so I removed the unit test as well.

[sky, 2012-11-09 21:30:53]

I don't see the DCHECKs. Did you intend to put them back?

[danakj, 2012-11-09 21:41:30]

Sorry, seems like i lost the change locally too. Uploaded now PTAL

[sky, 2012-11-09 22:03:28]

LGTM

[commit-bot: I haz the power, 2012-11-10 01:08:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/11365160/10001

[commit-bot: I haz the power, 2012-11-10 01:08:35]

Failed to apply patch for ui/gfx/size_base.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ui/gfx/size_base.h
  Hunk #2 FAILED at 33.
  1 out of 2 hunks FAILED -- saving rejects to file ui/gfx/size_base.h.rej

Patch:       ui/gfx/size_base.h
Index: ui/gfx/size_base.h
diff --git a/ui/gfx/size_base.h b/ui/gfx/size_base.h
index
cd7221c345ec701349fbfec0bd317fdc8020fa64..2f663d3744a513018c300650d0d8cabc38408430
100644
--- a/ui/gfx/size_base.h
+++ b/ui/gfx/size_base.h
@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/compiler_specific.h"
+#include "base/logging.h"
 #include "build/build_config.h"
 #include "ui/base/ui_export.h"
 
@@ -32,24 +33,26 @@ class UI_EXPORT SizeBase {
     set_height(height_ + height);
   }
 
-  void set_width(Type width) { width_ = width; }
-  void set_height(Type height) { height_ = height; }
-
-  bool IsEmpty() const {
-    return (width_ <= 0) || (height_ <= 0);
+  void set_width(Type width) {
+    DCHECK_GE(width, 0);
+    width_ = width < 0 ? 0 : width;
+  }
+  void set_height(Type height) {
+    DCHECK_GE(height, 0);
+    height_ = height < 0 ? 0 : height;
   }
 
-  void ClampToNonNegative() {
-    if (width_ < 0)
-      width_ = 0;
-    if (height_ < 0)
-      height_ = 0;
+  bool IsEmpty() const {
+    return (width_ == 0) || (height_ == 0);
   }
 
  protected:
   SizeBase(Type width, Type height)
-      : width_(width),
-        height_(height) {}
+      : width_(width < 0 ? 0 : width),
+        height_(height < 0 ? 0 : height) {
+    DCHECK_GE(width, 0);
+    DCHECK_GE(height, 0);
+  }
 
   // Destructor is intentionally made non virtual and protected.
   // Do not make this public.

[danakj, 2012-11-10 02:42:29]

In order to avoid LOG() macro conflicts from WebRect.h etc, I had to reinvoke
the size_base_impl.h header to hold methods that have DCHECKs in them.

[commit-bot: I haz the power, 2012-11-10 02:42:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/11365160/5012

[commit-bot: I haz the power, 2012-11-10 03:58:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/11365160/6010