Add gfx::Insets::ToString().

Add gfx::Insets::ToString().

This will be useful for debugging views and views applications
(I needed this when I was debugging my views application).
Along the way, added tests for Insets.

TEST=added tests; app_unittests; try
BUG=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=34650

[sky, 2009-12-14 19:58:28]

On 2009/12/14 05:55:10, satorux1 wrote:
> 

For the same reasons we didn't add this operator to TimeDelta, we shouldn't add
it to Insets.

[satorux1, 2009-12-14 23:47:52]

On 2009/12/14 19:58:28, sky wrote:
> On 2009/12/14 05:55:10, satorux1 wrote:
> > 
> 
> For the same reasons we didn't add this operator to TimeDelta, we shouldn't
add
> it to Insets.

I didn't know the decision about TimeDelta. Maybe because we don't want to
expose the internal states? I thought Insets were like Rect and Size so it
looked ok to have this operator, but I'm also fine without the operator.

I guess it would be nice to leave a comment why we intentionally don't have this
operator in insets.h, in case someone else tries to add this in the future.

[sky, 2009-12-14 23:58:38]

I cc'd you on the email to Andrew about this. Search for the thread,
"Overloading operator<< for TimeDelta" on chromium-dev. It basically
comes down to the style says don't do this.

  -Scott

On Mon, Dec 14, 2009 at 3:47 PM,  <satorux@chromium.org> wrote:
> On 2009/12/14 19:58:28, sky wrote:
>>
>> On 2009/12/14 05:55:10, satorux1 wrote:
>> >
>
>> For the same reasons we didn't add this operator to TimeDelta, we
>> shouldn't
>
> add
>>
>> it to Insets.
>
> I didn't know the decision about TimeDelta. Maybe because we don't want to
> expose the internal states? I thought Insets were like Rect and Size so it
> looked ok to have this operator, but I'm also fine without the operator.
>
> I guess it would be nice to leave a comment why we intentionally don't have
> this
> operator in insets.h, in case someone else tries to add this in the future.
>
>
> http://codereview.chromium.org/497004
>

[satorux1, 2009-12-15 00:34:42]

Thanks. I missed the thread. I agree that violating the style guide is not
great.

What about adding something like ToString() to Insets? So we could do LOG(INFO)
<< insets.ToString().


On 2009/12/14 23:58:38, sky wrote:
> I cc'd you on the email to Andrew about this. Search for the thread,
> "Overloading operator<< for TimeDelta" on chromium-dev. It basically
> comes down to the style says don't do this.
> 
>   -Scott
> 
> On Mon, Dec 14, 2009 at 3:47 PM,  <mailto:satorux@chromium.org> wrote:
> > On 2009/12/14 19:58:28, sky wrote:
> >>
> >> On 2009/12/14 05:55:10, satorux1 wrote:
> >> >
> >
> >> For the same reasons we didn't add this operator to TimeDelta, we
> >> shouldn't
> >
> > add
> >>
> >> it to Insets.
> >
> > I didn't know the decision about TimeDelta. Maybe because we don't want to
> > expose the internal states? I thought Insets were like Rect and Size so it
> > looked ok to have this operator, but I'm also fine without the operator.
> >
> > I guess it would be nice to leave a comment why we intentionally don't have
> > this
> > operator in insets.h, in case someone else tries to add this in the future.
> >
> >
> > http://codereview.chromium.org/497004
> >
>

[satorux1, 2009-12-15 01:43:19]

Changed it to ToString(). Could you have another look?

On 2009/12/15 00:34:42, satorux1 wrote:
> Thanks. I missed the thread. I agree that violating the style guide is not
> great.
> 
> What about adding something like ToString() to Insets? So we could do
LOG(INFO)
> << insets.ToString().
> 
> 
> On 2009/12/14 23:58:38, sky wrote:
> > I cc'd you on the email to Andrew about this. Search for the thread,
> > "Overloading operator<< for TimeDelta" on chromium-dev. It basically
> > comes down to the style says don't do this.
> > 
> >   -Scott
> > 
> > On Mon, Dec 14, 2009 at 3:47 PM,  <mailto:satorux@chromium.org> wrote:
> > > On 2009/12/14 19:58:28, sky wrote:
> > >>
> > >> On 2009/12/14 05:55:10, satorux1 wrote:
> > >> >
> > >
> > >> For the same reasons we didn't add this operator to TimeDelta, we
> > >> shouldn't
> > >
> > > add
> > >>
> > >> it to Insets.
> > >
> > > I didn't know the decision about TimeDelta. Maybe because we don't want to
> > > expose the internal states? I thought Insets were like Rect and Size so it
> > > looked ok to have this operator, but I'm also fine without the operator.
> > >
> > > I guess it would be nice to leave a comment why we intentionally don't
have
> > > this
> > > operator in insets.h, in case someone else tries to add this in the
future.
> > >
> > >
> > > http://codereview.chromium.org/497004
> > >
> >

[sky, 2009-12-15 16:44:13]

LGTM

http://codereview.chromium.org/497004/diff/5008/5011
File app/gfx/insets.cc (right):

http://codereview.chromium.org/497004/diff/5008/5011#newcode6
app/gfx/insets.cc:6: #include "base/string_util.h"
nit: newline between lines 5 and 6.

[satorux1, 2009-12-15 23:46:01]

Thank you for the review.

Fixed the line break.
Will submit after running try again.

http://codereview.chromium.org/497004/diff/5008/5011
File app/gfx/insets.cc (right):

http://codereview.chromium.org/497004/diff/5008/5011#newcode6
app/gfx/insets.cc:6: #include "base/string_util.h"
On 2009/12/15 16:44:13, sky wrote:
> nit: newline between lines 5 and 6.

Done.