Use RECT instead of CRect to reduce dependencies on ATL.

Use RECT instead of CRect to reduce dependencies on ATL.

BUG=5027
TEST=none

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

[Peter Kasting, 2009-09-09 17:49:36]

LGTM I guess, but I thought CRect was part of WTL (which is fine to keep as a
dependency) rather than ATL?  It always confuses me that both have headers like
"atlxxx.h".

http://codereview.chromium.org/195036/diff/1/2
File views/controls/table/table_view.cc (right):

http://codereview.chromium.org/195036/diff/1/2#newcode359
Line 359: DCHECK_EQ(all_columns_.count(col.id), 0);
Nit: Shouldn't the 0 be first here?

http://codereview.chromium.org/195036/diff/1/2#newcode1215
Line 1215: icon_rect.top - icon_rect.bottom, false);
Shouldn't this be bottom - top?

http://codereview.chromium.org/195036/diff/1/2#newcode1510
Line 1510: content_offset_ = origin.y + header_bounds.top -
header_bounds.bottom;
...And here

[Peter Kasting, 2009-09-09 17:53:48]

Hmm, maybe not, MSDN claims it comes from atltypes.h, which is part of ATL...
but it also claims it's part of MFC.  Gah

[James Hawkins, 2009-09-09 19:30:20]

http://codereview.chromium.org/195036/diff/1/2
File views/controls/table/table_view.cc (right):

http://codereview.chromium.org/195036/diff/1/2#newcode359
Line 359: DCHECK_EQ(all_columns_.count(col.id), 0);
On 2009/09/09 17:49:36, Peter Kasting wrote:
> Nit: Shouldn't the 0 be first here?

Done.  But I'm curious, why is that the precedent?

http://codereview.chromium.org/195036/diff/1/2#newcode1215
Line 1215: icon_rect.top - icon_rect.bottom, false);
On 2009/09/09 17:49:36, Peter Kasting wrote:
> Shouldn't this be bottom - top?

Done.

http://codereview.chromium.org/195036/diff/1/2#newcode1510
Line 1510: content_offset_ = origin.y + header_bounds.top -
header_bounds.bottom;
On 2009/09/09 17:49:36, Peter Kasting wrote:
> ...And here

Done.

[Peter Kasting, 2009-09-09 19:41:56]

On Wed, Sep 9, 2009 at 12:30 PM, <jhawkins@chromium.org> wrote:

> http://codereview.chromium.org/195036/diff/1/2#newcode359
> Line 359: DCHECK_EQ(all_columns_.count(col.id), 0);
> On 2009/09/09 17:49:36, Peter Kasting wrote:
>
>> Nit: Shouldn't the 0 be first here?
>>
>
> Done.  But I'm curious, why is that the precedent?


All the testing/logging macros whose arguments are commutative are
(expected, actual).  It affects how messages about their failure are dumped.

PK